[PATCH] D155955: [Clang] Improve the handling of large arrays evaluation.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D155955#4532385 <https://reviews.llvm.org/D155955#4532385>, @aaron.ballman wrote:

> In D155955#4528792 <https://reviews.llvm.org/D155955#4528792>, @efriedma wrote:
>
>>> Note sure what you mean. Making sure we use size_t for all array extents is not something that can be fixed overnight but more importantly, it does not help: Would it not overflow, the allocation would still fail.
>>
>> Oh, right, it would be sizeof(APValue) * UINT_MAX, which would break in any practical usage.
>>
>>> I don't think that would make sense in actual code, and having some sort of sparse array is something I considered. And we already delay allocation in some cases, but we do need to create elements to destroy them, read them, etc. So while it may be possible, it seems... complicated.
>>
>> Definitely not simple, sure.
>>
>>> I think a more viable long term fix might be to not crash on allocation failure, and/or to have a way to limit the allocation to some portion of the available memory.
>>
>> Making the compiler's behavior depend on the amount of memory installed in the user's computer seems like a non-starter.  I think we just have to stick with some combination of:
>>
>> - Try to reduce excessive memory usage in constant folding.
>
> 100% agreed
>
>> - Add strict limits memory usage limits for optional constant folding
>
> This seems reasonable to me, but I think this should be user controllable so that users with different resource constraints can override whatever default value we pick.
>
>> - Maybe consider disobeying the standard slightly in certain cases: the standard requires that we constant-fold the initializers for all global variables, but that might not really be viable for globals that are expensive to evaluate.
>
> If we can use an existing implementation limit (http://eel.is/c++draft/implimits) to justify our decision, great; otherwise, I think we should ask WG21 for an official escape hatch so that we don't have conformance issues.

Forgot to reply to that, it's already conforming

- A variable or temporary object o is constant-initialized if [..] the full-expression of its initialization is a constant expression http://eel.is/c++draft/expr.const#2
- An expression E is a core constant expression unless [...] it would exceed the implementation-defined limits http://eel.is/c++draft/expr.const#5
- Limits may constrain quantities that include those described below or others. http://eel.is/c++draft/implimits#2.sentence-1

On the last point, i think a *explicit* limit would be nice but i chatted with core and they agreed the current wording was sufficient - and they didn't seem super enthusiastic about adding limits.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155955/new/

https://reviews.llvm.org/D155955