sgilles added a comment.
Sure, if it still applies. I'm just a user and have no write access. Looks
like I patched this locally and then forgot all about it.
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
danielmarjamaki added a comment.
This is not committed as far as I see.. do you have write permission or do you
want that I commit it?
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
danielmarjamaki accepted this revision.
danielmarjamaki added a reviewer: danielmarjamaki.
danielmarjamaki added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
sgilles marked an inline comment as done.
sgilles added a comment.
Ping - if there are no comments, could this be accepted?
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
sgilles marked an inline comment as done.
sgilles added a comment.
Thanks to danielmarjamaki and rsmith for comments, which I think this diff
addresses. I have not done an extensive search of the codebase for places
where `isSyntacticForm()` would be useful, but there don't seem to be any
sgilles updated this revision to Diff 82849.
sgilles marked 2 inline comments as done.
sgilles added a comment.
Address danielmarjamaki's and rsmith's comments (creating
`InitListExpr::isSyntactic()` since it didn't already exist), as well as
correct syntax of test so that it actually runs.
rsmith added a comment.
Looks good to go once Daniel's and my comments are addressed. Thank you.
Comment at: lib/AST/Expr.cpp:1887
+bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions )
const {
+ assert(!getSyntacticForm() && "only test syntactic form as zero
danielmarjamaki added a comment.
Thanks for working on these. imo these are false positives.
Comment at: lib/AST/Expr.cpp:1893
+
+ const IntegerLiteral *lit = dyn_cast(getInit(0));
+ if (!lit) {
I would recommend capital first letter for this variable
sgilles updated this revision to Diff 82720.
sgilles added a comment.
Address rsmith's comments, in particular: factor out testing zero initializers
to a method of `InitListExpr`; use `ParentIList` instead of
`StructuredSubobjectInitList`.
The warning is (still) not relaxed for C++ code. I
sgilles added a comment.
Thank you for the comments, rsmith. I'm addressing them now, and I'll make
sure to add your examples to the test case. I don't think `isSyntactic()`
exists, so I'm using `!getSyntactic()` instead, which should have the desired
effect.
In
rjmccall added a comment.
In https://reviews.llvm.org/D28148#632541, @Quuxplusone wrote:
> Re the language option: In https://reviews.llvm.org/D27163, @rsmith wrote:
>
> > Perhaps some catch-all "I want defined behavior for things that C defines
> > even though I'm compiling in C++" flag would
Quuxplusone added subscribers: rjmccall, Quuxplusone.
Quuxplusone added a comment.
Re the language option: In https://reviews.llvm.org/D27163, @rsmith wrote:
> Perhaps some catch-all "I want defined behavior for things that C defines
> even though I'm compiling in C++" flag would make sense
fhahn added a comment.
Ops, it looks like I accidentally switched the diff back to the initial version
and I have no idea how to switch it back :(
I am very sorry about that, would you mind uploading the latest version again?
https://reviews.llvm.org/D28148
fhahn updated this revision to Diff 82626.
https://reviews.llvm.org/D28148
Files:
include/clang/Basic/LangOptions.def
include/clang/Frontend/LangStandard.h
include/clang/Frontend/LangStandards.def
lib/Frontend/CompilerInvocation.cpp
lib/Sema/SemaInit.cpp
test/Sema/zero-initializer.c
rsmith added inline comments.
Comment at: lib/Sema/SemaInit.cpp:884
+bool MissingBracesOkay = false;
+
+if (!SemaRef.getLangOpts().CPlusPlus &&
Remove empty line.
Comment at: lib/Sema/SemaInit.cpp:885-892
+if
sgilles updated the summary for this revision.
sgilles updated this revision to Diff 82638.
sgilles added a comment.
Instead of adding a language option to distinguish C, negatively check
against C++.
https://reviews.llvm.org/D28148
Files:
lib/Sema/SemaInit.cpp
test/Sema/zero-initializer.c
sgilles added a comment.
In https://reviews.llvm.org/D28148#632002, @rsmith wrote:
> Why are you adding a language option for this? Just use `!LangOpts.CPlusPlus`.
I didn't want to have this change accidentally apply to other, non-C++
languages, since I'm not sure which languages would go
rsmith added a comment.
Why are you adding a language option for this? Just use `!LangOpts.CPlusPlus`.
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
sgilles created this revision.
sgilles added reviewers: cfe-commits, rsmith, zaks.anna.
Add ZeroInitializer as a language option, attached to all standards of
C. Relax checks for -Wmissing-field-initializers and -Wmissing-braces so
that, for such languages, assigning to a structure with { 0 }
19 matches
Mail list logo
