Re: [PATCH] D19331: [Clang-tidy] Fix for crash in modernize-raw-string-literal check

[Marek via cfe-commits]

edyp87 removed rL LLVM as the repository for this revision.
edyp87 updated this revision to Diff 54401.
edyp87 added a comment.

Extended diff range + removed unnecessary variable.


http://reviews.llvm.org/D19331

Files:
  clang-tidy/modernize/RawStringLiteralCheck.cpp
  test/clang-tidy/modernize-raw-string-literal.cpp

Index: test/clang-tidy/modernize-raw-string-literal.cpp
===
--- test/clang-tidy/modernize-raw-string-literal.cpp
+++ test/clang-tidy/modernize-raw-string-literal.cpp
@@ -91,6 +91,10 @@
 // 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);
 
Index: clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -108,7 +108,10 @@
 }
 
 void RawStringLiteralCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(stringLiteral().bind("lit"), this);
+
+  Finder->addMatcher(
+  stringLiteral(unless(hasParent(predefinedExpr(.bind("lit"),
+  this);
 }
 
 void RawStringLiteralCheck::check(const MatchFinder::MatchResult &Result) {


Index: test/clang-tidy/modernize-raw-string-literal.cpp
===
--- test/clang-tidy/modernize-raw-string-literal.cpp
+++ test/clang-tidy/modernize-raw-string-literal.cpp
@@ -91,6 +91,10 @@
 // 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);
 
Index: clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -108,7 +108,10 @@
 }
 
 void RawStringLiteralCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(stringLiteral().bind("lit"), this);
+
+  Finder->addMatcher(
+  stringLiteral(unless(hasParent(predefinedExpr(.bind("lit"),
+  this);
 }
 
 void RawStringLiteralCheck::check(const MatchFinder::MatchResult &Result) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19331: [Clang-tidy] Fix for crash in modernize-raw-string-literal check

[Marek via cfe-commits]

edyp87 marked an inline comment as done.
edyp87 added a comment.

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  col:19 function 'const char *const' 
> callinit

>`-ImplicitCastExpr 0x2b274c0  'const char *' 

>  `-PredefinedExpr 0x2b27470  'const char [1]' lvalue __FUNCTION__

>`-StringLiteral 0x2b27448  'const char [1]' lvalue ""

> 

> Valid case:

> 

>   -VarDecl 0x2b26660  col:19 HexPrintable 'const char 
> *const' callinit

>`-ImplicitCastExpr 0x2b26718  'const char *' 

>  `-StringLiteral 0x2b266b8  'const char [3]' lvalue "@\\"


For `StringExpr` 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.


http://reviews.llvm.org/D19331



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19331: [Clang-tidy] Fix for crash in modernize-raw-string-literal check

[Marek via cfe-commits]

edyp87 added a comment.

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!


http://reviews.llvm.org/D19331



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits