The dependency mechanism for C has been implemented, and we have rolled out this to all internal users, didn't see crashy issues, we consider it is stable enough.
hmm, this doesn't exactly look right to me - we know the type of vsi so we should only be considering the signed case I thought.
However the existing diagnostic for vui indicates that it's considering the signed case, so I guess this is already broken/bad.
|424 ↗||(On Diff #296968)|
you may want to make these assertions fuzzier and possibly give a count rather than repeating them, but I guess the owners of these headers would be best to decide
nit: no need for xclang, this is cc1 already
I guess this is a crash test (and it also doesn't crash with recovery ast)?
this seems bad, am I missing something?
or just move the semicolon to suppress, pretty sure that's not what's being tested here.
what's up with this change?
yeah, we should know this is the signed case.
after the macro expansion, the code looks like
_Generic((vsi), vector int : (vector float)__builtin_altivec_vcfsx((vector int)(vsi), (index)), vector unsigned int : (vector float)__builtin_altivec_vcfux((vector unsigned int)(vsi), (index)));
it is a GenericSelectionExpr which contains a switch-like structure (see the AST below), so when we traverse it, we will traverse both __builtin_altivec_vcfsx and __builtin_altivec_vcfux.
`-GenericSelectionExpr 0x9527238 <line:31:3, line:33:88> '__vector float' contains-errors |-ImplicitCastExpr 0x9527220 <line:31:12, col:16> '__vector int' <LValueToRValue> | `-ParenExpr 0x9526980 <col:12, col:16> '__vector int' lvalue | `-DeclRefExpr 0x9526960 <col:13> '__vector int' lvalue Var 0x9526568 'vsi' '__vector int' non_odr_use_unevaluated |-VectorType 0x91923c0 '__vector int' altivec 4 | `-BuiltinType 0x91296e0 'int' |-case '__vector int' selected | |-VectorType 0x91923c0 '__vector int' altivec 4 | | `-BuiltinType 0x91296e0 'int' | `-CStyleCastExpr 0x9526db8 <line:32:14, col:78> '__vector float' contains-errors <Dependent> | `-RecoveryExpr 0x9526d70 <col:28, col:78> '<dependent type>' contains-errors lvalue | |-DeclRefExpr 0x9526bc0 <col:28> '<builtin fn type>' Function 0x95269e8 '__builtin_altivec_vcfsx' '__attribute__((__vector_size__(4 * sizeof(float)))) float (__attribute__((__vector_size__(4 * sizeof(int)))) int, int)' | |-CStyleCastExpr 0x9526c68 <col:52, col:68> '__vector int' <BitCast> | | `-ImplicitCastExpr 0x9526c50 <col:64, col:68> '__vector int' <LValueToRValue> part_of_explicit_cast | | `-ParenExpr 0x9526c30 <col:64, col:68> '__vector int' lvalue | | `-DeclRefExpr 0x9526be0 <col:65> '__vector int' lvalue Var 0x9526568 'vsi' '__vector int' | `-ParenExpr 0x9526cb0 <col:71, col:77> 'const int' lvalue | `-DeclRefExpr 0x9526c90 <col:72> 'const int' lvalue ParmVar 0x95267c8 'index' 'const int' `-case '__vector unsigned int' |-VectorType 0x9192700 '__vector unsigned int' altivec 4 | `-BuiltinType 0x9129780 'unsigned int' `-CStyleCastExpr 0x95271f8 <line:33:14, col:87> '__vector float' contains-errors <Dependent> `-RecoveryExpr 0x95271b0 <col:28, col:87> '<dependent type>' contains-errors lvalue |-DeclRefExpr 0x9527000 <col:28> '<builtin fn type>' Function 0x9526e28 '__builtin_altivec_vcfux' '__attribute__((__vector_size__(4 * sizeof(float)))) float (__attribute__((__vector_size__(4 * sizeof(unsigned int)))) unsigned int, int)' |-CStyleCastExpr 0x95270a8 <col:52, col:77> '__vector unsigned int' <BitCast> | `-ImplicitCastExpr 0x9527090 <col:73, col:77> '__vector int' <LValueToRValue> part_of_explicit_cast | `-ParenExpr 0x9527070 <col:73, col:77> '__vector int' lvalue | `-DeclRefExpr 0x9527020 <col:74> '__vector int' lvalue Var 0x9526568 'vsi' '__vector int' `-ParenExpr 0x95270f0 <col:80, col:86> 'const int' lvalue `-DeclRefExpr 0x95270d0 <col:81> 'const int' lvalue ParmVar 0x95267c8 'index' 'const int'
hmm, vsi and vui have the same type vector signed int, so I think there is a typo for vui, it should be vector unsigned int?
yes, the crash was caused by accesing a null-like ast node. With recovery ast, it is not true any more.
agree, __except makes it very confusing intuitively.
what's happening? - while at here, there is no preceding __try, clang parses __execept as a function call to an implicit function, so it expects a ; after __except (FilterExpression());
why don't we hit this before? -- because the FilterExpression() is invalid (missing an argument), and the whole function call expr is being dropped. With recovery-ast, we preserve the whole function call, then we emit the ; diagnostic.
not quite sure on how to make it clearer. I think this is not super critical.
I'd say this aligns with C++ behavior (the inner typo is not correct in C++ mode neither). The reason why it works here is due to the early typo-correction hack.
OK, that makes sense I guess.
Yep, that definitely looks to be the case.
Yes, this isn't critical and we were just getting a bit lucky before. No need for changes.