[PATCH] D149612: [Sema] avoid merge error type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D149612#4314536 <https://reviews.llvm.org/D149612#4314536>, @shafik wrote:

> In D149612#4314209 <https://reviews.llvm.org/D149612#4314209>, @HerrCai0907 wrote:
>
>> in SemaType.cpp#BuildArrayType, It will generate `DependentSizedArrayType` which cannot be merged. 
>> So we can either add additional check in `MergeVarDeclTypes` or directly ignore to generate this type in `BuildArrayType`.
>> Which one is better?
>>
>>   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
>>     T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
>
> I am not sure where the right place to fix this is, or when it is correct to use `containsErrors()`
>
> Maybe @aaron.ballman or @rsmith might have some advice

so `containsErrors` on an expression just means it has a `RecoveryExpr` in it.  MUCH of the time a `RecoveryExpr` is just created with a Dependent type, thus the Dependent Array type.  I would suggest rather than just returning 'no type' (via the empty qual type), if we find that the Array Size contains errors (I don't get the if CPlusPlus check?), we could either:
1- Make it an `IncompleteArrayType`.
2- Make it a `ConstantArrayType`, with a correctly-typed `RecoveryExpr` (that is, not dependent, still a size_t/etc expression, BUT with no value) for the bounds type.  I would think it is better to do this ~2584 though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612