alexfh added a comment.
Looks mostly good, a few nits.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:82
@@ +81,3 @@
+ ? std::string{R"lit()")lit"}
+ : (")" + Delimiter + R"(")")) != StringRef::npos;
+}
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > This is a wonderful demonstration of why I hate raw string literals on
> > shorter strings. I had to stare at that raw string for quite some time to
> > figure it out. :-(
> I used a raw string literal here because that's what this check would have
> recommended on this code :-).
>
> However, your feedback is useful. If I subsequently enhance this check with
> a parameter that says the minimum length a string literal must have before it
> is to be transformed into a raw string literal and the default value for that
> parameter is something like 5, would that address your concern?
That (a way to configure some thresholds of "sensitivity" of this check, so it
doesn't change `"\""` to `R"(")"` etc.) is roughly what I suggested a while
ago, and we decided that this can be addressed in a follow up.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:98
@@ +97,3 @@
+}
+
+} // namespace
----------------
> Working with the hard-coded list of delimiters is no more or less efficient
> than the general algorithm that I
> implemented, so efficiency is not the concern here.
The other concern is:
>> IMO, we don't need a universal algorithm that will hardly ever go further
>> than the second iteration, and even in
>> this case would produce a result that's likely undesirable (as I said,
>> R"lit0(....)lit0" is not what I would want to
>> see in my code).
However, given the low probability of this scenario, I don't want to waste more
of our time on this. The current algorithm is good enough for the first version.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:104
@@ +103,3 @@
+ : ClangTidyCheck(Name, Context),
+ DelimiterStem{Options.get("DelimiterStem", "lit")} {}
+
----------------
AFAIU, braced initializers in constructor initializer lists are not supported
on MSVC 2013.
================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:104
@@ +103,3 @@
+ : ClangTidyCheck(Name, Context),
+ DelimiterStem{Options.get("DelimiterStem", "lit")} {}
+
----------------
alexfh wrote:
> AFAIU, braced initializers in constructor initializer lists are not supported
> on MSVC 2013.
> Then the documentation needs to be updated. (Actually I couldn't find any
> documentation on these flags and how they relate to each other because they
> use preprocessor hell to generate bitfields in a structure.)
As with most things in open source, the documentation is missing, since nobody
yet needed it strongly enough to write it.
If you feel it's worth your time, send a patch to whoever maintains Clang
frontend code these days. The documentation should go somewhere around
include/clang/Frontend/LangStandards.def:115 or
include/clang/Basic/LangOptions.def:77.
http://reviews.llvm.org/D16529
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
