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

Remove the disabled >>> (SHR) operator #15839

Open
cameel opened this issue Feb 7, 2025 · 2 comments · May be fixed by #15848
Open

Remove the disabled >>> (SHR) operator #15839

cameel opened this issue Feb 7, 2025 · 2 comments · May be fixed by #15848
Labels
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. should have We like the idea but it’s not important enough to be a part of the roadmap.

Comments

@cameel
Copy link
Member

cameel commented Feb 7, 2025

Solidity parser accepts >>> and >>>= as valid operators. They're also present in the grammar. Looks like they have been present ever since the shift operators were introduced (#1487). They are, however, not documented and seem to be deliberately disabled:

// Disable >>> here.
if (_operator == Token::SHR)
return false;

The operators seem to have been meant to handle unsigned right shift (SHR), as opposed to >>, which performs a signed right shift (SAR). Note that both were introduced at the same time, in the constantinople EVM.

It's been a long time (>9 years) since the shift operators were added and I don't remember anyone actually requesting them or reporting them as broken so we should probably just remove this bit of legacy. But enabling and properly documenting them would not hurt either.

For the time being it would also be clearer to reject them with a proper error message (disabled? unimplemented? dangerous?) rather than make them incompatible with all types, which must be very confusing to anyone who encounters them (it was for me).

@cameel cameel added bug 🐛 and removed bug 🐛 labels Feb 7, 2025
@cameel cameel changed the title Enable or remove the >>> (SHR) operator Remove the disabled >>> (SHR) operator Feb 10, 2025
@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. should have We like the idea but it’s not important enough to be a part of the roadmap. labels Feb 10, 2025
@cameel
Copy link
Member Author

cameel commented Feb 10, 2025

Decision from the call today: we'll remove it. It's easy to add it back later if there's demand, but so far it doesn't seem like we'll ever need it.

@Lioncat2002
Copy link

can I work on this @cameel ?

@yash-sethi-24 yash-sethi-24 linked a pull request Feb 11, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. should have We like the idea but it’s not important enough to be a part of the roadmap.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants