================
Comment at: lib/Parse/ParseDecl.cpp:5198
@@ -5202,1 +5197,3 @@
+ isDeclarationSpecifier() || // 'int(int)' is a function.
+ isCXX11AttributeSpecifier());
----------------
LegalizeAdulthood wrote:
> majnemer wrote:
> > I think you are missing `// 'int([[]]int)' is a function.` on this line.
> Thanks, I did miss that comment. `clang-tidy` isn't preserving the comments
> on this check (I might enhance this new check further to do that), so I was
> backfilling the comments in by hand and I missed one.
>
> When I looked at this diff, I was thinking that this large, complicated
> boolean expression might be more understandable if it were broken down with
> some intention-revealing local variables; something like:
>
> ```
> } else {
> const bool FunctionReturnsInt = Tok.is(tok::r_paren);
> const bool VariadicFunctionReturnsInt = getLangOpts().CPlusPlus &&
> Tok.is(tok::ellipsis) && NextToken().is(tok::r_paren);
> isGrouping = !(FunctionReturnsInt || VariadicFunctionReturnsInt ||
> isDeclarationSpecifier() || isCXX11AttributeSpecifier());
> }
> ```
>
> The original author of this code seems to understand the complexity here,
> hence the sprinkling of comments to explain the logic. My own style is to
> prefer intention revealing names instead of comments, but my preferences are
> not the issue here. I recognized that the comments were important here, so I
> tried to preserve them in my diff. I'm not sure how the full-line comments
> should be reworded to express the same intent, so I left them as is. I'm
> happy to adjust them if someone can suggest an alternative text.
A more readable structure would be:
// 'int()' is a function.
} else if (Tok.is(tok::r_paren)) {
isGrouping = false;
// C++ 'int(...)' is a function.
} else if (getLangOpts().CPlusPlus && Tok.is(tok::ellipsis) &&
NextToken().is(tok::r_paren)) {
isGrouping = false;
// 'int(int)' is a function.
} else if (isDeclarationSpecifier()) {
isGrouping = false;
// C++11 'int([[...]])' is a function.
} else if (isCXX11AttributeSpecifier()) {
isGrouping = false;
// Otherwise, it's a grouping parenthesis.
} else {
isGrouping = true;
}
Note that putting the condition in an assignment to isGrouping is still not
helpful.
http://reviews.llvm.org/D8530
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
