Skip to content

Fix lexer to properly handle single quote strings #258

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

Merged
merged 1 commit into from
Dec 10, 2015

Conversation

Jumhyn
Copy link
Member

@Jumhyn Jumhyn commented Dec 6, 2015

In response to a FIXME comment, the lexer now suggests
replacing double quotes inside single quote strings
with a backslash followed by a single quote.

@gribozavr
Copy link
Contributor

Please add tests for the fixits. The file test/type/array.swift at the end shows how to test fixits:

typealias FixIt1 = Int[][] // expected-error{{array types are now written with the brackets around the element type}}{{20-20=[}}{{25-26=}}

@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 7, 2015

Updated the test in expressions.swift to expect escaped double quotes!

@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 8, 2015

Just tested compilation, tests are passing.

@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 9, 2015

@gribozavr Can this be merged?

@gribozavr
Copy link
Contributor

@Jumhyn Would you mind squashing everything into one commit and doing a push -f to the pull request?

@cwillmor cwillmor self-assigned this Dec 9, 2015
replacement += orig.slice(1, orig.size() - 1);
std::string str = orig.slice(1, orig.size() - 1).str();
std::string quot = "\"";
for (size_t pos = str.find(quot); pos != std::string::npos; pos = str.find(quot, pos)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reflow to 80-column limit.

@cwillmor
Copy link
Contributor

cwillmor commented Dec 9, 2015

There's one case where I think this will fail: it will suggest converting 'a\"b' to "a\\"b". You need to check to see whether the quote mark you're escaping is already escaped. It's subtle because you still want to convert 'a\\"b' to "a\\\"b".

@cwillmor
Copy link
Contributor

cwillmor commented Dec 9, 2015

I think it will be sufficient to step through the string, skipping all escaped sequences and replacing unescaped " with \".

@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 9, 2015

You're right. This approach also makes it easier to address un-escaping escaped single quotes. Will push a new patch shortly.

@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 10, 2015

This should work now -- added new tests for the cases noted above as well.

pos += 2;
}
}
else if (str.at(pos) == '"') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put on same line as preceding '}'.

Lexer now suggests escaping unescaped double quotes and unescaping escaped single quotes in single quote strings..
@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 10, 2015

Done. Are the style rules available on Swift.org (or otherwise)? I haven't seen an explicit style guide mentioned anywhere.

@lattner
Copy link
Contributor

lattner commented Dec 10, 2015

All of the C++ code in the swift compiler should follow the LLVM coding standards: http://llvm.org/docs/CodingStandards.html

@cwillmor
Copy link
Contributor

Sweet, thanks for putting this together!

cwillmor added a commit that referenced this pull request Dec 10, 2015
Fix lexer to properly handle single quote strings
@cwillmor cwillmor merged commit a69d0dc into swiftlang:master Dec 10, 2015
@Jumhyn
Copy link
Member Author

Jumhyn commented Dec 10, 2015

No problem, and thanks @lattner for the link to the coding standards! Those should be mentioned somewhere in reference to Swift in the contributing guide on Swift.org. Currently, the only line mentioning these is in the LLVM and Swift section, which says, "Contributions to Swift’s LLVM or Clang clones are governed by the LLVM Developer Policy and should follow the appropriate licensing and coding standards," but makes no mention of contributions to the Swift code itself. Probably worth a reference earlier on the page under the Contributing Code heading.

slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
Export objc_retainAutoreleasedReturnValue()
slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
Export objc_retainAutoreleasedReturnValue()

Signed-off-by: Daniel A. Steffen <[email protected]>
dabelknap added a commit to dabelknap/swift that referenced this pull request Jun 26, 2019
Fix parameter validation for functions with labeled arguments; add su…
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
Update swiftlang version to check against Xcode 10.0's
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.

4 participants