Don't ignore errors reported after analysis

[cameel]

Follow-up to #15168.

Originally this was a part of #15610, but I decided to extract it because it's really an issue of its own, even though before #15610 we weren't throwing any errors that could trigger it.

The issue is that CLI and Standard JSON interface don't check for errors after analysis and just happily proceed with assembling. CompilerStack also makes the same mistake when using YulStack internally. As a result, if an UnimplementedFeatureError were to be reported it would be ignored and we'd report empty bytecode, or run into an assertion/segfault in the process.

In addition to that UnimplementedFeatureErrors caught during parsing were not reported as a parsing failure.

[r0qs]

Nice, thanks for extracting this PR.

[r0qs]

Previously, stack.optimize() was called prior to invoking stack.parserResult(). However, now it is called afterward (on line 1264). If I’m not mistaken, the Object returned by parserResult() can also be modified by optimize(), meaning that hasContiguousSourceIndices will not be checked against the optimized object anymore, no? Is this intentional, or am I misunderstanding something?

[cameel]

I don't think that was intentional. The check was inserted after optimize() probably simply because it was more convenient to do it there. IMO it makes more sense to do such checks on the unoptimized code that the user is submitting rather than on some modified version so the change is good.

In any case, it should not affect solc behavior in practice - optimize() theoretically could change the code in an arbitrary way, but in practice it will never touch the @use-src annotation so this property should remain unchanged.

We also have a second validation against this in the asm export code, which would catch this if it did happen:

solidity/libevmasm/Assembly.cpp

Lines 509 to 514 in ea52aca

	solAssert(maxSourceIndex + 1 >= _sourceIndices.size());
	solRequire(
	_sourceIndices.size() == 0 || _sourceIndices.size() == maxSourceIndex + 1,
	AssemblyImportException,
	"The 'sourceList' array contains invalid 'null' item."
	);

We have the check in the CLI only because we don't catch AssemblyImportError there. Perhaps we should later change it into a CodeGenerationError so that it gets caught by YulStack and reported as a normal error.

[r0qs]

We have the check in the CLI only because we don't catch AssemblyImportError there. Perhaps we should later change it into a CodeGenerationError so that it gets caught by YulStack and reported as a normal error.

Yeah, I don't see why not do that.

[cameel]

ok, I'll keep it in mind for when we're done with this series of PRs and can actually handle those errors properly.

[r0qs]

It looks fine to me now ;)