diff options
-rw-r--r-- | libsolidity/analysis/DeclarationContainer.cpp | 33 | ||||
-rw-r--r-- | libsolidity/analysis/DeclarationContainer.h | 6 | ||||
-rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.cpp | 54 | ||||
-rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.h | 10 | ||||
-rw-r--r-- | libsolidity/analysis/ReferencesResolver.cpp | 10 | ||||
-rw-r--r-- | libsolidity/analysis/ReferencesResolver.h | 1 | ||||
-rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 20 | ||||
-rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 75 |
8 files changed, 176 insertions, 33 deletions
diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index 7508ad9e..c7ba78d6 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -79,6 +79,17 @@ Declaration const* DeclarationContainer::conflictingDeclaration( return nullptr; } +void DeclarationContainer::activateVariable(ASTString const& _name) +{ + solAssert( + m_invisibleDeclarations.count(_name) && m_invisibleDeclarations.at(_name).size() == 1, + "Tried to activate a non-inactive variable or multiple inactive variables with the same name." + ); + solAssert(m_declarations.count(_name) == 0 || m_declarations.at(_name).empty(), ""); + m_declarations[_name].emplace_back(m_invisibleDeclarations.at(_name).front()); + m_invisibleDeclarations.erase(_name); +} + bool DeclarationContainer::registerDeclaration( Declaration const& _declaration, ASTString const* _name, @@ -106,15 +117,17 @@ bool DeclarationContainer::registerDeclaration( return true; } -vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive) const +vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive, bool _alsoInvisible) const { solAssert(!_name.empty(), "Attempt to resolve empty name."); - auto result = m_declarations.find(_name); - if (result != m_declarations.end()) - return result->second; - if (_recursive && m_enclosingContainer) - return m_enclosingContainer->resolveName(_name, true); - return vector<Declaration const*>({}); + vector<Declaration const*> result; + if (m_declarations.count(_name)) + result = m_declarations.at(_name); + if (_alsoInvisible && m_invisibleDeclarations.count(_name)) + result += m_invisibleDeclarations.at(_name); + if (result.empty() && _recursive && m_enclosingContainer) + result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible); + return result; } vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) const @@ -129,6 +142,12 @@ vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) con if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) similar.push_back(declarationName); } + for (auto const& declaration: m_invisibleDeclarations) + { + string const& declarationName = declaration.first; + if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) + similar.push_back(declarationName); + } if (m_enclosingContainer) similar += m_enclosingContainer->similarNames(_name); diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h index f9b1bda4..e4b3320a 100644 --- a/libsolidity/analysis/DeclarationContainer.h +++ b/libsolidity/analysis/DeclarationContainer.h @@ -51,13 +51,17 @@ public: /// @param _update if true, replaces a potential declaration that is already present /// @returns false if the name was already declared. bool registerDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr, bool _invisible = false, bool _update = false); - std::vector<Declaration const*> resolveName(ASTString const& _name, bool _recursive = false) const; + std::vector<Declaration const*> resolveName(ASTString const& _name, bool _recursive = false, bool _alsoInvisible = false) const; ASTNode const* enclosingNode() const { return m_enclosingNode; } DeclarationContainer const* enclosingContainer() const { return m_enclosingContainer; } std::map<ASTString, std::vector<Declaration const*>> const& declarations() const { return m_declarations; } /// @returns whether declaration is valid, and if not also returns previous declaration. Declaration const* conflictingDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr) const; + /// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for + /// VariableDeclarationStatements. + void activateVariable(ASTString const& _name); + /// @returns existing declaration names similar to @a _name. /// Searches this and all parent containers. std::vector<ASTString> similarNames(ASTString const& _name) const; diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 40021771..2f675135 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -50,12 +50,13 @@ NameAndTypeResolver::NameAndTypeResolver( m_scopes[nullptr]->registerDeclaration(*declaration); } -bool NameAndTypeResolver::registerDeclarations(ASTNode& _sourceUnit, ASTNode const* _currentScope) +bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope) { + bool useC99Scoping = _sourceUnit.annotation().experimentalFeatures.count(ExperimentalFeature::V050); // The helper registers all declarations in m_scopes as a side-effect of its construction. try { - DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errorReporter, _currentScope); + DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, useC99Scoping, m_errorReporter, _currentScope); } catch (FatalError const&) { @@ -106,7 +107,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So else for (Declaration const* declaration: declarations) if (!DeclarationRegistrationHelper::registerDeclaration( - target, *declaration, alias.second.get(), &imp->location(), true, m_errorReporter + target, *declaration, alias.second.get(), &imp->location(), true, false, m_errorReporter )) error = true; } @@ -114,7 +115,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So for (auto const& nameAndDeclaration: scope->second->declarations()) for (auto const& declaration: nameAndDeclaration.second) if (!DeclarationRegistrationHelper::registerDeclaration( - target, *declaration, &nameAndDeclaration.first, &imp->location(), true, m_errorReporter + target, *declaration, &nameAndDeclaration.first, &imp->location(), true, false, m_errorReporter )) error = true; } @@ -151,6 +152,12 @@ bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration) return true; } +void NameAndTypeResolver::activateVariable(string const& _name) +{ + solAssert(m_currentScope, ""); + m_currentScope->activateVariable(_name); +} + vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _name, ASTNode const* _scope) const { auto iterator = m_scopes.find(_scope); @@ -159,9 +166,9 @@ vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _na return iterator->second->resolveName(_name, false); } -vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name) const +vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles) const { - return m_currentScope->resolveName(_name, true); + return m_currentScope->resolveName(_name, true, _includeInvisibles); } Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path) const @@ -229,7 +236,7 @@ void NameAndTypeResolver::warnVariablesNamedLikeInstructions() for (auto const& instruction: c_instructions) { string const instructionName{boost::algorithm::to_lower_copy(instruction.first)}; - auto declarations = nameFromCurrentScope(instructionName); + auto declarations = nameFromCurrentScope(instructionName, true); for (Declaration const* const declaration: declarations) { solAssert(!!declaration, ""); @@ -439,9 +446,11 @@ string NameAndTypeResolver::similarNameSuggestions(ASTString const& _name) const DeclarationRegistrationHelper::DeclarationRegistrationHelper( map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes, ASTNode& _astRoot, + bool _useC99Scoping, ErrorReporter& _errorReporter, ASTNode const* _currentScope ): + m_useC99Scoping(_useC99Scoping), m_scopes(_scopes), m_currentScope(_currentScope), m_errorReporter(_errorReporter) @@ -456,6 +465,7 @@ bool DeclarationRegistrationHelper::registerDeclaration( string const* _name, SourceLocation const* _errorLocation, bool _warnOnShadow, + bool _inactive, ErrorReporter& _errorReporter ) { @@ -465,10 +475,13 @@ bool DeclarationRegistrationHelper::registerDeclaration( string name = _name ? *_name : _declaration.name(); Declaration const* shadowedDeclaration = nullptr; if (_warnOnShadow && !name.empty() && _container.enclosingContainer()) - for (auto const* decl: _container.enclosingContainer()->resolveName(name, true)) + for (auto const* decl: _container.enclosingContainer()->resolveName(name, true, true)) shadowedDeclaration = decl; - if (!_container.registerDeclaration(_declaration, _name, !_declaration.isVisibleInContract())) + // We use "invisible" for both inactive variables in blocks and for members invisible in contracts. + // They cannot both be true at the same time. + solAssert(!(_inactive && !_declaration.isVisibleInContract()), ""); + if (!_container.registerDeclaration(_declaration, _name, !_declaration.isVisibleInContract() || _inactive)) { SourceLocation firstDeclarationLocation; SourceLocation secondDeclarationLocation; @@ -613,32 +626,28 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&) bool DeclarationRegistrationHelper::visit(Block& _block) { _block.setScope(m_currentScope); - // Enable C99-scoped variables. - if (_block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) + if (m_useC99Scoping) enterNewSubScope(_block); return true; } -void DeclarationRegistrationHelper::endVisit(Block& _block) +void DeclarationRegistrationHelper::endVisit(Block&) { - // Enable C99-scoped variables. - if (_block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) + if (m_useC99Scoping) closeCurrentScope(); } bool DeclarationRegistrationHelper::visit(ForStatement& _for) { _for.setScope(m_currentScope); - if (_for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) - // TODO special scoping rules for the init statement - if it is a block, then it should - // not open its own scope. + if (m_useC99Scoping) enterNewSubScope(_for); return true; } -void DeclarationRegistrationHelper::endVisit(ForStatement& _for) +void DeclarationRegistrationHelper::endVisit(ForStatement&) { - if (_for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) + if (m_useC99Scoping) closeCurrentScope(); } @@ -704,7 +713,12 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio if (fun->isConstructor()) warnAboutShadowing = false; - registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, m_errorReporter); + // Register declaration as inactive if we are in block scope and C99 mode. + bool inactive = + m_useC99Scoping && + (dynamic_cast<Block const*>(m_currentScope) || dynamic_cast<ForStatement const*>(m_currentScope)); + + registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter); _declaration.setScope(m_currentScope); if (_opensScope) diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 7a6e813e..3d10fbd8 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -56,7 +56,7 @@ public: /// @returns false in case of error. /// @param _currentScope should be nullptr but can be used to inject new declarations into /// existing scopes, used by the snippets feature. - bool registerDeclarations(ASTNode& _sourceUnit, ASTNode const* _currentScope = nullptr); + bool registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope = nullptr); /// Applies the effect of import directives. bool performImports(SourceUnit& _sourceUnit, std::map<std::string, SourceUnit const*> const& _sourceUnits); /// Resolves all names and types referenced from the given AST Node. @@ -69,6 +69,9 @@ public: /// that create their own scope. /// @returns false in case of error. bool updateDeclaration(Declaration const& _declaration); + /// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for + /// VariableDeclarationStatements. + void activateVariable(std::string const& _name); /// Resolves the given @a _name inside the scope @a _scope. If @a _scope is omitted, /// the global scope is used (i.e. the one containing only the pre-defined global variables). @@ -78,7 +81,7 @@ public: /// Resolves a name in the "current" scope, but also searches parent scopes. /// Should only be called during the initial resolving phase. - std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name) const; + std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles = false) const; /// Resolves a path starting from the "current" scope, but also searches parent scopes. /// Should only be called during the initial resolving phase. @@ -139,6 +142,7 @@ public: DeclarationRegistrationHelper( std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes, ASTNode& _astRoot, + bool _useC99Scoping, ErrorReporter& _errorReporter, ASTNode const* _currentScope = nullptr ); @@ -149,6 +153,7 @@ public: std::string const* _name, SourceLocation const* _errorLocation, bool _warnOnShadow, + bool _inactive, ErrorReporter& _errorReporter ); @@ -185,6 +190,7 @@ private: /// @returns the canonical name of the current scope. std::string currentCanonicalName() const; + bool m_useC99Scoping = false; std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& m_scopes; ASTNode const* m_currentScope = nullptr; VariableScope* m_currentFunction = nullptr; diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 4d919f2b..985c44d0 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -83,6 +83,16 @@ void ReferencesResolver::endVisit(ForStatement const& _for) m_resolver.setScope(_for.scope()); } +void ReferencesResolver::endVisit(VariableDeclarationStatement const& _varDeclStatement) +{ + if (!m_resolveInsideCode) + return; + if (m_experimental050Mode) + for (auto const& var: _varDeclStatement.declarations()) + if (var) + m_resolver.activateVariable(var->name()); +} + bool ReferencesResolver::visit(Identifier const& _identifier) { auto declarations = m_resolver.nameFromCurrentScope(_identifier.name()); diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index ab7c987e..4e8f54b5 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -61,6 +61,7 @@ private: virtual void endVisit(Block const& _block) override; virtual bool visit(ForStatement const& _for) override; virtual void endVisit(ForStatement const& _for) override; + virtual void endVisit(VariableDeclarationStatement const& _varDeclStatement) override; virtual bool visit(Identifier const& _identifier) override; virtual bool visit(ElementaryTypeName const& _typeName) override; virtual bool visit(FunctionDefinition const& _functionDefinition) override; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 3882e4ea..7bae3cd6 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -284,6 +284,26 @@ BOOST_AUTO_TEST_CASE(conditional_expression_functions) ABI_CHECK(callContractFunction("f(bool)", false), encodeArgs(u256(2))); } +BOOST_AUTO_TEST_CASE(C99_scoping_activation) +{ + char const* sourceCode = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public returns (uint) { + uint x = 7; + { + x = 3; // This should still assign to the outer variable + uint x; + x = 4; // This should assign to the new one + } + return x; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()"), encodeArgs(3)); +} + BOOST_AUTO_TEST_CASE(recursive_calls) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 8586ebf8..e5bd0103 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -135,19 +135,59 @@ BOOST_AUTO_TEST_CASE(scoping) CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); } -BOOST_AUTO_TEST_CASE(scoping_for) +BOOST_AUTO_TEST_CASE(scoping_activation_old) +{ + char const* text = R"( + contract test { + function f() pure public { + x = 3; + uint x; + } + } + )"; + CHECK_SUCCESS_NO_WARNINGS(text); +} + +BOOST_AUTO_TEST_CASE(scoping_activation) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { + x = 3; + uint x; + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); +} + +BOOST_AUTO_TEST_CASE(scoping_self_use) { char const* text = R"( pragma experimental "v0.5.0"; contract test { function f() public { + uint a = a; + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); +} + +BOOST_AUTO_TEST_CASE(scoping_for) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { for (uint x = 0; x < 10; x ++){ x = 2; } } } )"; - CHECK_SUCCESS(text); + CHECK_WARNING(text, "Experimental features"); } BOOST_AUTO_TEST_CASE(scoping_for2) @@ -155,7 +195,21 @@ BOOST_AUTO_TEST_CASE(scoping_for2) char const* text = R"( pragma experimental "v0.5.0"; contract test { - function f() public { + function f() pure public { + for (uint x = 0; x < 10; x ++) + x = 2; + } + } + )"; + CHECK_WARNING(text, "Experimental features"); +} + +BOOST_AUTO_TEST_CASE(scoping_for3) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { for (uint x = 0; x < 10; x ++){ x = 2; } @@ -166,6 +220,21 @@ BOOST_AUTO_TEST_CASE(scoping_for2) CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); } +BOOST_AUTO_TEST_CASE(scoping_for_decl_in_body) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { + for (;; y++){ + uint y = 3; + } + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); +} + BOOST_AUTO_TEST_CASE(name_shadowing) { char const* text = R"( |