Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable parser to accept transient as data location or identifier #15001

Open
wants to merge 18 commits into
base: catchSolUnimplementedFeatureErrors
Choose a base branch
from
Open

Enable parser to accept transient as data location or identifier #15001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
96760e4
Add unimplementedFeatureError method
matheusaaguiar May 31, 2024
e65fb17
Catch SolUnimplementedError
matheusaaguiar May 29, 2024
d196c33
Make SyntaxTest accept UnimplementedFeatureError after successfull co…
matheusaaguiar Jun 2, 2024
2e99c16
Update tests message
matheusaaguiar Jun 2, 2024
6a55279
fixup! separate catch blocks for each stage (parse/analysis/compilation)
matheusaaguiar Jun 4, 2024
7525a68
fixup! ignore error code
matheusaaguiar Jun 4, 2024
ce01cd3
fixup! style
matheusaaguiar Jun 4, 2024
6925866
Make parser accept transient as location (but not a keyword)
matheusaaguiar Apr 6, 2024
947a802
fixup! Storage layout solUnimplementedAssert
matheusaaguiar May 14, 2024
f5024f9
Remove storage layout cmd line test
matheusaaguiar May 16, 2024
4897478
Do not allow smt-checker to run if transient storage is present
matheusaaguiar May 14, 2024
22e3b5e
fixup! assert empty description string
matheusaaguiar May 16, 2024
34d3d01
solUnimplementedAssert codegen
matheusaaguiar May 31, 2024
59a4566
fixup! Changelog
matheusaaguiar May 31, 2024
d6289ff
fixup! remove error from ignore list
matheusaaguiar Jun 4, 2024
9b325bb
fixup! test
matheusaaguiar Jun 4, 2024
211070b
Revert "Remove storage layout cmd line test"
matheusaaguiar Jun 4, 2024
f5076cc
fixup! storage layout error message
matheusaaguiar Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.8.27 (unreleased)

Language Features:
* Accept declarations of state variables with ``transient`` data location (parser support only, no code generation yet).


Compiler Features:
Expand Down
1 change: 1 addition & 0 deletions docs/grammar/SolidityLexer.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ SignedIntegerType:
Storage: 'storage';
String: 'string';
Struct: 'struct';
Transient: 'transient'; // not a real keyword
True: 'true';
Try: 'try';
Type: 'type';
Expand Down
5 changes: 3 additions & 2 deletions docs/grammar/SolidityParser.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ userDefinedValueTypeDefinition:
* The declaration of a state variable.
*/
stateVariableDeclaration
locals [boolean constantnessSet = false, boolean visibilitySet = false, boolean overrideSpecifierSet = false]
locals [boolean constantnessSet = false, boolean visibilitySet = false, boolean overrideSpecifierSet = false, boolean locationSet = false]
:
type=typeName
(
Expand All @@ -272,6 +272,7 @@ locals [boolean constantnessSet = false, boolean visibilitySet = false, boolean
| {!$constantnessSet}? Constant {$constantnessSet = true;}
| {!$overrideSpecifierSet}? overrideSpecifier {$overrideSpecifierSet = true;}
| {!$constantnessSet}? Immutable {$constantnessSet = true;}
| {!$locationSet}? Transient {$locationSet = true;}
)*
name=identifier
(Assign initialValue=expression)?
Expand Down Expand Up @@ -419,7 +420,7 @@ inlineArrayExpression: LBrack (expression ( Comma expression)* ) RBrack;
/**
* Besides regular non-keyword Identifiers, some keywords like 'from' and 'error' can also be used as identifiers.
*/
identifier: Identifier | From | Error | Revert | Global;
identifier: Identifier | From | Error | Revert | Global | Transient;

literal: stringLiteral | numberLiteral | booleanLiteral | hexStringLiteral | unicodeStringLiteral;

Expand Down
10 changes: 10 additions & 0 deletions liblangutil/ErrorReporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,16 @@ void ErrorReporter::docstringParsingError(ErrorId _error, SourceLocation const&
);
}

void ErrorReporter::unimplementedFeatureError(ErrorId _error, SourceLocation const& _location, std::string const& _description)
{
error(
_error,
Error::Type::UnimplementedFeatureError,
_location,
_description
);
}

void ErrorReporter::info(
ErrorId _error,
SourceLocation const& _location,
Expand Down
2 changes: 2 additions & 0 deletions liblangutil/ErrorReporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ class ErrorReporter

void docstringParsingError(ErrorId _error, SourceLocation const& _location, std::string const& _description);

void unimplementedFeatureError(ErrorId _error, SourceLocation const& _location, std::string const& _description);

ErrorList const& errors() const;

void clear();
Expand Down
27 changes: 25 additions & 2 deletions libsolidity/analysis/DeclarationTypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
{
case Location::Memory: return "\"memory\"";
case Location::Storage: return "\"storage\"";
case Location::Transient: return "\"transient\"";
case Location::CallData: return "\"calldata\"";
case Location::Unspecified: return "none";
}
Expand Down Expand Up @@ -456,8 +457,24 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
}
else if (_variable.isStateVariable())
{
solAssert(varLoc == Location::Unspecified, "");
typeLoc = (_variable.isConstant() || _variable.immutable()) ? DataLocation::Memory : DataLocation::Storage;
switch (varLoc)
{
case Location::Unspecified:
typeLoc = (_variable.isConstant() || _variable.immutable()) ? DataLocation::Memory : DataLocation::Storage;
break;
case Location::Transient:
if (_variable.isConstant() || _variable.immutable())
m_errorReporter.declarationError(
2197_error,
_variable.location(),
"Transient cannot be used as data location for constant or immutable variables."
);
typeLoc = DataLocation::Transient;
nikola-matic marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
solAssert(false);
break;
}
}
else if (
dynamic_cast<StructDefinition const*>(_variable.scope()) ||
Expand All @@ -477,6 +494,9 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
case Location::CallData:
typeLoc = DataLocation::CallData;
break;
case Location::Transient:
solUnimplementedAssert(false, "Transient data location cannot be used in this kind of variable or parameter declaration.");
break;
case Location::Unspecified:
solAssert(!_variable.hasReferenceOrMappingType(), "Data location not properly set.");
}
Expand All @@ -497,6 +517,9 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
m_errorReporter.fatalTypeError(9259_error, _variable.location(), "Only constants of value type and byte array type are implemented.");
}

if (!type->isValueType())
solUnimplementedAssert(typeLoc != DataLocation::Transient, "Transient data location is only supported for value types.");

_variable.annotation().type = type;
}

Expand Down
4 changes: 3 additions & 1 deletion libsolidity/ast/AST.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,9 @@ std::set<VariableDeclaration::Location> VariableDeclaration::allowedDataLocation
{
using Location = VariableDeclaration::Location;

if (!hasReferenceOrMappingType() || isStateVariable() || isEventOrErrorParameter())
if (isStateVariable())
return std::set<Location>{Location::Unspecified, Location::Transient};
else if (!hasReferenceOrMappingType() || isEventOrErrorParameter())
return std::set<Location>{ Location::Unspecified };
else if (isCallableOrCatchParameter())
{
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/ast/AST.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ class FunctionDefinition: public CallableDeclaration, public StructurallyDocumen
class VariableDeclaration: public Declaration, public StructurallyDocumented
{
public:
enum Location { Unspecified, Storage, Memory, CallData };
enum Location { Unspecified, Storage, Transient, Memory, CallData };
enum class Mutability { Mutable, Immutable, Constant };
static std::string mutabilityToString(Mutability _mutability)
{
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/ast/ASTJsonExporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,8 @@ std::string ASTJsonExporter::location(VariableDeclaration::Location _location)
return "memory";
case VariableDeclaration::Location::CallData:
return "calldata";
case VariableDeclaration::Location::Transient:
return "transient";
}
// To make the compiler happy
return {};
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/ast/ASTJsonImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,8 @@ VariableDeclaration::Location ASTJsonImporter::location(Json const& _node)
return VariableDeclaration::Location::Memory;
else if (storageLocStr == "calldata")
return VariableDeclaration::Location::CallData;
else if (storageLocStr == "transient")
return VariableDeclaration::Location::Transient;
else
astAssert(false, "Unknown location declaration");

Expand Down
17 changes: 17 additions & 0 deletions libsolidity/ast/Types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,8 @@ TypeResult ReferenceType::unaryOperatorResult(Token _operator) const
return TypeProvider::emptyTuple();
case DataLocation::Storage:
return isPointer() ? nullptr : TypeProvider::emptyTuple();
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
}
return nullptr;
}
Expand Down Expand Up @@ -1571,6 +1573,9 @@ std::string ReferenceType::stringForReferencePart() const
return "calldata";
case DataLocation::Memory:
return "memory";
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
solAssert(false, "");
return "";
Expand All @@ -1584,6 +1589,9 @@ std::string ReferenceType::identifierLocationSuffix() const
case DataLocation::Storage:
id += "_storage";
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
id += "_memory";
break;
Expand Down Expand Up @@ -1748,6 +1756,9 @@ BoolResult ArrayType::validForLocation(DataLocation _loc) const
if (storageSizeUpperBound() >= bigint(1) << 256)
return BoolResult::err("Type too large for storage.");
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
return true;
}
Expand Down Expand Up @@ -1828,6 +1839,9 @@ std::vector<std::tuple<std::string, Type const*>> ArrayType::makeStackItems() co
case DataLocation::Storage:
// byte offset inside storage value is omitted
return {std::make_tuple("slot", TypeProvider::uint256())};
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
solAssert(false, "");
}
Expand Down Expand Up @@ -2568,6 +2582,9 @@ std::vector<std::tuple<std::string, Type const*>> StructType::makeStackItems() c
return {std::make_tuple("mpos", TypeProvider::uint256())};
case DataLocation::Storage:
return {std::make_tuple("slot", TypeProvider::uint256())};
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
solAssert(false, "");
}
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/ast/Types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ inline rational makeRational(bigint const& _numerator, bigint const& _denominato
return rational(_numerator, _denominator);
}

