[PATCH] Refactor: Simplify boolean expressions in lib/Parse

Richard legalize at xmission.com
Mon Mar 23 14:01:08 PDT 2015


================
Comment at: lib/Parse/ParseDecl.cpp:5198
@@ -5202,1 +5197,3 @@
+                   isDeclarationSpecifier() || // 'int(int)' is a function.
+                   isCXX11AttributeSpecifier());
 
----------------
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.

http://reviews.llvm.org/D8530

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list