zixuan-wu added inline comments.

================
Comment at: clang/test/Sema/builtin-alloca-with-align.c:32
 void test8(void) {
+#if defined(__csky__)
   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), 
__alignof__(__INT64_TYPE__)); // expected-warning {{second argument to 
__builtin_alloca_with_align is supposed to be in bits}}
----------------
aaron.ballman wrote:
> rengolin wrote:
> > zixuan-wu wrote:
> > > rengolin wrote:
> > > > This test is platform agnostic, perhaps the extra error could be in a 
> > > > new test, exclusively for csky?
> > > Then I prefer to split the file and add UNSUPPORTED for CSKY in the other 
> > > file which only contains test8
> > But then you wouldn't be testing the extra error you want... hmm.
> > 
> > Maybe it would be fine the way you did it originally.
> > 
> > Would be nice to get a clang person to give their opinion.
>   // Comment which explains why this is special.
> I think I would test this slightly differently:
> ```
> void test8(void) {
>   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), 
> __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to 
> __builtin_alloca_with_align is supposed to be in bits}}
> #ifdef __csky__
>   // expected-error@-2 {{requested alignment must be 8 or greater}}
>   // Comment which explains why this is special.
> #endif // __csky__
> }
> ```
Reusing the first warning is good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124977

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to