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

interp: builtins prefixed with backslash are handled by the exec handler #743

Open
cclerget opened this issue Oct 12, 2021 · 1 comment
Open

Comments

@cclerget
Copy link
Contributor

For example instead of having the export builtin handled by the interpreter, the builtin call is deferred to the ExecHandler registered for the interpreter:

\export FOO=BAR

I don't know if this is expected and/or fixable, the current workaround is to use a new parser and evaluate statements within the current interpreter, but would be nice to not have to :)

@mvdan
Copy link
Owner

mvdan commented Oct 12, 2021

Hmm, yup:

$ ''export foo=123
$ ${nope}export bar=456
$ echo $foo $bar
123 456

This is because Bash treats declare/export/let/etc as builtins, so it only checks for the word export when actually running the command.

declare and let can and often will contain tokens which are not valid word keywords, such as declare foo=(bar) or let i=(2+3), so our parser fully parses the program statically, assuming that declare and let are keywords, not builtins. This is briefly covered in the README under the caveats section.

Here I was going to say "we can't do anything significantly better", because our parser is static. However, look at this - Bash's parser is similarly crippled by this edge case where the first word isn't known statically:

$ declare bar=(baz)
$ ${nope}declare bar=(baz)
bash: syntax error near unexpected token `('
$ \declare bar=(baz)
bash: syntax error near unexpected token `('

Note how it only succeeds at parsing the rest of the line as a proper assignment if the first word looks like a keyword. Fun!

So I think we have multiple options:

  1. Keep the current logic. That is, treat words like \declare as a simple command.
  2. Reject these malformed builtins, for the sake of consistency and refusing to double-parse assignments.
  3. Imitate Bash automatically. When the parser encounters a simple command whose first field expanded to declare, re-parse the rest of the fields as assignments.

I think option 1 is clearly wrong. We shouldn't be trying to exec a declare program.

Option 2 is probably what I would choose if consistency and overall sanity were the goal. But I think Bash compatibility tends to take precedence.

So that leaves us with option 3. Which is a bit bonkers with its double-parsing, but the interpreter already hooks into the parser in four places, and we may choose to support re-parsing for arithmetic expressions as well.

@mvdan mvdan changed the title Shell builtin prefixed with backslash are handled by the exec handler interp: builtins prefixed with backslash are handled by the exec handler Oct 25, 2021
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

No branches or pull requests

2 participants