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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 1 08:34:16 PST 2016


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





More information about the cfe-commits mailing list