aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.
Thank you for working on this diagnostic! A few questions and comments below.
================
Comment at: lib/Sema/SemaExpr.cpp:10518
@@ +10517,3 @@
+ // Taking the address of a data member/field of a packed
+ // struct may be a problem if the pointer value is dereferenced
+ MemberExpr *ME = dyn_cast<MemberExpr>(op);
----------------
Missing a full stop at the end of the sentence.
================
Comment at: lib/Sema/SemaExpr.cpp:10519
@@ +10518,3 @@
+ // struct may be a problem if the pointer value is dereferenced
+ MemberExpr *ME = dyn_cast<MemberExpr>(op);
+ while (ME && isa<FieldDecl>(ME->getMemberDecl())) {
----------------
Can use `const auto *` here instead of repeating the type name.
================
Comment at: lib/Sema/SemaExpr.cpp:10527
@@ +10526,3 @@
+ ME->getMemberDecl()->hasAttr<PackedAttr>()) {
+ Diag(OpLoc, diag::warn_taking_address_of_packed_member)
+ << ME->getMemberDecl() << RD;
----------------
I wonder if we should diagnose only when the resulting pointer is actually
misaligned. For instance, the following code should be just fine:
```
struct __attribute__((packed)) S {
int i;
};
void f(S s) {
int *ip = &s.i; // Why should this warn?
}
```
Have you run this patch over any large code bases to see how high the
false-positive rate is? How do you envision users silencing this diagnostic if
they have a false-positive? Something like `&(s.x)` (since I you're not
ignoring paren imp casts)?
================
Comment at: test/Sema/address-packed.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -xc++ -fsyntax-only -verify %s
----------------
You should run clang-format over this file (or indeed, the entire patch).
================
Comment at: test/Sema/address-packed.c:2
@@ +1,3 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -xc++ -fsyntax-only -verify %s
+extern void f1(int *);
----------------
I would split this test case into two, one in Sema for C and one in SemaCXX for
C++.
http://reviews.llvm.org/D20561
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
