[PATCH] D32646: Fix a bug that -Wmissing-braces fires on system headers
Yuka Takahashi via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 30 04:38:51 PDT 2017
yamaguchi added inline comments.
================
Comment at: lib/Sema/SemaInit.cpp:892
+ SpellingLoc = SemaRef.getSourceManager().getSpellingLoc(SpellingLoc);
+ if (!(SpellingLoc.isValid() &&
+ SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))) {
----------------
yamaguchi wrote:
> v.g.vassilev wrote:
> > yamaguchi wrote:
> > > v.g.vassilev wrote:
> > > > I'd avoid double negations. Could you use `isInvalid` instead of `!isValid`. That would make the condition more readable.
> > > It's not (!SpellingLoc.isValid()) but is !((SpellingLoc.isValid() && SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))
> > >
> > I'd rewrite this using early returns:
> > ```
> > if (SpellingLoc.isInvalid() || SemaRef.getSourceManager().isInSystemHeader(SpellingLoc))
> > return;
> > ...
> > ```
> Do you think I should change this to (SpellingLoc.isInvalid() || !(SemaRef.getSourceManager().isInSystemHeader(SpellingLoc)) ??
Oh, I see.
Sorry I didn't see your reply because I was typing mine.
https://reviews.llvm.org/D32646
More information about the cfe-commits
mailing list