enum class DataLocation { Storage, CallData, Memory };
enum class DataLocation { Storage, Transient, CallData, Memory };


/**
Expand Down
6 changes: 6 additions & 0 deletions libsolidity/codegen/ArrayUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,9 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType, unsigned _stackDept
if (_arrayType.isByteArrayOrString())
m_context.callYulFunction(m_context.utilFunctions().extractByteArrayLengthFunction(), 1, 1);
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
}
}
Expand Down Expand Up @@ -1118,6 +1121,9 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType, bool _doBoundsCheck, b
m_context << endTag;
break;
}
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
}
}

Expand Down
9 changes: 9 additions & 0 deletions libsolidity/codegen/CompilerUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,9 @@ void CompilerUtils::convertType(
"Invalid conversion to storage type."
);
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
{
// Copy the array to a free position in memory, unless it is already in memory.
Expand Down Expand Up @@ -1168,6 +1171,9 @@ void CompilerUtils::convertType(
"Invalid conversion to storage type."
);
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
// Copy the array to a free position in memory, unless it is already in memory.
switch (typeOnStack.location())
Expand Down Expand Up @@ -1207,6 +1213,9 @@ void CompilerUtils::convertType(
conversionImpl(m_context);
break;
}
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::CallData:
{
if (typeOnStack.isDynamicallyEncoded())
Expand Down
6 changes: 6 additions & 0 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,9 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
ArrayUtils(m_context).retrieveLength(type);
m_context << Instruction::SWAP1 << Instruction::POP;
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
m_context << Instruction::MLOAD;
break;
Expand Down Expand Up @@ -2193,6 +2196,9 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
else
setLValueToStorageItem(_indexAccess);
break;
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
ArrayUtils(m_context).accessIndex(arrayType);
setLValue<MemoryItem>(_indexAccess, *_indexAccess.annotation().type, !arrayType.isByteArrayOrString());
Expand Down
3 changes: 3 additions & 0 deletions libsolidity/codegen/YulUtilFunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2563,6 +2563,9 @@ std::string YulUtilFunctions::nextArrayElementFunction(ArrayType const& _type)
templ("advance", toCompactHexWithPrefix(size));
break;
}
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::CallData:
{
u256 size = _type.calldataStride();
Expand Down
3 changes: 3 additions & 0 deletions libsolidity/codegen/ir/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2314,6 +2314,9 @@ void IRGeneratorForStatements::endVisit(IndexAccess const& _indexAccess)

break;
}
case DataLocation::Transient:
solUnimplementedAssert(false, "Transient data location is only supported for value types.");
break;
case DataLocation::Memory:
{
std::string const indexAccessFunction = m_utils.memoryArrayIndexAccessFunction(arrayType);
Expand Down
12 changes: 12 additions & 0 deletions libsolidity/formal/ModelChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ bool ModelChecker::isPragmaPresent(std::vector<std::shared_ptr<SourceUnit>> cons

void ModelChecker::checkRequestedSourcesAndContracts(std::vector<std::shared_ptr<SourceUnit>> const& _sources)
{
if (m_settings.engine.any())
{
struct TransientDataLocationChecker: ASTConstVisitor
Comment on lines +65 to +67
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this check would fit better in analyze() (somewhere after the if (m_settings.engine.none()) check there).

checkRequestedSourcesAndContracts() looks more like it was meant for validating settings, not sources. It does look at the AST but only to validate the contract names supplied in settings. Rejecting unsupported transient variables on the other hand feels more like it should be a part of the analysis process.

If you do it here, you're always checking it for all the sources. So you're making it impossible for the user to still use SMTChecker in presence of transient by just telling it to run on contracts that do not contain it. In fact, maybe it would even be better to move this check as far as CHC::visit(ContractDefinition) and BMC::visit(ContractDefinition) because otherwise we're still ignoring the contracts setting.

{
TransientDataLocationChecker(SourceUnit const& _sourceUnit) { _sourceUnit.accept(*this); }
void endVisit(VariableDeclaration const& _var) { solUnimplementedAssert(_var.referenceLocation() != VariableDeclaration::Location::Transient); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since unimplemented errors are not ICEs, we should provide a clear message for the user in each one.

};

for (auto const& source: _sources)
TransientDataLocationChecker checker(*source);
}

std::map<std::string, std::set<std::string>> exist;
for (auto const& source: _sources)
for (auto node: source->nodes())
Expand Down