[Clang-tidy] Fix for crash in modernize-raw-string-literal check

[edyp87]

Summary

Clang-tidy modernize-raw-string-literal check crashes on run-time assert while it is evaluating compiler predefined identifiers such as

• FUNCTION
• func
• PRETTY_FUNCTION

Check is asserting because it cannot find opening quote for such string literal. It occurs only on debug build config.
I think that it would be good to prune such cases by crossing off predefined expressions - there is no need to evaluate such matches.

[alexfh]

1. Please generate diffs with full context when sending patches. Use any of the methods described in http://llvm.org/docs/Phabricator.html.

clang-tidy/modernize/RawStringLiteralCheck.cpp

111

No need in this variable.

[alexfh]

(hit Submit early...)

2. How does AST look for these test cases? I wonder whether there are any similar cases not covered by PredefinedExpr.

And thank you for the patch!

[edyp87]

Extended diff range + removed unnecessary variable.

[edyp87]

1. Extended diff has been generated - sorry, I am new to Phabricator.
2. AST for this case looks like this:

AST for crashing case:
-VarDecl 0x2b27370 <line:96:1, col:28> col:19 function 'const char *const' callinit

`-ImplicitCastExpr 0x2b274c0 <col:28> 'const char *' <ArrayToPointerDecay>
  `-PredefinedExpr 0x2b27470 <col:28> 'const char [1]' lvalue __FUNCTION__
    `-StringLiteral 0x2b27448 <col:28> 'const char [1]' lvalue ""

Valid case:
-VarDecl 0x2b26660 <line:90:1, col:32> col:19 HexPrintable 'const char *const' callinit

`-ImplicitCastExpr 0x2b26718 <col:32> 'const char *' <ArrayToPointerDecay>
  `-StringLiteral 0x2b266b8 <col:32> 'const char [3]' lvalue "@\\"

For StringLiteral whose parent is PredefinedExpr Lexer::getSourceText returns this expr literally (__FUNCTION__) instead of evaluated function name.
I was wondering whether there is another case which results in such assert (lack of quote in string) but I could not came with an idea of such scenario.
Another approach would be just returning from check() method while evaluating "quote-less" string but I thought that it would be less elegant.

[alexfh]

Thank you for the explanation! The change looks good now. Do you need me to submit the patch for you?

As for other cases that can lead to this, it might be possible to achieve the same effect using macros. However, we could address this in a separate patch.

[edyp87]

Yes, please apply this patch for me.

As for macro cases - I also thought about this but check's author has taken care of macros in check() method :

void RawStringLiteralCheck::check(const MatchFinder::MatchResult &Result) {
   [...]
   if (Literal->getLocStart().isMacroID())
     return;
   [...]
}

Thank you for quick review!

clang-tidy/modernize/RawStringLiteralCheck.cpp

 }
 
 void RawStringLiteralCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(stringLiteral().bind("lit"), this);
+  const auto IsNotPredefinedExpr = unless(hasParent(predefinedExpr()));
+
+  Finder->addMatcher(stringLiteral(IsNotPredefinedExpr).bind("lit"), this);
 }
 
 void RawStringLiteralCheck::check(const MatchFinder::MatchResult &Result) {

test/clang-tidy/modernize-raw-string-literal.cpp

 // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: {{.*}} can be written as a raw string literal
 // CHECK-FIXES: {{^}}char const *const HexPrintable(R"(@\)");{{$}}
 
+char const *const prettyFunction(__PRETTY_FUNCTION__);
+char const *const function(__FUNCTION__);
+char const *const func(__func__);
+
 #define TRICK(arg_) #arg_
 char const *const MacroBody = TRICK(foo\\bar);