From 5470da4d9adc8ef07aa1c2a758b7062be843cca4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Aug 2017 19:48:34 +0200 Subject: View-pure checker. --- libsolidity/analysis/StaticAnalyzer.h | 4 +- libsolidity/analysis/ViewPureChecker.cpp | 227 +++++++++++++++++++++++++++++++ libsolidity/analysis/ViewPureChecker.h | 79 +++++++++++ libsolidity/ast/Types.h | 1 + libsolidity/interface/CompilerStack.cpp | 11 ++ 5 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 libsolidity/analysis/ViewPureChecker.cpp create mode 100644 libsolidity/analysis/ViewPureChecker.h (limited to 'libsolidity') diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index a3080b42..24ed119f 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -37,8 +37,8 @@ namespace solidity /** * The module that performs static analysis on the AST. * In this context, static analysis is anything that can produce warnings which can help - * programmers write cleaner code. For every warning generated eher, it has to be possible to write - * equivalent code that does generate the warning. + * programmers write cleaner code. For every warning generated here, it has to be possible to write + * equivalent code that does not generate the warning. */ class StaticAnalyzer: private ASTConstVisitor { diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp new file mode 100644 index 00000000..0e2cfacf --- /dev/null +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -0,0 +1,227 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ + +#include + +using namespace std; +using namespace dev; +using namespace dev::solidity; + +bool ViewPureChecker::check() +{ + vector contracts; + + for (auto const& node: m_ast) + { + SourceUnit const* source = dynamic_cast(node.get()); + solAssert(source, ""); + for (auto const& topLevelNode: source->nodes()) + { + ContractDefinition const* contract = dynamic_cast(topLevelNode.get()); + if (contract) + contracts.push_back(contract); + } + } + + // Check modifiers first to infer their state mutability. + for (auto const* contract: contracts) + for (ModifierDefinition const* mod: contract->functionModifiers()) + mod->accept(*this); + + for (auto const* contract: contracts) + contract->accept(*this); + + return !m_errors; +} + + + +bool ViewPureChecker::visit(FunctionDefinition const& _funDef) +{ + solAssert(!m_currentFunction, ""); + m_currentFunction = &_funDef; + m_currentBestMutability = StateMutability::Pure; + return true; +} + +void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) +{ + solAssert(m_currentFunction == &_funDef, ""); + if ( + m_currentBestMutability < _funDef.stateMutability() && + _funDef.stateMutability() != StateMutability::Payable && + _funDef.isImplemented() && + !_funDef.isConstructor() && + !_funDef.isFallback() && + !_funDef.isConstructor() && + !_funDef.annotation().superFunction + ) + m_errorReporter.warning( + _funDef.location(), + "Function state mutability can be restricted to " + stateMutabilityToString(m_currentBestMutability) + ); + m_currentFunction = nullptr; +} + +bool ViewPureChecker::visit(ModifierDefinition const&) +{ + solAssert(m_currentFunction == nullptr, ""); + m_currentBestMutability = StateMutability::Pure; + return true; +} + +void ViewPureChecker::endVisit(ModifierDefinition const& _modifierDef) +{ + solAssert(m_currentFunction == nullptr, ""); + m_inferredMutability[&_modifierDef] = m_currentBestMutability; +} + +void ViewPureChecker::endVisit(Identifier const& _identifier) +{ + Declaration const* declaration = _identifier.annotation().referencedDeclaration; + solAssert(declaration, ""); + + StateMutability mutability = StateMutability::Pure; + + bool writes = _identifier.annotation().lValueRequested; + if (VariableDeclaration const* varDecl = dynamic_cast(declaration)) + { + if (varDecl->isStateVariable()) + mutability = writes ? StateMutability::NonPayable : StateMutability::View; + } + else if (MagicVariableDeclaration const* magicVar = dynamic_cast(declaration)) + { + switch (magicVar->type()->category()) + { + case Type::Category::Contract: + solAssert(_identifier.name() == "this" || _identifier.name() == "super", ""); + if (!dynamic_cast(*magicVar->type()).isSuper()) + // reads the address + mutability = StateMutability::View; + break; + case Type::Category::Integer: + solAssert(_identifier.name() == "now", ""); + mutability = StateMutability::View; + break; + default: + break; + } + } + + reportMutability(mutability, _identifier); +} + +void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) +{ + // @TOOD we can and should analyze it further. + reportMutability(StateMutability::NonPayable, _inlineAssembly); +} + +void ViewPureChecker::reportMutability(StateMutability _mutability, const ASTNode& _node) +{ + if (m_currentFunction && m_currentFunction->stateMutability() < _mutability) + { + m_errors = true; + if (_mutability == StateMutability::View) + m_errorReporter.typeError( + _node.location(), + "Function declared as pure, but this expression reads from the environment or state and thus " + "requires \"view\"." + ); + else if (_mutability == StateMutability::NonPayable) + m_errorReporter.typeError( + _node.location(), + "Function declared as " + + stateMutabilityToString(m_currentFunction->stateMutability()) + + ", but this expression modifies the state and thus " + "requires non-payable (the default) or payable." + ); + else + solAssert(false, ""); + } + if (_mutability >= m_currentBestMutability) + m_currentBestMutability = _mutability; +} + +void ViewPureChecker::endVisit(FunctionCall const& _functionCall) +{ + if (_functionCall.annotation().kind != FunctionCallKind::FunctionCall) + return; + + StateMutability mut = dynamic_cast(*_functionCall.expression().annotation().type).stateMutability(); + // We only require "nonpayable" to call a payble function. + if (mut == StateMutability::Payable) + mut = StateMutability::NonPayable; + reportMutability(mut, _functionCall); +} + +void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) +{ + StateMutability mutability = StateMutability::Pure; + bool writes = _memberAccess.annotation().lValueRequested; + + ASTString const& member = _memberAccess.memberName(); + switch (_memberAccess.expression().annotation().type->category()) + { + case Type::Category::Contract: + case Type::Category::Integer: + if (member == "balance" && !_memberAccess.annotation().referencedDeclaration) + mutability = StateMutability::View; + break; + case Type::Category::Magic: + // we can ignore the kind of magic and only look at the name of the member + if (member != "data" && member != "sig" && member != "blockhash") + mutability = StateMutability::View; + break; + case Type::Category::Struct: + { + if (_memberAccess.expression().annotation().type->dataStoredIn(DataLocation::Storage)) + mutability = writes ? StateMutability::NonPayable : StateMutability::View; + break; + } + case Type::Category::Array: + { + auto const& type = dynamic_cast(*_memberAccess.expression().annotation().type); + if (member == "length" && type.isDynamicallySized() && type.dataStoredIn(DataLocation::Storage)) + mutability = writes ? StateMutability::NonPayable : StateMutability::View; + break; + } + default: + break; + } + reportMutability(mutability, _memberAccess); +} + +void ViewPureChecker::endVisit(IndexAccess const& _indexAccess) +{ + solAssert(_indexAccess.indexExpression(), ""); + + bool writes = _indexAccess.annotation().lValueRequested; + if (_indexAccess.baseExpression().annotation().type->dataStoredIn(DataLocation::Storage)) + reportMutability(writes ? StateMutability::NonPayable : StateMutability::View, _indexAccess); +} + +void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) +{ + solAssert(_modifier.name(), ""); + ModifierDefinition const* mod = dynamic_cast(_modifier.name()->annotation().referencedDeclaration); + solAssert(mod, ""); + solAssert(m_inferredMutability.count(mod), ""); + + reportMutability(m_inferredMutability.at(mod), _modifier); +} + diff --git a/libsolidity/analysis/ViewPureChecker.h b/libsolidity/analysis/ViewPureChecker.h new file mode 100644 index 00000000..6aedfa36 --- /dev/null +++ b/libsolidity/analysis/ViewPureChecker.h @@ -0,0 +1,79 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ + +#pragma once + +#include +#include +#include + +#include + +#include +#include + +namespace dev +{ +namespace solidity +{ + +class ASTNode; +class FunctionDefinition; +class ModifierDefinition; +class Identifier; +class MemberAccess; +class IndexAccess; +class ModifierInvocation; +class FunctionCall; +class InlineAssembly; + +class ViewPureChecker: private ASTConstVisitor +{ +public: + ViewPureChecker(std::vector> const& _ast, ErrorReporter& _errorReporter): + m_ast(_ast), m_errorReporter(_errorReporter) {} + + bool check(); + +private: + + virtual bool visit(FunctionDefinition const& _funDef) override; + virtual void endVisit(FunctionDefinition const& _funDef) override; + virtual bool visit(ModifierDefinition const& _modifierDef) override; + virtual void endVisit(ModifierDefinition const& _modifierDef) override; + virtual void endVisit(Identifier const& _identifier) override; + virtual void endVisit(MemberAccess const& _memberAccess) override; + virtual void endVisit(IndexAccess const& _indexAccess) override; + virtual void endVisit(ModifierInvocation const& _modifier) override; + virtual void endVisit(FunctionCall const& _functionCall) override; + virtual void endVisit(InlineAssembly const& _inlineAssembly) override; + + /// Called when an element of mutability @a _mutability is encountered. + /// Creates appropriate warnings and errors and sets @a m_currentBestMutability. + void reportMutability(StateMutability _mutability, ASTNode const& _node); + + std::vector> const& m_ast; + ErrorReporter& m_errorReporter; + + bool m_errors = false; + StateMutability m_currentBestMutability = StateMutability::Payable; + FunctionDefinition const* m_currentFunction = nullptr; + std::map m_inferredMutability; +}; + +} +} diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index de6dcee9..d4d6da69 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -1064,6 +1064,7 @@ public: { return _inLibrary ? shared_from_this() : TypePointer(); } + virtual bool dataStoredIn(DataLocation _location) const override { return _location == DataLocation::Storage; } TypePointer const& keyType() const { return m_keyType; } TypePointer const& valueType() const { return m_valueType; } diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 259694da..99ad061c 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -220,6 +221,16 @@ bool CompilerStack::analyze() noErrors = false; } + if (noErrors) + { + vector> ast; + for (Source const* source: m_sourceOrder) + ast.push_back(source->ast); + + if (!ViewPureChecker(ast, m_errorReporter).check()) + noErrors = false; + } + if (noErrors) { SMTChecker smtChecker(m_errorReporter, m_smtQuery); -- cgit v1.2.3 From eacee5b25c1edbcf3e32128bd28bce5e58fc2a74 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Aug 2017 16:29:28 +0200 Subject: Remove previous warning about pureness not being enforced. --- libsolidity/analysis/StaticAnalyzer.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 2f130414..ab1cbb52 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -57,8 +57,6 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function) solAssert(m_localVarUseCount.empty(), ""); m_nonPayablePublic = _function.isPublic() && !_function.isPayable(); m_constructor = _function.isConstructor(); - if (_function.stateMutability() == StateMutability::Pure) - m_errorReporter.warning(_function.location(), "Function is marked pure. Be careful, pureness is not enforced yet."); return true; } -- cgit v1.2.3 From 342367d5dcb897d1ef3bbc00853538859773ed37 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Aug 2017 17:07:32 +0200 Subject: Store super function. --- libsolidity/analysis/TypeChecker.cpp | 3 +++ libsolidity/analysis/TypeChecker.h | 1 + libsolidity/ast/ASTAnnotations.h | 3 +++ libsolidity/ast/ASTJsonConverter.cpp | 1 + 4 files changed, 8 insertions(+) (limited to 'libsolidity') diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c46485d8..966ca560 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -315,6 +315,9 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& function, Func if (!functionType.hasEqualArgumentTypes(superType)) return; + if (!function.annotation().superFunction) + function.annotation().superFunction = &super; + if (function.visibility() != super.visibility()) overrideError(function, super, "Overriding function visibility differs."); diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index f2e13765..0c6f54d3 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -63,6 +63,7 @@ private: void checkContractDuplicateFunctions(ContractDefinition const& _contract); void checkContractIllegalOverrides(ContractDefinition const& _contract); /// Reports a type error with an appropiate message if overriden function signature differs. + /// Also stores the direct super function in the AST annotations. void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super); void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message); void checkContractAbstractFunctions(ContractDefinition const& _contract); diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index fd9efb4d..3d4236cc 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -94,6 +94,9 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnota struct FunctionDefinitionAnnotation: ASTAnnotation, DocumentedAnnotation { + /// The function this function overrides, if any. This is always the closest + /// in the linearized inheritance hierarchy. + FunctionDefinition const* superFunction = nullptr; }; struct EventDefinitionAnnotation: ASTAnnotation, DocumentedAnnotation diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp index afc53bfe..6811d3e4 100644 --- a/libsolidity/ast/ASTJsonConverter.cpp +++ b/libsolidity/ast/ASTJsonConverter.cpp @@ -328,6 +328,7 @@ bool ASTJsonConverter::visit(FunctionDefinition const& _node) make_pair(m_legacy ? "constant" : "isDeclaredConst", _node.stateMutability() <= StateMutability::View), make_pair("payable", _node.isPayable()), make_pair("stateMutability", stateMutabilityToString(_node.stateMutability())), + make_pair("superFunction", idOrNull(_node.annotation().superFunction)), make_pair("visibility", Declaration::visibilityToString(_node.visibility())), make_pair("parameters", toJson(_node.parameterList())), make_pair("isConstructor", _node.isConstructor()), -- cgit v1.2.3 From ec27e569b0da6650238a9b48aa53d120184001ed Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Aug 2017 17:08:08 +0200 Subject: Do not report on overriding function and only warn for view. --- libsolidity/analysis/ViewPureChecker.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 0e2cfacf..b4b7f372 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -131,29 +131,36 @@ void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) reportMutability(StateMutability::NonPayable, _inlineAssembly); } -void ViewPureChecker::reportMutability(StateMutability _mutability, const ASTNode& _node) +void ViewPureChecker::reportMutability(StateMutability _mutability, ASTNode const& _node) { if (m_currentFunction && m_currentFunction->stateMutability() < _mutability) { - m_errors = true; + string text; if (_mutability == StateMutability::View) - m_errorReporter.typeError( - _node.location(), - "Function declared as pure, but this expression reads from the environment or state and thus " - "requires \"view\"." - ); + text = + "Function declared as pure, but this expression reads from the " + "environment or state and thus requires \"view\"."; else if (_mutability == StateMutability::NonPayable) - m_errorReporter.typeError( - _node.location(), + text = "Function declared as " + stateMutabilityToString(m_currentFunction->stateMutability()) + ", but this expression modifies the state and thus " - "requires non-payable (the default) or payable." - ); + "requires non-payable (the default) or payable."; + else + solAssert(false, ""); + + if (m_currentFunction->stateMutability() == StateMutability::View) + // Change this to error with 0.5.0 + m_errorReporter.warning(_node.location(), text); + else if (m_currentFunction->stateMutability() == StateMutability::Pure) + { + m_errors = true; + m_errorReporter.typeError(_node.location(), text); + } else solAssert(false, ""); } - if (_mutability >= m_currentBestMutability) + if (_mutability > m_currentBestMutability) m_currentBestMutability = _mutability; } -- cgit v1.2.3 From d6861d909c7dabb6993cfa7a8269f76c613428f7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Aug 2017 13:13:35 +0200 Subject: Analyze assembly. --- libsolidity/analysis/ViewPureChecker.cpp | 97 ++++++++++++++++++++++++++++---- libsolidity/analysis/ViewPureChecker.h | 2 +- 2 files changed, 88 insertions(+), 11 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index b4b7f372..60b07297 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -17,10 +17,86 @@ #include +#include + +#include + +#include + using namespace std; using namespace dev; using namespace dev::solidity; + +class AssemblyViewPureChecker: public boost::static_visitor +{ +public: + explicit AssemblyViewPureChecker(std::function _reportMutability): + m_reportMutability(_reportMutability) {} + + void operator()(assembly::Label const&) { } + void operator()(assembly::Instruction const& _instruction) + { + if (eth::SemanticInformation::invalidInViewFunctions(_instruction.instruction)) + m_reportMutability(StateMutability::NonPayable, _instruction.location); + else if (eth::SemanticInformation::invalidInPureFunctions(_instruction.instruction)) + m_reportMutability(StateMutability::View, _instruction.location); + } + void operator()(assembly::Literal const&) {} + void operator()(assembly::Identifier const&) {} + void operator()(assembly::FunctionalInstruction const& _instr) + { + (*this)(_instr.instruction); + for (auto const& arg: _instr.arguments) + boost::apply_visitor(*this, arg); + } + void operator()(assembly::StackAssignment const&) {} + void operator()(assembly::Assignment const& _assignment) + { + boost::apply_visitor(*this, *_assignment.value); + } + void operator()(assembly::VariableDeclaration const& _varDecl) + { + if (_varDecl.value) + boost::apply_visitor(*this, *_varDecl.value); + } + void operator()(assembly::FunctionDefinition const& _funDef) + { + (*this)(_funDef.body); + } + void operator()(assembly::FunctionCall const& _funCall) + { + for (auto const& arg: _funCall.arguments) + boost::apply_visitor(*this, arg); + } + void operator()(assembly::Switch const& _switch) + { + boost::apply_visitor(*this, *_switch.expression); + for (auto const& _case: _switch.cases) + { + if (_case.value) + (*this)(*_case.value); + (*this)(_case.body); + } + } + void operator()(assembly::ForLoop const& _for) + { + (*this)(_for.pre); + boost::apply_visitor(*this, *_for.condition); + (*this)(_for.body); + (*this)(_for.post); + } + void operator()(assembly::Block const& _block) + { + for (auto const& s: _block.statements) + boost::apply_visitor(*this, s); + } + +private: + std::function m_reportMutability; +}; + + bool ViewPureChecker::check() { vector contracts; @@ -122,16 +198,17 @@ void ViewPureChecker::endVisit(Identifier const& _identifier) } } - reportMutability(mutability, _identifier); + reportMutability(mutability, _identifier.location()); } void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) { - // @TOOD we can and should analyze it further. - reportMutability(StateMutability::NonPayable, _inlineAssembly); + AssemblyViewPureChecker{ + [=](StateMutability _mut, SourceLocation const& _loc) { reportMutability(_mut, _loc); } + }(_inlineAssembly.operations()); } -void ViewPureChecker::reportMutability(StateMutability _mutability, ASTNode const& _node) +void ViewPureChecker::reportMutability(StateMutability _mutability, SourceLocation const& _location) { if (m_currentFunction && m_currentFunction->stateMutability() < _mutability) { @@ -151,11 +228,11 @@ void ViewPureChecker::reportMutability(StateMutability _mutability, ASTNode cons if (m_currentFunction->stateMutability() == StateMutability::View) // Change this to error with 0.5.0 - m_errorReporter.warning(_node.location(), text); + m_errorReporter.warning(_location, text); else if (m_currentFunction->stateMutability() == StateMutability::Pure) { m_errors = true; - m_errorReporter.typeError(_node.location(), text); + m_errorReporter.typeError(_location, text); } else solAssert(false, ""); @@ -173,7 +250,7 @@ void ViewPureChecker::endVisit(FunctionCall const& _functionCall) // We only require "nonpayable" to call a payble function. if (mut == StateMutability::Payable) mut = StateMutability::NonPayable; - reportMutability(mut, _functionCall); + reportMutability(mut, _functionCall.location()); } void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) @@ -210,7 +287,7 @@ void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) default: break; } - reportMutability(mutability, _memberAccess); + reportMutability(mutability, _memberAccess.location()); } void ViewPureChecker::endVisit(IndexAccess const& _indexAccess) @@ -219,7 +296,7 @@ void ViewPureChecker::endVisit(IndexAccess const& _indexAccess) bool writes = _indexAccess.annotation().lValueRequested; if (_indexAccess.baseExpression().annotation().type->dataStoredIn(DataLocation::Storage)) - reportMutability(writes ? StateMutability::NonPayable : StateMutability::View, _indexAccess); + reportMutability(writes ? StateMutability::NonPayable : StateMutability::View, _indexAccess.location()); } void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) @@ -229,6 +306,6 @@ void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) solAssert(mod, ""); solAssert(m_inferredMutability.count(mod), ""); - reportMutability(m_inferredMutability.at(mod), _modifier); + reportMutability(m_inferredMutability.at(mod), _modifier.location()); } diff --git a/libsolidity/analysis/ViewPureChecker.h b/libsolidity/analysis/ViewPureChecker.h index 6aedfa36..ae303533 100644 --- a/libsolidity/analysis/ViewPureChecker.h +++ b/libsolidity/analysis/ViewPureChecker.h @@ -64,7 +64,7 @@ private: /// Called when an element of mutability @a _mutability is encountered. /// Creates appropriate warnings and errors and sets @a m_currentBestMutability. - void reportMutability(StateMutability _mutability, ASTNode const& _node); + void reportMutability(StateMutability _mutability, SourceLocation const& _location); std::vector> const& m_ast; ErrorReporter& m_errorReporter; -- cgit v1.2.3 From 7886c24d4003beb155b6dba119b92d2df7ec0140 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Aug 2017 16:03:50 +0200 Subject: Modifier invocation can be base constructor call --- libsolidity/analysis/ViewPureChecker.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 60b07297..55f391b7 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -302,10 +302,12 @@ void ViewPureChecker::endVisit(IndexAccess const& _indexAccess) void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) { solAssert(_modifier.name(), ""); - ModifierDefinition const* mod = dynamic_cast(_modifier.name()->annotation().referencedDeclaration); - solAssert(mod, ""); - solAssert(m_inferredMutability.count(mod), ""); - - reportMutability(m_inferredMutability.at(mod), _modifier.location()); + if (ModifierDefinition const* mod = dynamic_cast(_modifier.name()->annotation().referencedDeclaration)) + { + solAssert(m_inferredMutability.count(mod), ""); + reportMutability(m_inferredMutability.at(mod), _modifier.location()); + } + else + solAssert(dynamic_cast(_modifier.name()->annotation().referencedDeclaration), ""); } -- cgit v1.2.3 From 1a1db1ec963935bffcabd6115cf99d04fd1d633b Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Aug 2017 16:14:54 +0200 Subject: Tone down error message. --- libsolidity/analysis/ViewPureChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 55f391b7..e5b433c2 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -215,13 +215,13 @@ void ViewPureChecker::reportMutability(StateMutability _mutability, SourceLocati string text; if (_mutability == StateMutability::View) text = - "Function declared as pure, but this expression reads from the " + "Function declared as pure, but this expression (potentially) reads from the " "environment or state and thus requires \"view\"."; else if (_mutability == StateMutability::NonPayable) text = "Function declared as " + stateMutabilityToString(m_currentFunction->stateMutability()) + - ", but this expression modifies the state and thus " + ", but this expression (potentially) modifies the state and thus " "requires non-payable (the default) or payable."; else solAssert(false, ""); -- cgit v1.2.3 From c83768c4260979b6d30185ef19352d27b161c3b0 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 1 Sep 2017 12:31:24 +0200 Subject: Fix tests --- libsolidity/analysis/ViewPureChecker.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index e5b433c2..705d61e8 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -292,11 +292,14 @@ void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) void ViewPureChecker::endVisit(IndexAccess const& _indexAccess) { - solAssert(_indexAccess.indexExpression(), ""); - - bool writes = _indexAccess.annotation().lValueRequested; - if (_indexAccess.baseExpression().annotation().type->dataStoredIn(DataLocation::Storage)) - reportMutability(writes ? StateMutability::NonPayable : StateMutability::View, _indexAccess.location()); + if (!_indexAccess.indexExpression()) + solAssert(_indexAccess.annotation().type->category() == Type::Category::TypeType, ""); + else + { + bool writes = _indexAccess.annotation().lValueRequested; + if (_indexAccess.baseExpression().annotation().type->dataStoredIn(DataLocation::Storage)) + reportMutability(writes ? StateMutability::NonPayable : StateMutability::View, _indexAccess.location()); + } } void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) -- cgit v1.2.3 From 15bdc48a73d1cbebb9f75830fd1abbd29f798c12 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 1 Sep 2017 19:58:38 +0200 Subject: Rename and add anonymous namespace. --- libsolidity/analysis/ViewPureChecker.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 705d61e8..6621edb1 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -27,6 +27,8 @@ using namespace std; using namespace dev; using namespace dev::solidity; +namespace +{ class AssemblyViewPureChecker: public boost::static_visitor { @@ -39,7 +41,7 @@ public: { if (eth::SemanticInformation::invalidInViewFunctions(_instruction.instruction)) m_reportMutability(StateMutability::NonPayable, _instruction.location); - else if (eth::SemanticInformation::invalidInPureFunctions(_instruction.instruction)) + else if (eth::SemanticInformation::readsFromState(_instruction.instruction)) m_reportMutability(StateMutability::View, _instruction.location); } void operator()(assembly::Literal const&) {} @@ -96,6 +98,7 @@ private: std::function m_reportMutability; }; +} bool ViewPureChecker::check() { -- cgit v1.2.3 From b756274357c1e33517d535fdd2349a88221d4780 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 5 Sep 2017 18:33:52 +0200 Subject: Allow constant variables in pure functions. --- libsolidity/analysis/ViewPureChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 6621edb1..d4a6e96f 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -179,7 +179,7 @@ void ViewPureChecker::endVisit(Identifier const& _identifier) bool writes = _identifier.annotation().lValueRequested; if (VariableDeclaration const* varDecl = dynamic_cast(declaration)) { - if (varDecl->isStateVariable()) + if (varDecl->isStateVariable() && !varDecl->isConstant()) mutability = writes ? StateMutability::NonPayable : StateMutability::View; } else if (MagicVariableDeclaration const* magicVar = dynamic_cast(declaration)) -- cgit v1.2.3 From 66c01301fe5cb71a2bf66af2f7170043f088815c Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 13 Sep 2017 17:18:22 +0200 Subject: Rename to invalidInPureFunctions --- libsolidity/analysis/ViewPureChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index d4a6e96f..40ad6828 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -41,7 +41,7 @@ public: { if (eth::SemanticInformation::invalidInViewFunctions(_instruction.instruction)) m_reportMutability(StateMutability::NonPayable, _instruction.location); - else if (eth::SemanticInformation::readsFromState(_instruction.instruction)) + else if (eth::SemanticInformation::invalidInPureFunctions(_instruction.instruction)) m_reportMutability(StateMutability::View, _instruction.location); } void operator()(assembly::Literal const&) {} -- cgit v1.2.3 From e2f30ce9ca3bc8c6c6e64c074b98bafec7e2e5d5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 13 Sep 2017 17:29:27 +0200 Subject: Minor changes from review. --- libsolidity/analysis/ViewPureChecker.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 40ad6828..8f9d41c9 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -108,12 +108,7 @@ bool ViewPureChecker::check() { SourceUnit const* source = dynamic_cast(node.get()); solAssert(source, ""); - for (auto const& topLevelNode: source->nodes()) - { - ContractDefinition const* contract = dynamic_cast(topLevelNode.get()); - if (contract) - contracts.push_back(contract); - } + contracts += source->filteredNodes(source->nodes()); } // Check modifiers first to infer their state mutability. @@ -146,7 +141,6 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) _funDef.isImplemented() && !_funDef.isConstructor() && !_funDef.isFallback() && - !_funDef.isConstructor() && !_funDef.annotation().superFunction ) m_errorReporter.warning( @@ -207,7 +201,7 @@ void ViewPureChecker::endVisit(Identifier const& _identifier) void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) { AssemblyViewPureChecker{ - [=](StateMutability _mut, SourceLocation const& _loc) { reportMutability(_mut, _loc); } + [=](StateMutability _mutability, SourceLocation const& _location) { reportMutability(_mutability, _location); } }(_inlineAssembly.operations()); } @@ -230,7 +224,7 @@ void ViewPureChecker::reportMutability(StateMutability _mutability, SourceLocati solAssert(false, ""); if (m_currentFunction->stateMutability() == StateMutability::View) - // Change this to error with 0.5.0 + // TODO Change this to error with 0.5.0 m_errorReporter.warning(_location, text); else if (m_currentFunction->stateMutability() == StateMutability::Pure) { -- cgit v1.2.3