From 3c24d64be5619ed6e645790b89f9cf2a04efe897 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 07:55:35 +0200 Subject: [PATCH 01/23] Set version to 0.4.23 --- CMakeLists.txt | 2 +- Changelog.md | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 208e405d9daf..e6e177ac37ed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ include(EthPolicy) eth_policy() # project name and version should be set after cmake_policy CMP0048 -set(PROJECT_VERSION "0.4.22") +set(PROJECT_VERSION "0.4.23") project(solidity VERSION ${PROJECT_VERSION}) option(SOLC_LINK_STATIC "Link solc executable statically on supported platforms" OFF) diff --git a/Changelog.md b/Changelog.md index 0015c9499632..2279cd2b6847 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,12 @@ +### 0.4.23 (unreleased) + +Features: + + +Bugfixes: + + + ### 0.4.22 (2018-04-16) Features: From 6a747ed22957f5d14268518706d56d2ddd0b90ae Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 07:56:10 +0200 Subject: [PATCH 02/23] Support bionic. --- scripts/release_ppa.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/release_ppa.sh b/scripts/release_ppa.sh index ffb5717dd8f7..375b4d1b090e 100755 --- a/scripts/release_ppa.sh +++ b/scripts/release_ppa.sh @@ -54,7 +54,7 @@ keyid=703F83D0 email=builds@ethereum.org packagename=solc -for distribution in trusty vivid xenial zesty artful +for distribution in trusty vivid xenial zesty artful bionic do cd /tmp/ rm -rf $distribution From 797ce727bb7284cd4fa99fab677ca24d16f51a02 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 08:53:21 +0200 Subject: [PATCH 03/23] Report failed commandline tests. --- scripts/tests.sh | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/scripts/tests.sh b/scripts/tests.sh index 425a4ff4aad3..38073bf33f0c 100755 --- a/scripts/tests.sh +++ b/scripts/tests.sh @@ -42,13 +42,21 @@ else log_directory="" fi -echo "Running commandline tests..." +function printError() { echo "$(tput setaf 1)$1$(tput sgr0)"; } +function printTask() { echo "$(tput bold)$(tput setaf 2)$1$(tput sgr0)"; } + + +printTask "Running commandline tests..." "$REPO_ROOT/test/cmdlineTests.sh" & CMDLINE_PID=$! # Only run in parallel if this is run on CI infrastructure if [ -z "$CI" ] then - wait $CMDLINE_PID + if ! wait $CMDLINE_PID + then + printError "Commandline tests FAILED" + exit 1 + fi fi function download_eth() @@ -112,7 +120,7 @@ for optimize in "" "--optimize" do for vm in $EVM_VERSIONS do - echo "--> Running tests using "$optimize" --evm-version "$vm"..." + printTask "--> Running tests using "$optimize" --evm-version "$vm"..." log="" if [ -n "$log_directory" ] then @@ -127,7 +135,11 @@ do done done -wait $CMDLINE_PID +if ! wait $CMDLINE_PID +then + printError "Commandline tests FAILED" + exit 1 +fi pkill "$ETH_PID" || true sleep 4 From 4d1467eee2aa256e1f2df06fc10cc8f8303405d6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 16 Apr 2018 22:33:14 +0200 Subject: [PATCH 04/23] Enable travis tests on develop branch. --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 539e6088ee1c..e9d2dde6b4a4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -159,8 +159,8 @@ cache: install: - test $SOLC_INSTALL_DEPS_TRAVIS != On || (scripts/install_deps.sh) - test "$TRAVIS_OS_NAME" != "linux" || (scripts/install_cmake.sh) -# - if [ "$TRAVIS_BRANCH" != release -a -z "$TRAVIS_TAG" ]; then SOLC_TESTS=Off; fi - - SOLC_TESTS=Off + # Disable tests unless run on the release branch, on tags or with daily cron + - if [ "$TRAVIS_BRANCH" != release -a -z "$TRAVIS_TAG" -a "$TRAVIS_EVENT_TYPE" != cron ]; then SOLC_TESTS=Off; fi - if [ "$TRAVIS_BRANCH" = release -o -n "$TRAVIS_TAG" ]; then echo -n > prerelease.txt; else date -u +"nightly.%Y.%-m.%-d" > prerelease.txt; fi - echo -n "$TRAVIS_COMMIT" > commit_hash.txt From 0bf3db3fcf2f00f83b28d8d946df2865d047fcf1 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 17 Apr 2018 11:00:02 +0100 Subject: [PATCH 05/23] Add static_assert for the correct jsoncpp version --- libdevcore/JSON.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libdevcore/JSON.cpp b/libdevcore/JSON.cpp index 079d4d51f7f5..d99b3bc6f8e4 100644 --- a/libdevcore/JSON.cpp +++ b/libdevcore/JSON.cpp @@ -27,6 +27,11 @@ using namespace std; +static_assert( + (JSONCPP_VERSION_MAJOR == 1) && (JSONCPP_VERSION_MINOR == 7) && (JSONCPP_VERSION_PATCH == 7), + "Unexpected jsoncpp version: " JSONCPP_VERSION_STRING ". Expecting 1.7.7." +); + namespace dev { From 3710fb59f005073ea397b3d11df25f005cc663d0 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 09:09:07 +0200 Subject: [PATCH 06/23] Update security considerations. --- docs/security-considerations.rst | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index 49fd7ea4eefd..3e1c3a123b2e 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -225,9 +225,6 @@ Minor Details ============= - In ``for (var i = 0; i < arrayName.length; i++) { ... }``, the type of ``i`` will be ``uint8``, because this is the smallest type that is required to hold the value ``0``. If the array has more than 255 elements, the loop will not terminate. -- The ``constant`` keyword for functions is currently not enforced by the compiler. - Furthermore, it is not enforced by the EVM, so a contract function that "claims" - to be constant might still cause changes to the state. - Types that do not occupy the full 32 bytes might contain "dirty higher order bits". This is especially important if you access ``msg.data`` - it poses a malleability risk: You can craft transactions that call a function ``f(uint8 x)`` with a raw byte argument @@ -239,6 +236,22 @@ Minor Details Recommendations *************** +Take Warnings Seriously +======================= + +If the compiler warns you about something, you should better change it. +Even if you do not think that this particular warning has security +implications, there might be another issue buried beneath it. +Any compiler warning we issue can be silenced by slight changes to the +code. + +Also try to enable the "0.5.0" safety features as early as possible +by adding ``pragma experimental "v0.5.0";``. Note that in this case, +the word ``experimental`` does not mean that the safety features are in any +way risky, it is just a way to enable some features that are +not yet part of the latest version of Solidity due to backwards +compatibility. + Restrict the Amount of Ether ============================ From 1e55ec71a7ac02877092849e646759449975d6b8 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 16 Apr 2018 20:50:39 +0200 Subject: [PATCH 07/23] Update release checklist. --- ReleaseChecklist.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ReleaseChecklist.md b/ReleaseChecklist.md index 82b1308c7b10..ebdb75392c06 100644 --- a/ReleaseChecklist.md +++ b/ReleaseChecklist.md @@ -1,7 +1,8 @@ Checklist for making a release: - - [ ] Check that all "nextrelease" issues and pull requests are merged to ``develop``. - - [ ] Create a commit in ``develop`` that updates the ``Changelog`` to include a release date (run the tests locally to update the bug list). + - [ ] Ensure that a Github project exists for the release. + - [ ] Check that all issues and pull requests from the Github project to be released are merged to ``develop``. + - [ ] Create a commit in ``develop`` that updates the ``Changelog`` to include a release date (run ``./scripts/tests.sh`` to update the bug list). Sort the changelog entries alphabetically and correct any errors you notice. - [ ] Create a pull request and wait for the tests, merge it. - [ ] Create a pull request from ``develop`` to ``release``, wait for the tests, then merge it. - [ ] Make a final check that there are no platform-dependency issues in the ``solc-test-bytecode`` repository. @@ -9,7 +10,7 @@ Checklist for making a release: - [ ] Thank voluntary contributors in the Github release page (use ``git shortlog -s -n -e origin/release..origin/develop``). - [ ] Wait for the CI runs on the tag itself (they should push artefacts onto the Github release page). - [ ] Run ``scripts/release_ppa.sh release`` to create the PPA release (you need the relevant openssl key). - - [ ] Check that the Docker release was pushed to Docker Hub (this still seems to have problems). + - [ ] Check that the Docker release was pushed to Docker Hub (this still seems to have problems, run ``./scripts/docker_deploy_manual.sh release``). - [ ] Update the homebrew realease in https://github.com/ethereum/homebrew-ethereum/blob/master/solidity.rb (version and hash) - [ ] Update the default version on readthedocs. - [ ] Make a release of ``solc-js``: Increment the version number, create a pull request for that, merge it after tests succeeded. From ae3350ae0320d140a427d2fa318e7002745a73a5 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Fri, 6 Apr 2018 18:01:40 +0200 Subject: [PATCH 08/23] [SMTChecker] Integration with CVC4 --- Changelog.md | 4 +- cmake/FindCVC4.cmake | 4 + cmake/FindGMP.cmake | 3 + libsolidity/CMakeLists.txt | 26 +++- libsolidity/formal/CVC4Interface.cpp | 200 +++++++++++++++++++++++++++ libsolidity/formal/CVC4Interface.h | 62 +++++++++ libsolidity/formal/SMTChecker.cpp | 4 + libsolidity/formal/SolverInterface.h | 20 +++ libsolidity/formal/Z3Interface.cpp | 17 +-- libsolidity/formal/Z3Interface.h | 3 - 10 files changed, 318 insertions(+), 25 deletions(-) create mode 100644 cmake/FindCVC4.cmake create mode 100644 cmake/FindGMP.cmake create mode 100644 libsolidity/formal/CVC4Interface.cpp create mode 100644 libsolidity/formal/CVC4Interface.h diff --git a/Changelog.md b/Changelog.md index 2279cd2b6847..cfd23ad0ef0a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,12 +1,11 @@ ### 0.4.23 (unreleased) Features: - + * SMTChecker: Integration with CVC4 SMT solver Bugfixes: - ### 0.4.22 (2018-04-16) Features: @@ -34,7 +33,6 @@ Features: * Syntax Tests: Add source locations to syntax test expectations. * Type Checker: Improve documentation and warnings for accessing contract members inherited from ``address``. - Bugfixes: * Code Generator: Allow ``block.blockhash`` without being called. * Code Generator: Do not include internal functions in the runtime bytecode which are only referenced in the constructor. diff --git a/cmake/FindCVC4.cmake b/cmake/FindCVC4.cmake new file mode 100644 index 000000000000..0fb13196352e --- /dev/null +++ b/cmake/FindCVC4.cmake @@ -0,0 +1,4 @@ +find_path(CVC4_INCLUDE_DIR cvc4/cvc4.h) +find_library(CVC4_LIBRARY NAMES cvc4 ) +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(CVC4 DEFAULT_MSG CVC4_LIBRARY CVC4_INCLUDE_DIR) diff --git a/cmake/FindGMP.cmake b/cmake/FindGMP.cmake new file mode 100644 index 000000000000..8abe354caecb --- /dev/null +++ b/cmake/FindGMP.cmake @@ -0,0 +1,3 @@ +find_library(GMP_LIBRARY NAMES gmp ) +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(GMP DEFAULT_MSG GMP_LIBRARY) diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt index 99612c402c43..97b01c83c8f2 100644 --- a/libsolidity/CMakeLists.txt +++ b/libsolidity/CMakeLists.txt @@ -6,10 +6,25 @@ find_package(Z3 QUIET) if (${Z3_FOUND}) include_directories(${Z3_INCLUDE_DIR}) add_definitions(-DHAVE_Z3) - message("Z3 SMT solver found. This enables optional SMT checking.") + message("Z3 SMT solver found. This enables optional SMT checking with Z3.") + list(REMOVE_ITEM sources "${CMAKE_CURRENT_SOURCE_DIR}/formal/CVC4Interface.cpp") else() - message("Z3 SMT solver NOT found. Optional SMT checking will not be available. Please install Z3 if it is desired.") list(REMOVE_ITEM sources "${CMAKE_CURRENT_SOURCE_DIR}/formal/Z3Interface.cpp") + find_package(GMP QUIET) + find_package(CVC4 QUIET) + if (${CVC4_FOUND}) + if (${GMP_FOUND}) + include_directories(${CVC4_INCLUDE_DIR}) + add_definitions(-DHAVE_CVC4) + message("CVC4 SMT solver and GMP found. This enables optional SMT checking with CVC4.") + else() + message("CVC4 SMT solver found but its dependency GMP was NOT found. Optional SMT checking with CVC4 will not be available. Please install GMP if it is desired.") + list(REMOVE_ITEM sources "${CMAKE_CURRENT_SOURCE_DIR}/formal/CVC4Interface.cpp") + endif() + else() + message("No SMT solver found (Z3 or CVC4). Optional SMT checking will not be available. Please install Z3 or CVC4 if it is desired.") + list(REMOVE_ITEM sources "${CMAKE_CURRENT_SOURCE_DIR}/formal/CVC4Interface.cpp") + endif() endif() add_library(solidity ${sources} ${headers}) @@ -17,4 +32,9 @@ target_link_libraries(solidity PUBLIC evmasm devcore) if (${Z3_FOUND}) target_link_libraries(solidity PUBLIC ${Z3_LIBRARY}) -endif() \ No newline at end of file +endif() + +if (${CVC4_FOUND} AND ${GMP_FOUND}) + target_link_libraries(solidity PUBLIC ${CVC4_LIBRARY}) + target_link_libraries(solidity PUBLIC ${GMP_LIBRARY}) +endif() diff --git a/libsolidity/formal/CVC4Interface.cpp b/libsolidity/formal/CVC4Interface.cpp new file mode 100644 index 000000000000..dba5823aaec2 --- /dev/null +++ b/libsolidity/formal/CVC4Interface.cpp @@ -0,0 +1,200 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ + +#include + +#include + +#include + +using namespace std; +using namespace dev; +using namespace dev::solidity::smt; + +CVC4Interface::CVC4Interface(): + m_solver(&m_context) +{ + reset(); +} + +void CVC4Interface::reset() +{ + m_constants.clear(); + m_functions.clear(); + m_solver.reset(); + m_solver.setOption("produce-models", true); +} + +void CVC4Interface::push() +{ + m_solver.push(); +} + +void CVC4Interface::pop() +{ + m_solver.pop(); +} + +Expression CVC4Interface::newFunction(string _name, Sort _domain, Sort _codomain) +{ + CVC4::Type fType = m_context.mkFunctionType(cvc4Sort(_domain), cvc4Sort(_codomain)); + m_functions.insert({_name, m_context.mkVar(_name.c_str(), fType)}); + return SolverInterface::newFunction(move(_name), _domain, _codomain); +} + +Expression CVC4Interface::newInteger(string _name) +{ + m_constants.insert({_name, m_context.mkVar(_name.c_str(), m_context.integerType())}); + return SolverInterface::newInteger(move(_name)); +} + +Expression CVC4Interface::newBool(string _name) +{ + m_constants.insert({_name, m_context.mkVar(_name.c_str(), m_context.booleanType())}); + return SolverInterface::newBool(std::move(_name)); +} + +void CVC4Interface::addAssertion(Expression const& _expr) +{ + try + { + m_solver.assertFormula(toCVC4Expr(_expr)); + } + catch (CVC4::TypeCheckingException const&) + { + solAssert(false, ""); + } + catch (CVC4::LogicException const&) + { + solAssert(false, ""); + } + catch (CVC4::UnsafeInterruptException const&) + { + solAssert(false, ""); + } +} + +pair> CVC4Interface::check(vector const& _expressionsToEvaluate) +{ + CheckResult result; + vector values; + try + { + switch (m_solver.checkSat().isSat()) + { + case CVC4::Result::SAT: + result = CheckResult::SATISFIABLE; + break; + case CVC4::Result::UNSAT: + result = CheckResult::UNSATISFIABLE; + break; + case CVC4::Result::SAT_UNKNOWN: + result = CheckResult::UNKNOWN; + break; + default: + solAssert(false, ""); + } + + if (result != CheckResult::UNSATISFIABLE && !_expressionsToEvaluate.empty()) + { + for (Expression const& e: _expressionsToEvaluate) + values.push_back(toString(m_solver.getValue(toCVC4Expr(e)))); + } + } + catch (CVC4::Exception & e) + { + result = CheckResult::ERROR; + values.clear(); + } + + return make_pair(result, values); +} + +CVC4::Expr CVC4Interface::toCVC4Expr(Expression const& _expr) +{ + if (_expr.arguments.empty() && m_constants.count(_expr.name)) + return m_constants.at(_expr.name); + vector arguments; + for (auto const& arg: _expr.arguments) + arguments.push_back(toCVC4Expr(arg)); + + string const& n = _expr.name; + if (m_functions.count(n)) + return m_context.mkExpr(CVC4::kind::APPLY_UF, m_functions[n], arguments); + else if (m_constants.count(n)) + { + solAssert(arguments.empty(), ""); + return m_constants.at(n); + } + else if (arguments.empty()) + { + if (n == "true") + return m_context.mkConst(true); + else if (n == "false") + return m_context.mkConst(false); + else + // We assume it is an integer... + return m_context.mkConst(CVC4::Rational(n)); + } + + solAssert(_expr.hasCorrectArity(), ""); + if (n == "ite") + return arguments[0].iteExpr(arguments[1], arguments[2]); + else if (n == "not") + return arguments[0].notExpr(); + else if (n == "and") + return arguments[0].andExpr(arguments[1]); + else if (n == "or") + return arguments[0].orExpr(arguments[1]); + else if (n == "=") + return m_context.mkExpr(CVC4::kind::EQUAL, arguments[0], arguments[1]); + else if (n == "<") + return m_context.mkExpr(CVC4::kind::LT, arguments[0], arguments[1]); + else if (n == "<=") + return m_context.mkExpr(CVC4::kind::LEQ, arguments[0], arguments[1]); + else if (n == ">") + return m_context.mkExpr(CVC4::kind::GT, arguments[0], arguments[1]); + else if (n == ">=") + return m_context.mkExpr(CVC4::kind::GEQ, arguments[0], arguments[1]); + else if (n == "+") + return m_context.mkExpr(CVC4::kind::PLUS, arguments[0], arguments[1]); + else if (n == "-") + return m_context.mkExpr(CVC4::kind::MINUS, arguments[0], arguments[1]); + else if (n == "*") + return m_context.mkExpr(CVC4::kind::MULT, arguments[0], arguments[1]); + else if (n == "/") + return m_context.mkExpr(CVC4::kind::INTS_DIVISION_TOTAL, arguments[0], arguments[1]); + // Cannot reach here. + solAssert(false, ""); + return arguments[0]; +} + +CVC4::Type CVC4Interface::cvc4Sort(Sort _sort) +{ + switch (_sort) + { + case Sort::Bool: + return m_context.booleanType(); + case Sort::Int: + return m_context.integerType(); + default: + break; + } + solAssert(false, ""); + // Cannot be reached. + return m_context.integerType(); +} diff --git a/libsolidity/formal/CVC4Interface.h b/libsolidity/formal/CVC4Interface.h new file mode 100644 index 000000000000..cfaeb412973a --- /dev/null +++ b/libsolidity/formal/CVC4Interface.h @@ -0,0 +1,62 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ + +#pragma once + +#include + +#include + +#include + +namespace dev +{ +namespace solidity +{ +namespace smt +{ + +class CVC4Interface: public SolverInterface, public boost::noncopyable +{ +public: + CVC4Interface(); + + void reset() override; + + void push() override; + void pop() override; + + Expression newFunction(std::string _name, Sort _domain, Sort _codomain) override; + Expression newInteger(std::string _name) override; + Expression newBool(std::string _name) override; + + void addAssertion(Expression const& _expr) override; + std::pair> check(std::vector const& _expressionsToEvaluate) override; + +private: + CVC4::Expr toCVC4Expr(Expression const& _expr); + CVC4::Type cvc4Sort(smt::Sort _sort); + + CVC4::ExprManager m_context; + CVC4::SmtEngine m_solver; + std::map m_constants; + std::map m_functions; +}; + +} +} +} diff --git a/libsolidity/formal/SMTChecker.cpp b/libsolidity/formal/SMTChecker.cpp index 8f4abdc277c7..da6b632cc250 100644 --- a/libsolidity/formal/SMTChecker.cpp +++ b/libsolidity/formal/SMTChecker.cpp @@ -19,6 +19,8 @@ #ifdef HAVE_Z3 #include +#elif HAVE_CVC4 +#include #else #include #endif @@ -39,6 +41,8 @@ using namespace dev::solidity; SMTChecker::SMTChecker(ErrorReporter& _errorReporter, ReadCallback::Callback const& _readFileCallback): #ifdef HAVE_Z3 m_interface(make_shared()), +#elif HAVE_CVC4 + m_interface(make_shared()), #else m_interface(make_shared(_readFileCallback)), #endif diff --git a/libsolidity/formal/SolverInterface.h b/libsolidity/formal/SolverInterface.h index 0bdebb6c7a0b..e127bb5515c5 100644 --- a/libsolidity/formal/SolverInterface.h +++ b/libsolidity/formal/SolverInterface.h @@ -65,6 +65,26 @@ class Expression Expression& operator=(Expression const&) = default; Expression& operator=(Expression&&) = default; + bool hasCorrectArity() const + { + static std::map const operatorsArity{ + {"ite", 3}, + {"not", 1}, + {"and", 2}, + {"or", 2}, + {"=", 2}, + {"<", 2}, + {"<=", 2}, + {">", 2}, + {">=", 2}, + {"+", 2}, + {"-", 2}, + {"*", 2}, + {"/", 2} + }; + return operatorsArity.count(name) && operatorsArity.at(name) == arguments.size(); + } + static Expression ite(Expression _condition, Expression _trueValue, Expression _falseValue) { solAssert(_trueValue.sort == _falseValue.sort, ""); diff --git a/libsolidity/formal/Z3Interface.cpp b/libsolidity/formal/Z3Interface.cpp index 125da00d7cc2..41943c92e3e3 100644 --- a/libsolidity/formal/Z3Interface.cpp +++ b/libsolidity/formal/Z3Interface.cpp @@ -116,21 +116,6 @@ z3::expr Z3Interface::toZ3Expr(Expression const& _expr) for (auto const& arg: _expr.arguments) arguments.push_back(toZ3Expr(arg)); - static map arity{ - {"ite", 3}, - {"not", 1}, - {"and", 2}, - {"or", 2}, - {"=", 2}, - {"<", 2}, - {"<=", 2}, - {">", 2}, - {">=", 2}, - {"+", 2}, - {"-", 2}, - {"*", 2}, - {"/", 2} - }; string const& n = _expr.name; if (m_functions.count(n)) return m_functions.at(n)(arguments); @@ -150,7 +135,7 @@ z3::expr Z3Interface::toZ3Expr(Expression const& _expr) return m_context.int_val(n.c_str()); } - solAssert(arity.count(n) && arity.at(n) == arguments.size(), ""); + solAssert(_expr.hasCorrectArity(), ""); if (n == "ite") return z3::ite(arguments[0], arguments[1], arguments[2]); else if (n == "not") diff --git a/libsolidity/formal/Z3Interface.h b/libsolidity/formal/Z3Interface.h index 44d4bb2f2aae..354ded2598d8 100644 --- a/libsolidity/formal/Z3Interface.h +++ b/libsolidity/formal/Z3Interface.h @@ -51,9 +51,6 @@ class Z3Interface: public SolverInterface, public boost::noncopyable z3::expr toZ3Expr(Expression const& _expr); z3::sort z3Sort(smt::Sort _sort); - std::string checkSatAndGetValuesCommand(std::vector const& _expressionsToEvaluate); - std::vector parseValues(std::string::const_iterator _start, std::string::const_iterator _end); - z3::context m_context; z3::solver m_solver; std::map m_constants; From 1696c9f1630c7d5edaa2cb40e89b1db137781699 Mon Sep 17 00:00:00 2001 From: ankit raj Date: Tue, 17 Apr 2018 17:01:06 +0530 Subject: [PATCH 09/23] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 42c392e22b4b..8303131c84b0 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,6 @@ Solidity is still under development. So please do not hesitate and open an [issu See the [Solidity documentation](https://solidity.readthedocs.io/en/latest/installing-solidity.html#building-from-source) for build instructions. ## How to Contribute -Please see our contribution guidelines in [the Solidity documentation](https://solidity.readthedocs.io/en/latest/contributing.html). +Please see our [contribution guidelines](https://solidity.readthedocs.io/en/latest/contributing.html) in the Solidity documentation. Any contributions are welcome! From 78ba34608f9b587b5a481769ba6fee45a49fcf3a Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 18 Apr 2018 12:52:04 +0200 Subject: [PATCH 10/23] [SMTChecker] Using solUnimplementedAssert instead of solAssert when applicable --- libsolidity/formal/SMTChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsolidity/formal/SMTChecker.cpp b/libsolidity/formal/SMTChecker.cpp index da6b632cc250..777e57c3bc24 100644 --- a/libsolidity/formal/SMTChecker.cpp +++ b/libsolidity/formal/SMTChecker.cpp @@ -468,7 +468,7 @@ void SMTChecker::compareOperation(BinaryOperation const& _op) } else // Bool { - solAssert(SSAVariable::isBool(_op.annotation().commonType->category()), ""); + solUnimplementedAssert(SSAVariable::isBool(_op.annotation().commonType->category()), "Operation not yet supported"); value = make_shared( op == Token::Equal ? (left == right) : op == Token::NotEqual ? (left != right) : @@ -839,7 +839,7 @@ void SMTChecker::createExpr(Expression const& _e) m_expressions.emplace(&_e, m_interface->newBool(uniqueSymbol(_e))); break; default: - solAssert(false, "Type not implemented."); + solUnimplementedAssert(false, "Type not implemented."); } } } From f510348ff1d9f0839a4257d6e05f59304247b4e7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 11:39:40 +0200 Subject: [PATCH 11/23] Extract tests. --- .../SolidityNameAndTypeResolution.cpp | 182 ------------------ ...constructible_internal_constructor_new.sol | 6 + ...constructible_internal_constructor_old.sol | 9 + .../constructor/constructor_new.sol | 1 + .../constructor/constructor_old.sol | 3 + .../constructor/constructor_old_050.sol | 4 + .../constructor_state_mutability_new.sol | 13 ++ .../constructor_state_mutability_old.sol | 16 ++ .../constructor_visibility_new.sol | 12 ++ .../constructor_visibility_old.sol | 13 ++ ...constructor_without_implementation_new.sol | 5 + ...constructor_without_implementation_old.sol | 6 + .../constructor/external_constructor_new.sol | 5 + .../constructor/external_constructor_old.sol | 6 + ...ible_internal_constructor_inverted_new.sol | 13 ++ ...ible_internal_constructor_inverted_old.sol | 15 ++ ...constructible_internal_constructor_new.sol | 8 + ...constructible_internal_constructor_old.sol | 9 + .../constructor/interface_constructor_new.sol | 7 + .../constructor/interface_constructor_old.sol | 8 + .../constructor/library_constructor_new.sol | 6 + .../constructor/library_constructor_old.sol | 7 + .../constructor/overriding_constructor.sol | 6 + .../returns_in_constructor_new.sol | 5 + .../returns_in_constructor_old.sol | 6 + .../constructor/two_constructors_old.sol | 8 + 26 files changed, 197 insertions(+), 182 deletions(-) create mode 100644 test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_old_050.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/external_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/external_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/library_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/library_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/overriding_constructor.sol create mode 100644 test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_old.sol diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index d438a9dc16ee..301250f0a7d2 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -243,16 +243,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_struct) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(returns_in_constructor) -{ - char const* text = R"( - contract test { - function test() public returns (uint a) { } - } - )"; - CHECK_ERROR(text, TypeError, "Non-empty \"returns\" directive for constructor."); -} - BOOST_AUTO_TEST_CASE(forward_function_reference) { char const* text = R"( @@ -869,26 +859,6 @@ BOOST_AUTO_TEST_CASE(complex_inheritance) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(constructor_visibility) -{ - // The constructor of a base class should not be visible in the derived class - char const* text = R"( - contract A { function A() public { } } - contract B is A { function f() public { A x = A(0); } } - )"; - CHECK_SUCCESS(text); -} - -BOOST_AUTO_TEST_CASE(overriding_constructor) -{ - // It is fine to "override" constructor of a base class since it is invisible - char const* text = R"( - contract A { function A() public { } } - contract B is A { function A() public returns (uint8 r) {} } - )"; - CHECK_SUCCESS(text); -} - BOOST_AUTO_TEST_CASE(missing_base_constructor_arguments) { char const* text = R"( @@ -907,35 +877,6 @@ BOOST_AUTO_TEST_CASE(base_constructor_arguments_override) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(new_constructor_syntax) -{ - char const* text = R"( - contract A { constructor() public {} } - )"; - CHECK_SUCCESS_NO_WARNINGS(text); -} - -BOOST_AUTO_TEST_CASE(old_constructor_syntax) -{ - char const* text = R"( - contract A { function A() public {} } - )"; - CHECK_WARNING( - text, - "Defining constructors as functions with the same name as the contract is deprecated." - ); - - text = R"( - pragma experimental "v0.5.0"; - contract A { function A() public {} } - )"; - CHECK_ERROR( - text, - SyntaxError, - "Functions are not allowed to have the same name as the contract." - ); -} - BOOST_AUTO_TEST_CASE(implicit_derived_to_base_conversion) { char const* text = R"( @@ -2588,17 +2529,6 @@ BOOST_AUTO_TEST_CASE(override_changes_return_types) CHECK_ERROR(sourceCode, TypeError, "Overriding function return types differ"); } -BOOST_AUTO_TEST_CASE(multiple_constructors) -{ - char const* sourceCode = R"( - contract test { - function test(uint a) public { } - function test() public {} - } - )"; - CHECK_ERROR(sourceCode, DeclarationError, "More than one constructor defined"); -} - BOOST_AUTO_TEST_CASE(equal_overload) { char const* sourceCode = R"( @@ -3122,19 +3052,6 @@ BOOST_AUTO_TEST_CASE(library_having_variables) CHECK_ERROR(text, TypeError, "Library cannot have non-constant state variables"); } -BOOST_AUTO_TEST_CASE(library_constructor) -{ - char const* text = R"( - library Lib { - function Lib(); - } - )"; - CHECK_ERROR_ALLOW_MULTI(text, TypeError, (vector{ - "Constructor cannot be defined in libraries.", - "Constructor must be implemented if declared." - })); -} - BOOST_AUTO_TEST_CASE(valid_library) { char const* text = R"( @@ -5030,38 +4947,6 @@ BOOST_AUTO_TEST_CASE(unsatisfied_version) BOOST_CHECK(searchErrorMessage(*sourceAndError.second.front(), "Source file requires different compiler version")); } -BOOST_AUTO_TEST_CASE(invalid_constructor_statemutability) -{ - char const* text = R"( - contract test { - function test() constant {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); - text = R"( - contract test { - function test() view {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); - text = R"( - contract test { - function test() pure {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); -} - -BOOST_AUTO_TEST_CASE(external_constructor) -{ - char const* text = R"( - contract test { - function test() external {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be public or internal."); -} - BOOST_AUTO_TEST_CASE(invalid_array_as_statement) { char const* text = R"( @@ -5608,50 +5493,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_constant) CHECK_ERROR(text, TypeError, "Cannot assign to a constant variable."); } -BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor) -{ - char const* text = R"( - contract C { - function C() internal {} - } - contract D { - function f() public { var x = new C(); } - } - )"; - CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); -} - -BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor_inverted) -{ - // Previously, the type information for A was not yet available at the point of - // "new A". - char const* text = R"( - contract B { - A a; - function B() public { - a = new A(this); - } - } - contract A { - function A(address a) internal {} - } - )"; - CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); -} - -BOOST_AUTO_TEST_CASE(constructible_internal_constructor) -{ - char const* text = R"( - contract C { - function C() internal {} - } - contract D is C { - function D() public { } - } - )"; - CHECK_SUCCESS(text); -} - BOOST_AUTO_TEST_CASE(return_structs) { char const* text = R"( @@ -5813,19 +5654,6 @@ BOOST_AUTO_TEST_CASE(interface) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(interface_constructor) -{ - char const* text = R"( - interface I { - function I(); - } - )"; - CHECK_ERROR_ALLOW_MULTI(text, TypeError, (std::vector{ - "Constructor cannot be defined in interfaces", - "Constructor must be implemented if declared.", - })); -} - BOOST_AUTO_TEST_CASE(interface_functions) { char const* text = R"( @@ -6907,16 +6735,6 @@ BOOST_AUTO_TEST_CASE(builtin_reject_value) CHECK_ERROR(text, TypeError, "Member \"value\" not found or not visible after argument-dependent lookup"); } -BOOST_AUTO_TEST_CASE(constructor_without_implementation) -{ - char const* text = R"( - contract C { - function C(); - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared."); -} - BOOST_AUTO_TEST_CASE(large_storage_array_fine) { char const* text = R"( diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol new file mode 100644 index 000000000000..8dee4c7125bf --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol @@ -0,0 +1,6 @@ +contract C { + constructor() internal {} +} +contract D is C { + constructor() public { } +} diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol new file mode 100644 index 000000000000..144743e38339 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol @@ -0,0 +1,9 @@ +contract C { + function C() internal {} +} +contract D is C { + function D() public {} +} +// ---- +// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (60-82): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_new.sol new file mode 100644 index 000000000000..aa3422cc234a --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_new.sol @@ -0,0 +1 @@ +contract A { constructor() public {} } diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_old.sol new file mode 100644 index 000000000000..9ec6257ddf32 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_old.sol @@ -0,0 +1,3 @@ +contract A { function A() public {} } +// ---- +// Warning: (13-35): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol b/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol new file mode 100644 index 000000000000..19e46e793321 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol @@ -0,0 +1,4 @@ +pragma experimental "v0.5.0"; +contract A { function A() public {} } +// ---- +// SyntaxError: (43-65): Functions are not allowed to have the same name as the contract. If you intend this to be a constructor, use "constructor(...) { ... }" to define it. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol new file mode 100644 index 000000000000..15ed0e1e7c65 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol @@ -0,0 +1,13 @@ +contract test1 { + constructor() constant {} +} +contract test2 { + constructor() view {} +} +contract test3 { + constructor() pure {} +} +// ---- +// TypeError: (19-44): Constructor must be payable or non-payable, but is "view". +// TypeError: (66-87): Constructor must be payable or non-payable, but is "view". +// TypeError: (109-130): Constructor must be payable or non-payable, but is "pure". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol new file mode 100644 index 000000000000..6dbcbc974aef --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol @@ -0,0 +1,16 @@ +contract test1 { + function test1() constant {} +} +contract test2 { + function test2() view {} +} +contract test3 { + function test3() pure {} +} +// ---- +// Warning: (21-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (73-97): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (121-145): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (21-49): Constructor must be payable or non-payable, but is "view". +// TypeError: (73-97): Constructor must be payable or non-payable, but is "view". +// TypeError: (121-145): Constructor must be payable or non-payable, but is "pure". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol new file mode 100644 index 000000000000..502dc0295e50 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol @@ -0,0 +1,12 @@ +// The constructor of a base class should not be visible in the derived class +contract A { constructor(string) public { } } +contract B is A { + function f() pure public { + A x = A(0); // convert from address + string memory y = "ab"; + A(y); // call as a function is invalid + x; + } +} +// ---- +// TypeError: (243-247): Explicit type conversion not allowed from "string memory" to "contract A". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol new file mode 100644 index 000000000000..847ea27b1f96 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol @@ -0,0 +1,13 @@ +// The constructor of a base class should not be visible in the derived class +contract A { function A(string s) public { } } +contract B is A { + function f() pure public { + A x = A(0); // convert from address + string memory y = "ab"; + A(y); // call as a function is invalid + x; + } +} +// ---- +// Warning: (91-122): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (244-248): Explicit type conversion not allowed from "string memory" to "contract A". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol new file mode 100644 index 000000000000..5e61914303c3 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol @@ -0,0 +1,5 @@ +contract C { + constructor(); +} +// ---- +// TypeError: (14-28): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol new file mode 100644 index 000000000000..72458703de34 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol @@ -0,0 +1,6 @@ +contract C { + function C(); +} +// ---- +// Warning: (14-27): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (14-27): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol new file mode 100644 index 000000000000..30cf0668c128 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol @@ -0,0 +1,5 @@ +contract test { + constructor() external {} +} +// ---- +// TypeError: (17-42): Constructor must be public or internal. diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol new file mode 100644 index 000000000000..27869361573f --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol @@ -0,0 +1,6 @@ +contract test { + function test() external {} +} +// ---- +// Warning: (17-44): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (17-44): Constructor must be public or internal. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol new file mode 100644 index 000000000000..2a199b3a5fea --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol @@ -0,0 +1,13 @@ +// Previously, the type information for A was not yet available at the point of +// "new A". +contract B { + A a; + constructor() public { + a = new A(this); + } +} +contract A { + constructor(address a) internal {} +} +// ---- +// TypeError: (141-146): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol new file mode 100644 index 000000000000..0a27e9f85c54 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol @@ -0,0 +1,15 @@ +// Previously, the type information for A was not yet available at the point of +// "new A". +contract B { + A a; + function B() public { + a = new A(this); + } +} +contract A { + function A(address a) internal {} +} +// ---- +// Warning: (112-155): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (172-205): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (140-145): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol new file mode 100644 index 000000000000..2511c751c508 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol @@ -0,0 +1,8 @@ +contract C { + constructor() internal {} +} +contract D { + function f() public { C c = new C(); c; } +} +// ---- +// TypeError: (84-89): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol new file mode 100644 index 000000000000..2897e6f35a08 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol @@ -0,0 +1,9 @@ +contract C { + function C() internal {} +} +contract D { + function f() public { C x = new C(); x; } +} +// ---- +// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (83-88): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol new file mode 100644 index 000000000000..fa5d54c4f8c3 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol @@ -0,0 +1,7 @@ +interface I { + constructor(); +} +// ---- +// Warning: (15-29): Functions in interfaces should be declared external. +// TypeError: (15-29): Constructor cannot be defined in interfaces. +// TypeError: (15-29): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol new file mode 100644 index 000000000000..ddf549776b3c --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol @@ -0,0 +1,8 @@ +interface I { + function I(); +} +// ---- +// Warning: (15-28): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (15-28): Functions in interfaces should be declared external. +// TypeError: (15-28): Constructor cannot be defined in interfaces. +// TypeError: (15-28): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol new file mode 100644 index 000000000000..8db7e62a8a18 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol @@ -0,0 +1,6 @@ +library Lib { + constructor(); +} +// ---- +// TypeError: (15-29): Constructor cannot be defined in libraries. +// TypeError: (15-29): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol new file mode 100644 index 000000000000..d449904920b4 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol @@ -0,0 +1,7 @@ +library Lib { + function Lib(); +} +// ---- +// Warning: (15-30): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (15-30): Constructor cannot be defined in libraries. +// TypeError: (15-30): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol new file mode 100644 index 000000000000..3290a33b3e08 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol @@ -0,0 +1,6 @@ +// It is fine to "override" constructor of a base class since it is invisible +contract A { function A() public { } } +contract B is A { function A() public pure returns (uint8) {} } +// ---- +// Warning: (91-114): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (135-178): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol new file mode 100644 index 000000000000..e6a03014efad --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol @@ -0,0 +1,5 @@ +contract test { + constructor() public returns (uint a) { } +} +// ---- +// TypeError: (46-54): Non-empty "returns" directive for constructor. diff --git a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol new file mode 100644 index 000000000000..00b3974c6aff --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol @@ -0,0 +1,6 @@ +contract test { + function test() public returns (uint a) { } +} +// ---- +// Warning: (17-60): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (48-56): Non-empty "returns" directive for constructor. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol new file mode 100644 index 000000000000..40341c4d54f1 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol @@ -0,0 +1,8 @@ +contract test { + function test(uint a) public { } + function test() public {} +} +// ---- +// Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// DeclarationError: (17-49): More than one constructor defined. From 29a97f16411701c893b2887359524022c0fc6bd6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 11:40:02 +0200 Subject: [PATCH 12/23] Fix name clashes between constructor and fallback function. --- Changelog.md | 3 +- libsolidity/analysis/TypeChecker.cpp | 92 +++++++++---------- .../SolidityNameAndTypeResolution.cpp | 1 - .../constructor/two_constructors_mixed.sol | 7 ++ .../constructor/two_constructors_new.sol | 6 ++ .../constructor/two_constructors_old.sol | 2 +- 6 files changed, 61 insertions(+), 50 deletions(-) create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_new.sol diff --git a/Changelog.md b/Changelog.md index cfd23ad0ef0a..a0df396d6207 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,7 +4,8 @@ Features: * SMTChecker: Integration with CVC4 SMT solver Bugfixes: - + * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. + * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax. ### 0.4.22 (2018-04-16) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 47a551dc7f2a..87d69d97fac7 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -109,39 +109,28 @@ bool TypeChecker::visit(ContractDefinition const& _contract) m_errorReporter.typeError(function->location(), "Constructor must be public or internal."); } - FunctionDefinition const* fallbackFunction = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - { if (function->isFallback()) { - if (fallbackFunction) - { - m_errorReporter.declarationError(function->location(), "Only one fallback function is allowed."); - } - else - { - fallbackFunction = function; - if (_contract.isLibrary()) - m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions."); - if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) - m_errorReporter.typeError( - function->location(), - "Fallback function must be payable or non-payable, but is \"" + - stateMutabilityToString(function->stateMutability()) + - "\"." - ); - if (!fallbackFunction->parameters().empty()) - m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters."); - if (!fallbackFunction->returnParameters().empty()) - m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values."); - if ( - _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && - fallbackFunction->visibility() != FunctionDefinition::Visibility::External - ) - m_errorReporter.typeError(fallbackFunction->location(), "Fallback function must be defined as \"external\"."); - } + if (_contract.isLibrary()) + m_errorReporter.typeError(function->location(), "Libraries cannot have fallback functions."); + if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) + m_errorReporter.typeError( + function->location(), + "Fallback function must be payable or non-payable, but is \"" + + stateMutabilityToString(function->stateMutability()) + + "\"." + ); + if (!function->parameters().empty()) + m_errorReporter.typeError(function->parameterList().location(), "Fallback function cannot take parameters."); + if (!function->returnParameters().empty()) + m_errorReporter.typeError(function->returnParameterList()->location(), "Fallback function cannot return values."); + if ( + _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && + function->visibility() != FunctionDefinition::Visibility::External + ) + m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\"."); } - } for (auto const& n: _contract.subNodes()) if (!visited.count(n.get())) @@ -172,25 +161,34 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con /// Checks that two functions with the same name defined in this contract have different /// argument types and that there is at most one constructor. map> functions; + FunctionDefinition const* constructor = nullptr; + FunctionDefinition const* fallback = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - functions[function->name()].push_back(function); - - // Constructor - if (functions[_contract.name()].size() > 1) - { - SecondarySourceLocation ssl; - auto it = ++functions[_contract.name()].begin(); - for (; it != functions[_contract.name()].end(); ++it) - ssl.append("Another declaration is here:", (*it)->location()); - - string msg = "More than one constructor defined."; - ssl.limitSize(msg); - m_errorReporter.declarationError( - functions[_contract.name()].front()->location(), - ssl, - msg - ); - } + if (function->isConstructor()) + { + if (constructor) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", constructor->location()), + "More than one constructor defined." + ); + constructor = function; + } + else if (function->isFallback()) + { + if (fallback) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", fallback->location()), + "Only one fallback function is allowed." + ); + fallback = function; + } + else + { + solAssert(!function->name().empty(), ""); + functions[function->name()].push_back(function); + } findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 301250f0a7d2..7c0e86438bb0 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1140,7 +1140,6 @@ BOOST_AUTO_TEST_CASE(fallback_function_twice) } )"; CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, (vector{ - "Function with same name and arguments defined twice.", "Only one fallback function is" })); } diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol new file mode 100644 index 000000000000..c757354e816c --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol @@ -0,0 +1,7 @@ +contract test { + function test(uint) public { } + constructor() public {} +} +// ---- +// Warning: (17-47): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// DeclarationError: (49-72): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol new file mode 100644 index 000000000000..42c0de28f496 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol @@ -0,0 +1,6 @@ +contract test { + constructor(uint) public { } + constructor() public {} +} +// ---- +// DeclarationError: (47-70): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol index 40341c4d54f1..db632ced1536 100644 --- a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol @@ -5,4 +5,4 @@ contract test { // ---- // Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. // Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// DeclarationError: (17-49): More than one constructor defined. +// DeclarationError: (51-76): More than one constructor defined. From 64043ef970524ce4b9b9209b07c2bedd5d72cc0e Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 18 Apr 2018 14:56:54 +0200 Subject: [PATCH 13/23] Support ubuntu bionic source builds. --- Changelog.md | 1 + scripts/install_deps.sh | 14 +++++--------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/Changelog.md b/Changelog.md index cfd23ad0ef0a..df85ade9d54a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Features: * SMTChecker: Integration with CVC4 SMT solver + * Build system: Support Ubuntu Bionic. Bugfixes: diff --git a/scripts/install_deps.sh b/scripts/install_deps.sh index e884ed652725..fa5551bfe273 100755 --- a/scripts/install_deps.sh +++ b/scripts/install_deps.sh @@ -268,43 +268,39 @@ case $(uname -s) in install_z3="" case $(lsb_release -cs) in trusty|qiana|rebecca|rafaela|rosa) - #trusty echo "Installing solidity dependencies on Ubuntu Trusty Tahr (14.04)." echo "Or, you may also be running Linux Mint Qiana / Rebecca / Rafaela / Rosa (base: Ubuntu Trusty Tahr (14.04).)" ;; utopic) - #utopic echo "Installing solidity dependencies on Ubuntu Utopic Unicorn (14.10)." ;; vivid) - #vivid echo "Installing solidity dependencies on Ubuntu Vivid Vervet (15.04)." ;; wily) - #wily echo "Installing solidity dependencies on Ubuntu Wily Werewolf (15.10)." ;; xenial|sarah|serena|sonya|sylvia) - #xenial echo "Installing solidity dependencies on Ubuntu Xenial Xerus (16.04)." echo "Or, you may also be running Linux Mint Sarah / Serena / Sonya / Sylvia (base: Ubuntu Xenial Xerus (16.04).)" install_z3="libz3-dev" ;; yakkety) - #yakkety echo "Installing solidity dependencies on Ubuntu Yakkety Yak (16.10)." install_z3="libz3-dev" ;; zesty) - #zesty echo "Installing solidity dependencies on Ubuntu Zesty (17.04)." install_z3="libz3-dev" ;; artful) - #artful echo "Installing solidity dependencies on Ubuntu Artful (17.10)." install_z3="libz3-dev" ;; + bionic) + echo "Installing solidity dependencies on Ubuntu Bionic (18.04)." + install_z3="libz3-dev" + ;; betsy) #do not try anything for betsy. echo "Linux Mint Betsy is not supported at the moment as it runs off of Debian." @@ -319,7 +315,7 @@ case $(uname -s) in echo "ERROR - Unknown or unsupported Ubuntu version (" $(lsb_release -cs) ")" echo "ERROR - This might not work, but we are trying anyway." echo "Please drop us a message at https://gitter.im/ethereum/solidity-dev." - echo "We only support Trusty, Utopic, Vivid, Wily, Xenial, Yakkety, Zesty and Artful." + echo "We only support Trusty, Utopic, Vivid, Wily, Xenial, Yakkety, Zesty, Artful and Bionic." install_z3="libz3-dev" ;; esac From e22929ebb8f33048bfc3eed2b6909ae539b4e221 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 18 Apr 2018 18:14:45 +0200 Subject: [PATCH 14/23] Remove Zeppelin patches. --- test/externalTests.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/externalTests.sh b/test/externalTests.sh index 2a5ff7ef14f7..cd6deb7542b0 100755 --- a/test/externalTests.sh +++ b/test/externalTests.sh @@ -47,13 +47,6 @@ function test_truffle cd "$DIR" npm install find . -name soljson.js -exec cp "$SOLJSON" {} \; - if [ "$name" == "Zeppelin" ]; then - # Fix some things that look like bugs (only seemed to fail on Node 6 and not Node 8) - # FIXME: report upstream or to web3.js? - sed -i -e 's/let token = await ERC827TokenMock.new();//;' test/token/ERC827/ERC827Token.js - sed -i -e 's/CappedCrowdsale.new(this.startTime, this.endTime, rate, wallet, 0)/CappedCrowdsale.new(this.startTime, this.endTime, rate, wallet, 0, this.token.address)/' test/crowdsale/CappedCrowdsale.test.js - sed -i -e 's/RefundableCrowdsale.new(this.startTime, this.endTime, rate, wallet, 0, { from: owner })/RefundableCrowdsale.new(this.startTime, this.endTime, rate, wallet, 0, this.token.address, { from: owner })/' test/crowdsale/RefundableCrowdsale.test.js - fi if [ "$name" == "Gnosis" ]; then # Replace fixed-version pragmas in Gnosis (part of Consensys best practice) find contracts test -name '*.sol' -type f -print0 | xargs -0 sed -i -e 's/pragma solidity 0/pragma solidity ^0/' From 489586430202d17fb60d973503ab27670474143d Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 18 Apr 2018 14:28:53 +0200 Subject: [PATCH 15/23] Warn about functions named "constructor". --- Changelog.md | 3 ++- libsolidity/analysis/SyntaxChecker.cpp | 7 ++++++- .../syntaxTests/constructor/function_named_constructor.sol | 5 +++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/syntaxTests/constructor/function_named_constructor.sol diff --git a/Changelog.md b/Changelog.md index 230c1174a05c..8812bacefb45 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,8 +1,9 @@ ### 0.4.23 (unreleased) Features: - * SMTChecker: Integration with CVC4 SMT solver * Build system: Support Ubuntu Bionic. + * SMTChecker: Integration with CVC4 SMT solver + * Syntax Checker: Warn about functions named "constructor". Bugfixes: * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index f648e5b40beb..396058f44320 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -237,8 +237,13 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) if (v050) m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers."); else - m_errorReporter.warning( _function.location(), "Modifiers of functions without implementation are ignored." ); + m_errorReporter.warning(_function.location(), "Modifiers of functions without implementation are ignored." ); } + if (_function.name() == "constructor") + m_errorReporter.warning(_function.location(), + "This function is named \"constructor\" but is not the constructor of the contract. " + "If you intend this to be a constructor, use \"constructor(...) { ... }\" without the \"function\" keyword to define it." + ); return true; } diff --git a/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol b/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol new file mode 100644 index 000000000000..29784033e255 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol @@ -0,0 +1,5 @@ +contract C { + function constructor() public; +} +// ---- +// Warning: (17-47): This function is named "constructor" but is not the constructor of the contract. If you intend this to be a constructor, use "constructor(...) { ... }" without the "function" keyword to define it. From 17beac1e07fba46f14d458ee600502c1da723f4f Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 18 Apr 2018 20:40:08 +0200 Subject: [PATCH 16/23] Extract tests. --- .../libsolidity/SolidityNameAndTypeResolution.cpp | 15 --------------- .../memberLookup/memory_structs_with_mappings.sol | 10 ++++++++++ 2 files changed, 10 insertions(+), 15 deletions(-) create mode 100644 test/libsolidity/syntaxTests/memberLookup/memory_structs_with_mappings.sol diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 7c0e86438bb0..97cf0410b338 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2993,21 +2993,6 @@ BOOST_AUTO_TEST_CASE(literal_strings) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(memory_structs_with_mappings) -{ - char const* text = R"( - contract Test { - struct S { uint8 a; mapping(uint => uint) b; uint8 c; } - S s; - function f() public { - S memory x; - x.b[1]; - } - } - )"; - CHECK_ERROR(text, TypeError, "Member \"b\" is not available in struct Test.S memory outside of storage."); -} - BOOST_AUTO_TEST_CASE(string_bytes_conversion) { char const* text = R"( diff --git a/test/libsolidity/syntaxTests/memberLookup/memory_structs_with_mappings.sol b/test/libsolidity/syntaxTests/memberLookup/memory_structs_with_mappings.sol new file mode 100644 index 000000000000..bdafc754b0a1 --- /dev/null +++ b/test/libsolidity/syntaxTests/memberLookup/memory_structs_with_mappings.sol @@ -0,0 +1,10 @@ +contract Test { + struct S { uint8 a; mapping(uint => uint) b; uint8 c; } + S s; + function f() public { + S memory x; + x.b[1]; + } +} +// ---- +// TypeError: (118-121): Member "b" is not available in struct Test.S memory outside of storage. From a94945dfe40c879b6c3762620987a235582ccecf Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 18 Apr 2018 20:40:46 +0200 Subject: [PATCH 17/23] Improve error message for failed member lookup. --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 27 +++++++++++-------- .../memberLookup/failed_function_lookup.sol | 7 +++++ .../failed_function_lookup_in_library.sol | 9 +++++++ .../memberLookup/push_on_memory_types.sol | 8 ++++++ 5 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 test/libsolidity/syntaxTests/memberLookup/failed_function_lookup.sol create mode 100644 test/libsolidity/syntaxTests/memberLookup/failed_function_lookup_in_library.sol create mode 100644 test/libsolidity/syntaxTests/memberLookup/push_on_memory_types.sol diff --git a/Changelog.md b/Changelog.md index 8812bacefb45..3922c641de57 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ Features: * Syntax Checker: Warn about functions named "constructor". Bugfixes: + * Type Checker: Improve error message for failed function overload resolution. * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 87d69d97fac7..72d297621e70 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1904,7 +1904,8 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) // Retrieve the types of the arguments if this is used to call a function. auto const& argumentTypes = _memberAccess.annotation().argumentTypes; MemberList::MemberMap possibleMembers = exprType->members(m_scope).membersByName(memberName); - if (possibleMembers.size() > 1 && argumentTypes) + size_t const initialMemberCount = possibleMembers.size(); + if (initialMemberCount > 1 && argumentTypes) { // do overload resolution for (auto it = possibleMembers.begin(); it != possibleMembers.end();) @@ -1918,17 +1919,21 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) } if (possibleMembers.size() == 0) { - auto storageType = ReferenceType::copyForLocationIfReference( - DataLocation::Storage, - exprType - ); - if (!storageType->members(m_scope).membersByName(memberName).empty()) - m_errorReporter.fatalTypeError( - _memberAccess.location(), - "Member \"" + memberName + "\" is not available in " + - exprType->toString() + - " outside of storage." + if (initialMemberCount == 0) + { + // Try to see if the member was removed because it is only available for storage types. + auto storageType = ReferenceType::copyForLocationIfReference( + DataLocation::Storage, + exprType ); + if (!storageType->members(m_scope).membersByName(memberName).empty()) + m_errorReporter.fatalTypeError( + _memberAccess.location(), + "Member \"" + memberName + "\" is not available in " + + exprType->toString() + + " outside of storage." + ); + } m_errorReporter.fatalTypeError( _memberAccess.location(), "Member \"" + memberName + "\" not found or not visible " diff --git a/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup.sol b/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup.sol new file mode 100644 index 000000000000..c23992e999fa --- /dev/null +++ b/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup.sol @@ -0,0 +1,7 @@ +contract C { + function f(uint, uint) {} + function f(uint) {} + function g() { f(1, 2, 3); } +} +// ---- +// TypeError: (80-81): No matching declaration found after argument-dependent lookup. diff --git a/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup_in_library.sol b/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup_in_library.sol new file mode 100644 index 000000000000..310c4a103c35 --- /dev/null +++ b/test/libsolidity/syntaxTests/memberLookup/failed_function_lookup_in_library.sol @@ -0,0 +1,9 @@ +library L { + function f(uint, uint) {} + function f(uint) {} +} +contract C { + function g() { L.f(1, 2, 3); } +} +// ---- +// TypeError: (94-97): Member "f" not found or not visible after argument-dependent lookup in type(library L) diff --git a/test/libsolidity/syntaxTests/memberLookup/push_on_memory_types.sol b/test/libsolidity/syntaxTests/memberLookup/push_on_memory_types.sol new file mode 100644 index 000000000000..310c073f5c28 --- /dev/null +++ b/test/libsolidity/syntaxTests/memberLookup/push_on_memory_types.sol @@ -0,0 +1,8 @@ +contract Test { + function f() public pure { + uint[] memory x; + x.push(1); + } +} +// ---- +// TypeError: (77-83): Member "push" is not available in uint256[] memory outside of storage. From b53156b0395f85cc0c3280af4c8732b3d7241e32 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 18 Apr 2018 23:14:38 +0100 Subject: [PATCH 18/23] Remove -fpermissive --- cmake/EthCompilerSettings.cmake | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/cmake/EthCompilerSettings.cmake b/cmake/EthCompilerSettings.cmake index a9ed0a743fe3..d5f5789a61a0 100644 --- a/cmake/EthCompilerSettings.cmake +++ b/cmake/EthCompilerSettings.cmake @@ -43,27 +43,6 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA # TODO - Track down what breaks if we do NOT do this. add_compile_options(-Wno-unknown-pragmas) - # To get the code building on FreeBSD and Arch Linux we seem to need the following - # warning suppression to work around some issues in Boost headers. - # - # See the following reports: - # https://github.com/ethereum/webthree-umbrella/issues/384 - # https://github.com/ethereum/webthree-helpers/pull/170 - # - # The issue manifest as warnings-as-errors like the following: - # - # /usr/local/include/boost/multiprecision/cpp_int.hpp:181:4: error: - # right operand of shift expression '(1u << 63u)' is >= than the precision of the left operand - # - # -fpermissive is a pretty nasty way to address this. It is described as follows: - # - # Downgrade some diagnostics about nonconformant code from errors to warnings. - # Thus, using -fpermissive will allow some nonconforming code to compile. - # - # NB: Have to use this form for the setting, so that it only applies to C++ builds. - # Applying -fpermissive to a C command-line (ie. secp256k1) gives a build error. - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive") - # Configuration-specific compiler settings. set(CMAKE_CXX_FLAGS_DEBUG "-O0 -g -DETH_DEBUG") set(CMAKE_CXX_FLAGS_MINSIZEREL "-Os -DNDEBUG") From bff741b42f9c5774b3105ff6707e1fd447ec0478 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 18 Apr 2018 23:15:16 +0100 Subject: [PATCH 19/23] Remove obsolete warning supressions for clang --- cmake/EthCompilerSettings.cmake | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cmake/EthCompilerSettings.cmake b/cmake/EthCompilerSettings.cmake index d5f5789a61a0..c73536ada356 100644 --- a/cmake/EthCompilerSettings.cmake +++ b/cmake/EthCompilerSettings.cmake @@ -61,18 +61,6 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA # Additional Clang-specific compiler settings. elseif ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang") - - # A couple of extra warnings suppressions which we seemingly - # need when building with Clang. - # - # TODO - Nail down exactly where these warnings are manifesting and - # try to suppress them in a more localized way. Notes in this file - # indicate that the first is needed for sepc256k1 and that the - # second is needed for the (clog, cwarn) macros. These will need - # testing on at least OS X and Ubuntu. - add_compile_options(-Wno-unused-function) - add_compile_options(-Wno-dangling-else) - if ("${CMAKE_SYSTEM_NAME}" MATCHES "Darwin") # Set stack size to 16MB - by default Apple's clang defines a stack size of 8MB, some tests require more. set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-stack_size -Wl,0x1000000") From ae834e3dbe912dc0780ad2bebfc633d6d825c963 Mon Sep 17 00:00:00 2001 From: Thomas Sauvajon Date: Thu, 19 Apr 2018 10:32:48 +0200 Subject: [PATCH 20/23] Correct the style of coding style --- CODING_STYLE.md | 345 +++++++++++++++++++++++------------------------- 1 file changed, 163 insertions(+), 182 deletions(-) diff --git a/CODING_STYLE.md b/CODING_STYLE.md index 2cc9ac70993c..3244466b44d5 100644 --- a/CODING_STYLE.md +++ b/CODING_STYLE.md @@ -1,35 +1,24 @@ -0. Formatting - -GOLDEN RULE: Follow the style of the existing code when you make changes. - -a. Use tabs for leading indentation -- tab stops are every 4 characters (only relevant for line length). -- One indentation level -> exactly one byte (i.e. a tab character) in the source file. -b. Line widths: -- Lines should be at most 99 characters wide to make diff views readable and reduce merge conflicts. -- Lines of comments should be formatted according to ease of viewing, but simplicity is to be preferred over beauty. -c. Single-statement blocks should not have braces, unless required for clarity. -d. Never place condition bodies on same line as condition. -e. Space between keyword and opening parenthesis, but not following opening parenthesis or before final parenthesis. -f. No spaces for unary operators, `->` or `.`. -g. No space before ':' but one after it, except in the ternary operator: one on both sides. -h. Add spaces around all other operators. -i. Braces, when used, always have their own lines and are at same indentation level as "parent" scope. -j. If lines are broken, a list of elements enclosed with parentheses (of any kind) and separated by a - separator (of any kind) are formatted such that there is exactly one element per line, followed by - the separator, the opening parenthesis is on the first line, followed by a line break and the closing - parenthesis is on a line of its own (unindented). See example below. - -(WRONG) -if( a==b[ i ] ) { printf ("Hello\n"); } -foo->bar(someLongVariableName, - anotherLongVariableName, - anotherLongVariableName, - anotherLongVariableName, - anotherLongVariableName); -cout << "some very long string that contains completely irrelevant text that talks about this and that and contains the words \"lorem\" and \"ipsum\"" << endl; - -(RIGHT) +## 0. Formatting + +**GOLDEN RULE**: Follow the style of the existing code when you make changes. + +1. Use tabs for leading indentation: + - tab stops are every 4 characters (only relevant for line length). + - one indentation level -> exactly one byte (i.e. a tab character) in the source file. +2. Line widths: + - Lines should be at most 99 characters wide to make diff views readable and reduce merge conflicts. + - Lines of comments should be formatted according to ease of viewing, but simplicity is to be preferred over beauty. +3. Single-statement blocks should not have braces, unless required for clarity. +4. Never place condition bodies on same line as condition. +5. Space between keyword and opening parenthesis, but not following opening parenthesis or before final parenthesis. +6. No spaces for unary operators, `->` or `.`. +7. No space before `:` but one after it, except in the ternary operator: one on both sides. +8. Add spaces around all other operators. +9. Braces, when used, always have their own lines and are at same indentation level as "parent" scope. +10. If lines are broken, a list of elements enclosed with parentheses (of any kind) and separated by a separator (of any kind) are formatted such that there is exactly one element per line, followed by the separator, the opening parenthesis is on the first line, followed by a line break and the closing parenthesis is on a line of its own unindented). See example below. + +Yes: +```cpp if (a == b[i]) printf("Hello\n"); // NOTE spaces used instead of tab here for clarity - first byte should be '\t'. foo->bar( @@ -44,99 +33,92 @@ cout << "text that talks about this and that and contains the words " << "\"lorem\" and \"ipsum\"" << endl; +``` +No: +```cpp +if( a==b[ i ] ) { printf ("Hello\n"); } +foo->bar(someLongVariableName, + anotherLongVariableName, + anotherLongVariableName, + anotherLongVariableName, + anotherLongVariableName); +cout << "some very long string that contains completely irrelevant text that talks about this and that and contains the words \"lorem\" and \"ipsum\"" << endl; +``` +## 1. Namespaces -1. Namespaces; +1. No `using namespace` declarations in header files. +2. All symbols should be declared in a namespace except for final applications. +3. Use anonymous namespaces for helpers whose scope is a cpp file only. +4. Preprocessor symbols should be prefixed with the namespace in all-caps and an underscore. -a. No "using namespace" declarations in header files. -b. All symbols should be declared in a namespace except for final applications. -c. Use anonymous namespaces for helpers whose scope is a cpp file only. -d. Preprocessor symbols should be prefixed with the namespace in all-caps and an underscore. +Yes: +```cpp +#include +std::tuple meanAndSigma(std::vector const& _v); +``` -(WRONG) +No: +```cpp #include using namespace std; tuple meanAndSigma(vector const& _v); +``` -(CORRECT) -#include -std::tuple meanAndSigma(std::vector const& _v); - - - -2. Preprocessor; - -a. File comment is always at top, and includes: -- Copyright. -- License (e.g. see COPYING). -b. Never use #ifdef/#define/#endif file guards. Prefer #pragma once as first line below file comment. -c. Prefer static const variable to value macros. -d. Prefer inline constexpr functions to function macros. -e. Split complex macro on multiple lines with '\'. - - +## 2. Preprocessor -3. Capitalization; +1. File comment is always at top, and includes: + - Copyright + - License (e.g. see COPYING) +2. Never use `#ifdef`/`#define`/`#endif` file guards. Prefer `#pragma` once as first line below file comment. +3. Prefer static const variable to value macros. +4. Prefer inline constexpr functions to function macros. +5. Split complex macro on multiple lines with `\`. -GOLDEN RULE: Preprocessor: ALL_CAPS; C++: camelCase. +## 3. Capitalization -a. Use camelCase for splitting words in names, except where obviously extending STL/boost functionality in which case follow those naming conventions. -b. The following entities' first alpha is upper case: -- Type names. -- Template parameters. -- Enum members. -- static const variables that form an external API. -c. All preprocessor symbols (macros, macro arguments) in full uppercase with underscore word separation. +**GOLDEN RULE**: Preprocessor: `ALL_CAPS`; C++: `camelCase`. +1. Use camelCase for splitting words in names, except where obviously extending STL/boost functionality in which case follow those naming conventions. +2. The following entities' first alpha is upper case: + - Type names + - Template parameters + - Enum members + - static const variables that form an external API. +3. All preprocessor symbols (macros, macro arguments) in full uppercase with underscore word separation. All other entities' first alpha is lower case. +## 4. Variable prefixes +1. Leading underscore "_" to parameter names: + - Exception: "o_parameterName" when it is used exclusively for output. See 6(f). + - Exception: "io_parameterName" when it is used for both input and output. See 6(f). +2. Leading "g_" to global (non-const) variables. +3. Leading "s_" to static (non-const, non-global) variables. -4. Variable prefixes: - -a. Leading underscore "_" to parameter names. -- Exception: "o_parameterName" when it is used exclusively for output. See 6(f). -- Exception: "io_parameterName" when it is used for both input and output. See 6(f). -b. Leading "g_" to global (non-const) variables. -c. Leading "s_" to static (non-const, non-global) variables. - - - -5. Assertions: - -- use `solAssert` and `solUnimplementedAssert` generously to check assumptions - that span across different parts of the code base, for example before dereferencing - a pointer. - - -6. Declarations: - -a. {Typename} + {qualifiers} + {name}. -b. Only one per line. -c. Associate */& with type, not variable (at ends with parser, but more readable, and safe if in conjunction with (b)). -d. Favour declarations close to use; don't habitually declare at top of scope ala C. -e. Pass non-trivial parameters as const reference, unless the data is to be copied into the function, then either pass by const reference or by value and use std::move. -f. If a function returns multiple values, use std::tuple (std::pair acceptable) or better introduce a struct type. Do not use */& arguments. -g. Use parameters of pointer type only if ``nullptr`` is a valid argument, use references otherwise. Often, ``boost::optional`` is better suited than a raw pointer. -h. Never use a macro where adequate non-preprocessor C++ can be written. -i. Only use ``auto`` if the type is very long and rather irrelevant. -j. Do not pass bools: prefer enumerations instead. -k. Prefer enum class to straight enum. -l. Always initialize POD variables, even if their value is overwritten later. +## 5. Assertions +Use `solAssert` and `solUnimplementedAssert` generously to check assumptions that span across different parts of the code base, for example before dereferencing a pointer. -(WRONG) -const double d = 0; -int i, j; -char *s; -float meanAndSigma(std::vector _v, float* _sigma, bool _approximate); -Derived* x(dynamic_cast(base)); -for (map::iterator i = l.begin(); i != l.end(); ++l) {} +## 6. Declarations +1. {Typename} + {qualifiers} + {name}. +2. Only one per line. +3. Associate */& with type, not variable (at ends with parser, but more readable, and safe if in conjunction with (b)). +4. Favour declarations close to use; don't habitually declare at top of scope ala C. +5. Pass non-trivial parameters as const reference, unless the data is to be copied into the function, then either pass by const reference or by value and use std::move. +6. If a function returns multiple values, use std::tuple (std::pair acceptable) or better introduce a struct type. Do not use */& arguments. +7. Use parameters of pointer type only if ``nullptr`` is a valid argument, use references otherwise. Often, ``boost::optional`` is better suited than a raw pointer. +8. Never use a macro where adequate non-preprocessor C++ can be written. +9. Only use ``auto`` if the type is very long and rather irrelevant. +10. Do not pass bools: prefer enumerations instead. +11. Prefer enum class to straight enum. +12. Always initialize POD variables, even if their value is overwritten later. -(CORRECT) +Yes: +```cpp enum class Accuracy { Approximate, @@ -154,78 +136,78 @@ char* s; MeanAndSigma ms meanAndSigma(std::vector const& _v, Accuracy _a); Derived* x = dynamic_cast(base); for (auto i = x->begin(); i != x->end(); ++i) {} +``` +No: +```cp +const double d = 0; +int i, j; +char *s; +float meanAndSigma(std::vector _v, float* _sigma, bool _approximate); +Derived* x(dynamic_cast(base)); +for (map::iterator i = l.begin(); i != l.end(); ++l) {} +``` -7. Structs & classes - -a. Structs to be used when all members public and no virtual functions. -- In this case, members should be named naturally and not prefixed with 'm_' -b. Classes to be used in all other circumstances. - - - -8. Members: - -a. One member per line only. -b. Private, non-static, non-const fields prefixed with m_. -c. Avoid public fields, except in structs. -d. Use override, final and const as much as possible. -e. No implementations with the class declaration, except: -- template or force-inline method (though prefer implementation at bottom of header file). -- one-line implementation (in which case include it in same line as declaration). -f. For a property 'foo' -- Member: m_foo; -- Getter: foo() [ also: for booleans, isFoo() ]; -- Setter: setFoo(); - - - -9. Naming - -a. Avoid unpronouncable names -b. Names should be shortened only if they are extremely common, but shortening should be generally avoided -c. Avoid prefixes of initials (e.g. do not use IMyInterface, CMyImplementation) -c. Find short, memorable & (at least semi-) descriptive names for commonly used classes or name-fragments. -- A dictionary and thesaurus are your friends. -- Spell correctly. -- Think carefully about the class's purpose. -- Imagine it as an isolated component to try to decontextualise it when considering its name. -- Don't be trapped into naming it (purely) in terms of its implementation. - - - -10. Type-definitions - -a. Prefer 'using' to 'typedef'. e.g. using ints = std::vector; rather than typedef std::vector ints; -b. Generally avoid shortening a standard form that already includes all important information: -- e.g. stick to shared_ptr rather than shortening to ptr. -c. Where there are exceptions to this (due to excessive use and clear meaning), note the change prominently and use it consistently. -- e.g. using Guard = std::lock_guard; ///< Guard is used throughout the codebase since it is clear in meaning and used commonly. -d. In general expressions should be roughly as important/semantically meaningful as the space they occupy. -e. Avoid introducing aliases for types unless they are very complicated. Consider the number of items a brain can keep track of at the same time. - - - -11. Commenting - -a. Comments should be doxygen-compilable, using @notation rather than \notation. -b. Document the interface, not the implementation. -- Documentation should be able to remain completely unchanged, even if the method is reimplemented. -- Comment in terms of the method properties and intended alteration to class state (or what aspects of the state it reports). -- Be careful to scrutinise documentation that extends only to intended purpose and usage. -- Reject documentation that is simply an English transaction of the implementation. -c. Avoid in-code comments. Instead, try to extract blocks of functionality into functions. This often already eliminates the need for an in-code comment. - - -12. Include Headers - -Includes should go in increasing order of generality (libsolidity -> libevmasm -> libdevcore -> boost -> STL). -The corresponding .h file should be the first include in the respective .cpp file. -Insert empty lines between blocks of include files. +## 7. Structs & classes + +1. Structs to be used when all members public and no virtual functions: + - In this case, members should be named naturally and not prefixed with `m_`. +2. Classes to be used in all other circumstances. + +## 8. Members + +1. One member per line only. +2. Private, non-static, non-const fields prefixed with `m_`. +3. Avoid public fields, except in structs. +4. Use override, final and const as much as possible. +5. No implementations with the class declaration, except: + - template or force-inline method (though prefer implementation at bottom of header file). + - one-line implementation (in which case include it in same line as declaration). +6. For a property `foo` + - Member: `m_foo` + - Getter: `foo()` [ also: for booleans, `isFoo()` ] + - Setter: `setFoo()` + +## 9. Naming + +1. Avoid unpronouncable names. +2. Names should be shortened only if they are extremely common, but shortening should be generally avoided +3. Avoid prefixes of initials (e.g. do not use `IMyInterface`, `CMyImplementation`) +4. Find short, memorable & (at least semi-) descriptive names for commonly used classes or name-fragments: + - A dictionary and thesaurus are your friends; + - Spell correctly; + - Think carefully about the class's purpose; + - Imagine it as an isolated component to try to decontextualise it when considering its name; + - Don't be trapped into naming it (purely) in terms of its implementation. + +## 10. Type definitions + +1. Prefer `using` to `typedef`. e.g. `using ints = std::vector;` rather than typedef `std::vector ints;` +2. Generally avoid shortening a standard form that already includes all important information: + - e.g. stick to `shared_ptr` rather than shortening to `ptr`. +3. Where there are exceptions to this (due to excessive use and clear meaning), note the change prominently and use it consistently: + - e.g. `using Guard = std::lock_guard;` ///< Guard is used throughout the codebase since it is clear in meaning and used commonly. +4. In general expressions should be roughly as important/semantically meaningful as the space they occupy. +5. Avoid introducing aliases for types unless they are very complicated. Consider the number of items a brain can keep track of at the same time. + +## 11. Commenting + +1. Comments should be doxygen-compilable, using @notation rather than \notation. +2. Document the interface, not the implementation: + - Documentation should be able to remain completely unchanged, even if the method is reimplemented; + - Comment in terms of the method properties and intended alteration to class state (or what aspects of the state it reports); + - Be careful to scrutinise documentation that extends only to intended purpose and usage; + - Reject documentation that is simply an English transaction of the implementation. +3. Avoid in-code comments. Instead, try to extract blocks of functionality into functions. This often already eliminates the need for an in-code comment. + +## 12. Include Headers + +1. Includes should go in increasing order of generality (`libsolidity` -> `libevmasm` -> `libdevcore` -> `boost` -> `STL`). +2. The corresponding `.h` file should be the first include in the respective `.cpp` file. +3. Insert empty lines between blocks of include files. Example: - -``` +```cpp #include #include @@ -245,18 +227,17 @@ Example: #include ``` -See http://stackoverflow.com/questions/614302/c-header-order/614333#614333 for the reason: this makes it easier to find missing includes in header files. - +See [this issue](http://stackoverflow.com/questions/614302/c-header-order/614333#614333 "C header order") for the reason: this makes it easier to find missing includes in header files. -13. Recommended reading +## 13. Recommended reading -Herb Sutter and Bjarne Stroustrup -- "C++ Core Guidelines" (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) +- Herb Sutter and Bjarne Stroustrup: + - [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) -Herb Sutter and Andrei Alexandrescu -- "C++ Coding Standards: 101 Rules, Guidelines, and Best Practices" +- Herb Sutter and Andrei Alexandrescu: + - "C++ Coding Standards: 101 Rules, Guidelines, and Best Practices" -Scott Meyers -- "Effective C++: 55 Specific Ways to Improve Your Programs and Designs (3rd Edition)" -- "More Effective C++: 35 New Ways to Improve Your Programs and Designs" -- "Effective Modern C++: 42 Specific Ways to Improve Your Use of C++11 and C++14" +- Scott Meyers: + - "Effective C++: 55 Specific Ways to Improve Your Programs and Designs (3rd Edition)" + - "More Effective C++: 35 New Ways to Improve Your Programs and Designs" + - "Effective Modern C++: 42 Specific Ways to Improve Your Use of C++11 and C++14" From 754d79edfabd8a199a51411a865d75274fcb4169 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 18 Apr 2018 23:47:56 +0100 Subject: [PATCH 21/23] Disallow explicit conversion of bytesXX to contract --- Changelog.md | 1 + libsolidity/ast/Types.cpp | 1 - test/libsolidity/syntaxTests/types/bytes_to_contract.sol | 7 +++++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/types/bytes_to_contract.sol diff --git a/Changelog.md b/Changelog.md index 8812bacefb45..d6d5401f59ee 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Features: Bugfixes: * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax. + * Type Checker: Explicit conversion of ``bytesXX`` to ``contract`` is properly disallowed. ### 0.4.22 (2018-04-16) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 51739cb01947..425e5045b759 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1299,7 +1299,6 @@ bool FixedBytesType::isExplicitlyConvertibleTo(Type const& _convertTo) const { return _convertTo.category() == Category::Integer || _convertTo.category() == Category::FixedPoint || - _convertTo.category() == Category::Contract || _convertTo.category() == category(); } diff --git a/test/libsolidity/syntaxTests/types/bytes_to_contract.sol b/test/libsolidity/syntaxTests/types/bytes_to_contract.sol new file mode 100644 index 000000000000..2a3219ec57b7 --- /dev/null +++ b/test/libsolidity/syntaxTests/types/bytes_to_contract.sol @@ -0,0 +1,7 @@ +contract C { + function f() public pure { + C(bytes20(0x1234)); + } +} +// ---- +// TypeError: (64-82): Explicit type conversion not allowed from "bytes20" to "contract C". From a79c9a1dfebf08c3ab5bd6a56f72f3e84ba1ecef Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 19 Apr 2018 16:10:57 +0200 Subject: [PATCH 22/23] Prepare 0.4.23 release. --- Changelog.md | 2 +- docs/bugs.json | 8 ++++++++ docs/bugs_by_version.json | 8 +++++++- scripts/update_bugs_by_version.py | 8 ++++---- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Changelog.md b/Changelog.md index c5bad5aaddac..4cfa4385bae5 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,4 +1,4 @@ -### 0.4.23 (unreleased) +### 0.4.23 (2018-04-19) Features: * Build system: Support Ubuntu Bionic. diff --git a/docs/bugs.json b/docs/bugs.json index c642793a8c68..b464be18acac 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,12 @@ [ + { + "name": "OneOfTwoConstructorsSkipped", + "summary": "If a contract has both a new-style constructor (using the constructor keyword) and an old-style constructor (a function with the same name as the contract) at the same time, one of them will be ignored.", + "description": "If a contract has both a new-style constructor (using the constructor keyword) and an old-style constructor (a function with the same name as the contract) at the same time, one of them will be ignored. There will be a compiler warning about the old-style constructor, so contracts only using new-style constructors are fine.", + "introduced": "0.4.22", + "fixed": "0.4.23", + "severity": "very low" + }, { "name": "ZeroFunctionSelector", "summary": "It is possible to craft the name of a function such that it is executed instead of the fallback function in very specific circumstances.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 32f305c854e4..d96bfde34616 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -423,9 +423,15 @@ "released": "2018-03-07" }, "0.4.22": { - "bugs": [], + "bugs": [ + "OneOfTwoConstructorsSkipped" + ], "released": "2018-04-16" }, + "0.4.23": { + "bugs": [], + "released": "2018-04-19" + }, "0.4.3": { "bugs": [ "ZeroFunctionSelector", diff --git a/scripts/update_bugs_by_version.py b/scripts/update_bugs_by_version.py index c4bc0c9bfe6b..cbedf1a55399 100755 --- a/scripts/update_bugs_by_version.py +++ b/scripts/update_bugs_by_version.py @@ -35,9 +35,9 @@ def comp(version_string): continue versions[v]['bugs'] += [bug['name']] -with open(path + '/../docs/bugs_by_version.json', 'r+') as bugs_by_version: +new_contents = json.dumps(versions, sort_keys=True, indent=4) +with open(path + '/../docs/bugs_by_version.json', 'r') as bugs_by_version: old_contents = bugs_by_version.read() - new_contents = json.dumps(versions, sort_keys=True, indent=4) - bugs_by_version.seek(0) +with open(path + '/../docs/bugs_by_version.json', 'w') as bugs_by_version: bugs_by_version.write(new_contents) - sys.exit(old_contents != new_contents) \ No newline at end of file +sys.exit(old_contents != new_contents) From fb3f579ae21821acf06873767dc4bdbf64bab6d0 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 19 Apr 2018 18:31:58 +0200 Subject: [PATCH 23/23] Disable tests on travis again. --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e9d2dde6b4a4..3640d2e239be 100644 --- a/.travis.yml +++ b/.travis.yml @@ -160,7 +160,8 @@ install: - test $SOLC_INSTALL_DEPS_TRAVIS != On || (scripts/install_deps.sh) - test "$TRAVIS_OS_NAME" != "linux" || (scripts/install_cmake.sh) # Disable tests unless run on the release branch, on tags or with daily cron - - if [ "$TRAVIS_BRANCH" != release -a -z "$TRAVIS_TAG" -a "$TRAVIS_EVENT_TYPE" != cron ]; then SOLC_TESTS=Off; fi + #- if [ "$TRAVIS_BRANCH" != release -a -z "$TRAVIS_TAG" -a "$TRAVIS_EVENT_TYPE" != cron ]; then SOLC_TESTS=Off; fi + - SOLC_TESTS=Off - if [ "$TRAVIS_BRANCH" = release -o -n "$TRAVIS_TAG" ]; then echo -n > prerelease.txt; else date -u +"nightly.%Y.%-m.%-d" > prerelease.txt; fi - echo -n "$TRAVIS_COMMIT" > commit_hash.txt