[PATCH] D20853: [clang-tidy] misc-macro-parentheses: Don't insert parentheses in variable declarations. Fixes bugzilla 26273

Daniel Marjamäki via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 1 02:54:45 PDT 2016


danielmarjamaki created this revision.
danielmarjamaki added subscribers: cfe-commits, alexfh.
danielmarjamaki set the repository for this revision to rL LLVM.

This is a quick fix for bugzilla 26273. parentheses should not be inserted in variable declarations.

This patch will introduce false negatives when a macro looks like it could be variable declaration but is not.

For example:

    #define ABC   A*B*C

To avoid false positives in such expression, I am guessing that non-keywords such as "A", "B" and "C" are types or qualifiers so "A*B*C" is some variable declaration. My primary focus was to avoid FP.

Maybe there is a method that I am not aware of, to see if there is a type "A" during preprocessing.. that would be great.

The "possibleVarDecl" could be much more clever. For instance, variable declarations can at least contain :: < & also, I could handle those better also but that would mean more false negatives.


Repository:
  rL LLVM

http://reviews.llvm.org/D20853

Files:
  clang-tidy/misc/MacroParenthesesCheck.cpp
  test/clang-tidy/misc-macro-parentheses.cpp

Index: test/clang-tidy/misc-macro-parentheses.cpp
===================================================================
--- test/clang-tidy/misc-macro-parentheses.cpp
+++ test/clang-tidy/misc-macro-parentheses.cpp
@@ -8,6 +8,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: macro argument should be enclosed in parentheses [misc-macro-parentheses]
 #define BAD4(x)           ((unsigned char)(x & 0xff))
 // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: macro argument should be enclosed in parentheses [misc-macro-parentheses]
+#define BAD5(X)           A*B=(C*)X+2
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: macro argument should be enclosed in parentheses [misc-macro-parentheses]
 
 #define GOOD1             1
 #define GOOD2             (1+2)
@@ -39,6 +41,7 @@
 #define GOOD28(x)         namespace x {int b;}
 #define GOOD29(...)       std::cout << __VA_ARGS__;
 #define GOOD30(args...)   std::cout << args;
+#define GOOD31(X)         A*X=2
 
 // These are allowed for now..
 #define MAYBE1            *12.34
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===================================================================
--- clang-tidy/misc/MacroParenthesesCheck.cpp
+++ clang-tidy/misc/MacroParenthesesCheck.cpp
@@ -19,8 +19,7 @@
 namespace {
 class MacroParenthesesPPCallbacks : public PPCallbacks {
 public:
-  MacroParenthesesPPCallbacks(Preprocessor *PP,
-                              MacroParenthesesCheck *Check)
+  MacroParenthesesPPCallbacks(Preprocessor *PP, MacroParenthesesCheck *Check)
       : PP(PP), Check(Check) {}
 
   void MacroDefined(const Token &MacroNameTok,
@@ -67,8 +66,31 @@
                    tok::amp, tok::pipe, tok::caret);
 }
 
+static bool possibleVarDecl(const MacroInfo *MI) {
+  if (MI->tokens_empty())
+    return false;
+
+  const Token *Tok = MI->tokens_begin();
+
+  // Skip possible unknown types
+  while (Tok != MI->tokens_end() && !isKeyword(*Tok) &&
+         Tok->isOneOf(tok::identifier, tok::raw_identifier) &&
+         MI->getArgumentNum(Tok->getIdentifierInfo()) < 0) {
+    Tok++;
+  }
+
+  // Return true for 'UNKNOWN_ID*X', 'UNKNOWN_ID X' etc.
+  return Tok != MI->tokens_begin() && Tok != MI->tokens_end() && !isKeyword(*Tok) &&
+         Tok->isOneOf(tok::star, tok::identifier, tok::raw_identifier);
+}
+
 void MacroParenthesesPPCallbacks::replacementList(const Token &MacroNameTok,
                                                   const MacroInfo *MI) {
+  // Make sure macro replacement isn't a variable declaration, that begins
+  // with unknown identifier(s).
+  if (possibleVarDecl(MI))
+    return;
+
   // Count how deep we are in parentheses/braces/squares.
   int Count = 0;
 
@@ -117,6 +139,7 @@
 
 void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok,
                                            const MacroInfo *MI) {
+  bool VarDecl = possibleVarDecl(MI);
 
   for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
     // First token.
@@ -132,6 +155,13 @@
 
     const Token &Tok = *TI;
 
+    // Don't warn in possible variable declaration.
+    if (VarDecl) {
+      if (Tok.is(tok::equal))
+        VarDecl = false;
+      continue;
+    }
+
     // Only interested in identifiers.
     if (!Tok.isOneOf(tok::identifier, tok::raw_identifier))
       continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D20853.59180.patch
Type: text/x-patch
Size: 3284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160601/4e550cf9/attachment-0001.bin>


More information about the cfe-commits mailing list