hokein added inline comments.
================
Comment at: clang/test/CodeGen/builtins-ppc-error.c:51
void testCTF(int index) {
- vec_ctf(vsi, index); //expected-error {{argument to
'__builtin_altivec_vcfsx' must be a constant integer}}
- vec_ctf(vui, index); //expected-error {{argument to
'__builtin_altivec_vcfsx' must be a constant integer}}
+ vec_ctf(vsi, index); //expected-error {{argument to
'__builtin_altivec_vcfsx' must be a constant integer}} expected-error
{{argument to '__builtin_altivec_vcfux' must be a constant integer}}
+ vec_ctf(vui, index); //expected-error {{argument to
'__builtin_altivec_vcfsx' must be a constant integer}} expected-error
{{argument to '__builtin_altivec_vcfux' must be a constant integer}}
----------------
sammccall wrote:
> 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.
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'
```
> However the existing diagnostic for vui indicates that it's considering the
> signed case, so I guess this is already broken/bad.
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`?
================
Comment at: clang/test/Index/complete-switch.c:9
-// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:4:10 %s |
FileCheck %s -allow-empty
+// RUN: not %clang_cc1 -fsyntax-only -Xclang -fno-recovery-ast
-code-completion-at=%s:4:10 %s | FileCheck %s -allow-empty
// CHECK-NOT: COMPLETION: foo
----------------
sammccall wrote:
> 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)?
yes, the crash was caused by accesing a null-like ast node. With recovery ast,
it is not true any more.
================
Comment at: clang/test/Sema/__try.c:55
+ // expected-error{{too few arguments to function call, expected 1, have
0}} \
+ // expected-error{{expected ';' after expression}}
}
----------------
sammccall wrote:
> this seems bad, am I missing something?
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.
================
Comment at: clang/test/Sema/typo-correction.c:56
+ f(THIS_IS_AN_ERROR, // expected-error {{use of undeclared identifier
'THIS_IS_AN_ERROR'}}
+ afunction(afunction_)); // expected-error {{use of undeclared identifier
'afunction_'}}
}
----------------
sammccall wrote:
> what's up with this change?
> Do we see the LHS is dependent/contains-errors and then give up on correcting
> typos in the RHS?
> Should we?
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89046/new/
https://reviews.llvm.org/D89046
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
