Repository navigation
Conversation
blishko
changed the title
blishko
force-pushed
the
remove-virtual-modifiers
branch
6 times, most recently
from
November 4, 2026 18:54
b4ea04c to
f4cd946
Compare
blishko
commented
Nov 5, 2026
Comment on lines
-318
to
-320
|
|
||
| for (ModifierDefinition const* modifier: contract->functionModifiers()) | ||
| registerProxy(OverrideProxy(modifier)); |
Contributor
Author
There was a problem hiding this comment.
Modifiers cannot be unimplemented anymore, that's a syntax error now, so no need to check them here.
Comment on lines
+212
to
+213
| if (!_modifier.isImplemented()) | ||
| m_errorReporter.syntaxError(8063_error, _modifier.location(), "Modifiers without implementation are not allowed."); |
Contributor
Author
There was a problem hiding this comment.
Here I am changing the unimplemented modifier from a type error to syntax error. I think it makes more sense this way.
Comment on lines
-632
to
-640
| if ( | ||
| *_modifier.name().annotation().requiredLookup == VirtualLookup::Static && | ||
| !modifierDecl->isImplemented() | ||
| ) | ||
| m_errorReporter.typeError( | ||
| 1835_error, | ||
| _modifier.location(), | ||
| "Cannot call unimplemented modifier. The modifier has no implementation in the referenced contract. Refer to it by its unqualified name if you want to call the implementation from the most derived contract." | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
I think we don't need this check. Unimplemented modifier is a syntax error, I think that is sufficient.
| }; | ||
| if (!_node.annotation().baseFunctions.empty()) | ||
| attributes.emplace_back(std::make_pair("baseModifiers", getContainerIds(_node.annotation().baseFunctions, true))); | ||
| solAssert(_node.annotation().baseFunctions.empty()); |
Contributor
Author
There was a problem hiding this comment.
I am not not 100% sure about this, we should double-check.
blishko
marked this pull request as ready for review
November 5, 2026 08:35
Virtual modifiers has been deprecated and scheduled for removal. The mechanism of modifiers, in combination with inheritance is already quite complicated and allowing modifiers to be virtual as well complicates things further with little benefit. From #12483 "Modifiers are already a very complicated concept and we should not make it more difficult by allowing them to be virtual. Except for weird cases (adding another placeholder, putting it inside a loop, ...), you can get the same behaviour by adding a virtual function that is called from the modifier."
blishko
force-pushed
the
remove-virtual-modifiers
branch
from
November 9, 2026 18:28
66b9866 to
2fb3743
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Virtual modifiers has been deprecated and scheduled for removal. The mechanism of modifiers, in combination with inheritance is already quite complicated and allowing modifiers to be virtual as well complicates things further with little benefit.
From #12483 "Modifiers are already a very complicated concept and we should not make it more difficult by allowing them to be virtual. Except for weird cases (adding another placeholder, putting it inside a loop, ...), you can get the same behaviour by adding a virtual function that is called from the modifier."