sfertile added inline comments.
Herald added a project: All.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:5246
+
+ if (const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens())) {
+ if (const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr())) {
----------------
Nit: To avoid the deep nesting, can we instead have a series of casts, and if
the resulting pointer is null we return early?
================
Comment at: clang/test/Sema/aix-attr-align.c:10
+ int a[8] __attribute__((aligned(8))); // no-warning
+ int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16
bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX
16.1.0 or older}}
};
----------------
As far as I am aware, the layout of a 16-byte aligned member is exactly the
same between the all the compilers on AIX, and the only incompatibility is when
passing them byval. If that's true then warning on the declaration here is much
too verbose, as most uses will be fine and warning on the callsite (and later
on the function definition) calls attention to it only when there is indeed the
possibility of an incompatability.
================
Comment at: clang/test/Sema/aix-attr-align.c:31
+
+ baz(p1, p2, s.b, p3, b, p5, s); // expected-note {{'b' used with
potentially incompatible alignment here}}
+ jaz(p1, p2, a, p3, s.a, p5, t); // no-note
----------------
I get the following diagnostic from the compiler now:
```
baz(p1, p2, s.b, p3, b, p5, s); // expected-note {{'b' used with
potentially incompatible alignment here}}
^
```
I can't seem to get the formatting exact, but the caret is pointing to 's'
which is the correct argument to warn on, but the note uses the name 'b'.
Before adjusting the warning using the member name made sense, but I think now
that we are warning on the byval argument, we should be using `s`, and also
reword the note to something like
`passing byval argument 's' which is 16 byte aligned may be incompatible with
IBM XL C/C++ for AIX 16.1.0 or older`
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
