diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 1cff26c26..cb8694537 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -112,7 +112,7 @@ void ContractLevelChecker::checkDuplicateFunctions(ContractDefinition const& _co functions[function->name()].push_back(function); } - findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); + findDuplicateDefinitions(functions); } void ContractLevelChecker::checkDuplicateEvents(ContractDefinition const& _contract) @@ -123,11 +123,11 @@ void ContractLevelChecker::checkDuplicateEvents(ContractDefinition const& _contr for (EventDefinition const* event: _contract.events()) events[event->name()].push_back(event); - findDuplicateDefinitions(events, "Event with same name and arguments defined twice."); + findDuplicateDefinitions(events); } template -void ContractLevelChecker::findDuplicateDefinitions(map> const& _definitions, string _message) +void ContractLevelChecker::findDuplicateDefinitions(map> const& _definitions) { for (auto const& it: _definitions) { @@ -146,13 +146,27 @@ void ContractLevelChecker::findDuplicateDefinitions(map> const if (ssl.infos.size() > 0) { - ssl.limitSize(_message); + ErrorId error; + string message; + if constexpr (is_same_v) + { + error = 1686_error; + message = "Function with same name and arguments defined twice."; + } + else + { + static_assert(is_same_v, "Expected \"FunctionDefinition const*\" or \"EventDefinition const*\""); + error = 5883_error; + message = "Event with same name and arguments defined twice."; + } + + ssl.limitSize(message); m_errorReporter.declarationError( - 1686_error, + error, overloads[i]->location(), ssl, - _message + message ); } } @@ -222,9 +236,8 @@ void ContractLevelChecker::checkAbstractDefinitions(ContractDefinition const& _c 3656_error, _contract.location(), ssl, - "Contract \"" + _contract.annotation().canonicalName - + "\" should be marked as abstract."); - + "Contract \"" + _contract.annotation().canonicalName + "\" should be marked as abstract." + ); } } diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index 736b3d3d7..e40cb8b0d 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -60,7 +60,7 @@ private: void checkDuplicateFunctions(ContractDefinition const& _contract); void checkDuplicateEvents(ContractDefinition const& _contract); template - void findDuplicateDefinitions(std::map> const& _definitions, std::string _message); + void findDuplicateDefinitions(std::map> const& _definitions); /// Checks for unimplemented functions and modifiers. void checkAbstractDefinitions(ContractDefinition const& _contract); /// Checks that the base constructor arguments are properly provided. diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 1f4fa10a2..4cf2ff8f4 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -506,12 +506,13 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr ); if (!_overriding.overrides()) - overrideError(_overriding, _super, "Overriding " + _overriding.astNodeName() + " is missing \"override\" specifier."); + overrideError(_overriding, _super, 9456_error, "Overriding " + _overriding.astNodeName() + " is missing \"override\" specifier."); if (_super.isVariable()) overrideError( _super, _overriding, + 1452_error, "Cannot override public state variable.", "Overriding " + _overriding.astNodeName() + " is here:" ); @@ -519,6 +520,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr overrideError( _super, _overriding, + 4334_error, "Trying to override non-virtual " + _super.astNodeName() + ". Did you forget to add \"virtual\"?", "Overriding " + _overriding.astNodeName() + " is here:" ); @@ -526,7 +528,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr if (_overriding.isVariable()) { if (_super.visibility() != Visibility::External) - overrideError(_overriding, _super, "Public state variables can only override functions with external visibility."); + overrideError(_overriding, _super, 5225_error, "Public state variables can only override functions with external visibility."); solAssert(_overriding.visibility() == Visibility::External, ""); } else if (_overriding.visibility() != _super.visibility()) @@ -537,7 +539,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr _super.visibility() == Visibility::External && _overriding.visibility() == Visibility::Public )) - overrideError(_overriding, _super, "Overriding " + _overriding.astNodeName() + " visibility differs."); + overrideError(_overriding, _super, 9098_error, "Overriding " + _overriding.astNodeName() + " visibility differs."); } if (_super.isFunction()) @@ -548,7 +550,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr solAssert(functionType->hasEqualParameterTypes(*superType), "Override doesn't have equal parameters!"); if (!functionType->hasEqualReturnTypes(*superType)) - overrideError(_overriding, _super, "Overriding " + _overriding.astNodeName() + " return types differ."); + overrideError(_overriding, _super, 4822_error, "Overriding " + _overriding.astNodeName() + " return types differ."); // This is only relevant for a function overriding a function. if (_overriding.isFunction()) @@ -557,6 +559,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr overrideError( _overriding, _super, + 2837_error, "Overriding function changes state mutability from \"" + stateMutabilityToString(_super.stateMutability()) + "\" to \"" + @@ -568,6 +571,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr overrideError( _overriding, _super, + 4593_error, "Overriding an implemented function with an unimplemented function is not allowed." ); } @@ -577,6 +581,7 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr void OverrideChecker::overrideListError( OverrideProxy const& _item, set _secondary, + ErrorId _error, string const& _message1, string const& _message2 ) @@ -594,7 +599,7 @@ void OverrideChecker::overrideListError( contractSingularPlural = "contracts "; m_errorReporter.typeError( - 5883_error, + _error, _item.overrides() ? _item.overrides()->location() : _item.location(), ssl, _message1 + @@ -605,10 +610,10 @@ void OverrideChecker::overrideListError( ); } -void OverrideChecker::overrideError(Declaration const& _overriding, Declaration const& _super, string const& _message, string const& _secondaryMsg) +void OverrideChecker::overrideError(Declaration const& _overriding, Declaration const& _super, ErrorId _error, string const& _message, string const& _secondaryMsg) { m_errorReporter.typeError( - 9456_error, + _error, _overriding.location(), SecondarySourceLocation().append(_secondaryMsg, _super.location()), _message @@ -616,10 +621,10 @@ void OverrideChecker::overrideError(Declaration const& _overriding, Declaration } -void OverrideChecker::overrideError(OverrideProxy const& _overriding, OverrideProxy const& _super, string const& _message, string const& _secondaryMsg) +void OverrideChecker::overrideError(OverrideProxy const& _overriding, OverrideProxy const& _super, ErrorId _error, string const& _message, string const& _secondaryMsg) { m_errorReporter.typeError( - 1452_error, + _error, _overriding.location(), SecondarySourceLocation().append(_secondaryMsg, _super.location()), _message @@ -773,11 +778,11 @@ void OverrideChecker::checkOverrideList(OverrideProxy _item, OverrideProxyBySign 4520_error, list[i]->location(), ssl, - "Duplicate contract \"" + - joinHumanReadable(list[i]->namePath(), ".") + - "\" found in override list of \"" + - _item.name() + - "\"." + "Duplicate contract \"" + + joinHumanReadable(list[i]->namePath(), ".") + + "\" found in override list of \"" + + _item.name() + + "\"." ); } } @@ -810,6 +815,7 @@ void OverrideChecker::checkOverrideList(OverrideProxy _item, OverrideProxyBySign overrideListError( _item, missingContracts, + 4327_error, _item.astNodeNameCapitalized() + " needs to specify overridden ", "" ); @@ -819,6 +825,7 @@ void OverrideChecker::checkOverrideList(OverrideProxy _item, OverrideProxyBySign overrideListError( _item, surplusContracts, + 2353_error, "Invalid ", "specified in override list: " ); diff --git a/libsolidity/analysis/OverrideChecker.h b/libsolidity/analysis/OverrideChecker.h index 9d7776028..d47b89f3d 100644 --- a/libsolidity/analysis/OverrideChecker.h +++ b/libsolidity/analysis/OverrideChecker.h @@ -32,6 +32,7 @@ namespace solidity::langutil { class ErrorReporter; +struct ErrorId; struct SourceLocation; } @@ -160,18 +161,21 @@ private: void overrideListError( OverrideProxy const& _item, std::set _secondary, + langutil::ErrorId _error, std::string const& _message1, std::string const& _message2 ); void overrideError( Declaration const& _overriding, Declaration const& _super, + langutil::ErrorId _error, std::string const& _message, std::string const& _secondaryMsg = "Overridden function is here:" ); void overrideError( OverrideProxy const& _overriding, OverrideProxy const& _super, + langutil::ErrorId _error, std::string const& _message, std::string const& _secondaryMsg = "Overridden function is here:" ); diff --git a/libsolidity/formal/BMC.cpp b/libsolidity/formal/BMC.cpp index e8fc331fa..952eb1c50 100644 --- a/libsolidity/formal/BMC.cpp +++ b/libsolidity/formal/BMC.cpp @@ -21,8 +21,6 @@ #include #include -#include - using namespace std; using namespace solidity; using namespace solidity::util; @@ -594,8 +592,7 @@ void BMC::checkConstantCondition(BMCVerificationTarget& _target) *_target.expression, _target.constraints, _target.value, - _target.callStack, - "Condition is always $VALUE." + _target.callStack ); } @@ -613,6 +610,8 @@ void BMC::checkUnderflow(BMCVerificationTarget& _target, smt::Expression const& _target.callStack, _target.modelExpressions, _target.expression->location(), + 4144_error, + 8312_error, "Underflow (resulting value less than " + formatNumberReadable(intType->minValue()) + ")", "", &_target.value @@ -633,6 +632,8 @@ void BMC::checkOverflow(BMCVerificationTarget& _target, smt::Expression const& _ _target.callStack, _target.modelExpressions, _target.expression->location(), + 2661_error, + 8065_error, "Overflow (resulting value larger than " + formatNumberReadable(intType->maxValue()) + ")", "", &_target.value @@ -647,6 +648,8 @@ void BMC::checkDivByZero(BMCVerificationTarget& _target) _target.callStack, _target.modelExpressions, _target.expression->location(), + 3046_error, + 5272_error, "Division by zero", "", &_target.value @@ -661,6 +664,8 @@ void BMC::checkBalance(BMCVerificationTarget& _target) _target.callStack, _target.modelExpressions, _target.expression->location(), + 1236_error, + 4010_error, "Insufficient funds", "address(this).balance" ); @@ -675,6 +680,8 @@ void BMC::checkAssert(BMCVerificationTarget& _target) _target.callStack, _target.modelExpressions, _target.expression->location(), + 4661_error, + 7812_error, "Assertion violation" ); } @@ -705,9 +712,11 @@ void BMC::addVerificationTarget( void BMC::checkCondition( smt::Expression _condition, - vector const& callStack, + vector const& _callStack, pair, vector> const& _modelExpressions, SourceLocation const& _location, + ErrorId _errorHappens, + ErrorId _errorMightHappen, string const& _description, string const& _additionalValueName, smt::Expression const* _additionalValue @@ -719,7 +728,7 @@ void BMC::checkCondition( vector expressionsToEvaluate; vector expressionNames; tie(expressionsToEvaluate, expressionNames) = _modelExpressions; - if (callStack.size()) + if (_callStack.size()) if (_additionalValue) { expressionsToEvaluate.emplace_back(*_additionalValue); @@ -750,7 +759,7 @@ void BMC::checkCondition( { std::ostringstream message; message << _description << " happens here"; - if (callStack.size()) + if (_callStack.size()) { std::ostringstream modelMessage; modelMessage << " for:\n"; @@ -763,11 +772,11 @@ void BMC::checkCondition( for (auto const& eval: sortedModel) modelMessage << " " << eval.first << " = " << eval.second << "\n"; m_errorReporter.warning( - 4334_error, + _errorHappens, _location, message.str(), SecondarySourceLocation().append(modelMessage.str(), SourceLocation{}) - .append(SMTEncoder::callStackMessage(callStack)) + .append(SMTEncoder::callStackMessage(_callStack)) .append(move(secondaryLocation)) ); } @@ -781,7 +790,7 @@ void BMC::checkCondition( case smt::CheckResult::UNSATISFIABLE: break; case smt::CheckResult::UNKNOWN: - m_errorReporter.warning(5225_error, _location, _description + " might happen here.", secondaryLocation); + m_errorReporter.warning(_errorMightHappen, _location, _description + " might happen here.", secondaryLocation); break; case smt::CheckResult::CONFLICTING: m_errorReporter.warning(1584_error, _location, "At least two SMT solvers provided conflicting answers. Results might not be sound."); @@ -798,8 +807,7 @@ void BMC::checkBooleanNotConstant( Expression const& _condition, smt::Expression const& _constraints, smt::Expression const& _value, - vector const& _callStack, - string const& _description + vector const& _callStack ) { // Do not check for const-ness if this is a constant. @@ -832,22 +840,22 @@ void BMC::checkBooleanNotConstant( m_errorReporter.warning(2512_error, _condition.location(), "Condition unreachable.", SMTEncoder::callStackMessage(_callStack)); else { - string value; + string description; if (positiveResult == smt::CheckResult::SATISFIABLE) { solAssert(negatedResult == smt::CheckResult::UNSATISFIABLE, ""); - value = "true"; + description = "Condition is always true."; } else { solAssert(positiveResult == smt::CheckResult::UNSATISFIABLE, ""); solAssert(negatedResult == smt::CheckResult::SATISFIABLE, ""); - value = "false"; + description = "Condition is always false."; } m_errorReporter.warning( 6838_error, _condition.location(), - boost::algorithm::replace_all_copy(_description, "$VALUE", value), + description, SMTEncoder::callStackMessage(_callStack) ); } diff --git a/libsolidity/formal/BMC.h b/libsolidity/formal/BMC.h index f4b427ede..8b30f073d 100644 --- a/libsolidity/formal/BMC.h +++ b/libsolidity/formal/BMC.h @@ -44,6 +44,7 @@ using solidity::util::h256; namespace solidity::langutil { class ErrorReporter; +struct ErrorId; struct SourceLocation; } @@ -144,22 +145,22 @@ private: /// Check that a condition can be satisfied. void checkCondition( smt::Expression _condition, - std::vector const& callStack, + std::vector const& _callStack, std::pair, std::vector> const& _modelExpressions, langutil::SourceLocation const& _location, + langutil::ErrorId _errorHappens, + langutil::ErrorId _errorMightHappen, std::string const& _description, std::string const& _additionalValueName = "", smt::Expression const* _additionalValue = nullptr ); /// Checks that a boolean condition is not constant. Do not warn if the expression /// is a literal constant. - /// @param _description the warning string, $VALUE will be replaced by the constant value. void checkBooleanNotConstant( Expression const& _condition, smt::Expression const& _constraints, smt::Expression const& _value, - std::vector const& _callStack, - std::string const& _description + std::vector const& _callStack ); std::pair> checkSatisfiableAndGenerateModel(std::vector const& _expressionsToEvaluate);