-
Notifications
You must be signed in to change notification settings - Fork 261
[SUGGESTION][FIX] Add support for raw string literals in mixed mode #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SUGGESTION][FIX] Add support for raw string literals in mixed mode #69
Conversation
In the last push, I have added also support for
The only thing I did not check yet is the prefix. The current implementation can handle: auto r = R"abc(
this is raw string literal
\n
/* */
R"(this should be ignored)"
R"ab(this should be also ignored)ab"
//
i: int = 42;
)abc"; |
I have tested the prefixes and it seems to work, too. |
I actually completely forgot about raw string literals... thanks for doing this. At first blush this looks good, before merging let me take a deeper look with a fresh brain and a fresh cup of coffee... Did you check the three |
No, unfortunately I did not do this. Will check that. |
I think https://github.com/modern-cmake/cppfront does that. |
d942cbf
to
6feefab
Compare
I have rebased my changes to the main and done the passthrough tests. clang & GCC files are the same. MSVC ends with the error:
Need to check what caused the error. |
c4e1a86
to
4c91b78
Compare
OK, found a bug and fix it. The issue was here diff --git a/source/load.h b/source/load.h
index 71fd857..9954a02 100644
--- a/source/load.h
+++ b/source/load.h
@@ -248,7 +248,7 @@ auto process_cpp_line(
return r;
}
in_raw_string_literal = false;
- i += end_pos+d_char_sequence.size()-1;
+ i = end_pos+d_char_sequence.size()-1;
}
else {
r.all_comment_line = false; Small but nasty mistake. Now all files compiles and generate identical output. |
4c91b78
to
b628e33
Compare
Rebased to main - all passthrough-tests and regression-tests passed. |
96d319c
to
4945177
Compare
4945177
to
14b8cf7
Compare
I tried this out with the Running it under the debugger, I see that the crash is an out-of-bounds access attempt in the second-last line in
where I didn't investigate further, but this should give you a repro and a roadmap to find the bug... can you take a look? |
Hmm... I am surprised that it crashed. I will investigate |
I have trouble reproducing the crash. I even have added an assert before the line that you have spotted (lines 304-306): assert(i < line.size());
prev = line[i];
} There was a bug that I fixed (#69 (comment)), and it is already pushed. Can you check if line 251 uses i = end_pos+raw_string_closing_seq.size()-1; I have compiled my cppfront in clang
@hsutter Can you tell me which compiler you have used? |
The interesting fact is that only MSVC is using raw string literals in its lib. It is always inline and finalised with the string_view suffix. E.g.: _Result += R"(: ")"sv; so |
@hsutter I have rebased my changes on the newest main and still cannot reproduce. I will probably need to check on other compilers. |
14b8cf7
to
5bece77
Compare
@hsutter I am confused. I have tried it on MSVC and it works as well. I will need more information To be able to reproduce it. My very first guess is that it looks like the previous bug that I have fixed (check above). Some merge issues might also cause it. |
OK, I'll take another look. Thanks! |
@hsutter if it will not work, please send me the code you have via email I will verify what is wrong. |
5bece77
to
cd3160a
Compare
Good news: It works fine! Sorry for the noise: The test failure was almost certainly my error when I opened the branch in GitHub Desktop and must just not have had all the latest commits somehow. (I did try pulling the later commits, but there was a conflict and I'm just not familiar enough with switching to PR branches in GitHub Desktop, and I'm sadly not a git command-line person.) So I have verified the PR by the time-honored method of manually applying it to my main branch and running the passthrough tests, and they work fine. So now I'll revert that and merge this PR. :) Thanks again. |
Happy to hear that it's working. The good thing is that this situation motivated me to run sanitizers which helped in discovering another issue. |
…f-cpp1-rawstringliterals [SUGGESTION][FIX] Add support for raw string literals in mixed mode
Current implementation in mixed mode is not parsing raw string literals well.
Cpp2 source code
Compiles to
The proposed change makes cppfront parse raw string literals in a similar way as it parses
/* */
comments. After this change cppfront produces:Debug info shows that the lines are recognized as
/* R */
which was added symbol to mark raw string literals lines.Close #62