Skip to content

Reviewing how and when we call cleanModifierInvocationArguments#1444

Merged
Janther merged 1 commit intomainfrom
clean-modifier-invocations
Mar 8, 2026
Merged

Reviewing how and when we call cleanModifierInvocationArguments#1444
Janther merged 1 commit intomainfrom
clean-modifier-invocations

Conversation

@Janther
Copy link
Member

@Janther Janther commented Mar 6, 2026

a few details to view here:

  • in some cases this didn't need to be a public function since it's called straight from the constructor.
  • the check for typeof attribute !== 'string' is obsolete.
  • if the compiler is <0.5.0 constructors where functions with the same name as the contract which is the reason for this being a public function, but we had forgotten that libraries can also invoke modifiers. I also added a test case for this scenario.

@Janther Janther requested a review from fvictorio March 6, 2026 15:05
@Janther Janther added bug Something isn't working refactor reorganising the code without changes performance Results in smaller size, better memory usage, or faster execution format Addresses a change in the format and removed bug Something isn't working labels Mar 6, 2026
@Janther Janther force-pushed the clean-modifier-invocations branch from 0059d29 to 2cec6fb Compare March 7, 2026 21:04
@Janther Janther force-pushed the clean-modifier-invocations branch from 2cec6fb to 1d5fd1e Compare March 8, 2026 00:32
Copy link
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm tempted to say we should drop support for solidity versions before 0.5.0 entirely, but for now let's keep supporting whichever versions Slang support, I guess. There's a tiny bit of value in that.

@Janther
Copy link
Member Author

Janther commented Mar 8, 2026

Yeah, I just meant to clean up the function calls and then I noticed that use case when cleaning up. which meant a new format test which I was slightly annoyed by, but harmless.

@Janther Janther merged commit 2351319 into main Mar 8, 2026
7 checks passed
@Janther Janther deleted the clean-modifier-invocations branch March 8, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

format Addresses a change in the format performance Results in smaller size, better memory usage, or faster execution refactor reorganising the code without changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants