Some issues with modifier depth

[haltman-at]

Description

Modifier depth in source maps is difficult to use due to several issues. Mostly these issues take the form of, it stays the same when it shouldn't.

Specifically:

1. Modifier depth only changes on entering a modifier beyond the first one on a function. (The same applies to base constructors.) Modifier depth does not provide a way to distinguish whether you are in a modifier at all. Ideally, modifier depth would increment to 1 on the first modifier, then 2 on the second; rather than staying at 0 on the first, and then only incrementing to 1 on the second.

2. Modifier depth works differently with base constructors than with actual modifiers. (I'm not clear how it works if you mix the two.) With actual modifiers, the depth starts at 0, increments on each modifier beyond the first, until you reach the main function body at which point it increments again (rather than resetting to 0); it then decrements as the main function body exits and as each modifier exits (not going below 0, of course).

With base constructors, however... well, I'm unclear. One thing that seems clear is that the modifier depth doesn't go above 1. Another is that it may be 1 even in the main function body? But it's 0 when it's on a constructor declaration. Like it seems basically like it's "1 when in a constructor body, 0 when on a constructor declaration", except that's not always true -- sometimes it's 0 in a constructor body and I don't know why. It seems like having fewer base constructors leads to this, as does invoking those base constructors via an inheritance specifier on the contract rather than invoking it on the constructor. But I'm unclear on the details here.

I'm also not clear with how this interacts if you mix it with actual modifiers on the constructors; it looks like the modifier depth increments separately for each base constructor, except that sometimes it starts at 1 and sometimes it starts at 0? It's quite confusing.

It's not clear how much of this should be changed, to be honest. Doing so could potentially be breaking. (It wouldn't break anything Truffle's doing, I don't think, but it could be.) But ideally everything should work in a consistent manner and useful manner.

Most particularly:

1. Modifier depth should increment to 1 on the first modifer, not the second; and
2. Whether base constructors affect the modifier depth should not depend on how many of them there are or how they're invoked.

Beyond that it's hard to say what should be changed, but if those two were changed I could see how workable the result is. I don't know if base constructors vs modifiers really need to be made consistent with each other.

Basically, my real end goal here is to be able to tell when we are entering a given modifier, and the current system isn't much good for that due to the above issues.

Environment

• Compiler version: 0.6.6
• Target EVM version (as per compiler settings): default
• Framework/IDE (e.g. Truffle or Remix): Truffle

[chriseth]

I'm wondering - would it be better to start with the highest value and decreasing with each modifier, reaching 0 at the actual function?

[chriseth]

@haltman-at what do you think about the proposal above?

[haltman-at]

Sorry, I haven't had time to properly think about this. It's interesting, certainly. Not sure how it would apply to base constructors.

Remember, fundamentally the problem I want to solve is that I want to be able to tell when we're entering a modifier or base constructor -- the equivalent of a jump-in marking, since using actual jump-in markings has issues (including, obviously, the lack of an actual jump in). This is actually a little tricky to do with modifier depth, even if the problems I mention above are fixed, for a few reasons. For now I'm going to assume that modifier depth increases like it does currently rather than decreases, because I've thought more about that scenario, but you can I think make the appropriate conversions to the case of what if it decreases.

1. If modifier depth increases even on hitting the function body (like it does currently, mostly), then you can easily tell when you're entering a modifier, because a function body and a modifier body can be distinguished... except in the case of base constructors (which I'm treating as modifiers), where the main function body, and the modifier (read: base constructor) are both function bodies. I guess you could solve this by looking ahead a bunch, depending on how base constructors are handled?

2. If modifier depth resets to 0 on hitting the function body (which I think your decreasing-depth proposal is more similar to), this solves the problem above, but introduces a new one that an increase in depth doesn't always mean an entrance; it could mean a return to a modifier from the main function body. So, you have to keep track of the history somewhat in order to distinguish these two cases, and I'm not sure this is foolproof. Things that occur to me that could possibly screw this up:

• empty function bodies, maybe?
• modifiers with multiple _ statements, or with _ statements that are hit multiple times (should using multiple _ or putting a _ in a loop even be legal? They currently are!)
• maybe other things I'm not thinking of?

So, I don't know. I think this falls more into the pitfalls of approach (2). But it all depends on the details? Approach (2), including the decreasing version, could quite possibly work depending on how exactly source mapping is done in other ways, as well as the things above...

[haltman-at]

Sorry, quick clarification here: When I talk about when we're "entering" a given modifier, I really mean, when its arguments are being put on the stack (whether just before or just after), is what I really want to detect. For a normal modifier these are basically the same thing. But of course for a base constructor there can be quite a delay between when its arguments are put on the stack, and when the modifier body actually runs. Hm, that adds another wrinkle into approach (2), really. But a handleable one, probably...

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[haltman-at]

Not stale.