From 37257548d080f36c5eb6bd557593fd54783540a3 Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Tue, 28 Apr 2020 14:04:07 +0530 Subject: [PATCH 01/15] Fixed bug when two empty NatSpec comments led to scanning past EOL --- Changelog.md | 2 + liblangutil/Scanner.cpp | 17 +++-- liblangutil/Scanner.h | 2 +- test/libsolidity/SolidityNatspecJSON.cpp | 75 +++++++++++++++++++ test/libsolidity/SolidityScanner.cpp | 4 +- .../natspec/docstring_double_empty.sol | 7 ++ 6 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 test/libsolidity/syntaxTests/natspec/docstring_double_empty.sol diff --git a/Changelog.md b/Changelog.md index 649b2e147..4e0e677d7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,8 @@ Compiler Features: Bugfixes: * Optimizer: Fixed a bug in BlockDeDuplicator. * Type Checker: Disallow assignments to storage variables of type ``mapping``. + * NatSpec: DocString block is terminated when encountering an empty line. + * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. ### 0.6.8 (2020-05-14) diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index 9d748c570..6cf319067 100644 --- a/liblangutil/Scanner.cpp +++ b/liblangutil/Scanner.cpp @@ -267,10 +267,13 @@ bool Scanner::skipWhitespace() return sourcePos() != startPosition; } -void Scanner::skipWhitespaceExceptUnicodeLinebreak() +bool Scanner::skipWhitespaceExceptUnicodeLinebreak() { + int const startPosition = sourcePos(); while (isWhiteSpace(m_char) && !isUnicodeLinebreak()) advance(); + // Return whether or not we skipped any characters. + return sourcePos() != startPosition; } Token Scanner::skipSingleLineComment() @@ -321,7 +324,7 @@ int Scanner::scanSingleLineDocComment() { // Check if next line is also a single-line comment. // If any whitespaces were skipped, use source position before. - if (!skipWhitespace()) + if (!skipWhitespaceExceptUnicodeLinebreak()) endPosition = m_source->position(); if (!m_source->isPastEndOfInput(3) && @@ -329,8 +332,10 @@ int Scanner::scanSingleLineDocComment() m_source->get(1) == '/' && m_source->get(2) == '/') { - addCommentLiteralChar('\n'); m_char = m_source->advanceAndGet(3); + if (atEndOfLine()) + continue; + addCommentLiteralChar('\n'); } else break; // next line is not a documentation comment, we are done @@ -389,9 +394,11 @@ Token Scanner::scanMultiLineDocComment() } else if (!m_source->isPastEndOfInput(1) && m_source->get(0) == '*' && m_source->get(1) != '/') { // skip first '*' in subsequent lines + m_char = m_source->advanceAndGet(1); + if (atEndOfLine()) // ignores empty lines + continue; if (charsAdded) - addCommentLiteralChar('\n'); - m_char = m_source->advanceAndGet(2); + addCommentLiteralChar('\n'); // corresponds to the end of previous line } else if (!m_source->isPastEndOfInput(1) && m_source->get(0) == '*' && m_source->get(1) == '/') { // if after newline the comment ends, don't insert the newline diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h index 5b365f134..d54bb7532 100644 --- a/liblangutil/Scanner.h +++ b/liblangutil/Scanner.h @@ -214,7 +214,7 @@ private: /// Skips all whitespace and @returns true if something was skipped. bool skipWhitespace(); /// Skips all whitespace that are neither '\r' nor '\n'. - void skipWhitespaceExceptUnicodeLinebreak(); + bool skipWhitespaceExceptUnicodeLinebreak(); Token skipSingleLineComment(); Token skipMultiLineComment(); diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index 7ae794a83..fa1d2614d 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -78,6 +78,81 @@ private: BOOST_FIXTURE_TEST_SUITE(SolidityNatspecJSON, DocumentationChecker) +BOOST_AUTO_TEST_CASE(user_empty_natspec_test) +{ + char const* sourceCode = R"( + contract test { + /// + /// + function f() public { + } + } + )"; + + char const* natspec = R"( + { + "methods": {} + } + )"; + + checkNatspec(sourceCode, "test", natspec, true); +} + +BOOST_AUTO_TEST_CASE(user_newline_break) +{ + char const* sourceCode = R"( + contract test { + /// + /// @notice hello + + /// @notice world + function f() public { + } + } + )"; + + char const* natspec = R"ABCDEF( + { + "methods": { + "f()": + { + "notice": "world" + } + } + } + )ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, true); +} + +BOOST_AUTO_TEST_CASE(user_multiline_empty_lines) +{ + char const* sourceCode = R"( + contract test { + /** + * + * + * @notice hello world + */ + function f() public { + } + } + )"; + + char const* natspec = R"ABCDEF( + { + "methods": { + "f()": { + "notice": "hello world" + } + } + } + )ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, true); +} + + BOOST_AUTO_TEST_CASE(user_basic_test) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityScanner.cpp b/test/libsolidity/SolidityScanner.cpp index a80ef8b5c..dc628674a 100644 --- a/test/libsolidity/SolidityScanner.cpp +++ b/test/libsolidity/SolidityScanner.cpp @@ -361,7 +361,7 @@ BOOST_AUTO_TEST_CASE(multiline_documentation_comments_parsed) BOOST_CHECK_EQUAL(scanner.next(), Token::Identifier); BOOST_CHECK_EQUAL(scanner.next(), Token::Identifier); BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); - BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); + BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), " Send $(value / 1000) chocolates to the user"); } BOOST_AUTO_TEST_CASE(multiline_documentation_no_stars) @@ -385,7 +385,7 @@ BOOST_AUTO_TEST_CASE(multiline_documentation_whitespace_hell) BOOST_CHECK_EQUAL(scanner.next(), Token::Identifier); BOOST_CHECK_EQUAL(scanner.next(), Token::Identifier); BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); - BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); + BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), " Send $(value / 1000) chocolates to the user"); } BOOST_AUTO_TEST_CASE(comment_before_eos) diff --git a/test/libsolidity/syntaxTests/natspec/docstring_double_empty.sol b/test/libsolidity/syntaxTests/natspec/docstring_double_empty.sol new file mode 100644 index 000000000..a84d5d928 --- /dev/null +++ b/test/libsolidity/syntaxTests/natspec/docstring_double_empty.sol @@ -0,0 +1,7 @@ +contract C { + /// + /// + function vote(uint id) public { + } +} +// ---- From bd28fedd9b513b0887b50ba263eba19688bc0fe8 Mon Sep 17 00:00:00 2001 From: Juan Franco Date: Wed, 20 May 2020 13:05:40 -0300 Subject: [PATCH 02/15] Change build OSX CircleCI to Release --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 15f122e48..a34649d74 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -531,7 +531,7 @@ jobs: xcode: "11.0.0" environment: TERM: xterm - CMAKE_BUILD_TYPE: Debug + CMAKE_BUILD_TYPE: Release steps: - checkout - restore_cache: From 09433332764c053057d50a3e8168a960d726d9ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 13 May 2020 18:50:57 +0200 Subject: [PATCH 03/15] Simplify endVisit() for internal calls - Define IRHelpers::referencedFunctionDeclaration() to avoid repeating the same dynamic_casts over and over again. --- libsolidity/codegen/ir/Common.cpp | 10 ++++ libsolidity/codegen/ir/Common.h | 5 ++ .../codegen/ir/IRGeneratorForStatements.cpp | 51 +++++++------------ 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/libsolidity/codegen/ir/Common.cpp b/libsolidity/codegen/ir/Common.cpp index 1f1cc1cfe..2bf4a36cf 100644 --- a/libsolidity/codegen/ir/Common.cpp +++ b/libsolidity/codegen/ir/Common.cpp @@ -99,3 +99,13 @@ string IRNames::zeroValue(Type const& _type, string const& _variableName) { return "zero_value_for_type_" + _type.identifier() + _variableName; } + +FunctionDefinition const* IRHelpers::referencedFunctionDeclaration(Expression const& _expression) +{ + if (auto memberAccess = dynamic_cast(&_expression)) + return dynamic_cast(memberAccess->annotation().referencedDeclaration); + else if (auto identifier = dynamic_cast(&_expression)) + return dynamic_cast(identifier->annotation().referencedDeclaration); + else + return nullptr; +} diff --git a/libsolidity/codegen/ir/Common.h b/libsolidity/codegen/ir/Common.h index f1235553b..3f4ea64f4 100644 --- a/libsolidity/codegen/ir/Common.h +++ b/libsolidity/codegen/ir/Common.h @@ -62,6 +62,11 @@ struct IRNames static std::string zeroValue(Type const& _type, std::string const& _variableName); }; +struct IRHelpers +{ + static FunctionDefinition const* referencedFunctionDeclaration(Expression const& _expression); +}; + } // Overloading std::less() makes it possible to use YulArity as a map key. We could define operator< diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index b91ed9f2b..b06da63c1 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -625,7 +625,8 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) arguments.push_back(callArguments[std::distance(callArgumentNames.begin(), it)]); } - if (auto memberAccess = dynamic_cast(&_functionCall.expression())) + auto memberAccess = dynamic_cast(&_functionCall.expression()); + if (memberAccess) if (auto expressionType = dynamic_cast(memberAccess->expression().annotation().type)) if (auto contractType = dynamic_cast(expressionType->actualType())) solUnimplementedAssert( @@ -641,42 +642,26 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) break; case FunctionType::Kind::Internal: { - optional functionDef; - if (auto memberAccess = dynamic_cast(&_functionCall.expression())) - { - solUnimplementedAssert(!functionType->bound(), "Internal calls to bound functions are not yet implemented for libraries and not allowed for contracts"); + auto identifier = dynamic_cast(&_functionCall.expression()); + FunctionDefinition const* functionDef = IRHelpers::referencedFunctionDeclaration(_functionCall.expression()); - functionDef = dynamic_cast(memberAccess->annotation().referencedDeclaration); - if (functionDef.value() != nullptr) - solAssert(functionType->declaration() == *memberAccess->annotation().referencedDeclaration, ""); - else - { - solAssert(dynamic_cast(memberAccess->annotation().referencedDeclaration), ""); - solAssert(!functionType->hasDeclaration(), ""); - } - } - else if (auto identifier = dynamic_cast(&_functionCall.expression())) + if (functionDef) { - solAssert(!functionType->bound(), ""); + solAssert(memberAccess || identifier, ""); + solAssert(functionType->declaration() == *functionDef, ""); - if (auto unresolvedFunctionDef = dynamic_cast(identifier->annotation().referencedDeclaration)) - { - functionDef = &unresolvedFunctionDef->resolveVirtual(m_context.mostDerivedContract()); - solAssert(functionType->declaration() == *identifier->annotation().referencedDeclaration, ""); - } - else - { - functionDef = nullptr; - solAssert(dynamic_cast(identifier->annotation().referencedDeclaration), ""); - solAssert(!functionType->hasDeclaration(), ""); - } + if (identifier) + functionDef = &functionDef->resolveVirtual(m_context.mostDerivedContract()); + + solAssert(functionDef->isImplemented(), ""); } else - // Not a simple expression like x or A.x - functionDef = nullptr; + solAssert(!functionType->hasDeclaration(), ""); - solAssert(functionDef.has_value(), ""); - solAssert(functionDef.value() == nullptr || functionDef.value()->isImplemented(), ""); + if (memberAccess) + solUnimplementedAssert(!functionType->bound(), ""); + else + solAssert(!functionType->bound(), ""); vector args; for (size_t i = 0; i < arguments.size(); ++i) @@ -685,9 +670,9 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) else args += convert(*arguments[i], *parameterTypes[i]).stackSlots(); - if (functionDef.value() != nullptr) + if (functionDef) define(_functionCall) << - m_context.enqueueFunctionForCodeGeneration(*functionDef.value()) << + m_context.enqueueFunctionForCodeGeneration(*functionDef) << "(" << joinHumanReadable(args) << ")\n"; From 87c598863fdaa655ed8a5fd1a1ffe304ce1b08b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Tue, 19 May 2020 22:54:15 +0200 Subject: [PATCH 04/15] IRGeneratorForStatements: Remove unnecessary code for handling internal functions with arbitrary parameters - Internal functions cannot have arbitrary parameters --- libsolidity/codegen/ir/IRGeneratorForStatements.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index b06da63c1..4d0d6436a 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -663,12 +663,11 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) else solAssert(!functionType->bound(), ""); + solAssert(!functionType->takesArbitraryParameters(), ""); + vector args; for (size_t i = 0; i < arguments.size(); ++i) - if (functionType->takesArbitraryParameters()) - args += IRVariable(*arguments[i]).stackSlots(); - else - args += convert(*arguments[i], *parameterTypes[i]).stackSlots(); + args += convert(*arguments[i], *parameterTypes[i]).stackSlots(); if (functionDef) define(_functionCall) << From a68b4ab7ede1bf909b3922164d2cdb0bc33e83ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Tue, 19 May 2020 13:02:51 +0200 Subject: [PATCH 05/15] IRGeneratorForStatements: Slightly reorganize local variables in endVisit(MemberAccess) - Add memberFunctionType accessible in the whole function to avoid declaring it multiple times - Add objectCategory --- .../codegen/ir/IRGeneratorForStatements.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 4d0d6436a..50a9eadc2 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -1221,13 +1221,13 @@ void IRGeneratorForStatements::endVisit(FunctionCallOptions const& _options) void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess) { ASTString const& member = _memberAccess.memberName(); - if (auto funType = dynamic_cast(_memberAccess.annotation().type)) - if (funType->bound()) - { - solUnimplementedAssert(false, ""); - } + auto memberFunctionType = dynamic_cast(_memberAccess.annotation().type); + Type::Category objectCategory = _memberAccess.expression().annotation().type->category(); - switch (_memberAccess.expression().annotation().type->category()) + if (memberFunctionType && memberFunctionType->bound()) + solUnimplementedAssert(false, ""); + + switch (objectCategory) { case Type::Category::Contract: { @@ -1460,9 +1460,9 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess) { if (auto const* variable = dynamic_cast(_memberAccess.annotation().referencedDeclaration)) handleVariableReference(*variable, _memberAccess); - else if (auto const* funType = dynamic_cast(_memberAccess.annotation().type)) + else if (memberFunctionType) { - switch (funType->kind()) + switch (memberFunctionType->kind()) { case FunctionType::Kind::Declaration: break; @@ -1481,7 +1481,7 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess) break; case FunctionType::Kind::DelegateCall: define(IRVariable(_memberAccess).part("address"), _memberAccess.expression()); - define(IRVariable(_memberAccess).part("functionIdentifier")) << formatNumber(funType->externalIdentifier()) << "\n"; + define(IRVariable(_memberAccess).part("functionIdentifier")) << formatNumber(memberFunctionType->externalIdentifier()) << "\n"; break; case FunctionType::Kind::External: case FunctionType::Kind::Creation: From 4a2ce57bedc5c3a21652c880b69ba7413197e277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Tue, 19 May 2020 22:21:47 +0200 Subject: [PATCH 06/15] Add support for bound function calls in IR generator --- .../codegen/ir/IRGeneratorForStatements.cpp | 38 +++++++++++++++---- ...rnal_library_function_bound_to_address.sol | 18 +++++++++ ...n_bound_to_address_named_send_transfer.sol | 21 ++++++++++ ...function_bound_to_array_named_pop_push.sol | 19 ++++++++++ ...nternal_library_function_bound_to_bool.sol | 20 ++++++++++ ...nal_library_function_bound_to_contract.sol | 21 ++++++++++ ...ibrary_function_bound_to_dynamic_array.sol | 21 ++++++++++ ...nternal_library_function_bound_to_enum.sol | 21 ++++++++++ ...ry_function_bound_to_external_function.sol | 23 +++++++++++ ..._library_function_bound_to_fixed_array.sol | 21 ++++++++++ ..._library_function_bound_to_fixed_bytes.sol | 17 +++++++++ ...ction_bound_to_function_named_selector.sol | 22 +++++++++++ ...rnal_library_function_bound_to_integer.sol | 17 +++++++++ ...al_library_function_bound_to_interface.sol | 22 +++++++++++ ...ry_function_bound_to_internal_function.sol | 22 +++++++++++ ...rnal_library_function_bound_to_mapping.sol | 22 +++++++++++ ...ernal_library_function_bound_to_string.sol | 18 +++++++++ 17 files changed, 356 insertions(+), 7 deletions(-) create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address_named_send_transfer.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_array_named_pop_push.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_bool.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_contract.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_dynamic_array.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_enum.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_external_function.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_array.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_bytes.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_function_named_selector.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_integer.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_interface.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_internal_function.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_mapping.sol create mode 100644 test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_string.sol diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 50a9eadc2..5472ec7ed 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -627,14 +627,20 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) auto memberAccess = dynamic_cast(&_functionCall.expression()); if (memberAccess) + { if (auto expressionType = dynamic_cast(memberAccess->expression().annotation().type)) + { + solAssert(!functionType->bound(), ""); if (auto contractType = dynamic_cast(expressionType->actualType())) solUnimplementedAssert( !contractType->contractDefinition().isLibrary() || functionType->kind() == FunctionType::Kind::Internal, "Only internal function calls implemented for libraries" ); + } + } + else + solAssert(!functionType->bound(), ""); - solUnimplementedAssert(!functionType->bound(), ""); switch (functionType->kind()) { case FunctionType::Kind::Declaration: @@ -658,14 +664,18 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) else solAssert(!functionType->hasDeclaration(), ""); - if (memberAccess) - solUnimplementedAssert(!functionType->bound(), ""); - else - solAssert(!functionType->bound(), ""); - solAssert(!functionType->takesArbitraryParameters(), ""); vector args; + if (functionType->bound()) + { + solAssert(memberAccess && functionDef, ""); + solAssert(functionDef->parameters().size() == arguments.size() + 1, ""); + args += convert(memberAccess->expression(), *functionDef->parameters()[0]->type()).stackSlots(); + } + else + solAssert(!functionDef || functionDef->parameters().size() == arguments.size(), ""); + for (size_t i = 0; i < arguments.size(); ++i) args += convert(*arguments[i], *parameterTypes[i]).stackSlots(); @@ -1225,7 +1235,21 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess) Type::Category objectCategory = _memberAccess.expression().annotation().type->category(); if (memberFunctionType && memberFunctionType->bound()) - solUnimplementedAssert(false, ""); + { + solAssert((set{ + Type::Category::Contract, + Type::Category::Bool, + Type::Category::Integer, + Type::Category::Address, + Type::Category::Function, + Type::Category::Struct, + Type::Category::Enum, + Type::Category::Mapping, + Type::Category::Array, + Type::Category::FixedBytes, + }).count(objectCategory) > 0, ""); + return; + } switch (objectCategory) { diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address.sol new file mode 100644 index 000000000..08cc04346 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address.sol @@ -0,0 +1,18 @@ +library L { + function equals(address a, address b) internal pure returns (bool) { + return a == b; + } +} + +contract C { + using L for address; + + function foo(address a, address b) public returns (bool) { + return a.equals(b); + } +} +// ==== +// compileViaYul: also +// ---- +// foo(address, address): 0x111122223333444455556666777788889999aAaa, 0x111122223333444455556666777788889999aAaa -> true +// foo(address, address): 0x111122223333444455556666777788889999aAaa, 0x0000000000000000000000000000000000000000 -> false diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address_named_send_transfer.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address_named_send_transfer.sol new file mode 100644 index 000000000..bfc5a3d62 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_address_named_send_transfer.sol @@ -0,0 +1,21 @@ +library L { + function transfer(address a) internal {} + function send(address a) internal {} +} + +contract C { + using L for address; + + function useTransfer(address a) public { + a.transfer(); + } + + function useSend(address a) public { + a.send(); + } +} +// ==== +// compileViaYul: also +// ---- +// useTransfer(address): 0x111122223333444455556666777788889999aAaa -> +// useSend(address): 0x111122223333444455556666777788889999aAaa -> diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_array_named_pop_push.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_array_named_pop_push.sol new file mode 100644 index 000000000..c684cc77f --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_array_named_pop_push.sol @@ -0,0 +1,19 @@ +library L { + function pop(uint[2] memory a) internal {} + function push(uint[2] memory a) internal {} +} + +contract C { + using L for uint[2]; + + function test() public { + uint[2] memory input; + + input.push(); + input.pop(); + } +} +// ==== +// compileViaYul: also +// ---- +// test() -> diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_bool.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_bool.sol new file mode 100644 index 000000000..836f976bb --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_bool.sol @@ -0,0 +1,20 @@ +library L { + function xor(bool a, bool b) internal pure returns (bool) { + return a != b; + } +} + +contract C { + using L for bool; + + function foo(bool a, bool b) public returns (bool) { + return a.xor(b); + } +} +// ==== +// compileViaYul: also +// ---- +// foo(bool, bool): true, true -> false +// foo(bool, bool): true, false -> true +// foo(bool, bool): false, true -> true +// foo(bool, bool): false, false -> false diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_contract.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_contract.sol new file mode 100644 index 000000000..a9566ca59 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_contract.sol @@ -0,0 +1,21 @@ +contract E {} + +library L { + function foo(E e) internal pure returns (uint) { + return 42; + } +} + +contract C { + using L for E; + + function test() public returns (uint) { + E e = new E(); + return e.foo(); + } +} + +// ==== +// compileViaYul: also +// ---- +// test() -> 42 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_dynamic_array.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_dynamic_array.sol new file mode 100644 index 000000000..0ea7e4144 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_dynamic_array.sol @@ -0,0 +1,21 @@ +library L { + function at(uint[] memory a, uint i) internal pure returns (uint) { + return a[i]; + } +} + +contract C { + using L for uint[]; + + function secondItem() public returns (uint) { + uint[] memory input = new uint[](2); + input[0] = 0x11; + input[1] = 0x22; + + return input.at(1); + } +} +// ==== +// compileViaYul: also +// ---- +// secondItem() -> 0x22 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_enum.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_enum.sol new file mode 100644 index 000000000..93c5fd4a0 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_enum.sol @@ -0,0 +1,21 @@ +library L { + enum E { A, B } + + function equals(E a, E b) internal pure returns (bool) { + return a == b; + } +} + +contract C { + using L for L.E; + + function equalsA(uint choice) public returns (bool) { + L.E x = L.E.A; + return x.equals(L.E(choice)); + } +} +// ==== +// compileViaYul: also +// ---- +// equalsA(uint256): 0 -> true +// equalsA(uint256): 1 -> false diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_external_function.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_external_function.sol new file mode 100644 index 000000000..f68cb4e21 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_external_function.sol @@ -0,0 +1,23 @@ +library L { + // NOTE: External function takes up two stack slots + function double(function(uint) external pure returns (uint) f, uint x) internal pure returns (uint) { + return f(x) * 2; + } +} + +contract C { + using L for function(uint) external pure returns (uint); + + function identity(uint x) external pure returns (uint) { + return x; + } + + function test(uint value) public returns (uint) { + return this.identity.double(value); + } +} + +// ==== +// compileViaYul: also +// ---- +// test(uint256): 5 -> 10 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_array.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_array.sol new file mode 100644 index 000000000..842911079 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_array.sol @@ -0,0 +1,21 @@ +library L { + function at(uint[2] memory a, uint i) internal pure returns (uint) { + return a[i]; + } +} + +contract C { + using L for uint[2]; + + function secondItem() public returns (uint) { + uint[2] memory input; + input[0] = 0x11; + input[1] = 0x22; + + return input.at(1); + } +} +// ==== +// compileViaYul: also +// ---- +// secondItem() -> 0x22 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_bytes.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_bytes.sol new file mode 100644 index 000000000..62bd48beb --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_fixed_bytes.sol @@ -0,0 +1,17 @@ +library L { + function add(bytes2 a, bytes2 b) internal pure returns (bytes2) { + return bytes2(uint16(a) + uint16(b)); + } +} + +contract C { + using L for bytes2; + + function sum(bytes2 a, bytes2 b) public returns (bytes2) { + return a.add(b); + } +} +// ==== +// compileViaYul: also +// ---- +// sum(bytes2, bytes2): left(0x1100), left(0x0022) -> left(0x1122) diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_function_named_selector.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_function_named_selector.sol new file mode 100644 index 000000000..75c4e4cb8 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_function_named_selector.sol @@ -0,0 +1,22 @@ +library L { + function selector(function(uint) internal pure returns (uint) f, uint x) internal pure returns (uint) { + return f(x) * 2; + } +} + +contract C { + using L for function(uint) internal pure returns (uint); + + function identity(uint x) internal pure returns (uint) { + return x; + } + + function test(uint value) public returns (uint) { + return identity.selector(value); + } +} + +// ==== +// compileViaYul: also +// ---- +// test(uint256): 5 -> 10 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_integer.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_integer.sol new file mode 100644 index 000000000..b6793c3d7 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_integer.sol @@ -0,0 +1,17 @@ +library L { + function add(uint256 a, uint256 b) internal pure returns (uint256) { + return a + b; + } +} + +contract C { + using L for uint256; + + function foo(uint256 a, uint256 b) public returns (uint256) { + return a.add(b); + } +} +// ==== +// compileViaYul: also +// ---- +// foo(uint256, uint256): 8, 42 -> 50 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_interface.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_interface.sol new file mode 100644 index 000000000..47e58bb32 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_interface.sol @@ -0,0 +1,22 @@ +interface I {} +contract E is I {} + +library L { + function foo(I i) internal pure returns (uint) { + return 42; + } +} + +contract C { + using L for I; + + function test() public returns (uint) { + E e = new E(); + return I(e).foo(); + } +} + +// ==== +// compileViaYul: also +// ---- +// test() -> 42 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_internal_function.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_internal_function.sol new file mode 100644 index 000000000..519eafddd --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_internal_function.sol @@ -0,0 +1,22 @@ +library L { + function double(function(uint) internal pure returns (uint) f, uint x) internal pure returns (uint) { + return f(x) * 2; + } +} + +contract C { + using L for function(uint) internal pure returns (uint); + + function identity(uint x) internal pure returns (uint) { + return x; + } + + function test(uint value) public returns (uint) { + return identity.double(value); + } +} + +// ==== +// compileViaYul: also +// ---- +// test(uint256): 5 -> 10 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_mapping.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_mapping.sol new file mode 100644 index 000000000..ec1250a62 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_mapping.sol @@ -0,0 +1,22 @@ +library L { + function at(mapping(uint => uint) storage a, uint i) internal view returns (uint) { + return a[i]; + } +} + +contract C { + using L for mapping(uint => uint); + + mapping(uint => uint) map; + + function mapValue(uint a) public returns (uint) { + map[42] = 0x24; + map[66] = 0x66; + + return map.at(a); + } +} +// ==== +// compileViaYul: also +// ---- +// mapValue(uint256): 42 -> 0x24 diff --git a/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_string.sol b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_string.sol new file mode 100644 index 000000000..d8d420512 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/internal_library_function_bound_to_string.sol @@ -0,0 +1,18 @@ +library L { + function at(string memory a, uint i) internal pure returns (uint8) { + return uint8(bytes(a)[i]); + } +} + +contract C { + using L for string; + + function secondChar() public returns (uint8) { + string memory input = "abc"; + return input.at(1); + } +} +// ==== +// compileViaYul: also +// ---- +// secondChar() -> 98 From 3a75b1da4db39d20bc42f322b3b8b175923dca75 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Wed, 20 May 2020 23:56:25 +0200 Subject: [PATCH 07/15] Remove DeclarationTypeChecker class-specific error reporting functions --- .../analysis/DeclarationTypeChecker.cpp | 44 +++++++------------ libsolidity/analysis/DeclarationTypeChecker.h | 9 ---- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index a06679b97..ee5f8a020 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -50,7 +50,8 @@ bool DeclarationTypeChecker::visit(ElementaryTypeName const& _typeName) _typeName.annotation().type = TypeProvider::address(); break; default: - typeError( + m_errorReporter.typeError( + 2311_error, _typeName.location(), "Address types can only be payable or non-payable." ); @@ -102,7 +103,7 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct) auto visitor = [&](StructDefinition const& _struct, auto& _cycleDetector, size_t _depth) { if (_depth >= 256) - fatalDeclarationError(_struct.location(), "Struct definition exhausts cyclic dependency validator."); + m_errorReporter.fatalDeclarationError(5651_error, _struct.location(), "Struct definition exhausts cyclic dependency validator."); for (ASTPointer const& member: _struct.members()) { @@ -119,7 +120,7 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct) } }; if (util::CycleDetector(visitor).run(_struct) != nullptr) - fatalTypeError(_struct.location(), "Recursive struct definition."); + m_errorReporter.fatalTypeError(2046_error, _struct.location(), "Recursive struct definition."); return false; } @@ -145,7 +146,7 @@ void DeclarationTypeChecker::endVisit(UserDefinedTypeName const& _typeName) else { _typeName.annotation().type = TypeProvider::emptyTuple(); - fatalTypeError(_typeName.location(), "Name has to refer to a struct, enum or contract."); + m_errorReporter.fatalTypeError(9755_error, _typeName.location(), "Name has to refer to a struct, enum or contract."); } } bool DeclarationTypeChecker::visit(FunctionTypeName const& _typeName) @@ -165,13 +166,13 @@ bool DeclarationTypeChecker::visit(FunctionTypeName const& _typeName) case Visibility::External: break; default: - fatalTypeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); + m_errorReporter.fatalTypeError(7653_error, _typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); return false; } if (_typeName.isPayable() && _typeName.visibility() != Visibility::External) { - fatalTypeError(_typeName.location(), "Only external function types can be payable."); + m_errorReporter.fatalTypeError(6138_error, _typeName.location(), "Only external function types can be payable."); return false; } _typeName.annotation().type = TypeProvider::function(_typeName); @@ -226,7 +227,7 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) return; } if (baseType->storageBytes() == 0) - fatalTypeError(_typeName.baseType().location(), "Illegal base type of storage size zero for array."); + m_errorReporter.fatalTypeError(9390_error, _typeName.baseType().location(), "Illegal base type of storage size zero for array."); if (Expression const* length = _typeName.length()) { TypePointer& lengthTypeGeneric = length->annotation().type; @@ -235,13 +236,13 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) RationalNumberType const* lengthType = dynamic_cast(lengthTypeGeneric); u256 lengthValue = 0; if (!lengthType || !lengthType->mobileType()) - typeError(length->location(), "Invalid array length, expected integer literal or constant expression."); + m_errorReporter.typeError(8922_error, length->location(), "Invalid array length, expected integer literal or constant expression."); else if (lengthType->isZero()) - typeError(length->location(), "Array with zero length specified."); + m_errorReporter.typeError(1220_error, length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) - typeError(length->location(), "Array with fractional length specified."); + m_errorReporter.typeError(4323_error, length->location(), "Array with fractional length specified."); else if (lengthType->isNegative()) - typeError(length->location(), "Array with negative length specified."); + m_errorReporter.typeError(9308_error, length->location(), "Array with negative length specified."); else lengthValue = lengthType->literalValue(nullptr); _typeName.annotation().type = TypeProvider::array(DataLocation::Storage, baseType, lengthValue); @@ -309,7 +310,7 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable) errorString += " for variable"; } errorString += ", but " + locationToString(varLoc) + " was given."; - typeError(_variable.location(), errorString); + m_errorReporter.typeError(6160_error, _variable.location(), errorString); solAssert(!allowedDataLocations.empty(), ""); varLoc = *allowedDataLocations.begin(); @@ -359,24 +360,9 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable) } -void DeclarationTypeChecker::typeError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.typeError(2311_error, _location, _description); -} - -void DeclarationTypeChecker::fatalTypeError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.fatalTypeError(5651_error, _location, _description); -} - -void DeclarationTypeChecker::fatalDeclarationError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.fatalDeclarationError(2046_error, _location, _description); -} - bool DeclarationTypeChecker::check(ASTNode const& _node) { - unsigned errorCount = m_errorReporter.errorCount(); + auto watcher = m_errorReporter.errorWatcher(); _node.accept(*this); - return m_errorReporter.errorCount() == errorCount; + return watcher.ok(); } diff --git a/libsolidity/analysis/DeclarationTypeChecker.h b/libsolidity/analysis/DeclarationTypeChecker.h index e54075998..764a66e1d 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.h +++ b/libsolidity/analysis/DeclarationTypeChecker.h @@ -59,15 +59,6 @@ private: void endVisit(VariableDeclaration const& _variable) override; bool visit(StructDefinition const& _struct) override; - /// Adds a new error to the list of errors. - void typeError(langutil::SourceLocation const& _location, std::string const& _description); - - /// Adds a new error to the list of errors and throws to abort reference resolving. - void fatalTypeError(langutil::SourceLocation const& _location, std::string const& _description); - - /// Adds a new error to the list of errors and throws to abort reference resolving. - void fatalDeclarationError(langutil::SourceLocation const& _location, std::string const& _description); - langutil::ErrorReporter& m_errorReporter; langutil::EVMVersion m_evmVersion; bool m_insideFunctionType = false; From 66a8c7d1ab5b744736d1355ba46c9e1d5f090ac8 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 21 May 2020 00:04:26 +0200 Subject: [PATCH 08/15] Remove ReferencesResolver class-specific error reporting functions --- libsolidity/analysis/ReferencesResolver.cpp | 30 ++++++--------------- libsolidity/analysis/ReferencesResolver.h | 9 ------- 2 files changed, 8 insertions(+), 31 deletions(-) diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index ac3553f6b..073c0795a 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -119,7 +119,7 @@ bool ReferencesResolver::visit(Identifier const& _identifier) else errorMessage += " Did you mean " + std::move(suggestions) + "?"; } - declarationError(_identifier.location(), errorMessage); + m_errorReporter.declarationError(8051_error, _identifier.location(), errorMessage); } else if (declarations.size() == 1) _identifier.annotation().referencedDeclaration = declarations.front(); @@ -157,7 +157,7 @@ void ReferencesResolver::endVisit(UserDefinedTypeName const& _typeName) Declaration const* declaration = m_resolver.pathFromCurrentScope(_typeName.namePath()); if (!declaration) { - fatalDeclarationError(_typeName.location(), "Identifier not found or not unique."); + m_errorReporter.fatalDeclarationError(7556_error, _typeName.location(), "Identifier not found or not unique."); return; } @@ -209,14 +209,14 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier) )); if (realName.empty()) { - declarationError(_identifier.location, "In variable names _slot and _offset can only be used as a suffix."); + m_errorReporter.declarationError(9553_error, _identifier.location, "In variable names _slot and _offset can only be used as a suffix."); return; } declarations = m_resolver.nameFromCurrentScope(realName); } if (declarations.size() > 1) { - declarationError(_identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); + m_errorReporter.declarationError(8827_error, _identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); return; } else if (declarations.size() == 0) @@ -224,7 +224,7 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier) if (auto var = dynamic_cast(declarations.front())) if (var->isLocalVariable() && m_yulInsideFunction) { - declarationError(_identifier.location, "Cannot access local Solidity variables from inside an inline assembly function."); + m_errorReporter.declarationError(8477_error, _identifier.location, "Cannot access local Solidity variables from inside an inline assembly function."); return; } @@ -242,7 +242,7 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl) string namePrefix = identifier.name.str().substr(0, identifier.name.str().find('.')); if (isSlot || isOffset) - declarationError(identifier.location, "In variable declarations _slot and _offset can not be used as a suffix."); + m_errorReporter.declarationError(8820_error, identifier.location, "In variable declarations _slot and _offset can not be used as a suffix."); else if ( auto declarations = m_resolver.nameFromCurrentScope(namePrefix); !declarations.empty() @@ -252,7 +252,8 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl) for (auto const* decl: declarations) ssl.append("The shadowed declaration is here:", decl->location()); if (!ssl.infos.empty()) - declarationError( + m_errorReporter.declarationError( + 6005_error, identifier.location, ssl, namePrefix.size() < identifier.name.str().size() ? @@ -266,19 +267,4 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl) visit(*_varDecl.value); } -void ReferencesResolver::declarationError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.declarationError(8532_error, _location, _description); -} - -void ReferencesResolver::declarationError(SourceLocation const& _location, SecondarySourceLocation const& _ssl, string const& _description) -{ - m_errorReporter.declarationError(3881_error, _location, _ssl, _description); -} - -void ReferencesResolver::fatalDeclarationError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.fatalDeclarationError(6546_error, _location, _description); -} - } diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index 7b8c1fff5..7da745cc5 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -88,15 +88,6 @@ private: void operator()(yul::Identifier const& _identifier) override; void operator()(yul::VariableDeclaration const& _varDecl) override; - /// Adds a new error to the list of errors. - void declarationError(langutil::SourceLocation const& _location, std::string const& _description); - - /// Adds a new error to the list of errors. - void declarationError(langutil::SourceLocation const& _location, langutil::SecondarySourceLocation const& _ssl, std::string const& _description); - - /// Adds a new error to the list of errors and throws to abort reference resolving. - void fatalDeclarationError(langutil::SourceLocation const& _location, std::string const& _description); - langutil::ErrorReporter& m_errorReporter; NameAndTypeResolver& m_resolver; langutil::EVMVersion m_evmVersion; From 38e65a909ac86d0e2cea7d4c48dfe1a98f4f020a Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 21 May 2020 00:26:14 +0200 Subject: [PATCH 09/15] Remove DocStringAnalyzer and DocStringParser class-specific error reporting functions --- libsolidity/analysis/DocStringAnalyser.cpp | 27 +++++++++++----------- libsolidity/analysis/DocStringAnalyser.h | 2 -- libsolidity/parsing/DocStringParser.cpp | 11 +++------ libsolidity/parsing/DocStringParser.h | 2 -- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index a04c9314e..1d2d8f54a 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -68,13 +68,13 @@ bool DocStringAnalyser::visit(VariableDeclaration const& _variable) parseDocStrings(_variable, _variable.annotation(), validPublicTags, "non-public state variables"); if (_variable.annotation().docTags.count("notice") > 0) m_errorReporter.warning( - 9098_error, _variable.documentation()->location(), + 7816_error, _variable.documentation()->location(), "Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly." ); } if (_variable.annotation().docTags.count("title") > 0 || _variable.annotation().docTags.count("author") > 0) m_errorReporter.warning( - 4822_error, _variable.documentation()->location(), + 8532_error, _variable.documentation()->location(), "Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0." ); } @@ -110,7 +110,8 @@ void DocStringAnalyser::checkParameters( auto paramRange = _annotation.docTags.equal_range("param"); for (auto i = paramRange.first; i != paramRange.second; ++i) if (!validParams.count(i->second.paramName)) - appendError( + m_errorReporter.docstringParsingError( + 3881_error, _node.documentation()->location(), "Documented parameter \"" + i->second.paramName + @@ -159,7 +160,8 @@ void DocStringAnalyser::parseDocStrings( for (auto const& docTag: _annotation.docTags) { if (!_validTags.count(docTag.first)) - appendError( + m_errorReporter.docstringParsingError( + 6546_error, _node.documentation()->location(), "Documentation tag @" + docTag.first + " not valid for " + _nodeName + "." ); @@ -170,12 +172,14 @@ void DocStringAnalyser::parseDocStrings( if (auto* varDecl = dynamic_cast(&_node)) { if (!varDecl->isPublic()) - appendError( + m_errorReporter.docstringParsingError( + 9440_error, _node.documentation()->location(), "Documentation tag \"@" + docTag.first + "\" is only allowed on public state-variables." ); if (returnTagsVisited > 1) - appendError( + m_errorReporter.docstringParsingError( + 5256_error, _node.documentation()->location(), "Documentation tag \"@" + docTag.first + "\" is only allowed once on state-variables." ); @@ -186,7 +190,8 @@ void DocStringAnalyser::parseDocStrings( string firstWord = content.substr(0, content.find_first_of(" \t")); if (returnTagsVisited > function->returnParameters().size()) - appendError( + m_errorReporter.docstringParsingError( + 2604_error, _node.documentation()->location(), "Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" + " exceeds the number of return parameters." @@ -195,7 +200,8 @@ void DocStringAnalyser::parseDocStrings( { auto parameter = function->returnParameters().at(returnTagsVisited - 1); if (!parameter->name().empty() && parameter->name() != firstWord) - appendError( + m_errorReporter.docstringParsingError( + 5856_error, _node.documentation()->location(), "Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" + " does not contain the name of its return parameter." @@ -205,8 +211,3 @@ void DocStringAnalyser::parseDocStrings( } } } - -void DocStringAnalyser::appendError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.docstringParsingError(7816_error, _location, _description); -} diff --git a/libsolidity/analysis/DocStringAnalyser.h b/libsolidity/analysis/DocStringAnalyser.h index cddfc2f1d..9bc8ec578 100644 --- a/libsolidity/analysis/DocStringAnalyser.h +++ b/libsolidity/analysis/DocStringAnalyser.h @@ -81,8 +81,6 @@ private: std::string const& _nodeName ); - void appendError(langutil::SourceLocation const& _location, std::string const& _description); - langutil::ErrorReporter& m_errorReporter; }; diff --git a/libsolidity/parsing/DocStringParser.cpp b/libsolidity/parsing/DocStringParser.cpp index 3ce67a430..755eb0b50 100644 --- a/libsolidity/parsing/DocStringParser.cpp +++ b/libsolidity/parsing/DocStringParser.cpp @@ -96,7 +96,7 @@ void DocStringParser::parse(string const& _docString, ErrorReporter& _errorRepor auto tagNameEndPos = firstWhitespaceOrNewline(tagPos, end); if (tagNameEndPos == end) { - appendError("End of tag " + string(tagPos, tagNameEndPos) + " not found"); + m_errorReporter->docstringParsingError(9222_error, "End of tag " + string(tagPos, tagNameEndPos) + " not found"); break; } @@ -138,7 +138,7 @@ DocStringParser::iter DocStringParser::parseDocTagParam(iter _pos, iter _end) auto nameStartPos = skipWhitespace(_pos, _end); if (nameStartPos == _end) { - appendError("No param name given"); + m_errorReporter->docstringParsingError(3335_error, "No param name given"); return _end; } auto nameEndPos = firstNonIdentifier(nameStartPos, _end); @@ -149,7 +149,7 @@ DocStringParser::iter DocStringParser::parseDocTagParam(iter _pos, iter _end) if (descStartPos == nlPos) { - appendError("No description given for param " + paramName); + m_errorReporter->docstringParsingError(9942_error, "No description given for param " + paramName); return _end; } @@ -189,8 +189,3 @@ void DocStringParser::newTag(string const& _tagName) { m_lastTag = &m_docTags.insert(make_pair(_tagName, DocTag()))->second; } - -void DocStringParser::appendError(string const& _description) -{ - m_errorReporter->docstringParsingError(9440_error, _description); -} diff --git a/libsolidity/parsing/DocStringParser.h b/libsolidity/parsing/DocStringParser.h index b4c9186a7..eaf18503b 100644 --- a/libsolidity/parsing/DocStringParser.h +++ b/libsolidity/parsing/DocStringParser.h @@ -58,8 +58,6 @@ private: /// Creates and inserts a new tag and adjusts m_lastTag. void newTag(std::string const& _tagName); - void appendError(std::string const& _description); - /// Mapping tag name -> content. std::multimap m_docTags; DocTag* m_lastTag = nullptr; From cfdfa360658da1133463d5de9d0f7ce11f9afe75 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 20 May 2020 23:59:03 +0100 Subject: [PATCH 10/15] Rename wasm::Break(If) to wasm::Branch(If) for clarity --- libyul/backends/wasm/BinaryTransform.cpp | 10 +++++----- libyul/backends/wasm/BinaryTransform.h | 4 ++-- libyul/backends/wasm/TextTransform.cpp | 8 ++++---- libyul/backends/wasm/TextTransform.h | 4 ++-- libyul/backends/wasm/WasmAST.h | 10 +++++----- libyul/backends/wasm/WasmCodeTransform.cpp | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/libyul/backends/wasm/BinaryTransform.cpp b/libyul/backends/wasm/BinaryTransform.cpp index 249d87a33..43eced734 100644 --- a/libyul/backends/wasm/BinaryTransform.cpp +++ b/libyul/backends/wasm/BinaryTransform.cpp @@ -382,15 +382,15 @@ bytes BinaryTransform::operator()(Loop const& _loop) return result; } -bytes BinaryTransform::operator()(Break const& _break) +bytes BinaryTransform::operator()(Branch const& _branch) { - return toBytes(Opcode::Br) + encodeLabelIdx(_break.label.name); + return toBytes(Opcode::Br) + encodeLabelIdx(_branch.label.name); } -bytes BinaryTransform::operator()(BreakIf const& _breakIf) +bytes BinaryTransform::operator()(BranchIf const& _branchIf) { - bytes result = std::visit(*this, *_breakIf.condition); - result += toBytes(Opcode::BrIf) + encodeLabelIdx(_breakIf.label.name); + bytes result = std::visit(*this, *_branchIf.condition); + result += toBytes(Opcode::BrIf) + encodeLabelIdx(_branchIf.label.name); return result; } diff --git a/libyul/backends/wasm/BinaryTransform.h b/libyul/backends/wasm/BinaryTransform.h index e11c92bf6..c1c924dcc 100644 --- a/libyul/backends/wasm/BinaryTransform.h +++ b/libyul/backends/wasm/BinaryTransform.h @@ -48,8 +48,8 @@ public: bytes operator()(wasm::GlobalAssignment const& _assignment); bytes operator()(wasm::If const& _if); bytes operator()(wasm::Loop const& _loop); - bytes operator()(wasm::Break const& _break); - bytes operator()(wasm::BreakIf const& _break); + bytes operator()(wasm::Branch const& _branch); + bytes operator()(wasm::BranchIf const& _branchIf); bytes operator()(wasm::Return const& _return); bytes operator()(wasm::Block const& _block); bytes operator()(wasm::FunctionDefinition const& _function); diff --git a/libyul/backends/wasm/TextTransform.cpp b/libyul/backends/wasm/TextTransform.cpp index 2a2499b61..fd8eaf746 100644 --- a/libyul/backends/wasm/TextTransform.cpp +++ b/libyul/backends/wasm/TextTransform.cpp @@ -121,14 +121,14 @@ string TextTransform::operator()(wasm::Loop const& _loop) return "(loop" + move(label) + "\n" + indented(joinTransformed(_loop.statements, '\n')) + ")\n"; } -string TextTransform::operator()(wasm::Break const& _break) +string TextTransform::operator()(wasm::Branch const& _branch) { - return "(br $" + _break.label.name + ")\n"; + return "(br $" + _branch.label.name + ")\n"; } -string TextTransform::operator()(wasm::BreakIf const& _break) +string TextTransform::operator()(wasm::BranchIf const& _branchIf) { - return "(br_if $" + _break.label.name + " " + visit(*_break.condition) + ")\n"; + return "(br_if $" + _branchIf.label.name + " " + visit(*_branchIf.condition) + ")\n"; } string TextTransform::operator()(wasm::Return const&) diff --git a/libyul/backends/wasm/TextTransform.h b/libyul/backends/wasm/TextTransform.h index 10f9f3fbb..ebbca4e9e 100644 --- a/libyul/backends/wasm/TextTransform.h +++ b/libyul/backends/wasm/TextTransform.h @@ -48,9 +48,9 @@ public: std::string operator()(wasm::GlobalAssignment const& _assignment); std::string operator()(wasm::If const& _if); std::string operator()(wasm::Loop const& _loop); - std::string operator()(wasm::Break const& _break); + std::string operator()(wasm::Branch const& _branch); + std::string operator()(wasm::BranchIf const& _branchIf); std::string operator()(wasm::Return const& _return); - std::string operator()(wasm::BreakIf const& _break); std::string operator()(wasm::Block const& _block); private: diff --git a/libyul/backends/wasm/WasmAST.h b/libyul/backends/wasm/WasmAST.h index 635b7c4ea..7ba7066cb 100644 --- a/libyul/backends/wasm/WasmAST.h +++ b/libyul/backends/wasm/WasmAST.h @@ -41,13 +41,13 @@ struct GlobalAssignment; struct Block; struct If; struct Loop; -struct Break; -struct BreakIf; +struct Branch; +struct BranchIf; struct Return; using Expression = std::variant< Literal, StringLiteral, LocalVariable, GlobalVariable, FunctionCall, BuiltinCall, LocalAssignment, GlobalAssignment, - Block, If, Loop, Break, BreakIf, Return + Block, If, Loop, Branch, BranchIf, Return >; struct Literal { uint64_t value; }; @@ -66,9 +66,9 @@ struct If { std::unique_ptr> elseStatements; }; struct Loop { std::string labelName; std::vector statements; }; -struct Break { Label label; }; +struct Branch { Label label; }; struct Return {}; -struct BreakIf { Label label; std::unique_ptr condition; }; +struct BranchIf { Label label; std::unique_ptr condition; }; struct VariableDeclaration { std::string variableName; }; struct GlobalVariableDeclaration { std::string variableName; }; diff --git a/libyul/backends/wasm/WasmCodeTransform.cpp b/libyul/backends/wasm/WasmCodeTransform.cpp index f2e624552..308171c44 100644 --- a/libyul/backends/wasm/WasmCodeTransform.cpp +++ b/libyul/backends/wasm/WasmCodeTransform.cpp @@ -256,26 +256,26 @@ wasm::Expression WasmCodeTransform::operator()(ForLoop const& _for) wasm::Loop loop; loop.labelName = newLabel(); loop.statements = visit(_for.pre.statements); - loop.statements.emplace_back(wasm::BreakIf{wasm::Label{breakLabel}, make_unique( + loop.statements.emplace_back(wasm::BranchIf{wasm::Label{breakLabel}, make_unique( wasm::BuiltinCall{"i64.eqz", make_vector( visitReturnByValue(*_for.condition) )} )}); loop.statements.emplace_back(wasm::Block{continueLabel, visit(_for.body.statements)}); loop.statements += visit(_for.post.statements); - loop.statements.emplace_back(wasm::Break{wasm::Label{loop.labelName}}); + loop.statements.emplace_back(wasm::Branch{wasm::Label{loop.labelName}}); return { wasm::Block{breakLabel, make_vector(move(loop))} }; } wasm::Expression WasmCodeTransform::operator()(Break const&) { - return wasm::Break{wasm::Label{m_breakContinueLabelNames.top().first}}; + return wasm::Branch{wasm::Label{m_breakContinueLabelNames.top().first}}; } wasm::Expression WasmCodeTransform::operator()(Continue const&) { - return wasm::Break{wasm::Label{m_breakContinueLabelNames.top().second}}; + return wasm::Branch{wasm::Label{m_breakContinueLabelNames.top().second}}; } wasm::Expression WasmCodeTransform::operator()(Leave const&) From 5870253b0079edb672a3c8a494905f7fdb5daec5 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 21 May 2020 17:38:47 +0200 Subject: [PATCH 11/15] Shorten a couple of lines --- .../analysis/DeclarationTypeChecker.cpp | 51 ++++++++++++++++--- libsolidity/analysis/DocStringAnalyser.cpp | 6 ++- libsolidity/analysis/ReferencesResolver.cpp | 24 +++++++-- libsolidity/parsing/DocStringParser.cpp | 5 +- 4 files changed, 71 insertions(+), 15 deletions(-) diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index ee5f8a020..ca565b53a 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -103,7 +103,11 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct) auto visitor = [&](StructDefinition const& _struct, auto& _cycleDetector, size_t _depth) { if (_depth >= 256) - m_errorReporter.fatalDeclarationError(5651_error, _struct.location(), "Struct definition exhausts cyclic dependency validator."); + m_errorReporter.fatalDeclarationError( + 5651_error, + _struct.location(), + "Struct definition exhausts cyclic dependency validator." + ); for (ASTPointer const& member: _struct.members()) { @@ -146,9 +150,14 @@ void DeclarationTypeChecker::endVisit(UserDefinedTypeName const& _typeName) else { _typeName.annotation().type = TypeProvider::emptyTuple(); - m_errorReporter.fatalTypeError(9755_error, _typeName.location(), "Name has to refer to a struct, enum or contract."); + m_errorReporter.fatalTypeError( + 9755_error, + _typeName.location(), + "Name has to refer to a struct, enum or contract." + ); } } + bool DeclarationTypeChecker::visit(FunctionTypeName const& _typeName) { if (_typeName.annotation().type) @@ -166,18 +175,27 @@ bool DeclarationTypeChecker::visit(FunctionTypeName const& _typeName) case Visibility::External: break; default: - m_errorReporter.fatalTypeError(7653_error, _typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); + m_errorReporter.fatalTypeError( + 7653_error, + _typeName.location(), + "Invalid visibility, can only be \"external\" or \"internal\"." + ); return false; } if (_typeName.isPayable() && _typeName.visibility() != Visibility::External) { - m_errorReporter.fatalTypeError(6138_error, _typeName.location(), "Only external function types can be payable."); + m_errorReporter.fatalTypeError( + 6138_error, + _typeName.location(), + "Only external function types can be payable." + ); return false; } _typeName.annotation().type = TypeProvider::function(_typeName); return false; } + void DeclarationTypeChecker::endVisit(Mapping const& _mapping) { if (_mapping.annotation().type) @@ -227,7 +245,11 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) return; } if (baseType->storageBytes() == 0) - m_errorReporter.fatalTypeError(9390_error, _typeName.baseType().location(), "Illegal base type of storage size zero for array."); + m_errorReporter.fatalTypeError( + 9390_error, + _typeName.baseType().location(), + "Illegal base type of storage size zero for array." + ); if (Expression const* length = _typeName.length()) { TypePointer& lengthTypeGeneric = length->annotation().type; @@ -236,7 +258,11 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) RationalNumberType const* lengthType = dynamic_cast(lengthTypeGeneric); u256 lengthValue = 0; if (!lengthType || !lengthType->mobileType()) - m_errorReporter.typeError(8922_error, length->location(), "Invalid array length, expected integer literal or constant expression."); + m_errorReporter.typeError( + 8922_error, + length->location(), + "Invalid array length, expected integer literal or constant expression." + ); else if (lengthType->isZero()) m_errorReporter.typeError(1220_error, length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) @@ -250,15 +276,24 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) else _typeName.annotation().type = TypeProvider::array(DataLocation::Storage, baseType); } + void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable) { if (_variable.annotation().type) return; if (_variable.isConstant() && !_variable.isStateVariable()) - m_errorReporter.declarationError(1788_error, _variable.location(), "The \"constant\" keyword can only be used for state variables."); + m_errorReporter.declarationError( + 1788_error, + _variable.location(), + "The \"constant\" keyword can only be used for state variables." + ); if (_variable.immutable() && !_variable.isStateVariable()) - m_errorReporter.declarationError(8297_error, _variable.location(), "The \"immutable\" keyword can only be used for state variables."); + m_errorReporter.declarationError( + 8297_error, + _variable.location(), + "The \"immutable\" keyword can only be used for state variables." + ); if (!_variable.typeName()) { diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index 1d2d8f54a..7d8a8a43f 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -69,13 +69,15 @@ bool DocStringAnalyser::visit(VariableDeclaration const& _variable) if (_variable.annotation().docTags.count("notice") > 0) m_errorReporter.warning( 7816_error, _variable.documentation()->location(), - "Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly." + "Documentation tag on non-public state variables will be disallowed in 0.7.0. " + "You will need to use the @dev tag explicitly." ); } if (_variable.annotation().docTags.count("title") > 0 || _variable.annotation().docTags.count("author") > 0) m_errorReporter.warning( 8532_error, _variable.documentation()->location(), - "Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0." + "Documentation tag @title and @author is only allowed on contract definitions. " + "It will be disallowed in 0.7.0." ); } return false; diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 073c0795a..f92f8dc30 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -209,14 +209,22 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier) )); if (realName.empty()) { - m_errorReporter.declarationError(9553_error, _identifier.location, "In variable names _slot and _offset can only be used as a suffix."); + m_errorReporter.declarationError( + 9553_error, + _identifier.location, + "In variable names _slot and _offset can only be used as a suffix." + ); return; } declarations = m_resolver.nameFromCurrentScope(realName); } if (declarations.size() > 1) { - m_errorReporter.declarationError(8827_error, _identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); + m_errorReporter.declarationError( + 8827_error, + _identifier.location, + "Multiple matching identifiers. Resolving overloaded identifiers is not supported." + ); return; } else if (declarations.size() == 0) @@ -224,7 +232,11 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier) if (auto var = dynamic_cast(declarations.front())) if (var->isLocalVariable() && m_yulInsideFunction) { - m_errorReporter.declarationError(8477_error, _identifier.location, "Cannot access local Solidity variables from inside an inline assembly function."); + m_errorReporter.declarationError( + 8477_error, + _identifier.location, + "Cannot access local Solidity variables from inside an inline assembly function." + ); return; } @@ -242,7 +254,11 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl) string namePrefix = identifier.name.str().substr(0, identifier.name.str().find('.')); if (isSlot || isOffset) - m_errorReporter.declarationError(8820_error, identifier.location, "In variable declarations _slot and _offset can not be used as a suffix."); + m_errorReporter.declarationError( + 8820_error, + identifier.location, + "In variable declarations _slot and _offset can not be used as a suffix." + ); else if ( auto declarations = m_resolver.nameFromCurrentScope(namePrefix); !declarations.empty() diff --git a/libsolidity/parsing/DocStringParser.cpp b/libsolidity/parsing/DocStringParser.cpp index 755eb0b50..33cdbd937 100644 --- a/libsolidity/parsing/DocStringParser.cpp +++ b/libsolidity/parsing/DocStringParser.cpp @@ -96,7 +96,10 @@ void DocStringParser::parse(string const& _docString, ErrorReporter& _errorRepor auto tagNameEndPos = firstWhitespaceOrNewline(tagPos, end); if (tagNameEndPos == end) { - m_errorReporter->docstringParsingError(9222_error, "End of tag " + string(tagPos, tagNameEndPos) + " not found"); + m_errorReporter->docstringParsingError( + 9222_error, + "End of tag " + string(tagPos, tagNameEndPos) + " not found" + ); break; } From a499ef16faf9a24bb5e73c90d8c7012c7e406bf9 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Sun, 24 May 2020 19:41:15 +0200 Subject: [PATCH 12/15] Fix spelling errors --- docs/060-breaking-changes.rst | 2 +- libevmasm/Assembly.cpp | 6 +++--- libsolidity/ast/ASTAnnotations.h | 2 +- libsolidity/codegen/CompilerUtils.h | 2 +- libsolidity/codegen/ExpressionCompiler.cpp | 2 +- libsolidity/interface/StandardCompiler.cpp | 4 ++-- scripts/codespell_whitelist.txt | 1 + scripts/release_ppa.sh | 2 +- test/compilationTests/corion/ico.sol | 6 +++--- test/libevmasm/Optimiser.cpp | 20 ++++++++++---------- 10 files changed, 24 insertions(+), 23 deletions(-) diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index 99e969e07..a1ee96d95 100644 --- a/docs/060-breaking-changes.rst +++ b/docs/060-breaking-changes.rst @@ -166,7 +166,7 @@ This section gives detailed instructions on how to update prior code for every b documentation so long as the notices are in the order they appear in the tuple return type. * Choose unique identifiers for variable declarations in inline assembly that do not conflict - with declartions outside the inline assembly block. + with declarations outside the inline assembly block. * Add ``virtual`` to every non-interface function you intend to override. Add ``virtual`` to all functions without implementation outside interfaces. For single inheritance, add diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index fa6ea4f16..296a330ab 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -435,10 +435,10 @@ map Assembly::optimiseInternal( // This only modifies PushTags, we have to run again to actually remove code. if (_settings.runDeduplicate) { - BlockDeduplicator dedup{m_items}; - if (dedup.deduplicate()) + BlockDeduplicator deduplicator{m_items}; + if (deduplicator.deduplicate()) { - for (auto const& replacement: dedup.replacedTags()) + for (auto const& replacement: deduplicator.replacedTags()) { assertThrow( replacement.first <= size_t(-1) && replacement.second <= size_t(-1), diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index eb8f7a263..507559ca6 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -250,7 +250,7 @@ struct ExpressionAnnotation: ASTAnnotation bool lValueOfOrdinaryAssignment = false; /// Types and - if given - names of arguments if the expr. is a function - /// that is called, used for overload resoultion + /// that is called, used for overload resolution std::optional arguments; }; diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index 3d5d85825..a3c266bde 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -286,7 +286,7 @@ public: /// Appends code that computes the Keccak-256 hash of the topmost stack element of 32 byte type. void computeHashStatic(); - /// Apppends code that copies the code of the given contract to memory. + /// Appends code that copies the code of the given contract to memory. /// Stack pre: Memory position /// Stack post: Updated memory position /// @param creation if true, copies creation code, if false copies runtime code. diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index f05ffda8f..6397c7492 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -1912,7 +1912,7 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier) // If the identifier is called right away, this code is executed in visit(FunctionCall...), because // we want to avoid having a reference to the runtime function entry point in the // constructor context, since this would force the compiler to include unreferenced - // internal functions in the runtime contex. + // internal functions in the runtime context. utils().pushCombinedFunctionEntryLabel(functionDef->resolveVirtual(m_context.mostDerivedContract())); else if (auto variable = dynamic_cast(declaration)) appendVariable(*variable, static_cast(_identifier)); diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp index feda8fc53..e04b0b2f1 100644 --- a/libsolidity/interface/StandardCompiler.cpp +++ b/libsolidity/interface/StandardCompiler.cpp @@ -178,7 +178,7 @@ bool isArtifactRequested(Json::Value const& _outputSelection, string const& _art } /// -/// @a _outputSelection is a JSON object containining a two-level hashmap, where the first level is the filename, +/// @a _outputSelection is a JSON object containing a two-level hashmap, where the first level is the filename, /// the second level is the contract name and the value is an array of artifact names to be requested for that contract. /// @a _file is the current file /// @a _contract is the current contract @@ -229,7 +229,7 @@ bool isBinaryRequested(Json::Value const& _outputSelection) if (!_outputSelection.isObject()) return false; - // This does not inculde "evm.methodIdentifiers" on purpose! + // This does not include "evm.methodIdentifiers" on purpose! static vector const outputsThatRequireBinaries{ "*", "ir", "irOptimized", diff --git a/scripts/codespell_whitelist.txt b/scripts/codespell_whitelist.txt index 0409dc1a2..f1111719f 100644 --- a/scripts/codespell_whitelist.txt +++ b/scripts/codespell_whitelist.txt @@ -11,3 +11,4 @@ compilability errorstring hist otion +keypair diff --git a/scripts/release_ppa.sh b/scripts/release_ppa.sh index 6d50e0066..267346e41 100755 --- a/scripts/release_ppa.sh +++ b/scripts/release_ppa.sh @@ -6,7 +6,7 @@ ## You can pass a branch name as argument to this script (which, if no argument is given, ## will default to "develop"). ## -## If the gien branch is "release", the resulting package will be uplaoded to +## If the given branch is "release", the resulting package will be uploaded to ## ethereum/ethereum PPA, or ethereum/ethereum-dev PPA otherwise. ## ## The gnupg key for "builds@ethereum.org" has to be present in order to sign diff --git a/test/compilationTests/corion/ico.sol b/test/compilationTests/corion/ico.sol index 26858e9d6..a8804fd4c 100644 --- a/test/compilationTests/corion/ico.sol +++ b/test/compilationTests/corion/ico.sol @@ -193,7 +193,7 @@ contract ico is safeMath { function setICOEthPrice(uint256 value) external { /* - Setting of the ICO ETC USD rates which can only be calle by a pre-defined address. + Setting of the ICO ETC USD rates which can only be called by a pre-defined address. After this function is completed till the call of the next function (which is at least an exchangeRateDelay array) this rate counts. With this process avoiding the sudden rate changes. @@ -221,8 +221,8 @@ contract ico is safeMath { /* Closing the ICO. It is only possible when the ICO period passed and only by the owner. - The 96% of the whole amount of the token is generated to the address of the fundation. - Ethers which are situated in this contract will be sent to the address of the fundation. + The 96% of the whole amount of the token is generated to the address of the foundation. + Ethers which are situated in this contract will be sent to the address of the foundation. */ require( msg.sender == owner ); require( block.number > icoDelay ); diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index 782b687fc..536eaa951 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -826,8 +826,8 @@ BOOST_AUTO_TEST_CASE(block_deduplicator) Instruction::JUMP, AssemblyItem(Tag, 3) }; - BlockDeduplicator dedup(input); - dedup.deduplicate(); + BlockDeduplicator deduplicator(input); + deduplicator.deduplicate(); set pushTags; for (AssemblyItem const& item: input) @@ -857,8 +857,8 @@ BOOST_AUTO_TEST_CASE(block_deduplicator_assign_immutable_same) AssemblyItem(PushTag, 1), AssemblyItem(PushTag, 1), } + blocks; - BlockDeduplicator dedup(input); - dedup.deduplicate(); + BlockDeduplicator deduplicator(input); + deduplicator.deduplicate(); BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); } @@ -876,8 +876,8 @@ BOOST_AUTO_TEST_CASE(block_deduplicator_assign_immutable_different_value) AssemblyItem{AssignImmutable, 0x1234}, Instruction::JUMP }; - BlockDeduplicator dedup(input); - BOOST_CHECK(!dedup.deduplicate()); + BlockDeduplicator deduplicator(input); + BOOST_CHECK(!deduplicator.deduplicate()); } BOOST_AUTO_TEST_CASE(block_deduplicator_assign_immutable_different_hash) @@ -894,8 +894,8 @@ BOOST_AUTO_TEST_CASE(block_deduplicator_assign_immutable_different_hash) AssemblyItem{AssignImmutable, 0xABCD}, Instruction::JUMP }; - BlockDeduplicator dedup(input); - BOOST_CHECK(!dedup.deduplicate()); + BlockDeduplicator deduplicator(input); + BOOST_CHECK(!deduplicator.deduplicate()); } BOOST_AUTO_TEST_CASE(block_deduplicator_loops) @@ -920,8 +920,8 @@ BOOST_AUTO_TEST_CASE(block_deduplicator_loops) AssemblyItem(PushTag, 2), Instruction::JUMP, }; - BlockDeduplicator dedup(input); - dedup.deduplicate(); + BlockDeduplicator deduplicator(input); + deduplicator.deduplicate(); set pushTags; for (AssemblyItem const& item: input) From 0fda5fe077b6a2a42b64c75ae6a5a9217da795c7 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Sat, 23 May 2020 21:51:59 +0200 Subject: [PATCH 13/15] [SMTChecker] Add test that has an unused mapping --- .../smtCheckerTests/types/unused_mapping.sol | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 test/libsolidity/smtCheckerTests/types/unused_mapping.sol diff --git a/test/libsolidity/smtCheckerTests/types/unused_mapping.sol b/test/libsolidity/smtCheckerTests/types/unused_mapping.sol new file mode 100644 index 000000000..f12cd41de --- /dev/null +++ b/test/libsolidity/smtCheckerTests/types/unused_mapping.sol @@ -0,0 +1,17 @@ +pragma experimental SMTChecker; + +contract C { + uint x; + uint y; + mapping (address => bool) public never_used; + + function inc() public { + require(x < 10); + require(y < 10); + + if(x == 0) x = 0; // noop state var read + x++; + y++; + assert(y == x); + } +} From d45bb2aa07677a53a0219b93cde0414e3a1ab937 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Tue, 19 May 2020 13:12:07 +0200 Subject: [PATCH 14/15] Add proper unipmlemented errors for array copying --- Changelog.md | 2 +- libsolidity/codegen/ArrayUtils.cpp | 7 +++++++ libsolidity/codegen/CompilerUtils.cpp | 8 ++++++++ .../syntaxTests/array/nested_calldata_memory.sol | 13 +++++++++++++ .../syntaxTests/array/nested_calldata_memory2.sol | 13 +++++++++++++ .../syntaxTests/array/nested_calldata_memory3.sol | 13 +++++++++++++ .../syntaxTests/array/nested_calldata_storage.sol | 9 +++++++++ .../syntaxTests/array/nested_calldata_storage2.sol | 9 +++++++++ 8 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/array/nested_calldata_memory.sol create mode 100644 test/libsolidity/syntaxTests/array/nested_calldata_memory2.sol create mode 100644 test/libsolidity/syntaxTests/array/nested_calldata_memory3.sol create mode 100644 test/libsolidity/syntaxTests/array/nested_calldata_storage.sol create mode 100644 test/libsolidity/syntaxTests/array/nested_calldata_storage2.sol diff --git a/Changelog.md b/Changelog.md index 4e0e677d7..518c78963 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,7 +14,7 @@ Bugfixes: * Type Checker: Disallow assignments to storage variables of type ``mapping``. * NatSpec: DocString block is terminated when encountering an empty line. * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. - + * Code Generator: Trigger proper unimplemented errors on certain array copy operations. ### 0.6.8 (2020-05-14) diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 089d0f5cf..732deca6f 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -185,6 +185,13 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons { solAssert(byteOffsetSize == 0, "Byte offset for array as base type."); auto const& sourceBaseArrayType = dynamic_cast(*sourceBaseType); + + solUnimplementedAssert( + _sourceType.location() != DataLocation::CallData || + !_sourceType.isDynamicallyEncoded() || + !sourceBaseArrayType.isDynamicallySized(), + "Copying nested calldata dynamic arrays to storage is not implemented in the old code generator." + ); _context << Instruction::DUP3; if (sourceBaseArrayType.location() == DataLocation::Memory) _context << Instruction::MLOAD; diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 8bf0a21e2..25c91f0a3 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -974,6 +974,14 @@ void CompilerUtils::convertType( } else { + if (auto baseType = dynamic_cast(typeOnStack.baseType())) + solUnimplementedAssert( + typeOnStack.location() != DataLocation::CallData || + !typeOnStack.isDynamicallyEncoded() || + !baseType->isDynamicallySized(), + "Copying nested dynamic calldata arrays to memory is not implemented in the old code generator." + ); + m_context << u256(0) << Instruction::SWAP1; // stack: (variably sized) auto repeat = m_context.newTag(); diff --git a/test/libsolidity/syntaxTests/array/nested_calldata_memory.sol b/test/libsolidity/syntaxTests/array/nested_calldata_memory.sol new file mode 100644 index 000000000..b8361e115 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/nested_calldata_memory.sol @@ -0,0 +1,13 @@ +pragma experimental ABIEncoderV2; + +contract Test { + struct shouldBug { + bytes[2] deadly; + } + function killer(bytes[2] calldata weapon) pure external { + shouldBug(weapon); + } +} + +// ---- +// UnimplementedFeatureError: Copying nested dynamic calldata arrays to memory is not implemented in the old code generator. diff --git a/test/libsolidity/syntaxTests/array/nested_calldata_memory2.sol b/test/libsolidity/syntaxTests/array/nested_calldata_memory2.sol new file mode 100644 index 000000000..c886c8eee --- /dev/null +++ b/test/libsolidity/syntaxTests/array/nested_calldata_memory2.sol @@ -0,0 +1,13 @@ +pragma experimental ABIEncoderV2; + +contract Test { + struct shouldBug { + uint256[][2] deadly; + } + function killer(uint256[][2] calldata weapon) pure external { + shouldBug(weapon); + } +} + +// ---- +// UnimplementedFeatureError: Copying nested dynamic calldata arrays to memory is not implemented in the old code generator. diff --git a/test/libsolidity/syntaxTests/array/nested_calldata_memory3.sol b/test/libsolidity/syntaxTests/array/nested_calldata_memory3.sol new file mode 100644 index 000000000..d8716d18e --- /dev/null +++ b/test/libsolidity/syntaxTests/array/nested_calldata_memory3.sol @@ -0,0 +1,13 @@ +pragma experimental ABIEncoderV2; + +contract Test { + struct shouldBug { + uint256[][] deadly; + } + function killer(uint256[][] calldata weapon) pure external { + shouldBug(weapon); + } +} + +// ---- +// UnimplementedFeatureError: Copying nested dynamic calldata arrays to memory is not implemented in the old code generator. diff --git a/test/libsolidity/syntaxTests/array/nested_calldata_storage.sol b/test/libsolidity/syntaxTests/array/nested_calldata_storage.sol new file mode 100644 index 000000000..bae103321 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/nested_calldata_storage.sol @@ -0,0 +1,9 @@ +pragma experimental ABIEncoderV2; + +contract C { + uint[][2] tmp_i; + function i(uint[][2] calldata s) external { tmp_i = s; } +} + +// ---- +// UnimplementedFeatureError: Copying nested calldata dynamic arrays to storage is not implemented in the old code generator. diff --git a/test/libsolidity/syntaxTests/array/nested_calldata_storage2.sol b/test/libsolidity/syntaxTests/array/nested_calldata_storage2.sol new file mode 100644 index 000000000..d922d717f --- /dev/null +++ b/test/libsolidity/syntaxTests/array/nested_calldata_storage2.sol @@ -0,0 +1,9 @@ +pragma experimental ABIEncoderV2; + +contract C { + uint[][] tmp_i; + function i(uint[][] calldata s) external { tmp_i = s; } +} + +// ---- +// UnimplementedFeatureError: Copying nested calldata dynamic arrays to storage is not implemented in the old code generator. From 7f3d437ffeb9710a3e3aadd5c9eefa477bb57d52 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 25 May 2020 12:22:11 +0200 Subject: [PATCH 15/15] Fix caret position for errors with utf source --- liblangutil/SourceReferenceFormatterHuman.cpp | 6 ++++-- libsolutil/UTF8.cpp | 9 +++++++++ libsolutil/UTF8.h | 2 ++ scripts/isolate_tests.py | 2 +- scripts/wasm-rebuild/docker-scripts/isolate_tests.py | 2 +- test/cmdlineTests/message_format_utf16/err | 11 +++++++++++ test/cmdlineTests/message_format_utf16/input.sol | 3 +++ 7 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 test/cmdlineTests/message_format_utf16/err create mode 100644 test/cmdlineTests/message_format_utf16/input.sol diff --git a/liblangutil/SourceReferenceFormatterHuman.cpp b/liblangutil/SourceReferenceFormatterHuman.cpp index ca947314e..cbf24cf35 100644 --- a/liblangutil/SourceReferenceFormatterHuman.cpp +++ b/liblangutil/SourceReferenceFormatterHuman.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include using namespace std; @@ -103,12 +104,13 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ m_stream << leftpad << ' '; frameColored() << '|'; m_stream << ' '; + for_each( _ref.text.cbegin(), - _ref.text.cbegin() + _ref.startColumn, + _ref.text.cbegin() + numCodepoints(_ref.text.substr(0, _ref.startColumn)), [this](char ch) { m_stream << (ch == '\t' ? '\t' : ' '); } ); - diagColored() << string(locationLength, '^'); + diagColored() << string(numCodepoints(_ref.text.substr(_ref.startColumn, locationLength)), '^'); m_stream << '\n'; } else diff --git a/libsolutil/UTF8.cpp b/libsolutil/UTF8.cpp index a7d55af6c..82c87bb44 100644 --- a/libsolutil/UTF8.cpp +++ b/libsolutil/UTF8.cpp @@ -138,4 +138,13 @@ bool validateUTF8(std::string const& _input, size_t& _invalidPosition) return validateUTF8(reinterpret_cast(_input.c_str()), _input.length(), _invalidPosition); } +size_t numCodepoints(std::string const& _utf8EncodedInput) +{ + size_t codepoint = 0; + for (char c: _utf8EncodedInput) + codepoint += (c & 0xc0) != 0x80; + + return codepoint; +} + } diff --git a/libsolutil/UTF8.h b/libsolutil/UTF8.h index cd84c3982..fb5ee376c 100644 --- a/libsolutil/UTF8.h +++ b/libsolutil/UTF8.h @@ -38,4 +38,6 @@ inline bool validateUTF8(std::string const& _input) return validateUTF8(_input, invalidPos); } +size_t numCodepoints(std::string const& _utf8EncodedInput); + } diff --git a/scripts/isolate_tests.py b/scripts/isolate_tests.py index 768f94801..0f2bc1d79 100755 --- a/scripts/isolate_tests.py +++ b/scripts/isolate_tests.py @@ -13,7 +13,7 @@ import hashlib from os.path import join, isfile def extract_test_cases(path): - lines = open(path, mode='r', encoding='utf8').read().splitlines() + lines = open(path, encoding="utf8", errors='ignore', mode='r').read().splitlines() inside = False delimiter = '' diff --git a/scripts/wasm-rebuild/docker-scripts/isolate_tests.py b/scripts/wasm-rebuild/docker-scripts/isolate_tests.py index eab461bff..0a7571891 100755 --- a/scripts/wasm-rebuild/docker-scripts/isolate_tests.py +++ b/scripts/wasm-rebuild/docker-scripts/isolate_tests.py @@ -8,7 +8,7 @@ from os.path import join, isfile def extract_test_cases(path): - lines = open(path, 'rb').read().splitlines() + lines = open(path, encoding="utf8", errors='ignore', mode='rb').read().splitlines() inside = False delimiter = '' diff --git a/test/cmdlineTests/message_format_utf16/err b/test/cmdlineTests/message_format_utf16/err new file mode 100644 index 000000000..abe92d2e5 --- /dev/null +++ b/test/cmdlineTests/message_format_utf16/err @@ -0,0 +1,11 @@ +Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information. +--> message_format_utf16/input.sol + +Warning: Source file does not specify required compiler version! +--> message_format_utf16/input.sol + +Warning: Statement has no effect. + --> message_format_utf16/input.sol:2:58: + | +2 | /* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } + | ^^^^^^^^^^^^ diff --git a/test/cmdlineTests/message_format_utf16/input.sol b/test/cmdlineTests/message_format_utf16/input.sol new file mode 100644 index 000000000..2c1f9007a --- /dev/null +++ b/test/cmdlineTests/message_format_utf16/input.sol @@ -0,0 +1,3 @@ +contract Foo { +/* ©©©©ﾩ©©©©© 2017 */ constructor () public { "©©©©ﾩ©©©©©" ; } +}