aaron.ballman added a comment.
This is getting really close, I mostly only have nits left.
================
Comment at: lib/Sema/SemaExpr.cpp:10529
@@ +10528,3 @@
+ bool ByteAligned = Context.getTypeAlignInChars(MD->getType()).isOne();
+ if (ByteAligned) // packed has had not any effect on it
+ break;
----------------
Comments should be complete sentences including capitalization and punctuation,
so perhaps instead: `// Packed does not have any effect.`
================
Comment at: test/Sema/address-packed.c:21
@@ +20,3 @@
+
+extern void f3(void());
+
----------------
This is unused and can be removed.
================
Comment at: test/Sema/address-packed.c:27
@@ +26,3 @@
+
+void g0() {
+ {
----------------
Function should take `(void)` since this is C.
================
Comment at: test/Sema/address-packed.c:47
@@ +46,3 @@
+ f2(&arguable.c0); // no-warning
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or
structure }}
+ f2(&arguable.c1); // no-warning
----------------
Extra space before `}}`.
================
Comment at: test/Sema/address-packed.c:51
@@ +50,3 @@
+ {
+ struct Arguable *arguable = get_arguable();
+ f2(&(arguable->c0)); // no-warning
----------------
Should put in a comment explaining that this produces no diagnostic because of
the parens.
================
Comment at: test/SemaCXX/address-packed.cpp:13
@@ +12,3 @@
+
+typedef struct Arguable ArguableT;
+
----------------
This is unused and can be removed.
================
Comment at: test/SemaCXX/address-packed.cpp:77
@@ +76,2 @@
+ }
+};
----------------
Can we get one further test?
```
template <typename Ty>
class __attribute__((packed)) S {
Ty X;
public:
Ty *get() const {
return &X; // no warning?
}
};
```
I am fine if this does not warn. I am also fine if it only warns when there are
instantiations of S for which Ty should warn (e.g., do an explicit
instantiation with `char` that does not warn and another one with `int` that
does).
http://reviews.llvm.org/D20561
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
