LegalizeAdulthood updated this revision to Diff 51738.
LegalizeAdulthood added a comment.
Remove brace initializers from test code.
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/RawStringLiteralC
LegalizeAdulthood updated this revision to Diff 51727.
LegalizeAdulthood added a comment.
Update release notes
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/RawStringLiteralCheck.cpp
clang-tidy
LegalizeAdulthood added a comment.
Looks like I forgot to remove brace initializers from the test files. I will
fix that.
Chris Lattner has given me commit access now, so I can commit on my own.
http://reviews.llvm.org/D16529
___
cfe-commits maili
aaron.ballman added a comment.
When running the tests from this patch locally (Win10, MSVC 2015, debug, x64),
I get:
1>-- Build started: Project: intrinsics_gen, Configuration: Debug x64 --
2>-- Build started: Project: ClangDiagnosticLex, Configuration: Debug x64
--
3>-- Bui
LegalizeAdulthood updated this revision to Diff 51105.
LegalizeAdulthood marked an inline comment as done.
LegalizeAdulthood added a comment.
Update from comments.
Update documentation to reflect changes in the implementation.
I do not have commit access.
Patch by Richard Thomson
http://review
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;
+}
L
LegalizeAdulthood added a comment.
I do not have commit access, so I will need someone to commit this for me.
Patch by Richard Thomson
thanks.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:82
@@ +81,3 @@
+? std::string{R"lit()")lit"}
+
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
In http://reviews.llvm.org/D16529#366433, @LegalizeAdulthood wrote:
> Squeak. This has been waiting on review for over two weeks
Sorry for the delay, I was at WG21 meetings
LegalizeAdulthood added a comment.
Squeak. This has been waiting on review for over two weeks
http://reviews.llvm.org/D16529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
LegalizeAdulthood updated this revision to Diff 47929.
LegalizeAdulthood marked an inline comment as done.
LegalizeAdulthood added a comment.
Add FIXME for macro argument test case.
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyM
LegalizeAdulthood updated this revision to Diff 47928.
LegalizeAdulthood added a comment.
Add FIXME for non-ASCII string literals.
Allow delimiter stem to be configured.
Avoid some string concatenation.
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/mod
LegalizeAdulthood updated this revision to Diff 47927.
LegalizeAdulthood marked 4 inline comments as done.
LegalizeAdulthood added a comment.
Update from review comments.
Avoid some string concatenation when delimiter is empty.
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeL
LegalizeAdulthood added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:97
@@ +96,3 @@
+}
+
+} // namespace
alexfh wrote:
> > I believe we agree on the following: ...
>
> Yes.
>
> > Where we seem to disagree is what algorithm should b
alexfh added a comment.
Sorry for the delay. I'm trying to prioritize reviews that are taking less time
per-iteration. Unfortunately, here we have a bunch of disagreements and I have
to spend significant time to read through your arguments and address your
points.
Comment at:
LegalizeAdulthood added a comment.
Squeak?
http://reviews.llvm.org/D16529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
LegalizeAdulthood added a comment.
I wrote:
> Repeated searching of the string for the literal delimiter could be avoided
> by changing the routine to search for the delimiter. If present, it could
> examine the characters following the literal to make the literal more unique
> and then contin
LegalizeAdulthood updated this revision to Diff 47157.
LegalizeAdulthood marked 2 inline comments as done.
LegalizeAdulthood added a comment.
Update from review comments.
Added a bunch of test cases for non-printing characters, templates and macros.
Removed string literals containing newline (`\
LegalizeAdulthood marked 5 inline comments as done.
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:
LegalizeAdulthood added a comment.
I agree it needs more testing.
I think also my current approach to newlines is overly aggressive and will
result in more raw string literals than people would prefer.
It's really the Windows style path separators and regex ones that are not
controversial tran
alexfh added a comment.
There are still a few comments open. One more important thing is to try running
this check over a large enough project (LLVM + Clang, for example), apply
fixes, look at the results and try to build the fixed code and run all tests.
You can use the clang-tidy/tool/run-cla
LegalizeAdulthood added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:88
@@ +87,3 @@
+}
+
+bool containsDelimiter(StringRef Bytes, const std::string &Delimiter) {
alexfh wrote:
> aaron.ballman wrote:
> > I think Alex's point is: why n
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.
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:80
@@ +79,3 @@
+return false;
+
+ const size_t NewLinePos = Text.find(R"(\n)");
This is why I would still prefer to block on fixing StringLiteral. This is
functio
LegalizeAdulthood updated this revision to Diff 46504.
LegalizeAdulthood added a comment.
Update from comments
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/RawStringLiteralCheck.cpp
clang-tidy
LegalizeAdulthood added inline comments.
Comment at: docs/clang-tidy/checks/modernize-raw-string-literal.rst:46
@@ +45,3 @@
+editor. Trailing whitespace is likely to be stripped by editors and other
+tools, changing the meaning of the literal.
+
LegalizeAdulthood
LegalizeAdulthood marked an inline comment as done.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:79
@@ +78,3 @@
+ if (Text.find('R') < Text.find('"'))
+return false;
+
Take a look at http://en.cppreference.com/w/cpp/language/string_literal
Ther
alexfh added a comment.
In http://reviews.llvm.org/D16529#340240, @alexfh wrote:
> Also, please remove unrelated files from the patch.
Please ignore this part, I missed the new diff.
http://reviews.llvm.org/D16529
___
cfe-commits mailing list
cfe
alexfh added a comment.
Thank you for addressing (most of) the comments!
I have a few more comments and the comment about the implementation of
`asRawStringLiteral` still applies.
Also, please remove unrelated files from the patch.
Comment at: clang-tidy/modernize/RawStringLi
LegalizeAdulthood updated this revision to Diff 46466.
LegalizeAdulthood added a comment.
Re-upload diff
http://reviews.llvm.org/D16529
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/RawStringLiteralCheck.cpp
clang-tidy/moder
LegalizeAdulthood added a comment.
In http://reviews.llvm.org/D16529#339192, @bkramer wrote:
> Why are you re-adding all those Makefiles?
Ugh, I didn't even notice they were in there. It must have errantly slipped in
from rebasing.
http://reviews.llvm.org/D16529
_
bkramer added a subscriber: bkramer.
bkramer added a comment.
Why are you re-adding all those Makefiles?
http://reviews.llvm.org/D16529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
LegalizeAdulthood updated this revision to Diff 46333.
LegalizeAdulthood marked 3 inline comments as done.
LegalizeAdulthood added a comment.
Herald added a subscriber: klimek.
Update from review comments.
Extend analysis of newlines to prevent pathological raw strings from being
introduced.
Exte
alexfh added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27
@@ +26,3 @@
+ // we only transform ASCII string literals
+ if (!Literal->isAscii())
+return false;
LegalizeAdulthood wrote:
> alexfh wrote:
> > Why can't the check co
LegalizeAdulthood added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:107
@@ +106,3 @@
+ if (const auto *Literal = Result.Nodes.getNodeAs("lit"))
+preferRawStringLiterals(Result, Literal);
+}
alexfh wrote:
> I don't see a reason
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:57
@@ +56,3 @@
+
+ // already a raw string literal if R comes before "
+ if (Text.find_first_of("R") < Text.find_first_of(R"(")"))
LegalizeAdulthood wrote:
> alexfh w
LegalizeAdulthood added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27
@@ +26,3 @@
+ // we only transform ASCII string literals
+ if (!Literal->isAscii())
+return false;
alexfh wrote:
> Why can't the check convert non-ascii st
alexfh added inline comments.
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27
@@ +26,3 @@
+ // we only transform ASCII string literals
+ if (!Literal->isAscii())
+return false;
Why can't the check convert non-ascii string literals to correspond
LegalizeAdulthood updated this revision to Diff 46101.
LegalizeAdulthood marked 2 inline comments as done.
LegalizeAdulthood added a comment.
Update from comments:
- leave existing raw string literals unchanged
- don't change UTF-8, UTF-16, UTF-32 or wide character string literals
- apply changes
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.fi
aaron.ballman added a subscriber: aaron.ballman.
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 =
LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: alexfh.
LegalizeAdulthood added a subscriber: cfe-commits.
This check examines string literals and looks for those that could be more
directly expressed as a raw string literal.
Example:
```
char const *const BackSlash
41 matches
Mail list logo
