Skip to content
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

Add handling of escaped backslashes in expanded string - closes #461 #464

Conversation

filipsajdak
Copy link
Contributor

@filipsajdak filipsajdak commented May 22, 2023

The current cppfront (c84bf47) does not handle escaped backslashes in an expanded string. That makes the following code fail with an assertion:

main: () = {
  std::cout << "\"\"" << '\n'; // Prints `""`.
  std::cout << $R"("")" << '\n'; // Prints `""`.
  std::cout << "\"(\"\")$\"" << '\n'; // Prints `""`.
  std::cout << "(\"\\\"\\\"\")$" << '\n'; // cppfront asserts.
}

This PR removes all redundant backslashes - Closes #461

All regression tests pass.

@filipsajdak filipsajdak force-pushed the fsajdak-fix-escaped-backslash-in-expanded-string branch 2 times, most recently from bd22c59 to f94ef0b Compare May 22, 2023 21:08
@filipsajdak filipsajdak changed the title Add handling of escaped backslashes in expanded string Add handling of escaped backslashes in expanded string - closes #461 May 22, 2023
@filipsajdak
Copy link
Contributor Author

@hsutter fix ready

@filipsajdak filipsajdak force-pushed the fsajdak-fix-escaped-backslash-in-expanded-string branch from f94ef0b to 617b161 Compare May 23, 2023 11:57
@filipsajdak
Copy link
Contributor Author

After sleeping over, I replaced the raw for-loop with the copy_if algorithm.

The current change iterate over entire string end removes extra backslashes
that are used to indicate escape sequence.

Closes hsutter#461
@filipsajdak filipsajdak force-pushed the fsajdak-fix-escaped-backslash-in-expanded-string branch from 617b161 to c2bbcd2 Compare May 23, 2023 12:15
@hsutter
Copy link
Owner

hsutter commented May 29, 2023

Looks good, thanks!

@hsutter hsutter merged commit e294d17 into hsutter:main May 29, 2023
@filipsajdak filipsajdak deleted the fsajdak-fix-escaped-backslash-in-expanded-string branch May 31, 2023 21:25
filipsajdak added a commit to filipsajdak/cppfront that referenced this pull request May 31, 2023
Original fix uses std::copy_if algorithm with overlapping input and output
ranges - that behaviour is undefined.
After reviewing it with @Maiqel and @lukasz-matysiak we have replace it
with std::remove_if algorithm - which better describe what is happening
and does not excercise undefined behaviour.

From https://en.cppreference.com/w/cpp/algorithm/copy:

> The behavior is undefined if d_first is within the range [first, last)
hsutter pushed a commit that referenced this pull request May 31, 2023
Original fix uses std::copy_if algorithm with overlapping input and output
ranges - that behaviour is undefined.
After reviewing it with @Maiqel and @lukasz-matysiak we have replace it
with std::remove_if algorithm - which better describe what is happening
and does not excercise undefined behaviour.

From https://en.cppreference.com/w/cpp/algorithm/copy:

> The behavior is undefined if d_first is within the range [first, last)
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
The current change iterate over entire string end removes extra backslashes
that are used to indicate escape sequence.

Closes hsutter#461
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
…sutter#481)

Original fix uses std::copy_if algorithm with overlapping input and output
ranges - that behaviour is undefined.
After reviewing it with @Maiqel and @lukasz-matysiak we have replace it
with std::remove_if algorithm - which better describe what is happening
and does not excercise undefined behaviour.

From https://en.cppreference.com/w/cpp/algorithm/copy:

> The behavior is undefined if d_first is within the range [first, last)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Escaping '"' in string in interpolation fails assertion
2 participants