teemperor added a comment.
Everything beside this last test case seems to be handled. And from what I
remember there was a longer discussion how to properly handle this case and so
this review got stuck.
Can we add this last test case with a FIXME and then get this merged?
https://reviews.llv
yamaguchi added inline comments.
Comment at: lib/Sema/SemaInit.cpp:875
if (!VerifyOnly) {
StructuredSubobjectInitList->setType(T);
ruiu wrote:
> Is it intentional that you run the new code only when !VerifyOnly?
I think VerifyOnly is used to check if it
yamaguchi updated this revision to Diff 97398.
yamaguchi marked 2 inline comments as done.
yamaguchi added a comment.
Fixed spaces and changed SpellingLoc from two lines to one line.
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missing-braces.c
Index: test/S
thakis added inline comments.
Comment at: test/Sema/warn-missing-braces.c:21
+
+#endif
This looks shorter than the test I pasted on the bug. Does it get the corner
case right that my patch on that bug didn't get right?
https://reviews.llvm.org/D32646
__
ruiu added inline comments.
Comment at: lib/Sema/SemaInit.cpp:897
+SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))
+ return;
SemaRef.Diag(StructuredSubobjectInitList->getLocStart(),
Please use clang-format to format your cod
ruiu added inline comments.
Comment at: lib/Sema/SemaInit.cpp:875
if (!VerifyOnly) {
StructuredSubobjectInitList->setType(T);
Is it intentional that you run the new code only when !VerifyOnly?
Comment at: lib/Sema/SemaInit.cpp:890-891
yamaguchi updated this revision to Diff 97228.
yamaguchi added a comment.
Add check if there is -Wmissing-braces enabled or not.
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missing-braces.c
Index: test/Sema/warn-missing-braces.c
v.g.vassilev added a comment.
Did you incorporate the comment from here:
https://bugs.llvm.org/show_bug.cgi?id=24007#c2
https://reviews.llvm.org/D32646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
yamaguchi updated this revision to Diff 97227.
yamaguchi marked 5 inline comments as done.
yamaguchi added a comment.
Changed to early return.
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missing-braces.c
Index: test/Sema/warn-missing-braces.c
==
v.g.vassilev added a comment.
I don't think we should worry about that. Please also see:
http://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
https://reviews.llvm.org/D32646
___
cfe-commits mailing list
cfe-commit
yamaguchi updated this revision to Diff 97224.
yamaguchi added a comment.
@v.g.vassilev
I don't think early return is good idea, because someone might want to add some
code under this function in the future.
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missi
yamaguchi added inline comments.
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))) {
ya
yamaguchi added inline comments.
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))) {
v.
v.g.vassilev added inline comments.
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))) {
yamaguchi marked 2 inline comments as done.
yamaguchi added inline comments.
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+SemaRef.getSourceManager().isInSyste
v.g.vassilev added a comment.
LGTM, modulo the comment.
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+SemaRef.getSourceManager().isInSystemHeader(SpellingLoc
yamaguchi updated this revision to Diff 97182.
yamaguchi added a comment.
Update testcase. Made it minimal.
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missing-braces.c
Index: test/Sema/warn-missing-braces.c
=
ruiu added inline comments.
Comment at: test/Sema/warn-missing-braces.c:6-11
+typedef struct _GUID {
+ unsigned Data1;
+ unsigned short Data2;
+ unsigned short Data3;
+ unsigned char Data4[8];
+} GUID;
You can simplify this, can't you? It seems something li
yamaguchi created this revision.
This is an update patch for bug [1].
-Wmissing-braces should not fire for system headers.
[1] https://bugs.llvm.org/show_bug.cgi?id=24007
https://reviews.llvm.org/D32646
Files:
lib/Sema/SemaInit.cpp
test/Sema/warn-missing-braces.c
Index: test/Sema/warn-m
19 matches
Mail list logo
