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



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

Reply via email to