This revision was automatically updated to reflect the committed changes.
Closed by commit rC345558: [analyzer] Allow padding checker to traverse simple
class hierarchies (authored by alexshap, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Ch
tekknolagi updated this revision to Diff 171568.
tekknolagi added a comment.
Skip non-definitions in `VisitRecord`
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analysis/padding_inherit.cpp
Index: test/Analysis/padding_in
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81
+// We need to be looking at a definition, not just any pointer to the
+// declaration.
+if (!(RD = RD->getDefinition()))
+ return;
tekknolagi wrote:
> te
tekknolagi added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81
+// We need to be looking at a definition, not just any pointer to the
+// declaration.
+if (!(RD = RD->getDefinition()))
+ return;
tekknolagi wrot
tekknolagi added a comment.
@NoQ I think @alexshap will be committing this. Thanks
Repository:
rC Clang
https://reviews.llvm.org/D53206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
NoQ accepted this revision.
NoQ added a comment.
Thanks! Also do you have commit access or do you need someone to commit this
for you?
Repository:
rC Clang
https://reviews.llvm.org/D53206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
tekknolagi updated this revision to Diff 171234.
tekknolagi added a comment.
Add comments suggested by @NoQ and @bcraig
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analysis/padding_inherit.cpp
Index: test/Analysis/paddi
tekknolagi added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81
+// We need to be looking at a definition, not just any pointer to the
+// declaration.
+if (!(RD = RD->getDefinition()))
+ return;
NoQ wrote:
> Th
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Patch looks great, thanks!
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81
+// We need to be looking at a definition, not just any pointer to the
+// decla
george.karpenkov added a comment.
I'll try to take a look this week.
Repository:
rC Clang
https://reviews.llvm.org/D53206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tekknolagi added a comment.
Ping @NoQ @george.karpenkov
Repository:
rC Clang
https://reviews.llvm.org/D53206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bcraig added a comment.
add a comment, and it will be fine in my eyes. You'll need signoff from the
code owner though.
Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+ char
tekknolagi updated this revision to Diff 170475.
tekknolagi added a comment.
Make the test case a little easier to read
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analysis/padding_inherit.cpp
Index: test/Analysis/paddi
tekknolagi added inline comments.
Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+ char c1;
+ int i;
+ char c2;
+};
+
tekknolagi wrote:
> bcraig wrote:
> > Lo
tekknolagi updated this revision to Diff 170464.
tekknolagi edited the summary of this revision.
tekknolagi added a comment.
Remove useless test case in `padding_cpp.cpp`
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analy
tekknolagi added inline comments.
Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+ char c1;
+ int i;
+ char c2;
+};
+
bcraig wrote:
> Looking at this again...
george.karpenkov added a comment.
@bcraig comments look valid, marking as "Needs Changes"
Repository:
rC Clang
https://reviews.llvm.org/D53206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
bcraig added inline comments.
Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+ char c1;
+ int i;
+ char c2;
+};
+
Looking at this again... what new cases does
tekknolagi updated this revision to Diff 169501.
tekknolagi added a comment.
Fix confusing & wrong if-statements, add new test suggested by @bcraig
Repository:
rC Clang
https://reviews.llvm.org/D53206
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analysis/padding_cpp.cpp
t
NoQ added a reviewer: george.karpenkov.
NoQ added a comment.
I'll take care of this review as an [analyzer]-responsible person, maybe not
immediately because i'm a bit overwhelmed right now - sorry! - and George may
want to have a look (also or instead).
Repository:
rC Clang
https://reviews
tekknolagi added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:167-171
}
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+ return true;
+
bcraig wrote:
> I think this should just b
tekknolagi edited reviewers, added: dcoughlin; removed: malcolm.parsons.
tekknolagi added a comment.
Sorry @malcolm.parsons -- I misunderstood code owners. Should not have just
looked at a blame of the file...
Repository:
rC Clang
https://reviews.llvm.org/D53206
__
bcraig added a comment.
You should probably add whoever the current code owner of that static analyzer
to this review. I have very little involvement with Clang these days, so a
"LGTM" from me doesn't carry much weight.
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.
tekknolagi created this revision.
tekknolagi added reviewers: bcraig, NoQ.
tekknolagi added a project: clang.
The existing padding checker skips classes that have any base classes. This
patch allows the checker to traverse very simple cases: classes that have no
fields and have one base class.
24 matches
Mail list logo
