alexfh added inline comments.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:80
@@ +79,3 @@
+ return false;
+
+ const size_t NewLinePos = Text.find(R"(\n)");
----------------
aaron.ballman wrote:
> This is why I would still prefer to block on fixing StringLiteral. This is
> functional, but really kind of nasty -- in the common case (not already a raw
> string literal), this does two linear searches through the entire string
> literal.
Richard, you're right, it's `u8R"(...)"`, not `Ru8"(...)"` or something.
Nevertheless, doing a full scan for `R` (the search for `"` wouldn't result in
a full scan) in this condition shouldn't be necessary, it could first find a
`"` and then check if the previous character is `R`. It would be a bit more
code, but less needless work at runtime.
Alternatively, D16012 might be a better answer, if it ever makes it into Clang.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:88
@@ +87,3 @@
+}
+
+bool containsDelimiter(StringRef Bytes, const std::string &Delimiter) {
----------------
aaron.ballman wrote:
> I think Alex's point is: why not R"('\"?x01)" (removing the need for lit)?
Exactly, I was only talking about `lit`, not about using the raw string literal.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:96
@@ +95,3 @@
+ std::string Delimiter;
+ for (int Counter = 0; containsDelimiter(Bytes, Delimiter); ++Counter) {
+ Delimiter = (Counter == 0) ? "lit" : "lit" + std::to_string(Counter);
----------------
Please address my comment above that relates to this code. Specifically, I find
this generic algorithm unnecessarily inefficient and too rigid, i.e. one can't
configure a custom set of delimiters that don't follow the <prefix><number>
pattern. I suggest using a list of pre-concatenated pairs of strings (like
`R"lit(` and `)lit"`).
http://reviews.llvm.org/D16529
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
