aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:5212-5215
+ const Expr *AArg = Arg->IgnoreParens();
+ if (const auto *ICE = dyn_cast<ImplicitCastExpr>(AArg)) {
+ AArg = ICE->getSubExpr();
+ if (const auto *ME = dyn_cast<MemberExpr>(AArg)) {
----------------
I don't see a need for `AArg`, so I'd sink it into the uses instead.
================
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;
+ }
----------------
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).
================
Comment at: clang/lib/Sema/SemaChecking.cpp:5341-5342
+ if (Context.getTargetInfo().getTriple().isOSAIX() && Arg &&
+ (FDecl->hasLinkage()) &&
+ !(FDecl->getFormalLinkage() == InternalLinkage))
+ checkAIXMemberAlignment((Arg->getExprLoc()), FDecl,
----------------
================
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}}
+
----------------
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?
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
