YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.37k stars 874 forks source link

Support quoted strings as arguments to passes #4511

Open gussmith23 opened 1 month ago

gussmith23 commented 1 month ago

Feature Description

I'd like to be able to call a pass like

pass_name "some string" "some other string maybe having \"escaped quotes\""

and have the args list appear as

["pass_name", "some string", "some other string maybe having \"escaped quotes\""]

It currently seems like that might not be possible with the parser. Is that true, or am I missing something?

povik commented 1 month ago

Related: #4429

whitequark commented 1 month ago

IIRC this also affects bugpoint.

gussmith23 commented 1 month ago

It seems like this might be supported by the long_strings arg of next_token -- when true, it seems like next_token will grab an entire quoted string as one token (though it looks like it will break on escaped quotes \"). The relevant arg parsing code even sets next_token to true. However, despite this, quoted strings still don't seem to be parsed correctly. I'm wondering if there's bug here:

https://github.com/YosysHQ/yosys/blob/960bca019639402df3211ad0687b01522a01ba22/kernel/yosys.cc#L201-L208

Essentially, if I pass command "some string", next_token will grab command as the first token and return "some string" (note the space) as the rest of the string. Because this string doesn't start with ", it won't trigger the long_strings case I think.

KrystalDelusion commented 1 month ago

Oh huh, I was looking into this for log but it seemed like it was already being handled well enough at the kernel level. I certainly wasn't getting any issues of spaces at the start of strings in log.

I wonder if it's a difference between void Pass::call(RTLIL::Design *design, std::string command) and void Pass::call(RTLIL::Design *design, std::vector<std::string> args)? The former appears to skip spaces (and tabs) in the cmd_buf directly before attempting to grab the next token. That is probably why it works there but not other places?

https://github.com/YosysHQ/yosys/blob/960bca019639402df3211ad0687b01522a01ba22/kernel/register.cc#L280-L291

gussmith23 commented 1 month ago

Am I able to choose which one should be used when implementing a Pass? I override execute, which i assume is called by call.

KrystalDelusion commented 1 month ago

That I do not know

KrystalDelusion commented 4 weeks ago

The difference between which call is used is whether it is called with a string (which is the one you want for quote parsing), or as a list of strings (which does not). The only way to get the latter (as far as I can tell), is via tee, debug, and trace, all of which are called via the former and so have already had quotes parsed; or if your pass is a frontend, because for some reason frontends are handled slightly differently.

KrystalDelusion commented 4 weeks ago

Oh actually backends also run their own arg parsing, and don't use call at all. See https://github.com/YosysHQ/yosys/blob/960bca019639402df3211ad0687b01522a01ba22/kernel/register.cc#L712-L720