From cc6314cd019be80a9212f6bd5b0c86206be16c88 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 21 Jun 2016 14:36:23 +0200 Subject: [PATCH 1/2] Warn about unused return values. --- libsolidity/analysis/TypeChecker.cpp | 17 +++++++++++++ libsolidity/analysis/TypeChecker.h | 3 +++ .../SolidityNameAndTypeResolution.cpp | 25 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ce55de000..c2fd3e7ee 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -830,6 +830,13 @@ void TypeChecker::endVisit(ExpressionStatement const& _statement) if (type(_statement.expression())->category() == Type::Category::RationalNumber) if (!dynamic_cast(*type(_statement.expression())).mobileType()) typeError(_statement.expression().location(), "Invalid rational number."); + + bool unusedReturnValue = true; + if (auto t = dynamic_cast(type(_statement.expression()).get())) + if (t->components().empty()) + unusedReturnValue = false; + if (unusedReturnValue) + warning(_statement.location(), "Unused return value."); } bool TypeChecker::visit(Conditional const& _conditional) @@ -1570,6 +1577,16 @@ void TypeChecker::typeError(SourceLocation const& _location, string const& _desc m_errors.push_back(err); } +void TypeChecker::warning(SourceLocation const& _location, string const& _description) +{ + auto err = make_shared(Error::Type::Warning); + *err << + errinfo_sourceLocation(_location) << + errinfo_comment(_description); + + m_errors.push_back(err); +} + void TypeChecker::fatalTypeError(SourceLocation const& _location, string const& _description) { typeError(_location, _description); diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 48f8285a3..be1e02be5 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -59,6 +59,9 @@ private: /// Adds a new error to the list of errors. void typeError(SourceLocation const& _location, std::string const& _description); + /// Adds a new warning to the list of errors. + void warning(SourceLocation const& _location, std::string const& _description); + /// Adds a new error to the list of errors and throws to abort type checking. void fatalTypeError(SourceLocation const& _location, std::string const& _description); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 697b3fa98..1f42c23ed 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3750,6 +3750,31 @@ BOOST_AUTO_TEST_CASE(one_divided_by_three_integer_conversion) BOOST_CHECK(!success(text)); } +BOOST_AUTO_TEST_CASE(unused_return_value) +{ + char const* text = R"( + contract test { + function g() returns (uint) {} + function f() { + g(); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + +BOOST_AUTO_TEST_CASE(unused_return_value_send) +{ + char const* text = R"( + contract test { + function f() { + address(0x12).send(1); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + BOOST_AUTO_TEST_SUITE_END() } From 25a64c7f8f35833b9dec6068232be6e0ce9d2e78 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 24 Jun 2016 16:41:17 +0200 Subject: [PATCH 2/2] Only warn about unused return in low-level functions. --- libsolidity/analysis/TypeChecker.cpp | 21 +++++--- .../SolidityNameAndTypeResolution.cpp | 50 ++++++++++++++++++- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c2fd3e7ee..6b2c1cb8e 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -831,12 +831,21 @@ void TypeChecker::endVisit(ExpressionStatement const& _statement) if (!dynamic_cast(*type(_statement.expression())).mobileType()) typeError(_statement.expression().location(), "Invalid rational number."); - bool unusedReturnValue = true; - if (auto t = dynamic_cast(type(_statement.expression()).get())) - if (t->components().empty()) - unusedReturnValue = false; - if (unusedReturnValue) - warning(_statement.location(), "Unused return value."); + if (auto call = dynamic_cast(&_statement.expression())) + { + if (auto callType = dynamic_cast(type(call->expression()).get())) + { + using Location = FunctionType::Location; + Location location = callType->location(); + if ( + location == Location::Bare || + location == Location::BareCallCode || + location == Location::BareDelegateCall || + location == Location::Send + ) + warning(_statement.location(), "Return value of low-level calls not used."); + } + } } bool TypeChecker::visit(Conditional const& _conditional) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1f42c23ed..7e81bd7ec 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3760,7 +3760,7 @@ BOOST_AUTO_TEST_CASE(unused_return_value) } } )"; - BOOST_CHECK(expectError(text, true) == Error::Type::Warning); + BOOST_CHECK(success(text)); } BOOST_AUTO_TEST_CASE(unused_return_value_send) @@ -3775,6 +3775,54 @@ BOOST_AUTO_TEST_CASE(unused_return_value_send) BOOST_CHECK(expectError(text, true) == Error::Type::Warning); } +BOOST_AUTO_TEST_CASE(unused_return_value_call) +{ + char const* text = R"( + contract test { + function f() { + address(0x12).call("abc"); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + +BOOST_AUTO_TEST_CASE(unused_return_value_call_value) +{ + char const* text = R"( + contract test { + function f() { + address(0x12).call.value(2)("abc"); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + +BOOST_AUTO_TEST_CASE(unused_return_value_callcode) +{ + char const* text = R"( + contract test { + function f() { + address(0x12).callcode("abc"); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + +BOOST_AUTO_TEST_CASE(unused_return_value_delegatecall) +{ + char const* text = R"( + contract test { + function f() { + address(0x12).delegatecall("abc"); + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + BOOST_AUTO_TEST_SUITE_END() }