aaron.ballman added a comment. In http://reviews.llvm.org/D20561#442051, @rogfer01 wrote:
> Only warn if the expression is of the form &a.x this way &(a.x) can be used > to silence the warning. I think this is a reasonable idea, but would still like to make sure we have a low false-positive rate by default. ================ Comment at: lib/Sema/SemaExpr.cpp:10519 @@ +10518,3 @@ + // struct may be a problem if the pointer value is dereferenced. + Expr* rhs = OrigOp.get(); + const auto *ME = dyn_cast<MemberExpr>(rhs); ---------------- The * binds to the identifier instead of the type; you should run the patch through clang-format (formatting issues are also in the test files). ================ Comment at: lib/Sema/SemaExpr.cpp:10527 @@ +10526,3 @@ + if (RD->hasAttr<PackedAttr>() || + ME->getMemberDecl()->hasAttr<PackedAttr>()) { + Diag(OpLoc, diag::warn_taking_address_of_packed_member) ---------------- Ah, I forgot that the attribute also affected the alignment of the struct itself. Amending my false-positive idea, the following should be fine, shouldn't it?: ``` struct __attribute__((packed)) S { char c; int i; char c2; }; void f(S s) { char *cp = &s.c; char *cp2 = &s.c2; } ``` I think perhaps we should not diagnose if the member's type also has a one-byte alignment (or, for instance, uses alignas(1) to achieve that). What do you think? ================ Comment at: test/SemaCXX/address-packed.cpp:1 @@ +1,2 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +extern void f1(int *); ---------------- This test should only really test the parts that are different from the C test (such as inheritance). The goal is to have the maximum test coverage with the minimum amount of testing code. http://reviews.llvm.org/D20561 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits