diff options
| -rw-r--r-- | Changelog.md | 1 | ||||
| -rw-r--r-- | docs/control-structures.rst | 61 | ||||
| -rw-r--r-- | libsolidity/analysis/DeclarationContainer.cpp | 33 | ||||
| -rw-r--r-- | libsolidity/analysis/DeclarationContainer.h | 6 | ||||
| -rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.cpp | 93 | ||||
| -rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.h | 30 | ||||
| -rw-r--r-- | libsolidity/analysis/ReferencesResolver.cpp | 50 | ||||
| -rw-r--r-- | libsolidity/analysis/ReferencesResolver.h | 7 | ||||
| -rw-r--r-- | libsolidity/ast/AST.cpp | 41 | ||||
| -rw-r--r-- | libsolidity/ast/AST.h | 47 | ||||
| -rw-r--r-- | test/libsolidity/SMTChecker.cpp | 6 | ||||
| -rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 48 | ||||
| -rw-r--r-- | test/libsolidity/SolidityExpressionCompiler.cpp | 19 | ||||
| -rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 245 |
14 files changed, 598 insertions, 89 deletions
diff --git a/Changelog.md b/Changelog.md index acd3aa54..db4f9786 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.4.21 (unreleased) Features: + * C99/C++-style scoping rules (instead of JavaScript function scoping) take effect as experimental v0.5.0 feature. * Code Generator: Assert that ``k != 0`` for ``molmod(a, b, k)`` and ``addmod(a, b, k)`` as experimental 0.5.0 feature. * Standard JSON: Reject badly formatted invalid JSON inputs. * Type Checker: Disallow uninitialized storage pointers as experimental 0.5.0 feature. diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 7be92cfa..46e076e5 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -324,7 +324,8 @@ is ``false``. The default value for the ``uint`` or ``int`` types is ``0``. For element will be initialized to the default value corresponding to its type. Finally, for dynamically-sized arrays, ``bytes`` and ``string``, the default value is an empty array or string. -A variable declared anywhere within a function will be in scope for the *entire function*, regardless of where it is declared. +A variable declared anywhere within a function will be in scope for the *entire function*, regardless of where it is declared +(this will change soon, see below). This happens because Solidity inherits its scoping rules from JavaScript. This is in contrast to many languages where variables are only scoped where they are declared until the end of the semantic block. As a result, the following code is illegal and cause the compiler to throw an error, ``Identifier already declared``:: @@ -366,7 +367,9 @@ As a result, the following code is illegal and cause the compiler to throw an er } In addition to this, if a variable is declared, it will be initialized at the beginning of the function to its default value. -As a result, the following code is legal, despite being poorly written:: +As a result, the following code is legal, despite being poorly written: + +:: pragma solidity ^0.4.0; @@ -383,6 +386,60 @@ As a result, the following code is legal, despite being poorly written:: } } +Scoping starting from Version 0.5.0 +----------------------------------- + +Starting from version 0.5.0, Solidity will change to the more widespread scoping rules of C99 +(and many other languages): Variables are visible from the point right after their declaration +until the end of a ``{ }``-block. As an exception to this rule, variables declared in the +initialization part of a for-loop are only visible until the end of the for-loop. + +Variables and other items declared outside of a code block, for example functions, contracts, +user-defined types, etc., do not change their scoping behaviour. This means you can +use state variables before they are declared and call functions recursively. + +These rules are already introduced now as an experimental feature. + +As a consequence, the following examples will compile without warnings, since +the two variables have the same name but disjoint scopes. In non-0.5.0-mode, +they have the same scope (the function ``minimalScoping``) and thus it does +not compile there. + +:: + + pragma solidity ^0.4.0; + pragma experimental "v0.5.0"; + contract C { + function minimalScoping() pure public { + { + uint same2 = 0; + } + + { + uint same2 = 0; + } + } + } + +As a special example of the C99 scoping rules, note that in the following, +the first assignment to ``x`` will actually assign the outer and not the inner variable. +In any case, you will get a warning about the outer variable being shadowed. + +:: + + pragma solidity ^0.4.0; + pragma experimental "v0.5.0"; + contract C { + function f() pure public returns (uint) { + uint x = 1; + { + x = 2; // this will assign to the outer variable + uint x; + } + return x; // x has value 2 + } + } + .. index:: ! exception, ! throw, ! assert, ! require, ! revert Error handling: Assert, Require, Revert and Exceptions 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 662792a3..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,15 +166,15 @@ vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _na return iterator->second->resolveName(_name, false); } -vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _recursive) const +vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles) const { - return m_currentScope->resolveName(_name, _recursive); + return m_currentScope->resolveName(_name, true, _includeInvisibles); } -Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path, bool _recursive) const +Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path) const { solAssert(!_path.empty(), ""); - vector<Declaration const*> candidates = m_currentScope->resolveName(_path.front(), _recursive); + vector<Declaration const*> candidates = m_currentScope->resolveName(_path.front(), true); for (size_t i = 1; i < _path.size() && candidates.size() == 1; i++) { if (!m_scopes.count(candidates.front())) @@ -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, ""); @@ -244,19 +251,24 @@ void NameAndTypeResolver::warnVariablesNamedLikeInstructions() } } +void NameAndTypeResolver::setScope(ASTNode const* _node) +{ + m_currentScope = m_scopes[_node].get(); +} + bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode) { if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(&_node)) { bool success = true; - m_currentScope = m_scopes[contract->scope()].get(); + setScope(contract->scope()); solAssert(!!m_currentScope, ""); for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts()) if (!resolveNamesAndTypes(*baseContract, true)) success = false; - m_currentScope = m_scopes[contract].get(); + setScope(contract); if (success) { @@ -273,7 +285,7 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res // these can contain code, only resolve parameters for now for (ASTPointer<ASTNode> const& node: contract->subNodes()) { - m_currentScope = m_scopes[contract].get(); + setScope(contract); if (!resolveNamesAndTypes(*node, false)) { success = false; @@ -287,12 +299,12 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res if (!_resolveInsideCode) return success; - m_currentScope = m_scopes[contract].get(); + setScope(contract); // now resolve references inside the code for (ASTPointer<ASTNode> const& node: contract->subNodes()) { - m_currentScope = m_scopes[contract].get(); + setScope(contract); if (!resolveNamesAndTypes(*node, true)) success = false; } @@ -301,7 +313,7 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res else { if (m_scopes.count(&_node)) - m_currentScope = m_scopes[&_node].get(); + setScope(&_node); return ReferencesResolver(m_errorReporter, *this, _resolveInsideCode).resolve(_node); } } @@ -434,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) @@ -451,6 +465,7 @@ bool DeclarationRegistrationHelper::registerDeclaration( string const* _name, SourceLocation const* _errorLocation, bool _warnOnShadow, + bool _inactive, ErrorReporter& _errorReporter ) { @@ -460,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; @@ -605,6 +623,34 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&) closeCurrentScope(); } +bool DeclarationRegistrationHelper::visit(Block& _block) +{ + _block.setScope(m_currentScope); + if (m_useC99Scoping) + enterNewSubScope(_block); + return true; +} + +void DeclarationRegistrationHelper::endVisit(Block&) +{ + if (m_useC99Scoping) + closeCurrentScope(); +} + +bool DeclarationRegistrationHelper::visit(ForStatement& _for) +{ + _for.setScope(m_currentScope); + if (m_useC99Scoping) + enterNewSubScope(_for); + return true; +} + +void DeclarationRegistrationHelper::endVisit(ForStatement&) +{ + if (m_useC99Scoping) + closeCurrentScope(); +} + void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _variableDeclarationStatement) { // Register the local variables with the function @@ -632,14 +678,14 @@ void DeclarationRegistrationHelper::endVisit(EventDefinition&) closeCurrentScope(); } -void DeclarationRegistrationHelper::enterNewSubScope(Declaration const& _declaration) +void DeclarationRegistrationHelper::enterNewSubScope(ASTNode& _subScope) { map<ASTNode const*, shared_ptr<DeclarationContainer>>::iterator iter; bool newlyAdded; shared_ptr<DeclarationContainer> container(new DeclarationContainer(m_currentScope, m_scopes[m_currentScope].get())); - tie(iter, newlyAdded) = m_scopes.emplace(&_declaration, move(container)); + tie(iter, newlyAdded) = m_scopes.emplace(&_subScope, move(container)); solAssert(newlyAdded, "Unable to add new scope."); - m_currentScope = &_declaration; + m_currentScope = &_subScope; } void DeclarationRegistrationHelper::closeCurrentScope() @@ -667,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 9aea07ab..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,20 +69,24 @@ 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). /// @returns a pointer to the declaration on success or nullptr on failure. + /// SHOULD only be used for testing. std::vector<Declaration const*> resolveName(ASTString const& _name, ASTNode const* _scope = nullptr) const; - /// Resolves a name in the "current" scope. Should only be called during the initial - /// resolving phase. - std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name, bool _recursive = true) const; + /// 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, bool _includeInvisibles = false) const; - /// Resolves a path starting from the "current" scope. Should only be called during the initial - /// resolving phase. + /// Resolves a path starting from the "current" scope, but also searches parent scopes. + /// Should only be called during the initial resolving phase. /// @note Returns a null pointer if any component in the path was not unique or not found. - Declaration const* pathFromCurrentScope(std::vector<ASTString> const& _path, bool _recursive = true) const; + Declaration const* pathFromCurrentScope(std::vector<ASTString> const& _path) const; /// returns the vector of declarations without repetitions std::vector<Declaration const*> cleanedDeclarations( @@ -96,6 +100,9 @@ public: /// @returns a list of similar identifiers in the current and enclosing scopes. May return empty string if no suggestions. std::string similarNameSuggestions(ASTString const& _name) const; + /// Sets the current scope. + void setScope(ASTNode const* _node); + private: /// Internal version of @a resolveNamesAndTypes (called from there) throws exceptions on fatal errors. bool resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode = true); @@ -135,6 +142,7 @@ public: DeclarationRegistrationHelper( std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes, ASTNode& _astRoot, + bool _useC99Scoping, ErrorReporter& _errorReporter, ASTNode const* _currentScope = nullptr ); @@ -145,6 +153,7 @@ public: std::string const* _name, SourceLocation const* _errorLocation, bool _warnOnShadow, + bool _inactive, ErrorReporter& _errorReporter ); @@ -163,12 +172,16 @@ private: void endVisit(FunctionDefinition& _function) override; bool visit(ModifierDefinition& _modifier) override; void endVisit(ModifierDefinition& _modifier) override; + bool visit(Block& _block) override; + void endVisit(Block& _block) override; + bool visit(ForStatement& _forLoop) override; + void endVisit(ForStatement& _forLoop) override; void endVisit(VariableDeclarationStatement& _variableDeclarationStatement) override; bool visit(VariableDeclaration& _declaration) override; bool visit(EventDefinition& _event) override; void endVisit(EventDefinition& _event) override; - void enterNewSubScope(Declaration const& _declaration); + void enterNewSubScope(ASTNode& _subScope); void closeCurrentScope(); void registerDeclaration(Declaration& _declaration, bool _opensScope); @@ -177,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 0bb5e3fe..985c44d0 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -43,6 +43,56 @@ bool ReferencesResolver::resolve(ASTNode const& _root) return !m_errorOccurred; } +bool ReferencesResolver::visit(Block const& _block) +{ + if (!m_resolveInsideCode) + return false; + m_experimental050Mode = _block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); + // C99-scoped variables + if (m_experimental050Mode) + m_resolver.setScope(&_block); + return true; +} + +void ReferencesResolver::endVisit(Block const& _block) +{ + if (!m_resolveInsideCode) + return; + + // C99-scoped variables + if (m_experimental050Mode) + m_resolver.setScope(_block.scope()); +} + +bool ReferencesResolver::visit(ForStatement const& _for) +{ + if (!m_resolveInsideCode) + return false; + m_experimental050Mode = _for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); + // C99-scoped variables + if (m_experimental050Mode) + m_resolver.setScope(&_for); + return true; +} + +void ReferencesResolver::endVisit(ForStatement const& _for) +{ + if (!m_resolveInsideCode) + return; + if (m_experimental050Mode) + 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 fef2e73f..4e8f54b5 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -57,7 +57,11 @@ public: bool resolve(ASTNode const& _root); private: - virtual bool visit(Block const&) override { return m_resolveInsideCode; } + virtual bool visit(Block const& _block) override; + 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; @@ -90,6 +94,7 @@ private: std::vector<ParameterList const*> m_returnParameters; bool const m_resolveInsideCode; bool m_errorOccurred = false; + bool m_experimental050Mode = false; }; } diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 8da6964e..27220b1f 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -96,20 +96,6 @@ set<SourceUnit const*> SourceUnit::referencedSourceUnits(bool _recurse, set<Sour return sourceUnits; } -SourceUnit const& Declaration::sourceUnit() const -{ - solAssert(!!m_scope, ""); - ASTNode const* scope = m_scope; - while (dynamic_cast<Declaration const*>(scope) && dynamic_cast<Declaration const*>(scope)->m_scope) - scope = dynamic_cast<Declaration const*>(scope)->m_scope; - return dynamic_cast<SourceUnit const&>(*scope); -} - -string Declaration::sourceUnitName() const -{ - return sourceUnit().annotation().path; -} - ImportAnnotation& ImportDirective::annotation() const { if (!m_annotation) @@ -408,12 +394,36 @@ UserDefinedTypeNameAnnotation& UserDefinedTypeName::annotation() const return dynamic_cast<UserDefinedTypeNameAnnotation&>(*m_annotation); } +SourceUnit const& Scopable::sourceUnit() const +{ + ASTNode const* s = scope(); + solAssert(s, ""); + // will not always be a declaratoion + while (dynamic_cast<Scopable const*>(s) && dynamic_cast<Scopable const*>(s)->scope()) + s = dynamic_cast<Scopable const*>(s)->scope(); + return dynamic_cast<SourceUnit const&>(*s); +} + +string Scopable::sourceUnitName() const +{ + return sourceUnit().annotation().path; +} + bool VariableDeclaration::isLValue() const { // External function parameters and constant declared variables are Read-Only return !isExternalCallableParameter() && !m_isConstant; } +bool VariableDeclaration::isLocalVariable() const +{ + auto s = scope(); + return + dynamic_cast<CallableDeclaration const*>(s) || + dynamic_cast<Block const*>(s) || + dynamic_cast<ForStatement const*>(s); +} + bool VariableDeclaration::isCallableParameter() const { auto const* callable = dynamic_cast<CallableDeclaration const*>(scope()); @@ -459,8 +469,7 @@ bool VariableDeclaration::isExternalCallableParameter() const bool VariableDeclaration::canHaveAutoType() const { - auto const* callable = dynamic_cast<CallableDeclaration const*>(scope()); - return (!!callable && !isCallableParameter()); + return isLocalVariable() && !isCallableParameter(); } TypePointer VariableDeclaration::type() const diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index c0d55aec..863ad2fe 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -140,9 +140,32 @@ private: }; /** + * Abstract class that is added to each AST node that is stored inside a scope + * (including scopes). + */ +class Scopable +{ +public: + /// @returns the scope this declaration resides in. Can be nullptr if it is the global scope. + /// Available only after name and type resolution step. + ASTNode const* scope() const { return m_scope; } + void setScope(ASTNode const* _scope) { m_scope = _scope; } + + /// @returns the source unit this scopable is present in. + SourceUnit const& sourceUnit() const; + + /// @returns the source name this scopable is present in. + /// Can be combined with annotation().canonicalName (if present) to form a globally unique name. + std::string sourceUnitName() const; + +protected: + ASTNode const* m_scope = nullptr; +}; + +/** * Abstract AST class for a declaration (contract, function, struct, variable, import directive). */ -class Declaration: public ASTNode +class Declaration: public ASTNode, public Scopable { public: /// Visibility ordered from restricted to unrestricted. @@ -171,7 +194,7 @@ public: ASTPointer<ASTString> const& _name, Visibility _visibility = Visibility::Default ): - ASTNode(_location), m_name(_name), m_visibility(_visibility), m_scope(nullptr) {} + ASTNode(_location), m_name(_name), m_visibility(_visibility) {} /// @returns the declared name. ASTString const& name() const { return *m_name; } @@ -181,17 +204,6 @@ public: virtual bool isVisibleInContract() const { return visibility() != Visibility::External; } bool isVisibleInDerivedContracts() const { return isVisibleInContract() && visibility() >= Visibility::Internal; } - /// @returns the scope this declaration resides in. Can be nullptr if it is the global scope. - /// Available only after name and type resolution step. - ASTNode const* scope() const { return m_scope; } - void setScope(ASTNode const* _scope) { m_scope = _scope; } - - /// @returns the source unit this declaration is present in. - SourceUnit const& sourceUnit() const; - - /// @returns the source name this declaration is present in. - /// Can be combined with annotation().canonicalName to form a globally unique name. - std::string sourceUnitName() const; std::string fullyQualifiedName() const { return sourceUnitName() + ":" + name(); } virtual bool isLValue() const { return false; } @@ -213,7 +225,6 @@ protected: private: ASTPointer<ASTString> m_name; Visibility m_visibility; - ASTNode const* m_scope; }; /** @@ -289,6 +300,8 @@ private: /** * Abstract class that is added to each AST node that can store local variables. + * Local variables in functions are always added to functions, even though they are not + * in scope for the whole function. */ class VariableScope { @@ -662,7 +675,7 @@ public: virtual bool isLValue() const override; virtual bool isPartOfExternalInterface() const override { return isPublic(); } - bool isLocalVariable() const { return !!dynamic_cast<CallableDeclaration const*>(scope()); } + bool isLocalVariable() const; /// @returns true if this variable is a parameter or return parameter of a function. bool isCallableParameter() const; /// @returns true if this variable is a return parameter of a function. @@ -1004,7 +1017,7 @@ private: /** * Brace-enclosed block containing zero or more statements. */ -class Block: public Statement +class Block: public Statement, public Scopable { public: Block( @@ -1111,7 +1124,7 @@ private: /** * For loop statement */ -class ForStatement: public BreakableStatement +class ForStatement: public BreakableStatement, public Scopable { public: ForStatement( diff --git a/test/libsolidity/SMTChecker.cpp b/test/libsolidity/SMTChecker.cpp index 5088ab94..12b5f439 100644 --- a/test/libsolidity/SMTChecker.cpp +++ b/test/libsolidity/SMTChecker.cpp @@ -466,7 +466,8 @@ BOOST_AUTO_TEST_CASE(for_loop) text = R"( contract C { function f(uint x) public pure { - for (uint y = 2; x < 10; ) { + uint y; + for (y = 2; x < 10; ) { y = 3; } assert(y == 3); @@ -477,7 +478,8 @@ BOOST_AUTO_TEST_CASE(for_loop) text = R"( contract C { function f(uint x) public pure { - for (uint y = 2; x < 10; ) { + uint y; + for (y = 2; x < 10; ) { y = 3; } assert(y == 2); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 3882e4ea..c352a2c2 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -284,6 +284,54 @@ 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; + } + function g() pure public returns (uint x) { + x = 7; + { + x = 3; + uint x; + return x; // This returns the new variable, i.e. 0 + } + } + function h() pure public returns (uint x, uint a, uint b) { + x = 7; + { + x = 3; + a = x; // This should read from the outer + uint x = 4; + b = x; + } + } + function i() pure public returns (uint x, uint a) { + x = 7; + { + x = 3; + uint x = x; // This should read from the outer and assign to the inner + a = x; + } + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()"), encodeArgs(3)); + ABI_CHECK(callContractFunction("g()"), encodeArgs(0)); + ABI_CHECK(callContractFunction("h()"), encodeArgs(3, 3, 4)); + ABI_CHECK(callContractFunction("i()"), encodeArgs(3, 3)); +} + BOOST_AUTO_TEST_CASE(recursive_calls) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index 67747386..e2a0c3cd 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/test/libsolidity/SolidityExpressionCompiler.cpp @@ -322,10 +322,10 @@ BOOST_AUTO_TEST_CASE(arithmetics) { char const* sourceCode = R"( contract test { - function f(uint y) { var x = ((((((((y ^ 8) & 7) | 6) - 5) + 4) % 3) / 2) * 1); } + function f(uint y) { ((((((((y ^ 8) & 7) | 6) - 5) + 4) % 3) / 2) * 1); } } )"; - bytes code = compileFirstExpression(sourceCode, {}, {{"test", "f", "y"}, {"test", "f", "x"}}); + bytes code = compileFirstExpression(sourceCode, {}, {{"test", "f", "y"}}); bytes expectation({byte(Instruction::PUSH1), 0x1, byte(Instruction::PUSH1), 0x2, byte(Instruction::PUSH1), 0x3, @@ -334,7 +334,7 @@ BOOST_AUTO_TEST_CASE(arithmetics) byte(Instruction::PUSH1), 0x6, byte(Instruction::PUSH1), 0x7, byte(Instruction::PUSH1), 0x8, - byte(Instruction::DUP10), + byte(Instruction::DUP9), byte(Instruction::XOR), byte(Instruction::AND), byte(Instruction::OR), @@ -364,13 +364,13 @@ BOOST_AUTO_TEST_CASE(unary_operators) { char const* sourceCode = R"( contract test { - function f(int y) { var x = !(~+- y == 2); } + function f(int y) { !(~+- y == 2); } } )"; - bytes code = compileFirstExpression(sourceCode, {}, {{"test", "f", "y"}, {"test", "f", "x"}}); + bytes code = compileFirstExpression(sourceCode, {}, {{"test", "f", "y"}}); bytes expectation({byte(Instruction::PUSH1), 0x2, - byte(Instruction::DUP3), + byte(Instruction::DUP2), byte(Instruction::PUSH1), 0x0, byte(Instruction::SUB), byte(Instruction::NOT), @@ -383,7 +383,7 @@ BOOST_AUTO_TEST_CASE(unary_inc_dec) { char const* sourceCode = R"( contract test { - function f(uint a) { var x = --a ^ (a-- ^ (++a ^ a++)); } + function f(uint a) returns (uint x) { x = --a ^ (a-- ^ (++a ^ a++)); } } )"; bytes code = compileFirstExpression(sourceCode, {}, {{"test", "f", "a"}, {"test", "f", "x"}}); @@ -426,7 +426,10 @@ BOOST_AUTO_TEST_CASE(unary_inc_dec) byte(Instruction::POP), // second ++ // Stack here: a x a^(a+2)^(a+2) byte(Instruction::DUP3), // will change - byte(Instruction::XOR)}); + byte(Instruction::XOR), + byte(Instruction::SWAP1), + byte(Instruction::POP), + byte(Instruction::DUP1)}); // Stack here: a x a^(a+2)^(a+2)^a BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end()); } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index be147e48..419e46a7 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -76,15 +76,236 @@ BOOST_AUTO_TEST_CASE(double_function_declaration) BOOST_AUTO_TEST_CASE(double_variable_declaration) { - char const* text = R"( + string text = R"( contract test { - function f() public { + function f() pure public { uint256 x; if (true) { uint256 x; } } } )"; - CHECK_ERROR(text, DeclarationError, "Identifier already declared."); + CHECK_ERROR(text, DeclarationError, "Identifier already declared"); +} + +BOOST_AUTO_TEST_CASE(double_variable_declaration_050) +{ + string text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { + uint256 x; + if (true) { uint256 x; } + } + } + )"; + CHECK_WARNING_ALLOW_MULTI(text, (vector<string>{ + "This declaration shadows an existing declaration.", + "Experimental features", + "Unused local variable", + "Unused local variable" + })); +} + +BOOST_AUTO_TEST_CASE(double_variable_declaration_disjoint_scope) +{ + string text = R"( + contract test { + function f() pure public { + { uint x; } + { uint x; } + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Identifier already declared"); +} + +BOOST_AUTO_TEST_CASE(double_variable_declaration_disjoint_scope_050) +{ + string text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { + { uint x; } + { uint x; } + } + } + )"; + CHECK_WARNING_ALLOW_MULTI(text, (vector<string>{ + "Experimental features", + "Unused local variable", + "Unused local variable" + })); +} + +BOOST_AUTO_TEST_CASE(double_variable_declaration_disjoint_scope_activation) +{ + string text = R"( + contract test { + function f() pure public { + { uint x; } + uint x; + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Identifier already declared"); +} + +BOOST_AUTO_TEST_CASE(double_variable_declaration_disjoint_scope_activation_050) +{ + string text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure public { + { uint x; } + uint x; + } + } + )"; + CHECK_WARNING_ALLOW_MULTI(text, (vector<string>{ + "Experimental features", + "Unused local variable", + "Unused local variable" + })); +} +BOOST_AUTO_TEST_CASE(scoping_old) +{ + char const* text = R"( + contract test { + function f() pure public { + x = 4; + uint256 x = 2; + } + } + )"; + CHECK_SUCCESS_NO_WARNINGS(text); +} + +BOOST_AUTO_TEST_CASE(scoping) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() public { + { + uint256 x; + } + x = 2; + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Undeclared identifier"); +} + +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"( + contract test { + function f() pure public { + uint a = a; + } + } + )"; + CHECK_SUCCESS_NO_WARNINGS(text); +} + +BOOST_AUTO_TEST_CASE(scoping_self_use_050) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract test { + function f() pure 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_WARNING(text, "Experimental features"); +} + +BOOST_AUTO_TEST_CASE(scoping_for2) +{ + 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_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; + } + x = 4; + } + } + )"; + 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) @@ -1004,7 +1225,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation) { char const* text = R"( contract B { - function f() mod1(2, true) mod2("0123456") public { } + function f() mod1(2, true) mod2("0123456") pure public { } modifier mod1(uint a, bool b) { if (b) _; } modifier mod2(bytes7 a) { while (a == "1234567") _; } } @@ -1039,11 +1260,23 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation_local_variables) { char const* text = R"( contract B { - function f() mod(x) public { uint x = 7; } + function f() mod(x) pure public { uint x = 7; } modifier mod(uint a) { if (a > 0) _; } } )"; - CHECK_SUCCESS(text); + CHECK_SUCCESS_NO_WARNINGS(text); +} + +BOOST_AUTO_TEST_CASE(function_modifier_invocation_local_variables050) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract B { + function f() mod(x) pure public { uint x = 7; } + modifier mod(uint a) { if (a > 0) _; } + } + )"; + CHECK_ERROR(text, DeclarationError, "Undeclared identifier."); } BOOST_AUTO_TEST_CASE(function_modifier_double_invocation) |