[PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

Richard via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 26 18:13:50 PST 2016


LegalizeAdulthood added inline comments.

================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:35
@@ +34,3 @@
+  const bool HasBackSlash = Text.find(R"(\\)") != StringRef::npos;
+  const bool HasNewLine = Text.find(R"(\n)") != StringRef::npos;
+  const bool HasQuote = Text.find(R"(\')") != StringRef::npos;
----------------
aaron.ballman wrote:
> Newlines are hard. Should this also handle \r (for CRLF code like \r\n)?
The standard [2.14.5 lex.string, clauses 4 and 5] explicitly says that a source file new-line in a raw string literal results in a new-line in the resulting execution string-literal.

The examples shown in the standard are consistent with what I've written and VS interprets CRLF in a raw string literal in the source file as `\n` in the string literal.

`\r` is not handled similarly and the presence of `\r` in the string literal prevents this check from generating any fix-it.

================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:66
@@ +65,3 @@
+void RawStringLiteralCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(stringLiteral().bind("lit"), this);
+}
----------------
aaron.ballman wrote:
> Please limit registration of the checker to just C++11 or later.
Good catch.  I meant to do this, but overlooked it.

================
Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:18
@@ +17,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} can be written as a raw string literal
+// CHECK-FIXES: {{^}}char const *const NewLine{R"(goink{{$}}
+// CHECK-FIXES-NEXT: {{^}}frob)"};{{$}}
----------------
aaron.ballman wrote:
> Is there a way that this fix-it can mangle the source file in scary ways, though? For instance, say I have a source file saved with CRLF and a string literal of "foo\nbar" -- this will either make the string literal contain a CRLF instead of just LF, or it will make the string literal contain an LF right up until the user's editor saves the file and converts the line ending (I think).
See above.  This doesn't work the way you think it does.

If it did (no newline interpretation), then the feature would be utterly useless for multi-line raw string literals, where simply checking out the source code on Windows with CRLF EOL interpretation would change the meaning of source that was originally committed on Unix with LF EOL interpretation.

================
Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:55
@@ +54,2 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: {{.*}} can be written as a raw string literal
+// CHECK-FIXES: {{^}}char const *const HexPrintable{R"(@\)"};{{$}}
----------------
aaron.ballman wrote:
> Can you also add a test that ensures we don't mangle already-raw string literals (by re-rawifying them)?
Another good catch, I will add something for this.


http://reviews.llvm.org/D16529





More information about the cfe-commits mailing list