ZarkoCA marked 4 inline comments as done.
ZarkoCA added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:5220-5221
+ Context.toCharUnitsFromBits(AA->getAlignment(Context));
+ if (Alignment.getQuantity() >= 16)
+ Diag(Loc, diag::warn_not_xl_compatible) << FD;
+ }
----------------
aaron.ballman wrote:
> I think it'd probably be helpful to tell the user which alignment was
> calculated (it may not be obvious from the context because the alignment
> could be hidden behind a macro or something).
I tried to address in slightly modifying the warning message to emit the
offending alignment and also adding a note for the declaration as you suggested
elsewhere.
================
Comment at: clang/test/Sema/aix-attr-align.c:34-35
+ baz(s.a); // no-warning
+ baz(s.b); // expected-warning {{requesting an alignment of 16 bytes or
greater for struct members is not binary compatible with IBM XL C/C++ for AIX
16.1.0 and older}}
+ baz(s.c); // expected-warning {{requesting an alignment of 16 bytes or
greater for struct members is not binary compatible with IBM XL C/C++ for AIX
16.1.0 and older}}
+
----------------
aaron.ballman wrote:
> This diagnostic is a bit odd to me. It says there's a request for alignment,
> but there's no such request on this line. So it's not clear how the user is
> supposed to react to the diagnostic. While the current code makes it somewhat
> obvious because there's only one field in the expression, imagine code like
> `quux(s.a, s.b);` where it's less clear as to which field causes the
> diagnostic from looking at the call site.
>
> Personally, I found the old diagnostics to be more clear as to what the issue
> is. I think we should put the warning on the declaration involving the
> alignment request, and we should add a note to the call site where the
> diagnostic is generated from (or vice versa). WDYT?
That's a good point actually, there's nothing on the line that would be obvious
to a user.
I opted to warn at the use of struct member and to make a note where it was
declared. This will hopefully help with determining which struct member is
causing this warning instead of searching the code for its cause. I have a
slight preference for doing it this way instead of the other way but can change
it if preferred.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118350/new/
https://reviews.llvm.org/D118350
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
