UnimplementedFeatureError with no line indication when copying an array of structs to storage

[barakman]

Description

I am getting an error with no indication of which line it refers to:

UnimplementedFeatureError: Copying of type struct MyStruct memory[] memory to storage not yet supported.

Environment

• Compiler version: 0.8.12

Steps to Reproduce

Well, I need to dig through my code in order to find it, but on the compiler side, the problem is probably in ArrayUtils.cpp:

solUnimplemented("Copying of type " + _sourceType.toString(false) + " to storage not yet supported.");

Thanks :)

[wechman]

@barakman Thank you for your ticket! Could you share the code we can use to reproduce the issue? Hope you have managed to find it. Otherwise, even if we made adjustments you suggest, we wouldn't be sure the problem is fixed.

[cameel]

This looks like the same thing as #3446.

And it probably won't be implemented in the current syntax but #2435 will provide a different mechanism for the same thing.

I think we should replace the UnimplementedFeatureError with a proper error at the analysis stage. Letting it blow up in the codegen is not a great user experience. And even when we have copyof syntax will be different so will still need to block this case.

[barakman]

@barakman Thank you for your ticket! Could you share the code we can use to reproduce the issue? Hope you have managed to find it. Otherwise, even if we made adjustments you suggest, we wouldn't be sure the problem is fixed.

With no line indication on your side, all I can do is paste my entire project here, which you'll probably agree is senseless.
I suggest that you add a line indication on your side (as pointed out in my initial message), then similar issues in the future will also include a short snippet of the client code (solidity) causing the problem.

As you might understand, the main issue in this context is not the compilation error itself, but rather the message issued by the compiler, which lacks the information required for the developer to fix (or workaround) the error.

[cameel]

Yeah, the error should be better.

But just to help you locate it in your particular case - you can try commenting out lines that assign something of type MyStruct stored in memory to a storage variable. If doing this makes the error go away, it's the same issue.

[timweri]

I'm interested in fixing this. Will start digging to see what I can find.

[barakman]

So what's the status on this? Still getting it on solc 0.8.17.

[cameel]

Sorry, no one is working actively on this due to other tasks with higher priorites.

If anyone wants to pick it up and work on it, feel free. It should be relatively easy to plug with an extra check in the type checker.

[barakman]

There are actually two different problems embedded in this issue:

1. UnimplementedFeatureError reported by the compiler with no line indication
2. No support by the compiler for copying an array of structs into storage

One the one hand, I guess that you could consider both as low-priority, since 1 only requires a bit of a "dig through" in order to find the error in the code, and 2 can be worked-around by copying struct by struct into storage.

On the other hand, both are rather annoying when you (I) encounter them...

Thanks

[cameel]

I'm only speaking about the error message here. Copying is actually high priority and is one of the roadmap issues. It's just that it we don't think it's a good idea to allow doing it implicitly through an assignment. In #2435 we want to introduce special syntax for it to make it explicit.

[grapevinegizmos]

Just to add my feedback that, for developers, getting an error message which gives no indication of the line of source that spawns the error can be quite tedious in a large code base. You spend a lot of time hunting in the code trying to figure out where you did such a thing,

This error was opened about a year ago and appears to be relatively low effort fix (with significant impact for those who run into it). Hope that it gets some attention.

[ekpyron]

The error should really just tell people to use --via-ir these days (which has the feature implemented for a long time).

[cameel]

@ekpyron But it still should not be a failing assert. It should at the very least go through an error reporter.

[ekpyron]

If the message for the assert is good enough, it is fine to keep it. We plan to drop legacy code generation anyways. The assertion message just has to be clear about what can and should be done to get around it.

[cameel]

I don't think an assert is ever good enough when it comes to reporting errors. It's an internal mechanism to catch bugs and very unfriendly as a way of reporting errors to tools and users.

The error should really just tell people to use --via-ir these days (which has the feature implemented for a long time).

By the way, I just realized that you must have confused this issue with #12449 (maybe I referred to it as such by mistake somewhere? I thought it was that too until I took a closer look now). This one is about the inability to copy structs, not "stack too deep". Here it's very easy for us to give the user the exact location of the error if we detect this during analysis and issue a proper error. For whatever reason we're not doing that and letting it blow up in the codegen.

[barakman]

Closed as not planned? Why not just fix it?

This issue (a compilation error with no line indication) is clearly a compiler issue.

[dannydoritoeth]

The error message is not helpful at all at guiding you for where to start looking for in your code for what is causing it.

It would be good to just make the error bit clearer even its not going to be fixed.

[PaulRBerg]

Why has this issue been closed as not planned, @cameel, @NunoFilipeSantos?

[peexledev]

fix this