diff options
Diffstat (limited to 'libsolidity/analysis')
25 files changed, 471 insertions, 428 deletions
diff --git a/libsolidity/analysis/ConstantEvaluator.cpp b/libsolidity/analysis/ConstantEvaluator.cpp index 9d041ce5..e637795a 100644 --- a/libsolidity/analysis/ConstantEvaluator.cpp +++ b/libsolidity/analysis/ConstantEvaluator.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/ConstantEvaluator.h> + #include <libsolidity/ast/AST.h> #include <liblangutil/ErrorReporter.h> @@ -41,7 +42,7 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) auto right = type(_operation.rightExpression()); if (left && right) { - auto commonType = left->binaryOperatorResult(_operation.getOperator(), right); + TypePointer commonType = left->binaryOperatorResult(_operation.getOperator(), right); if (!commonType) m_errorReporter.fatalTypeError( _operation.location(), diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 6dc564de..96b9611e 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -20,10 +20,9 @@ */ #include <libsolidity/analysis/ContractLevelChecker.h> -#include <libsolidity/ast/AST.h> +#include <libsolidity/ast/AST.h> #include <liblangutil/ErrorReporter.h> - #include <boost/range/adaptor/reversed.hpp> @@ -227,7 +226,7 @@ void ContractLevelChecker::checkAbstractFunctions(ContractDefinition const& _con return _type->hasEqualParameterTypes(*_funAndFlag.first); }); if (it == overloads.end()) - overloads.push_back(make_pair(_type, _implemented)); + overloads.emplace_back(_type, _implemented); else if (it->second) { if (!_implemented) @@ -409,8 +408,8 @@ void ContractLevelChecker::checkExternalTypeClashes(ContractDefinition const& _c auto functionType = make_shared<FunctionType>(*f); // under non error circumstances this should be true if (functionType->interfaceFunctionType()) - externalDeclarations[functionType->externalSignature()].push_back( - make_pair(f, functionType->asCallableFunction(false)) + externalDeclarations[functionType->externalSignature()].emplace_back( + f, functionType->asCallableFunction(false) ); } for (VariableDeclaration const* v: contract->stateVariables()) @@ -419,8 +418,8 @@ void ContractLevelChecker::checkExternalTypeClashes(ContractDefinition const& _c auto functionType = make_shared<FunctionType>(*v); // under non error circumstances this should be true if (functionType->interfaceFunctionType()) - externalDeclarations[functionType->externalSignature()].push_back( - make_pair(v, functionType->asCallableFunction(false)) + externalDeclarations[functionType->externalSignature()].emplace_back( + v, functionType->asCallableFunction(false) ); } } diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index 15cbf45d..d754687a 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -22,7 +22,6 @@ #pragma once #include <libsolidity/ast/ASTForward.h> - #include <map> namespace langutil diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp index fe58f0aa..3adf6318 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.cpp +++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp @@ -16,7 +16,9 @@ */ #include <libsolidity/analysis/ControlFlowAnalyzer.h> + #include <liblangutil/SourceLocation.h> +#include <boost/range/algorithm/sort.hpp> using namespace std; using namespace langutil; @@ -33,131 +35,112 @@ bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function) if (_function.isImplemented()) { auto const& functionFlow = m_cfg.functionFlow(_function); - checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit); + checkUninitializedAccess(functionFlow.entry, functionFlow.exit); } return false; } -set<VariableDeclaration const*> ControlFlowAnalyzer::variablesAssignedInNode(CFGNode const *node) +void ControlFlowAnalyzer::checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const { - set<VariableDeclaration const*> result; - for (auto expression: node->block.expressions) + struct NodeInfo { - if (auto const* assignment = dynamic_cast<Assignment const*>(expression)) + set<VariableDeclaration const*> unassignedVariablesAtEntry; + set<VariableDeclaration const*> unassignedVariablesAtExit; + set<VariableOccurrence const*> uninitializedVariableAccesses; + /// Propagate the information from another node to this node. + /// To be used to propagate information from a node to its exit nodes. + /// Returns true, if new variables were added and thus the current node has + /// to be traversed again. + bool propagateFrom(NodeInfo const& _entryNode) { - stack<Expression const*> expressions; - expressions.push(&assignment->leftHandSide()); - while (!expressions.empty()) - { - Expression const* expression = expressions.top(); - expressions.pop(); - - if (auto const *tuple = dynamic_cast<TupleExpression const*>(expression)) - for (auto const& component: tuple->components()) - expressions.push(component.get()); - else if (auto const* identifier = dynamic_cast<Identifier const*>(expression)) - if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>( - identifier->annotation().referencedDeclaration - )) - result.insert(variableDeclaration); - } + size_t previousUnassignedVariablesAtEntry = unassignedVariablesAtEntry.size(); + size_t previousUninitializedVariableAccessess = uninitializedVariableAccesses.size(); + unassignedVariablesAtEntry += _entryNode.unassignedVariablesAtExit; + uninitializedVariableAccesses += _entryNode.uninitializedVariableAccesses; + return + unassignedVariablesAtEntry.size() > previousUnassignedVariablesAtEntry || + uninitializedVariableAccesses.size() > previousUninitializedVariableAccessess + ; } - } - return result; -} - -void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( - FunctionDefinition const& _function, - CFGNode const* _functionEntry, - CFGNode const* _functionExit -) const -{ - if (_function.returnParameterList()->parameters().empty()) - return; - - map<CFGNode const*, set<VariableDeclaration const*>> unassigned; - - { - auto& unassignedAtFunctionEntry = unassigned[_functionEntry]; - for (auto const& returnParameter: _function.returnParameterList()->parameters()) - if ( - returnParameter->type()->dataStoredIn(DataLocation::Storage) || - returnParameter->type()->category() == Type::Category::Mapping - ) - unassignedAtFunctionEntry.insert(returnParameter.get()); - } - - stack<CFGNode const*> nodesToTraverse; - nodesToTraverse.push(_functionEntry); - - // walk all paths from entry with maximal set of unassigned return values + }; + map<CFGNode const*, NodeInfo> nodeInfos; + set<CFGNode const*> nodesToTraverse; + nodesToTraverse.insert(_entry); + + // Walk all paths starting from the nodes in ``nodesToTraverse`` until ``NodeInfo::propagateFrom`` + // returns false for all exits, i.e. until all paths have been walked with maximal sets of unassigned + // variables and accesses. while (!nodesToTraverse.empty()) { - auto node = nodesToTraverse.top(); - nodesToTraverse.pop(); - - auto& unassignedAtNode = unassigned[node]; + CFGNode const* currentNode = *nodesToTraverse.begin(); + nodesToTraverse.erase(nodesToTraverse.begin()); - if (node->block.returnStatement != nullptr) - if (node->block.returnStatement->expression()) - unassignedAtNode.clear(); - if (!unassignedAtNode.empty()) + auto& nodeInfo = nodeInfos[currentNode]; + auto unassignedVariables = nodeInfo.unassignedVariablesAtEntry; + for (auto const& variableOccurrence: currentNode->variableOccurrences) { - // kill all return values to which a value is assigned - for (auto const* variableDeclaration: variablesAssignedInNode(node)) - unassignedAtNode.erase(variableDeclaration); - - // kill all return values referenced in inline assembly - // a reference is enough, checking whether there actually was an assignment might be overkill - for (auto assembly: node->block.inlineAssemblyStatements) - for (auto const& ref: assembly->annotation().externalReferences) - if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(ref.second.declaration)) - unassignedAtNode.erase(variableDeclaration); + switch (variableOccurrence.kind()) + { + case VariableOccurrence::Kind::Assignment: + unassignedVariables.erase(&variableOccurrence.declaration()); + break; + case VariableOccurrence::Kind::InlineAssembly: + // We consider all variables referenced in inline assembly as accessed. + // So far any reference is enough, but we might want to actually analyze + // the control flow in the assembly at some point. + case VariableOccurrence::Kind::Access: + case VariableOccurrence::Kind::Return: + if (unassignedVariables.count(&variableOccurrence.declaration())) + { + if (variableOccurrence.declaration().type()->dataStoredIn(DataLocation::Storage)) + // Merely store the unassigned access. We do not generate an error right away, since this + // path might still always revert. It is only an error if this is propagated to the exit + // node of the function (i.e. there is a path with an uninitialized access). + nodeInfo.uninitializedVariableAccesses.insert(&variableOccurrence); + } + break; + case VariableOccurrence::Kind::Declaration: + unassignedVariables.insert(&variableOccurrence.declaration()); + break; + } } + nodeInfo.unassignedVariablesAtExit = std::move(unassignedVariables); - for (auto const& exit: node->exits) - { - auto& unassignedAtExit = unassigned[exit]; - auto oldSize = unassignedAtExit.size(); - unassignedAtExit.insert(unassignedAtNode.begin(), unassignedAtNode.end()); - // (re)traverse an exit, if we are on a path with new unassigned return values to consider - // this will terminate, since there is only a finite number of unassigned return values - if (unassignedAtExit.size() > oldSize) - nodesToTraverse.push(exit); - } + // Propagate changes to all exits and queue them for traversal, if needed. + for (auto const& exit: currentNode->exits) + if (nodeInfos[exit].propagateFrom(nodeInfo)) + nodesToTraverse.insert(exit); } - if (!unassigned[_functionExit].empty()) + auto const& exitInfo = nodeInfos[_exit]; + if (!exitInfo.uninitializedVariableAccesses.empty()) { - vector<VariableDeclaration const*> unassignedOrdered( - unassigned[_functionExit].begin(), - unassigned[_functionExit].end() - ); - sort( - unassignedOrdered.begin(), - unassignedOrdered.end(), - [](VariableDeclaration const* lhs, VariableDeclaration const* rhs) -> bool { - return lhs->id() < rhs->id(); + vector<VariableOccurrence const*> uninitializedAccessesOrdered( + exitInfo.uninitializedVariableAccesses.begin(), + exitInfo.uninitializedVariableAccesses.end() + ); + boost::range::sort( + uninitializedAccessesOrdered, + [](VariableOccurrence const* lhs, VariableOccurrence const* rhs) -> bool + { + return *lhs < *rhs; } ); - for (auto const* returnVal: unassignedOrdered) + + for (auto const* variableOccurrence: uninitializedAccessesOrdered) { SecondarySourceLocation ssl; - for (CFGNode* lastNodeBeforeExit: _functionExit->entries) - if (unassigned[lastNodeBeforeExit].count(returnVal)) - { - if (!!lastNodeBeforeExit->block.returnStatement) - ssl.append("Problematic return:", lastNodeBeforeExit->block.returnStatement->location()); - else - ssl.append("Problematic end of function:", _function.location()); - } + if (variableOccurrence->occurrence()) + ssl.append("The variable was declared here.", variableOccurrence->declaration().location()); m_errorReporter.typeError( - returnVal->location(), + variableOccurrence->occurrence() ? + variableOccurrence->occurrence()->location() : + variableOccurrence->declaration().location(), ssl, - "This variable is of storage pointer type and might be returned without assignment and " - "could be used uninitialized. Assign the variable (potentially from itself) " - "to fix this error." + string("This variable is of storage pointer type and can be ") + + (variableOccurrence->kind() == VariableOccurrence::Kind::Return ? "returned" : "accessed") + + " without prior assignment." ); } } diff --git a/libsolidity/analysis/ControlFlowAnalyzer.h b/libsolidity/analysis/ControlFlowAnalyzer.h index 411d57ff..7761817a 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.h +++ b/libsolidity/analysis/ControlFlowAnalyzer.h @@ -18,7 +18,6 @@ #pragma once #include <libsolidity/analysis/ControlFlowGraph.h> - #include <set> namespace dev @@ -37,12 +36,8 @@ public: bool visit(FunctionDefinition const& _function) override; private: - static std::set<VariableDeclaration const*> variablesAssignedInNode(CFGNode const *node); - void checkUnassignedStorageReturnValues( - FunctionDefinition const& _function, - CFGNode const* _functionEntry, - CFGNode const* _functionExit - ) const; + /// Checks for uninitialized variable accesses in the control flow between @param _entry and @param _exit. + void checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const; CFG const& m_cfg; langutil::ErrorReporter& m_errorReporter; diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp index 5bd39da3..3dab8b16 100644 --- a/libsolidity/analysis/ControlFlowBuilder.cpp +++ b/libsolidity/analysis/ControlFlowBuilder.cpp @@ -22,7 +22,10 @@ using namespace solidity; using namespace std; ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow): - m_nodeContainer(_nodeContainer), m_currentFunctionFlow(_functionFlow), m_currentNode(_functionFlow.entry) + m_nodeContainer(_nodeContainer), + m_currentNode(_functionFlow.entry), + m_returnNode(_functionFlow.exit), + m_revertNode(_functionFlow.revert) { } @@ -37,26 +40,8 @@ unique_ptr<FunctionFlow> ControlFlowBuilder::createFunctionFlow( functionFlow->revert = _nodeContainer.newNode(); ControlFlowBuilder builder(_nodeContainer, *functionFlow); builder.appendControlFlow(_function); - connect(builder.m_currentNode, functionFlow->exit); - return functionFlow; -} - -unique_ptr<ModifierFlow> ControlFlowBuilder::createModifierFlow( - CFG::NodeContainer& _nodeContainer, - ModifierDefinition const& _modifier -) -{ - auto modifierFlow = unique_ptr<ModifierFlow>(new ModifierFlow()); - modifierFlow->entry = _nodeContainer.newNode(); - modifierFlow->exit = _nodeContainer.newNode(); - modifierFlow->revert = _nodeContainer.newNode(); - modifierFlow->placeholderEntry = _nodeContainer.newNode(); - modifierFlow->placeholderExit = _nodeContainer.newNode(); - ControlFlowBuilder builder(_nodeContainer, *modifierFlow); - builder.appendControlFlow(_modifier); - connect(builder.m_currentNode, modifierFlow->exit); - return modifierFlow; + return functionFlow; } bool ControlFlowBuilder::visit(BinaryOperation const& _operation) @@ -219,64 +204,24 @@ bool ControlFlowBuilder::visit(Continue const&) bool ControlFlowBuilder::visit(Throw const&) { solAssert(!!m_currentNode, ""); - solAssert(!!m_currentFunctionFlow.revert, ""); - connect(m_currentNode, m_currentFunctionFlow.revert); + solAssert(!!m_revertNode, ""); + connect(m_currentNode, m_revertNode); m_currentNode = newLabel(); return false; } -bool ControlFlowBuilder::visit(Block const&) -{ - solAssert(!!m_currentNode, ""); - createLabelHere(); - return true; -} - -void ControlFlowBuilder::endVisit(Block const&) -{ - solAssert(!!m_currentNode, ""); - createLabelHere(); -} - -bool ControlFlowBuilder::visit(Return const& _return) -{ - solAssert(!!m_currentNode, ""); - solAssert(!!m_currentFunctionFlow.exit, ""); - solAssert(!m_currentNode->block.returnStatement, ""); - m_currentNode->block.returnStatement = &_return; - connect(m_currentNode, m_currentFunctionFlow.exit); - m_currentNode = newLabel(); - return true; -} - - bool ControlFlowBuilder::visit(PlaceholderStatement const&) { solAssert(!!m_currentNode, ""); - auto modifierFlow = dynamic_cast<ModifierFlow const*>(&m_currentFunctionFlow); - solAssert(!!modifierFlow, ""); - - connect(m_currentNode, modifierFlow->placeholderEntry); + solAssert(!!m_placeholderEntry, ""); + solAssert(!!m_placeholderExit, ""); + connect(m_currentNode, m_placeholderEntry); m_currentNode = newLabel(); - - connect(modifierFlow->placeholderExit, m_currentNode); + connect(m_placeholderExit, m_currentNode); return false; } -bool ControlFlowBuilder::visitNode(ASTNode const& node) -{ - solAssert(!!m_currentNode, ""); - if (auto const* expression = dynamic_cast<Expression const*>(&node)) - m_currentNode->block.expressions.emplace_back(expression); - else if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(&node)) - m_currentNode->block.variableDeclarations.emplace_back(variableDeclaration); - else if (auto const* assembly = dynamic_cast<InlineAssembly const*>(&node)) - m_currentNode->block.inlineAssemblyStatements.emplace_back(assembly); - - return true; -} - bool ControlFlowBuilder::visit(FunctionCall const& _functionCall) { solAssert(!!m_currentNode, ""); @@ -286,19 +231,19 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall) switch (functionType->kind()) { case FunctionType::Kind::Revert: - solAssert(!!m_currentFunctionFlow.revert, ""); + solAssert(!!m_revertNode, ""); _functionCall.expression().accept(*this); ASTNode::listAccept(_functionCall.arguments(), *this); - connect(m_currentNode, m_currentFunctionFlow.revert); + connect(m_currentNode, m_revertNode); m_currentNode = newLabel(); return false; case FunctionType::Kind::Require: case FunctionType::Kind::Assert: { - solAssert(!!m_currentFunctionFlow.revert, ""); + solAssert(!!m_revertNode, ""); _functionCall.expression().accept(*this); ASTNode::listAccept(_functionCall.arguments(), *this); - connect(m_currentNode, m_currentFunctionFlow.revert); + connect(m_currentNode, m_revertNode); auto nextNode = newLabel(); connect(m_currentNode, nextNode); m_currentNode = nextNode; @@ -310,6 +255,183 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall) return ASTConstVisitor::visit(_functionCall); } +bool ControlFlowBuilder::visit(ModifierInvocation const& _modifierInvocation) +{ + if (auto arguments = _modifierInvocation.arguments()) + for (auto& argument: *arguments) + appendControlFlow(*argument); + + auto modifierDefinition = dynamic_cast<ModifierDefinition const*>( + _modifierInvocation.name()->annotation().referencedDeclaration + ); + if (!modifierDefinition) return false; + solAssert(!!modifierDefinition, ""); + solAssert(!!m_returnNode, ""); + + m_placeholderEntry = newLabel(); + m_placeholderExit = newLabel(); + + appendControlFlow(*modifierDefinition); + connect(m_currentNode, m_returnNode); + + m_currentNode = m_placeholderEntry; + m_returnNode = m_placeholderExit; + + m_placeholderEntry = nullptr; + m_placeholderExit = nullptr; + + return false; +} + +bool ControlFlowBuilder::visit(FunctionDefinition const& _functionDefinition) +{ + for (auto const& parameter: _functionDefinition.parameters()) + appendControlFlow(*parameter); + + for (auto const& returnParameter: _functionDefinition.returnParameters()) + { + appendControlFlow(*returnParameter); + m_returnNode->variableOccurrences.emplace_back( + *returnParameter, + VariableOccurrence::Kind::Return, + nullptr + ); + + } + + for (auto const& modifier: _functionDefinition.modifiers()) + appendControlFlow(*modifier); + + appendControlFlow(_functionDefinition.body()); + + connect(m_currentNode, m_returnNode); + m_currentNode = nullptr; + + return false; +} + +bool ControlFlowBuilder::visit(Return const& _return) +{ + solAssert(!!m_currentNode, ""); + solAssert(!!m_returnNode, ""); + if (_return.expression()) + { + appendControlFlow(*_return.expression()); + // Returns with return expression are considered to be assignments to the return parameters. + for (auto returnParameter: _return.annotation().functionReturnParameters->parameters()) + m_currentNode->variableOccurrences.emplace_back( + *returnParameter, + VariableOccurrence::Kind::Assignment, + &_return + ); + } + connect(m_currentNode, m_returnNode); + m_currentNode = newLabel(); + return true; +} + +bool ControlFlowBuilder::visit(FunctionTypeName const&) +{ + // Do not visit the parameters and return values of a function type name. + // We do not want to consider them as variable declarations for the control flow graph. + return false; +} + +bool ControlFlowBuilder::visit(InlineAssembly const& _inlineAssembly) +{ + solAssert(!!m_currentNode, ""); + for (auto const& ref: _inlineAssembly.annotation().externalReferences) + { + if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(ref.second.declaration)) + m_currentNode->variableOccurrences.emplace_back( + *variableDeclaration, + VariableOccurrence::Kind::InlineAssembly, + &_inlineAssembly + ); + } + return true; +} + +bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration) +{ + solAssert(!!m_currentNode, ""); + + m_currentNode->variableOccurrences.emplace_back( + _variableDeclaration, + VariableOccurrence::Kind::Declaration, + nullptr + ); + + // Handle declaration with immediate assignment. + if (_variableDeclaration.value()) + m_currentNode->variableOccurrences.emplace_back( + _variableDeclaration, + VariableOccurrence::Kind::Assignment, + _variableDeclaration.value().get() + ); + // Function arguments are considered to be immediately assigned as well (they are "externally assigned"). + else if (_variableDeclaration.isCallableParameter() && !_variableDeclaration.isReturnParameter()) + m_currentNode->variableOccurrences.emplace_back( + _variableDeclaration, + VariableOccurrence::Kind::Assignment, + nullptr + ); + return true; +} + +bool ControlFlowBuilder::visit(VariableDeclarationStatement const& _variableDeclarationStatement) +{ + solAssert(!!m_currentNode, ""); + + for (auto const& var: _variableDeclarationStatement.declarations()) + if (var) + var->accept(*this); + if (_variableDeclarationStatement.initialValue()) + { + _variableDeclarationStatement.initialValue()->accept(*this); + for (size_t i = 0; i < _variableDeclarationStatement.declarations().size(); i++) + if (auto const& var = _variableDeclarationStatement.declarations()[i]) + { + auto expression = _variableDeclarationStatement.initialValue(); + if (auto tupleExpression = dynamic_cast<TupleExpression const*>(expression)) + if (tupleExpression->components().size() > 1) + { + solAssert(tupleExpression->components().size() > i, ""); + expression = tupleExpression->components()[i].get(); + } + while (auto tupleExpression = dynamic_cast<TupleExpression const*>(expression)) + if (tupleExpression->components().size() == 1) + expression = tupleExpression->components().front().get(); + else + break; + m_currentNode->variableOccurrences.emplace_back( + *var, + VariableOccurrence::Kind::Assignment, + expression + ); + } + } + return false; +} + +bool ControlFlowBuilder::visit(Identifier const& _identifier) +{ + solAssert(!!m_currentNode, ""); + + if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration)) + m_currentNode->variableOccurrences.emplace_back( + *variableDeclaration, + static_cast<Expression const&>(_identifier).annotation().lValueRequested ? + VariableOccurrence::Kind::Assignment : + VariableOccurrence::Kind::Access, + &_identifier + ); + + return true; +} + + + void ControlFlowBuilder::appendControlFlow(ASTNode const& _node) { _node.accept(*this); diff --git a/libsolidity/analysis/ControlFlowBuilder.h b/libsolidity/analysis/ControlFlowBuilder.h index 40605e00..f196e5fc 100644 --- a/libsolidity/analysis/ControlFlowBuilder.h +++ b/libsolidity/analysis/ControlFlowBuilder.h @@ -38,14 +38,11 @@ public: CFG::NodeContainer& _nodeContainer, FunctionDefinition const& _function ); - static std::unique_ptr<ModifierFlow> createModifierFlow( - CFG::NodeContainer& _nodeContainer, - ModifierDefinition const& _modifier - ); private: explicit ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow); + // Visits for constructing the control flow. bool visit(BinaryOperation const& _operation) override; bool visit(Conditional const& _conditional) override; bool visit(IfStatement const& _ifStatement) override; @@ -54,12 +51,20 @@ private: bool visit(Break const&) override; bool visit(Continue const&) override; bool visit(Throw const&) override; - bool visit(Block const&) override; - void endVisit(Block const&) override; - bool visit(Return const& _return) override; bool visit(PlaceholderStatement const&) override; bool visit(FunctionCall const& _functionCall) override; + bool visit(ModifierInvocation const& _modifierInvocation) override; + + // Visits for constructing the control flow as well as filling variable occurrences. + bool visit(FunctionDefinition const& _functionDefinition) override; + bool visit(Return const& _return) override; + // Visits for filling variable occurrences. + bool visit(FunctionTypeName const& _functionTypeName) override; + bool visit(InlineAssembly const& _inlineAssembly) override; + bool visit(VariableDeclaration const& _variableDeclaration) override; + bool visit(VariableDeclarationStatement const& _variableDeclarationStatement) override; + bool visit(Identifier const& _identifier) override; /// Appends the control flow of @a _node to the current control flow. void appendControlFlow(ASTNode const& _node); @@ -73,9 +78,6 @@ private: static void connect(CFGNode* _from, CFGNode* _to); -protected: - bool visitNode(ASTNode const& node) override; - private: /// Splits the control flow starting at the current node into n paths. @@ -114,17 +116,18 @@ private: CFG::NodeContainer& m_nodeContainer; - /// The control flow of the function that is currently parsed. - /// Note: this can also be a ModifierFlow - FunctionFlow const& m_currentFunctionFlow; - CFGNode* m_currentNode = nullptr; + CFGNode* m_returnNode = nullptr; + CFGNode* m_revertNode = nullptr; /// The current jump destination of break Statements. CFGNode* m_breakJump = nullptr; /// The current jump destination of continue Statements. CFGNode* m_continueJump = nullptr; + CFGNode* m_placeholderEntry = nullptr; + CFGNode* m_placeholderExit = nullptr; + /// Helper class that replaces the break and continue jump destinations for the /// current scope and restores the originals at the end of the scope. class BreakContinueScope diff --git a/libsolidity/analysis/ControlFlowGraph.cpp b/libsolidity/analysis/ControlFlowGraph.cpp index b8860158..8960166a 100644 --- a/libsolidity/analysis/ControlFlowGraph.cpp +++ b/libsolidity/analysis/ControlFlowGraph.cpp @@ -16,10 +16,9 @@ */ #include <libsolidity/analysis/ControlFlowGraph.h> -#include <libsolidity/analysis/ControlFlowBuilder.h> +#include <libsolidity/analysis/ControlFlowBuilder.h> #include <boost/range/adaptor/reversed.hpp> - #include <algorithm> using namespace std; @@ -29,20 +28,14 @@ using namespace dev::solidity; bool CFG::constructFlow(ASTNode const& _astRoot) { _astRoot.accept(*this); - applyModifiers(); return Error::containsOnlyWarnings(m_errorReporter.errors()); } -bool CFG::visit(ModifierDefinition const& _modifier) -{ - m_modifierControlFlow[&_modifier] = ControlFlowBuilder::createModifierFlow(m_nodeContainer, _modifier); - return false; -} - bool CFG::visit(FunctionDefinition const& _function) { - m_functionControlFlow[&_function] = ControlFlowBuilder::createFunctionFlow(m_nodeContainer, _function); + if (_function.isImplemented()) + m_functionControlFlow[&_function] = ControlFlowBuilder::createFunctionFlow(m_nodeContainer, _function); return false; } @@ -57,81 +50,3 @@ CFGNode* CFG::NodeContainer::newNode() m_nodes.emplace_back(new CFGNode()); return m_nodes.back().get(); } - -void CFG::applyModifiers() -{ - for (auto const& function: m_functionControlFlow) - { - for (auto const& modifierInvocation: boost::adaptors::reverse(function.first->modifiers())) - { - if (auto modifierDefinition = dynamic_cast<ModifierDefinition const*>( - modifierInvocation->name()->annotation().referencedDeclaration - )) - { - solAssert(m_modifierControlFlow.count(modifierDefinition), ""); - applyModifierFlowToFunctionFlow(*m_modifierControlFlow[modifierDefinition], function.second.get()); - } - } - } -} - -void CFG::applyModifierFlowToFunctionFlow( - ModifierFlow const& _modifierFlow, - FunctionFlow* _functionFlow -) -{ - solAssert(!!_functionFlow, ""); - - map<CFGNode*, CFGNode*> copySrcToCopyDst; - - // inherit the revert node of the function - copySrcToCopyDst[_modifierFlow.revert] = _functionFlow->revert; - - // replace the placeholder nodes by the function entry and exit - copySrcToCopyDst[_modifierFlow.placeholderEntry] = _functionFlow->entry; - copySrcToCopyDst[_modifierFlow.placeholderExit] = _functionFlow->exit; - - stack<CFGNode*> nodesToCopy; - nodesToCopy.push(_modifierFlow.entry); - - // map the modifier entry to a new node that will become the new function entry - copySrcToCopyDst[_modifierFlow.entry] = m_nodeContainer.newNode(); - - while (!nodesToCopy.empty()) - { - CFGNode* copySrcNode = nodesToCopy.top(); - nodesToCopy.pop(); - - solAssert(copySrcToCopyDst.count(copySrcNode), ""); - - CFGNode* copyDstNode = copySrcToCopyDst[copySrcNode]; - - copyDstNode->block = copySrcNode->block; - for (auto const& entry: copySrcNode->entries) - { - if (!copySrcToCopyDst.count(entry)) - { - copySrcToCopyDst[entry] = m_nodeContainer.newNode(); - nodesToCopy.push(entry); - } - copyDstNode->entries.emplace_back(copySrcToCopyDst[entry]); - } - for (auto const& exit: copySrcNode->exits) - { - if (!copySrcToCopyDst.count(exit)) - { - copySrcToCopyDst[exit] = m_nodeContainer.newNode(); - nodesToCopy.push(exit); - } - copyDstNode->exits.emplace_back(copySrcToCopyDst[exit]); - } - } - - // if the modifier control flow never reached its exit node, - // we need to create a new (disconnected) exit node now - if (!copySrcToCopyDst.count(_modifierFlow.exit)) - copySrcToCopyDst[_modifierFlow.exit] = m_nodeContainer.newNode(); - - _functionFlow->entry = copySrcToCopyDst[_modifierFlow.entry]; - _functionFlow->exit = copySrcToCopyDst[_modifierFlow.exit]; -} diff --git a/libsolidity/analysis/ControlFlowGraph.h b/libsolidity/analysis/ControlFlowGraph.h index 8fe9fe8e..cc0113d8 100644 --- a/libsolidity/analysis/ControlFlowGraph.h +++ b/libsolidity/analysis/ControlFlowGraph.h @@ -31,25 +31,57 @@ namespace dev namespace solidity { -/** Basic Control Flow Block. - * Basic block of control flow. Consists of a set of (unordered) AST nodes - * for which control flow is always linear. A basic control flow block - * encompasses at most one scope. Reverts are considered to break the control - * flow. - * @todo Handle function calls correctly. So far function calls are not considered - * to change the control flow. - */ -struct ControlFlowBlock +/** Occurrence of a variable in a block of control flow. + * Stores the declaration of the referenced variable, the + * kind of the occurrence and possibly the node at which + * it occurred. + */ +class VariableOccurrence { - /// All variable declarations inside this control flow block. - std::vector<VariableDeclaration const*> variableDeclarations; - /// All expressions inside this control flow block (this includes all subexpressions!). - std::vector<Expression const*> expressions; - /// All inline assembly statements inside in this control flow block. - std::vector<InlineAssembly const*> inlineAssemblyStatements; - /// If control flow returns in this node, the return statement is stored in returnStatement, - /// otherwise returnStatement is nullptr. - Return const* returnStatement = nullptr; +public: + enum class Kind + { + Declaration, + Access, + Return, + Assignment, + InlineAssembly + }; + VariableOccurrence(VariableDeclaration const& _declaration, Kind _kind, ASTNode const* _occurrence): + m_declaration(_declaration), m_occurrenceKind(_kind), m_occurrence(_occurrence) + { + } + + /// Defines a deterministic order on variable occurrences. + bool operator<(VariableOccurrence const& _rhs) const + { + if (m_occurrence && _rhs.m_occurrence) + { + if (m_occurrence->id() < _rhs.m_occurrence->id()) return true; + if (_rhs.m_occurrence->id() < m_occurrence->id()) return false; + } + else if (_rhs.m_occurrence) + return true; + else if (m_occurrence) + return false; + + using KindCompareType = std::underlying_type<VariableOccurrence::Kind>::type; + return + std::make_pair(m_declaration.id(), static_cast<KindCompareType>(m_occurrenceKind)) < + std::make_pair(_rhs.m_declaration.id(), static_cast<KindCompareType>(_rhs.m_occurrenceKind)) + ; + } + + VariableDeclaration const& declaration() const { return m_declaration; } + Kind kind() const { return m_occurrenceKind; }; + ASTNode const* occurrence() const { return m_occurrence; } +private: + /// Declaration of the occurring variable. + VariableDeclaration const& m_declaration; + /// Kind of occurrence. + Kind m_occurrenceKind = Kind::Access; + /// AST node at which the variable occurred, if available (may be nullptr). + ASTNode const* m_occurrence = nullptr; }; /** Node of the Control Flow Graph. @@ -64,14 +96,15 @@ struct CFGNode /// Exit nodes. All CFG nodes to which control flow may continue after this node. std::vector<CFGNode*> exits; - /// Control flow in the node. - ControlFlowBlock block; + /// Variable occurrences in the node. + std::vector<VariableOccurrence> variableOccurrences; }; /** Describes the control flow of a function. */ struct FunctionFlow { - virtual ~FunctionFlow() {} + virtual ~FunctionFlow() = default; + /// Entry node. Control flow of the function starts here. /// This node is empty and does not have any entries. CFGNode* entry = nullptr; @@ -85,19 +118,6 @@ struct FunctionFlow CFGNode* revert = nullptr; }; -/** Describes the control flow of a modifier. - * Every placeholder breaks the control flow. The node preceding the - * placeholder is assigned placeholderEntry as exit and the node - * following the placeholder is assigned placeholderExit as entry. - */ -struct ModifierFlow: FunctionFlow -{ - /// Control flow leading towards a placeholder exit in placeholderEntry. - CFGNode* placeholderEntry = nullptr; - /// Control flow coming from a placeholder enter from placeholderExit. - CFGNode* placeholderExit = nullptr; -}; - class CFG: private ASTConstVisitor { public: @@ -105,7 +125,6 @@ public: bool constructFlow(ASTNode const& _astRoot); - bool visit(ModifierDefinition const& _modifier) override; bool visit(FunctionDefinition const& _function) override; FunctionFlow const& functionFlow(FunctionDefinition const& _function) const; @@ -118,20 +137,6 @@ public: std::vector<std::unique_ptr<CFGNode>> m_nodes; }; private: - /// Initially the control flow for all functions *ignoring* modifiers and for - /// all modifiers is constructed. Afterwards the control flow of functions - /// is adjusted by applying all modifiers. - void applyModifiers(); - - /// Creates a copy of the modifier flow @a _modifierFlow, while replacing the - /// placeholder entry and exit with the function entry and exit, as well as - /// replacing the modifier revert node with the function's revert node. - /// The resulting control flow is the new function flow with the modifier applied. - /// @a _functionFlow is updated in-place. - void applyModifierFlowToFunctionFlow( - ModifierFlow const& _modifierFlow, - FunctionFlow* _functionFlow - ); langutil::ErrorReporter& m_errorReporter; @@ -141,7 +146,6 @@ private: NodeContainer m_nodeContainer; std::map<FunctionDefinition const*, std::unique_ptr<FunctionFlow>> m_functionControlFlow; - std::map<ModifierDefinition const*, std::unique_ptr<ModifierFlow>> m_modifierControlFlow; }; } diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index cf12a49d..d0657898 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/DeclarationContainer.h> + #include <libsolidity/ast/AST.h> #include <libsolidity/ast/Types.h> #include <libdevcore/StringUtils.h> diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h index 9d7a17a3..e26f5891 100644 --- a/libsolidity/analysis/DeclarationContainer.h +++ b/libsolidity/analysis/DeclarationContainer.h @@ -22,11 +22,10 @@ #pragma once +#include <libsolidity/ast/ASTForward.h> +#include <boost/noncopyable.hpp> #include <map> #include <set> -#include <boost/noncopyable.hpp> - -#include <libsolidity/ast/ASTForward.h> namespace dev { diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index 69a7a43c..0878b550 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -22,9 +22,10 @@ */ #include <libsolidity/analysis/DocStringAnalyser.h> + #include <libsolidity/ast/AST.h> -#include <liblangutil/ErrorReporter.h> #include <libsolidity/parsing/DocStringParser.h> +#include <liblangutil/ErrorReporter.h> using namespace std; using namespace dev; diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index cba2655c..cd5fe07d 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -21,10 +21,11 @@ * Container of the (implicit and explicit) global objects. */ -#include <memory> #include <libsolidity/analysis/GlobalContext.h> + #include <libsolidity/ast/AST.h> #include <libsolidity/ast/Types.h> +#include <memory> using namespace std; diff --git a/libsolidity/analysis/GlobalContext.h b/libsolidity/analysis/GlobalContext.h index 4ed08711..09611c41 100644 --- a/libsolidity/analysis/GlobalContext.h +++ b/libsolidity/analysis/GlobalContext.h @@ -22,12 +22,12 @@ #pragma once -#include <string> -#include <vector> +#include <libsolidity/ast/ASTForward.h> +#include <boost/noncopyable.hpp> #include <map> #include <memory> -#include <boost/noncopyable.hpp> -#include <libsolidity/ast/ASTForward.h> +#include <string> +#include <vector> namespace dev { diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 0528a200..95bc69fe 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -22,11 +22,10 @@ #include <libsolidity/analysis/NameAndTypeResolver.h> -#include <libsolidity/ast/AST.h> #include <libsolidity/analysis/TypeChecker.h> +#include <libsolidity/ast/AST.h> #include <liblangutil/ErrorReporter.h> #include <libdevcore/StringUtils.h> - #include <boost/algorithm/string.hpp> using namespace std; diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 1b034ef4..89c53932 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -22,13 +22,15 @@ #pragma once -#include <map> -#include <list> -#include <boost/noncopyable.hpp> #include <libsolidity/analysis/DeclarationContainer.h> #include <libsolidity/analysis/ReferencesResolver.h> -#include <libsolidity/ast/ASTVisitor.h> #include <libsolidity/ast/ASTAnnotations.h> +#include <libsolidity/ast/ASTVisitor.h> + +#include <boost/noncopyable.hpp> + +#include <list> +#include <map> namespace langutil { diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index 27cbcd45..6a7e5c7e 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -16,15 +16,14 @@ */ #include <libsolidity/analysis/PostTypeChecker.h> -#include <libsolidity/ast/AST.h> + #include <libsolidity/analysis/SemVerHandler.h> -#include <liblangutil/ErrorReporter.h> +#include <libsolidity/ast/AST.h> #include <libsolidity/interface/Version.h> - +#include <liblangutil/ErrorReporter.h> #include <libdevcore/Algorithms.h> #include <boost/range/adaptor/map.hpp> - #include <memory> using namespace std; diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index c4931d98..4e6d7f59 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -21,12 +21,15 @@ */ #include <libsolidity/analysis/ReferencesResolver.h> -#include <libsolidity/ast/AST.h> #include <libsolidity/analysis/NameAndTypeResolver.h> #include <libsolidity/analysis/ConstantEvaluator.h> +#include <libsolidity/ast/AST.h> + #include <libyul/AsmAnalysis.h> #include <libyul/AsmAnalysisInfo.h> #include <libyul/AsmData.h> +#include <libyul/backends/evm/EVMDialect.h> + #include <liblangutil/ErrorReporter.h> #include <liblangutil/Exceptions.h> @@ -316,7 +319,14 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) // We use the latest EVM version because we will re-run it anyway. yul::AsmAnalysisInfo analysisInfo; boost::optional<Error::Type> errorTypeForLoose = Error::Type::SyntaxError; - yul::AsmAnalyzer(analysisInfo, errorsIgnored, EVMVersion(), errorTypeForLoose, yul::AsmFlavour::Loose, resolver).analyze(_inlineAssembly.operations()); + yul::AsmAnalyzer( + analysisInfo, + errorsIgnored, + EVMVersion(), + errorTypeForLoose, + yul::EVMDialect::looseAssemblyForEVM(), + resolver + ).analyze(_inlineAssembly.operations()); return false; } diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index 32c0553f..b3de9458 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -22,12 +22,13 @@ #pragma once -#include <map> -#include <list> -#include <boost/noncopyable.hpp> #include <libsolidity/ast/ASTVisitor.h> #include <libsolidity/ast/ASTAnnotations.h> +#include <boost/noncopyable.hpp> +#include <list> +#include <map> + namespace langutil { class ErrorReporter; diff --git a/libsolidity/analysis/SemVerHandler.cpp b/libsolidity/analysis/SemVerHandler.cpp index 64fa17b3..7c6ba91f 100644 --- a/libsolidity/analysis/SemVerHandler.cpp +++ b/libsolidity/analysis/SemVerHandler.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/SemVerHandler.h> + #include <functional> using namespace std; diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 38391841..aaaa4f9f 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/StaticAnalyzer.h> + #include <libsolidity/analysis/ConstantEvaluator.h> #include <libsolidity/ast/AST.h> #include <liblangutil/ErrorReporter.h> @@ -63,21 +64,21 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function) void StaticAnalyzer::endVisit(FunctionDefinition const&) { - m_currentFunction = nullptr; - m_constructor = false; - for (auto const& var: m_localVarUseCount) - if (var.second == 0) - { - if (var.first.second->isCallableParameter()) - m_errorReporter.warning( - var.first.second->location(), - "Unused function parameter. Remove or comment out the variable name to silence this warning." - ); - else - m_errorReporter.warning(var.first.second->location(), "Unused local variable."); - } - + if (m_currentFunction && !m_currentFunction->body().statements().empty()) + for (auto const& var: m_localVarUseCount) + if (var.second == 0) + { + if (var.first.second->isCallableParameter()) + m_errorReporter.warning( + var.first.second->location(), + "Unused function parameter. Remove or comment out the variable name to silence this warning." + ); + else + m_errorReporter.warning(var.first.second->location(), "Unused local variable."); + } m_localVarUseCount.clear(); + m_constructor = false; + m_currentFunction = nullptr; } bool StaticAnalyzer::visit(Identifier const& _identifier) diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index a73d7e5c..066b5004 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -16,15 +16,18 @@ */ #include <libsolidity/analysis/SyntaxChecker.h> -#include <memory> + +#include <libsolidity/analysis/SemVerHandler.h> #include <libsolidity/ast/AST.h> #include <libsolidity/ast/ExperimentalFeatures.h> -#include <libsolidity/analysis/SemVerHandler.h> -#include <liblangutil/ErrorReporter.h> #include <libsolidity/interface/Version.h> -#include <boost/algorithm/cxx11/all_of.hpp> +#include <liblangutil/ErrorReporter.h> + +#include <boost/algorithm/cxx11/all_of.hpp> #include <boost/algorithm/string.hpp> + +#include <memory> #include <string> using namespace std; @@ -111,7 +114,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma) vector<string> literals(_pragma.literals().begin() + 1, _pragma.literals().end()); SemVerMatchExpressionParser parser(tokens, literals); auto matchExpression = parser.parse(); - SemVerVersion currentVersion{string(VersionString)}; + static SemVerVersion const currentVersion{string(VersionString)}; if (!matchExpression.matches(currentVersion)) m_errorReporter.syntaxError( _pragma.location(), diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 9350df05..507a2c94 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -26,6 +26,7 @@ #include <libyul/AsmAnalysis.h> #include <libyul/AsmAnalysisInfo.h> #include <libyul/AsmData.h> +#include <libyul/backends/evm/EVMDialect.h> #include <liblangutil/ErrorReporter.h> @@ -33,8 +34,8 @@ #include <libdevcore/StringUtils.h> #include <boost/algorithm/cxx11/all_of.hpp> -#include <boost/algorithm/string/predicate.hpp> #include <boost/algorithm/string/join.hpp> +#include <boost/algorithm/string/predicate.hpp> #include <memory> #include <vector> @@ -658,7 +659,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) m_errorReporter, m_evmVersion, Error::Type::SyntaxError, - yul::AsmFlavour::Loose, + yul::EVMDialect::looseAssemblyForEVM(), identifierAccess ); if (!analyzer.analyze(_inlineAssembly.operations())) @@ -935,30 +936,32 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) var.accept(*this); if (!valueComponentType->isImplicitlyConvertibleTo(*var.annotation().type)) { + auto errorMsg = "Type " + + valueComponentType->toString() + + " is not implicitly convertible to expected type " + + var.annotation().type->toString(); if ( valueComponentType->category() == Type::Category::RationalNumber && dynamic_cast<RationalNumberType const&>(*valueComponentType).isFractional() && valueComponentType->mobileType() ) - m_errorReporter.typeError( - _statement.location(), - "Type " + - valueComponentType->toString() + - " is not implicitly convertible to expected type " + - var.annotation().type->toString() + - ". Try converting to type " + - valueComponentType->mobileType()->toString() + - " or use an explicit conversion." - ); + { + if (var.annotation().type->operator==(*valueComponentType->mobileType())) + m_errorReporter.typeError( + _statement.location(), + errorMsg + ", but it can be explicitly converted." + ); + else + m_errorReporter.typeError( + _statement.location(), + errorMsg + + ". Try converting to type " + + valueComponentType->mobileType()->toString() + + " or use an explicit conversion." + ); + } else - m_errorReporter.typeError( - _statement.location(), - "Type " + - valueComponentType->toString() + - " is not implicitly convertible to expected type " + - var.annotation().type->toString() + - "." - ); + m_errorReporter.typeError(_statement.location(), errorMsg + "."); } } } @@ -1252,7 +1255,8 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) { TypePointer const& leftType = type(_operation.leftExpression()); TypePointer const& rightType = type(_operation.rightExpression()); - TypePointer commonType = leftType->binaryOperatorResult(_operation.getOperator(), rightType); + TypeResult result = leftType->binaryOperatorResult(_operation.getOperator(), rightType); + TypePointer commonType = result.get(); if (!commonType) { m_errorReporter.typeError( @@ -1262,7 +1266,8 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) " not compatible with types " + leftType->toString() + " and " + - rightType->toString() + rightType->toString() + + (!result.message().empty() ? ". " + result.message() : "") ); commonType = leftType; } @@ -2329,30 +2334,32 @@ bool TypeChecker::expectType(Expression const& _expression, Type const& _expecte _expression.accept(*this); if (!type(_expression)->isImplicitlyConvertibleTo(_expectedType)) { + auto errorMsg = "Type " + + type(_expression)->toString() + + " is not implicitly convertible to expected type " + + _expectedType.toString(); if ( type(_expression)->category() == Type::Category::RationalNumber && dynamic_pointer_cast<RationalNumberType const>(type(_expression))->isFractional() && type(_expression)->mobileType() ) - m_errorReporter.typeError( - _expression.location(), - "Type " + - type(_expression)->toString() + - " is not implicitly convertible to expected type " + - _expectedType.toString() + - ". Try converting to type " + - type(_expression)->mobileType()->toString() + - " or use an explicit conversion." - ); + { + if (_expectedType.operator==(*type(_expression)->mobileType())) + m_errorReporter.typeError( + _expression.location(), + errorMsg + ", but it can be explicitly converted." + ); + else + m_errorReporter.typeError( + _expression.location(), + errorMsg + + ". Try converting to type " + + type(_expression)->mobileType()->toString() + + " or use an explicit conversion." + ); + } else - m_errorReporter.typeError( - _expression.location(), - "Type " + - type(_expression)->toString() + - " is not implicitly convertible to expected type " + - _expectedType.toString() + - "." - ); + m_errorReporter.typeError(_expression.location(), errorMsg + "."); return false; } return true; diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index ebfcdadc..b60c571a 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -24,10 +24,10 @@ #include <liblangutil/EVMVersion.h> -#include <libsolidity/ast/Types.h> #include <libsolidity/ast/ASTAnnotations.h> #include <libsolidity/ast/ASTForward.h> #include <libsolidity/ast/ASTVisitor.h> +#include <libsolidity/ast/Types.h> namespace langutil { diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 1112d682..eb019481 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -16,14 +16,10 @@ */ #include <libsolidity/analysis/ViewPureChecker.h> - -#include <libevmasm/SemanticInformation.h> - #include <libsolidity/ast/ExperimentalFeatures.h> #include <libyul/AsmData.h> - #include <liblangutil/ErrorReporter.h> - +#include <libevmasm/SemanticInformation.h> #include <functional> using namespace std; @@ -156,6 +152,7 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) m_bestMutabilityAndLocation.mutability < _funDef.stateMutability() && _funDef.stateMutability() != StateMutability::Payable && _funDef.isImplemented() && + !_funDef.body().statements().empty() && !_funDef.isConstructor() && !_funDef.isFallback() && !_funDef.annotation().superFunction |