Unify handling of quoted strings in pass args #5385
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
i.e. they should be part of the quote instead of starting a comment
Use `std::quoted()` from `<iomanip>` to quote/unquote pass arguments. In order to maintain handling of (unquoted) comments and semicolons this occurs when adding the token to the arg list, rather than in the tokenization itself. Passes no longer receive tokenized-but-raw arguments and are instead pre-unquoted. This will cause problems for some passes (e.g. `setparam`) which rely on arguments having quotation marks for disambiguation. In order to maintain reproducibility with `echo on`, arguments which require quoting will automatically quote (arguments which were quoted but didn't need to be will not be quoted in the echo).
Add other special characters to `needs_quote()` check. Fix `"\" ` being treated as a complete quoted string because of the space after the `"` even though it's escaped.
Single quoted strings have no escape character and are treated verbatim. This is useful for minimizing the number of backslashes in (for example) `logger -expect` regexps (e.g. `"\\\""` -> `'\"'`). `Yosys::quote()` will use single quotes by default, unless the string contains a single quote and then it will use `std::quoted()`. Fix strange behaviour arrising from always using `std::quoted(result)`. Because of the way the function interacts with streams, if there is only one quotation mark then the result would end at the first whitespace.
Don't strip runner/suffix quotes. Quote script/command argument.
Don't rely on strings being quoted, instead we introduce `-setstr`. Change the help text formatting for `setattr` and `setparam` to avoid the line being too long. Instead use a generic `[options]` and list the options separately.
Replaces double quotes on problematic regex strings (mostly ones that have escape sequences that are easier to preserve in single quotes). Necessitates also changing single quotes to `.`, i.e match any. For some (mostly ones that only have a single escaped character, or were using `\.` to match a literal fullstop) keep the double quotes and fix the regex instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the reasons/motivation for this change?
https://yosyshq.discourse.group/t/quoted-strings-in-pass-arguments/83
Explain how this is achieved.
std::quoted()from<iomanip>to quote/unquote pass arguments. In order to maintain handling of (unquoted) comments and semicolons this occurs when adding the token to the arg list, rather than in the tokenization itself. Passes no longer receive tokenized-but-raw arguments and are instead pre-unquoted. Should resolve Support quoted strings as arguments to passes #4511.logger -expectregexps (e.g."\\\""->'\"').echo on, arguments which require quoting will automatically quote (arguments which were quoted but didn't need to be will not be quoted in the echo). Single quotes are used by default, unless the arg contains a single quote and then it will usestd::quoted()(which uses double quotes and can escape with backslash).setattrwhich rely on arguments having quotation marks for disambiguation.If applicable, please suggest to reviewers how they can test the change.
PR includes tests for checking behavior of arg parsing in
tests/scripts. Many of these utilize a plugin that gets compiled during testing and provides thetest_argspass, which is basically justlogbut each argument is separated by a newline.