diff --git a/Changelog.md b/Changelog.md index d1384095b..3ff3c2cd0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Compiler Features: Bugfixes: + * Type system: Detect if a contract's base uses types that require the experimental abi encoder while the contract still uses the old encoder Build System: diff --git a/liblangutil/ErrorReporter.cpp b/liblangutil/ErrorReporter.cpp index fb01847d5..0bf3a4167 100644 --- a/liblangutil/ErrorReporter.cpp +++ b/liblangutil/ErrorReporter.cpp @@ -118,6 +118,12 @@ bool ErrorReporter::checkForExcessiveErrors(Error::Type _type) return false; } +void ErrorReporter::fatalError(Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description) +{ + error(_type, _location, _secondaryLocation, _description); + BOOST_THROW_EXCEPTION(FatalError()); +} + void ErrorReporter::fatalError(Error::Type _type, SourceLocation const& _location, string const& _description) { error(_type, _location, _description); @@ -207,6 +213,15 @@ void ErrorReporter::typeError(SourceLocation const& _location, string const& _de ); } +void ErrorReporter::fatalTypeError(SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description) +{ + fatalError( + Error::Type::TypeError, + _location, + _secondaryLocation, + _description + ); +} void ErrorReporter::fatalTypeError(SourceLocation const& _location, string const& _description) { diff --git a/liblangutil/ErrorReporter.h b/liblangutil/ErrorReporter.h index d7cea4cda..8b70aabbf 100644 --- a/liblangutil/ErrorReporter.h +++ b/liblangutil/ErrorReporter.h @@ -104,6 +104,7 @@ public: } void fatalTypeError(SourceLocation const& _location, std::string const& _description); + void fatalTypeError(SourceLocation const& _location, SecondarySourceLocation const& _secondLocation, std::string const& _description); void docstringParsingError(std::string const& _description); @@ -118,12 +119,20 @@ public: } private: - void error(Error::Type _type, + void error( + Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, std::string const& _description = std::string()); - void fatalError(Error::Type _type, + void fatalError( + Error::Type _type, + SourceLocation const& _location, + SecondarySourceLocation const& _secondaryLocation, + std::string const& _description = std::string()); + + void fatalError( + Error::Type _type, SourceLocation const& _location = SourceLocation(), std::string const& _description = std::string()); diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 96b9611e5..1c6383b8b 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -44,6 +45,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract) checkExternalTypeClashes(_contract); checkHashCollisions(_contract); checkLibraryRequirements(_contract); + checkBaseABICompatibility(_contract); return Error::containsOnlyWarnings(m_errorReporter.errors()); } @@ -460,3 +462,50 @@ void ContractLevelChecker::checkLibraryRequirements(ContractDefinition const& _c if (!var->isConstant()) m_errorReporter.typeError(var->location(), "Library cannot have non-constant state variables"); } + +void ContractLevelChecker::checkBaseABICompatibility(ContractDefinition const& _contract) +{ + if (_contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2)) + return; + + if (_contract.isLibrary()) + { + solAssert( + _contract.baseContracts().empty() || m_errorReporter.hasErrors(), + "Library is not allowed to inherit" + ); + return; + } + + SecondarySourceLocation errors; + + // interfaceFunctionList contains all inherited functions as well + for (auto const& func: _contract.interfaceFunctionList()) + { + solAssert(func.second->hasDeclaration(), "Function has no declaration?!"); + + if (!func.second->declaration().sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2)) + continue; + + auto const& currentLoc = func.second->declaration().location(); + + for (TypePointer const& paramType: func.second->parameterTypes() + func.second->parameterTypes()) + if (!TypeChecker::typeSupportedByOldABIEncoder(*paramType, false)) + { + errors.append("Type only supported by the new experimental ABI encoder", currentLoc); + break; + } + } + + if (!errors.infos.empty()) + m_errorReporter.fatalTypeError( + _contract.location(), + errors, + std::string("Contract \"") + + _contract.name() + + "\" does not use the new experimental ABI encoder but wants to inherit from a contract " + + "which uses types that require it. " + + "Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature." + ); + +} diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index d754687ae..f2cd9887f 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -78,6 +78,8 @@ private: void checkHashCollisions(ContractDefinition const& _contract); /// Checks that all requirements for a library are fulfilled if this is a library. void checkLibraryRequirements(ContractDefinition const& _contract); + /// Checks base contracts for ABI compatibility + void checkBaseABICompatibility(ContractDefinition const& _contract); langutil::ErrorReporter& m_errorReporter; }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 5378f15fb..0109a7b03 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -45,10 +45,7 @@ using namespace dev; using namespace langutil; using namespace dev::solidity; -namespace -{ - -bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) +bool TypeChecker::typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) { if (_isLibraryCall && _type.dataStoredIn(DataLocation::Storage)) return true; @@ -64,9 +61,6 @@ bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall) return true; } -} - - bool TypeChecker::checkTypeRequirements(ASTNode const& _contract) { _contract.accept(*this); @@ -94,6 +88,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract) for (auto const& n: _contract.subNodes()) n->accept(*this); + return false; } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 09879322e..8f532a89c 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -63,6 +63,8 @@ public: /// (this can happen for variables with non-explicit types before their types are resolved) TypePointer const& type(VariableDeclaration const& _variable) const; + static bool typeSupportedByOldABIEncoder(Type const& _type, bool _isLibraryCall); + private: bool visit(ContractDefinition const& _contract) override; diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 60cbc7dce..62170284c 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -429,7 +429,6 @@ private: std::vector> m_subNodes; ContractKind m_contractKind; - std::vector m_linearizedBaseContracts; mutable std::unique_ptr, FunctionTypePointer>>> m_interfaceFunctionList; mutable std::unique_ptr> m_interfaceEvents; mutable std::unique_ptr> m_inheritableMembers; @@ -589,6 +588,7 @@ public: } std::vector> const& parameters() const { return m_parameters->parameters(); } + std::vector> const& returnParameters() const { return m_returnParameters->parameters(); } ParameterList const& parameterList() const { return *m_parameters; } ASTPointer const& returnParameterList() const { return m_returnParameters; } @@ -629,7 +629,6 @@ public: bool isFallback() const { return !m_isConstructor && name().empty(); } bool isPayable() const { return m_stateMutability == StateMutability::Payable; } std::vector> const& modifiers() const { return m_functionModifiers; } - std::vector> const& returnParameters() const { return m_returnParameters->parameters(); } Block const& body() const { solAssert(m_body, ""); return *m_body; } bool isVisibleInContract() const override { diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index abd659b61..ebdb46545 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -320,6 +320,153 @@ BOOST_AUTO_TEST_CASE(shadowing_builtins_with_alias) } } +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_mismatch_1) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(!c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + + string const* msg = e->comment(); + BOOST_REQUIRE(msg); + BOOST_CHECK_EQUAL(*msg, std::string("Contract \"C\" does not use the new experimental ABI encoder but wants to inherit from a contract which uses types that require it. Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature.")); + } + BOOST_CHECK_EQUAL(typeErrors, 1); +} + +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_mismatch_2) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(!c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + + string const* msg = e->comment(); + BOOST_REQUIRE(msg); + BOOST_CHECK_EQUAL(*msg, std::string("Contract \"B\" does not use the new experimental ABI encoder but wants to inherit from a contract which uses types that require it. Use \"pragma experimental ABIEncoderV2;\" for the inheriting contract as well to enable the feature.")); + } + BOOST_CHECK_EQUAL(typeErrors, 1); +} + +BOOST_AUTO_TEST_CASE(inheritance_abi_encoder_match) +{ + CompilerStack c; + c.addSource("A.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + contract A + { + struct S { uint a; } + S public s; + function f(S memory _s) public returns (S memory,S memory) { } + } + )"); + + c.addSource("B.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./A.sol"; + contract B is A { } + )"); + + c.addSource("C.sol", R"( + pragma solidity >=0.0; + pragma experimental ABIEncoderV2; + + import "./B.sol"; + contract C is B { } + )"); + + c.setEVMVersion(dev::test::Options::get().evmVersion()); + BOOST_CHECK(c.compile()); + + int typeErrors = 0; + + // Sometimes we get the prerelease warning, sometimes not. + for (auto const& e: c.errors()) + { + if (e->type() != langutil::Error::Type::TypeError) + continue; + + typeErrors++; + } + + BOOST_CHECK_EQUAL(typeErrors, 0); +} + BOOST_AUTO_TEST_SUITE_END() }