Empty tuple components should not be possible #3976
Conversation
libsolidity/analysis/TypeChecker.cpp
Outdated
| @@ -1418,6 +1418,10 @@ bool TypeChecker::visit(TupleExpression const& _tuple) | |||
| components[i]->accept(*this); | |||
| types.push_back(type(*components[i])); | |||
|
|
|||
| if (types[i]->category() == Type::Category::Tuple) | |||
| if (dynamic_cast<TupleType const&>(*types[i]).components().empty()) | |||
| m_errorReporter.fatalTypeError(components[i]->location(), "Type of tuple component cannot be null."); | |||
There was a problem hiding this comment.
I know I used the world null in the description of the issue, but this is actually the first place this appears. Perhaps we should say something like "non-empty value"?
There was a problem hiding this comment.
Also, I think it should only be a warning pre-0.5.0
There was a problem hiding this comment.
I'd be up for "cannot be empty". Do you suggest this to be a warning rather than an error? I think we should treat this as an error.
There was a problem hiding this comment.
Let's again make it a warning pre-050 and an error starting from 050
| pragma solidity ^0.4.3; | ||
| contract C { | ||
| function a() public { | ||
| (a(), 7); |
There was a problem hiding this comment.
I don't think this test case adds to coverage.
libsolidity/analysis/TypeChecker.cpp
Outdated
| if (types[i]->category() == Type::Category::Tuple) | ||
| if (dynamic_cast<TupleType const&>(*types[i]).components().empty()) | ||
| { | ||
| if (!v050) |
There was a problem hiding this comment.
Can you swap this? Usually we have if (v050).
libsolidity/analysis/TypeChecker.cpp
Outdated
| if (!v050) | ||
| m_errorReporter.warning(components[i]->location(), "Tuple component cannot be empty."); | ||
| else | ||
| m_errorReporter.fatalTypeError(components[i]->location(), "Tuple component cannot be empty."); |
There was a problem hiding this comment.
Yes, because we cannot assign a proper type.
| } | ||
| } | ||
| // ---- | ||
| // Warning: (146-149): Tuple component cannot be empty. |
There was a problem hiding this comment.
The message is wrong, it is not a tuple, it is an array. (the same checks should apply, though, so only the masseg is to be changed).
| // ---- | ||
| // Warning: (146-149): Tuple component cannot be empty. | ||
| // Warning: (151-154): Tuple component cannot be empty. | ||
| // TypeError: (145-155): Type tuple()[2] memory is not implicitly convertible to expected type tuple(uint256,uint256). |
There was a problem hiding this comment.
Because of that, I think we can turn into an error for arrays right away.
There was a problem hiding this comment.
Do you mean assigning it to a tuple should throw an error even before the components are being checked?
There was a problem hiding this comment.
No, the tuple ast node is used both for tuple expressions and for inline arrays. Empty component should be a warning/error for tuples but an error for arrays.
There was a problem hiding this comment.
Ok, I see. Will change it accordingly.
erak
force-pushed
the
emptyTupleComponent
branch
from
f2c93bf to
b2ff9bc
Compare
Fixes #3889