This revision was automatically updated to reflect the committed changes.
Closed by commit rC319904: Stringizing raw string literals containing newline
(authored by twoh).
Changed prior to commit:
https://reviews.llvm.org/D39279?vs=125564=125745#toc
Repository:
rC Clang
twoh updated this revision to Diff 125564.
twoh added a comment.
clang-format
https://reviews.llvm.org/D39279
Files:
include/clang/Lex/Lexer.h
lib/Lex/Lexer.cpp
test/Preprocessor/macro_raw_string.cpp
unittests/Lex/LexerTest.cpp
Index: unittests/Lex/LexerTest.cpp
twoh updated this revision to Diff 125563.
twoh added a comment.
@jkorous-apple Got it. I agree that it would be better to move the comments to
the header. Will land it soon. Thanks!
https://reviews.llvm.org/D39279
Files:
include/clang/Lex/Lexer.h
lib/Lex/Lexer.cpp
jkorous-apple added a comment.
I see your concern. Would you just consider moving the annotation to header
file then? It looks more like documentation of public interface than
implementation details to me. I imagine any programmer interested in these
methods needs to know these details so they
twoh updated this revision to Diff 125368.
twoh added a comment.
Thanks @jkorous-apple for the comment. I think your suggestion is a more
precise description for the implementation, and adjusted the comments
accordingly.
I intentionally didn't add the details to the comments for the header,
as
jkorous-apple added a comment.
Thank you for your patience @twoh and sorry for the delay.
I have few suggestions about doxygen annotations but otherwise LGTM.
Comment at: include/clang/Lex/Lexer.h:247
+ /// add surrounding ""'s to the string. If Charify is true, this escapes
twoh added a comment.
@jkorous-apple Thanks for the comments! Yeah, I was thinking of
O(lenght_of_string) approach, but considering the complicatedness of the
implementation (I guess the real implementation would be a bit more complex
than your pseudo implementation to handle quote and '\n\r'
jkorous-apple added a comment.
I am sorry I wasn't really clear. What I meant was to do something like this
(pseudo-code, dealing only with newlines):
if( Str.size() == 0)
return;
// Calculate all the extra space needed first.
typename T::size_type extra_space = 0;
bool
twoh updated this revision to Diff 124515.
twoh added a comment.
Thanks @jkorous-apple for your comments. I modified the type for the variables
and replaced unnecessary inserts and erases with updates.
https://reviews.llvm.org/D39279
Files:
include/clang/Lex/Lexer.h
lib/Lex/Lexer.cpp
jkorous-apple added inline comments.
Comment at: lib/Lex/Lexer.cpp:217
+void StringifyImpl(T& Str, char Quote) {
+ unsigned i = 0, e = Str.size();
+ while (i < e) {
Wouldn't **auto** or **typename T::size_type** instead of **unsigned** be more
appropriate
twoh updated this revision to Diff 123658.
twoh added a comment.
Addressing @vsapsai's comments. Thank you for the suggestion! Added test case
actually finds an off-by-one error in the original patch. I improved the
comments as well.
https://reviews.llvm.org/D39279
Files:
vsapsai added a comment.
On the first glance string erase and insert seem to be done correctly. But we
can still improve the tests. Can you please add tests for tricky cases where
new line or backslash appear in unexpected places? Some of the examples are
R"ab\
cd"
R"ab
c"
twoh added a comment.
Ping. Thanks!
https://reviews.llvm.org/D39279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
twoh updated this revision to Diff 120195.
twoh added a comment.
clang-format
https://reviews.llvm.org/D39279
Files:
lib/Lex/Lexer.cpp
test/Preprocessor/macro_raw_string.cpp
unittests/Lex/LexerTest.cpp
Index: unittests/Lex/LexerTest.cpp
twoh created this revision.
This patch implements 4.3 of
http://open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4220.pdf. If a raw string
contains a newline character, replace each newline character with the \n escape
code. Without this patch, included test case (macro_raw_string.cpp) results
15 matches
Mail list logo
