Disallow modifier declarations and definitions in interfaces #11577
Conversation
cameel
force-pushed
the
disallow-modifiers-in-interfaces
branch
from
c164491
to
9c34d20
Compare
|
Shouldn't this be breaking? |
|
We discussed it on the chat and @chriseth's opinion was that it should not. |
|
Strictly it is breaking. @axic what do you think? |
| @@ -1114,7 +1114,6 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts) | |||
| unique_ptr<CompilerStack> compilerStack = parseAndAnalyzeContracts(R"( | |||
| interface I { | |||
| event Ev(uint); | |||
| modifier m() virtual; | |||
There was a problem hiding this comment.
Yeah, though when working on the FunctionCallGraph I just tried everything that compiled so it's not like it was by design.
|
It is a though question. Since we never intended this to happen and rather started out with a very restricted feature set for interfaces (recall #3420 and many other related issues), it is easy to see that this is an oversight and a bug. However given this likely existed for 2 years and that some forms worked, makes me wonder if anyone relies on it? And if they do, what does that mean? Likely only that they are locked in to the last version if they insist on this feature. If we want to be careful, we could ask others from the ecosystem (like the tooling channel). If we are more confident that this is something nobody ever should have used, then we should go ahead in a non-breaking release and create a blog post to spread the word. I'm leaning towards not making this a breaking change. |
|
I'm also fine with calling it a bugfix. |
|
The documentation does not mention modifiers, but it says that functions cannot be implemented, so I hope nobody thought that modifiers can be implemented... https://docs.soliditylang.org/en/v0.8.6/contracts.html#interfaces Can you also update the documentation? It also seems that the bullet list somehow does not render properly. |
I mentioned this on matrix two weeks ago, none of the bullet points render anywhere. |
cameel
force-pushed
the
disallow-modifiers-in-interfaces
branch
2 times, most recently
from
5fc8e50
to
8ce0dd3
Compare
|
Rebased to resolve the changelog conflict and update docs.
I updated the page to mention modifiers. The bullet list issue seems to be a general issue with the docs now so I'm going to work on it in a separate PR. No need to hold back this one because of it. |
docs/contracts/interfaces.rst
Outdated
| @@ -6,12 +6,14 @@ | |||
| Interfaces | |||
| ********** | |||
|
|
|||
| Interfaces are similar to abstract contracts, but they cannot have any functions implemented. There are further restrictions: | |||
| Interfaces are similar to abstract contracts, but they cannot have any functions or modifiers implemented. | |||
There was a problem hiding this comment.
If you say "modifiers implemented" here, it looks like it is OK to declare them.
There was a problem hiding this comment.
There's They cannot declare modifiers below. I wanted to emphasize that they cannot be defined either.
There was a problem hiding this comment.
I reworded it. How about now?
cameel
force-pushed
the
disallow-modifiers-in-interfaces
branch
from
8ce0dd3
to
239e324
Compare
|
Now it's inconsistent with e.g. constructors, because there you only write "declare". |
cameel
force-pushed
the
disallow-modifiers-in-interfaces
branch
from
239e324
to
ec60673
Compare
|
Changed to just "declare". |
Marenz
dismissed
their stale review
Needs a rebase and the changelog needs to add it to the latest unreleased
Marenz
force-pushed
the
disallow-modifiers-in-interfaces
branch
from
ec60673
to
d07b796
Compare
|
Rebased & fixed changelog entry position. |
Fixes #11557.