Properly detect which array and struct types are unsupported by the old ABI encoder.#3549
Properly detect which array and struct types are unsupported by the old ABI encoder.#3549
Conversation
chriseth
force-pushed
the
fixmultidim
branch
2 times, most recently
from
a5bc2ea to
4ef8f24
Compare
| char const* text = R"( | ||
| contract C { | ||
| struct S { string[] s; } | ||
| function f() public pure returns (S x) {} |
There was a problem hiding this comment.
Hm, why does this one needs a named return parameter while the other one doesn't?
There was a problem hiding this comment.
Probably a rebasing problem.
| m_errorReporter.typeError( | ||
| var->location(), | ||
| "Structs are only supported in the new experimental ABI encoder. " | ||
| "This type is only supported in the new experimental ABI encoder. " |
There was a problem hiding this comment.
Were the current (not the new ones) tests only checking for "only supported" or we didn't had tests to cover this?
There was a problem hiding this comment.
We only had tests for structs, not structs inside arrays or arrays containing arrays etc.
There was a problem hiding this comment.
Yeah but none of the test expectations had to change so either there was no text or it didn't expect the entire message.
There was a problem hiding this comment.
It seems we did not have tests for that.
There was a problem hiding this comment.
Is this a blocker for merging? Please make clear statements.
There was a problem hiding this comment.
Grepped the sources, there was no test for this before.
There was a problem hiding this comment.
No, there are tests now hopefully covering all of it.
| else if (!m_baseType->canBeUsedExternally(_inLibrary)) | ||
| return false; | ||
| else if (m_baseType->category() == Category::Array && m_baseType->isDynamicallySized()) | ||
| return false; |
There was a problem hiding this comment.
So basically we enable multi level arrays now for non-experimental mode. Will it be still disabled due to no support for returndatacopy? Do we have a test for this?
There was a problem hiding this comment.
It is still disabled by the type checker.
There was a problem hiding this comment.
The function typeSupportedByOldABIEncoder is much stricter than what was there before.
chriseth
force-pushed
the
fixmultidim
branch
from
4ef8f24 to
4d26a34
Compare
|
@axic anything still to be done here? |
|
Failure on travis is unrelated. |
2 participants
Fixes #3176