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

Yul sources are given id 1 instead of 0 in compiler output (inconsistent with source map & ast) #14453

Closed
haltman-at opened this issue Jul 26, 2023 · 2 comments · Fixed by #14455
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.

Comments

@haltman-at
Copy link
Contributor

Description

In the sources output in Yul mode (when present), the ID of each source is given as 1, instead of 0. This is inconsistent with Solidity's general practice of 0-indexing sources. Worse yet, it's inconsistent with the numbering elsewhere in the output, namely, the source map and the AST; both of these will give the ID as 0.

Environment

  • Compiler version: 0.8.21
  • Target EVM version (as per compiler settings): default

Steps to Reproduce

Any Yul source code will do, just make sure to request ASTs in the output (about which see #14452) so that you get sources output. Also good idea to request the source map as well for comparison.

@haltman-at haltman-at changed the title Yul sources are given id 1 instead of 0 in compiler output (inconsisten with source map & ast) Yul sources are given id 1 instead of 0 in compiler output (inconsistent with source map & ast) Jul 26, 2023
@r0qs
Copy link
Member

r0qs commented Jul 28, 2023

Hi @haltman-at, thank you very much! Indeed the IDs are inconsistent. The wrong ID was added here: https://github.com/ethereum/solidity/pull/14177/files#diff-ad23dac446ab50b79251aacbae1a5230bacd344588c52241eca0f0b1d815e8acR1528.

I opened a PR to fix it :)

@cameel
Copy link
Member

cameel commented Aug 1, 2023

Yes, just like #14452 this is just an oversight from #14177. The IDs are arbitrary, but if they don't match numbers in the AST or source map, it's a bug. And there's no reason for it not to start at 0.

@cameel cameel added low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. labels Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants