[PATCH] D20561: Warn when taking address of packed member

[Roger Ferrer Ibanez via cfe-commits]

rogfer01 marked 3 inline comments as done.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5392
@@ -5388,1 +5391,3 @@
+def note_address_of_packed_member_silence : Note<
+  "place parentheses around the '%0' expression to silence this warning">;
 
----------------
aaron.ballman wrote:
> I don't think this note adds value. Placing parens around the expression does silence the warning for false-positives, but that seems more like a documentation issue than a diagnostic.
Ditto.

================
Comment at: lib/Sema/SemaExpr.cpp:10537-10538
@@ +10536,4 @@
+      Diag(rhs->getLocStart(), diag::note_address_of_packed_member_silence)
+          << rhs << FixItHint::CreateInsertion(rhs->getLocStart(), "(")
+          << FixItHint::CreateInsertion(rhs->getLocEnd(), ")");
+      break;
----------------
aaron.ballman wrote:
> I do not think this diagnostic should have a FixIt hint. This isn't actually *fixing* the problem, it is simply a way to silence the diagnostic while retaining the same behavior.
OK. I will remove it.

================
Comment at: test/SemaCXX/address-packed.cpp:92-93
@@ +91,4 @@
+   // expected-warning at -1 {{packed member 'X' of class or structure 'S<float>'}}
+   // expected-note at -2 {{place parentheses around the 'this->X'}}
+   // expected-note at -3 {{place parentheses around the 'this->X'}}
+  }
----------------
aaron.ballman wrote:
> Why `this->X` in the diagnostic when that's not what the user wrote?
Probably clang introduces it internally when parsing an id-expression that happens to be a nonstatic data member because the diagnostic does include it.

That said this is going away as I'm removing the fix-it.


http://reviews.llvm.org/D20561