[clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

[IdrissRio]

Summary

Hello, i would like to suggest a fix for one of the checks in clang-tidy. The bug was reported in https://bugs.llvm.org/show_bug.cgi?id=28406 where you can find more information.

With the following code:

#define BODY {}
void foo(void) {
  [] () BODY;
}

we have got this crash:

clang-tidy -checks='-*,modernize-redundant-void-arg' t.cc -- -std=c++11
clang-tidy: tools/clang/include/clang/Basic/SourceManager.h:414: const clang::SrcMgr::ExpansionInfo &clang::SrcMgr::SLocEntry::getExpansion() const: Assertion `isExpansion() && "Not a macro expansion SLocEntry!

Hope this can help.
I also have add a new test.

[alexfh]

Thank you for working on this!

clang-tidy/modernize/RedundantVoidArgCheck.cpp

241 ↗

(On Diff #157272)

Let's pull Lambda->getBody()->getLocStart() to a variable to avoid repetition.

242–244 ↗

(On Diff #157272)

The fix looks limited to the specific test case. Let's try other cases (see the comment below).

test/clang-tidy/modernize-redundant-void-arg.cpp

454 ↗

(On Diff #157272)

I'd like to see more tests here with different macro-related cases. These come to mind, but if you come up with more distinct cases, that would be even better:

#define LAMBDA1 [](void){}
(void)LAMBDA1;
#define LAMBDA2 [](void)BODY
(void)LAMBDA2;
#define LAMBDA3(captures, args, body) [captures](args){body}
(void)LAMBDA3(=, void, BODY);
#define LAMBDA4(captures, args, body) captures args body
(void)LAMBDA4([], (void), BODY);
#define LAMBDAS1 \
  (void)LAMBDA1; \
  (void)LAMBDA2; \
  (void)LAMBDA3(=, void, BODY); \
  (void)LAMBDA4([], (void), BODY)

LAMBDAS1;
#define WRAP(...) __VA_ARGS__
WRAP((void)WRAP(WRAP(LAMBDA4(WRAP([]), WRAP((void)), WRAP(BODY)))));
(void)WRAP([](void){});

[IdrissRio]

Thank you @alexfh. You where right. This update is more general. And I also have modified the removeVoidArgumentTokens. This because if we consider the following case:

WRAP((void)WRAP(WRAP(LAMBDA3(WRAP([]), WRAP((void)), WRAP(BODY)))));

We find the correct range, and the DeclText variable inside the function is WRAP((void)).
In the while loop we fine the first occurrence of l_paren and so State = SawLeftParent, and when we find the second occurrence of l_paren this is lost because the case SawLeftParent
doesn't handle a second occurrence of l_paren.

Hope this can help.

[alexfh]

clang-tidy/modernize/RedundantVoidArgCheck.cpp

246–247 ↗

(On Diff #159179)

Will this code also work in the non-macro case?

[IdrissRio]

Yes it works also for the non macro. It is actually more general.
This update have passed all the tests.

[alexfh]

LG with a couple of nits. Do you need someone to commit the patch for you?

clang-tidy/modernize/RedundantVoidArgCheck.cpp

241 ↗

(On Diff #160039)

s/auto /SourceManager */

242 ↗

(On Diff #160039)

Use the actual type name here as well.

[alexfh]

I've fixed the comments and committed the patch myself. Hope that's fine by you.

clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp

case NothingYet:
         State = SawLeftParen;
       }
       break;
     case SawLeftParen:
       if (ProtoToken.is(tok::TokenKind::raw_identifier) &&
           ProtoToken.getRawIdentifier() == "void") {
         State = SawVoid;
         VoidToken = ProtoToken;
+      } else if (ProtoToken.is(tok::TokenKind::l_paren)) {
+        State = SawLeftParen;
       } else {
         State = NothingYet;
       }
       break;
     case SawVoid:
       State = NothingYet;
       if (ProtoToken.is(tok::TokenKind::r_paren)) {
         removeVoidToken(VoidToken, Diagnostic);
removeVoidArgumentTokens(Result, ExplicitCast->getSourceRange(),
                              "cast expression");
   }
 }
 
 void RedundantVoidArgCheck::processLambdaExpr(
     const MatchFinder::MatchResult &Result, const LambdaExpr *Lambda) {
   if (Lambda->getLambdaClass()->getLambdaCallOperator()->getNumParams() == 0 &&
       Lambda->hasExplicitParameters()) {
-    SourceLocation Begin =
-        Lambda->getIntroducerRange().getEnd().getLocWithOffset(1);
-    SourceLocation End = Lambda->getBody()->getBeginLoc().getLocWithOffset(-1);
-    removeVoidArgumentTokens(Result, SourceRange(Begin, End),
+    SourceManager *SM = Result.SourceManager;
+    TypeLoc TL = Lambda->getLambdaClass()->getLambdaTypeInfo()->getTypeLoc();
+    removeVoidArgumentTokens(Result,
+                             {SM->getSpellingLoc(TL.getBeginLoc()),
+                              SM->getSpellingLoc(TL.getEndLoc())},
                              "lambda expression");
   }
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/test/clang-tidy/modernize-redundant-void-arg.cpp

 
 }
 
 struct DefinitionWithNoBody {
   DefinitionWithNoBody(void) = delete;
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in function definition
   // CHECK-FIXES: DefinitionWithNoBody() = delete;
 };
+
+
+
+#define BODY {}
+#define LAMBDA1 [](void){}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA1 [](){}
+
+#define LAMBDA2 [](void)BODY
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA2 []()BODY
+
+#define LAMBDA3(captures, args, body) captures args body
+#define WRAP(...) __VA_ARGS__
+
+#define LAMBDA4 (void)LAMBDA3([],(void),BODY)
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA4 (void)LAMBDA3([],(),BODY)
+
+#define LAMBDA5 []() -> void (*)(void) {return BODY;}
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA5 []() -> void (*)() {return BODY;}
+void lambda_expression_with_macro_test(){
+  (void)LAMBDA1;
+  (void)LAMBDA2;
+  (void)LAMBDA3([], (void), BODY);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: (void)LAMBDA3([], (), BODY);
+
+  LAMBDA4;
+  LAMBDA5;
+  WRAP((void)WRAP(WRAP(LAMBDA3(WRAP([]), WRAP((void)), WRAP(BODY)))));
+  // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: WRAP((void)WRAP(WRAP(LAMBDA3(WRAP([]), WRAP(()), WRAP(BODY)))));
+
+  (void)WRAP([](void) {});
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: (void)WRAP([]() {});
+
+  [](void) BODY;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: []() BODY;
+}