From 9fc6eccc26eea6f5a70f2960df9d53d49392151e Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 27 Jul 2017 20:55:55 +0100 Subject: [PATCH 1/3] Add isFallback() helper --- libsolidity/analysis/TypeChecker.cpp | 4 ++-- libsolidity/ast/AST.cpp | 2 +- libsolidity/ast/AST.h | 5 +++-- libsolidity/interface/CompilerStack.cpp | 4 +++- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 1ee827d4c..5419db2d8 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -93,7 +93,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract) FunctionDefinition const* fallbackFunction = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) { - if (function->name().empty()) + if (function->isFallback()) { if (fallbackFunction) { @@ -482,7 +482,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function) { if (isLibraryFunction) m_errorReporter.typeError(_function.location(), "Library functions cannot be payable."); - if (!_function.isConstructor() && !_function.name().empty() && !_function.isPartOfExternalInterface()) + if (!_function.isConstructor() && !_function.isFallback() && !_function.isPartOfExternalInterface()) m_errorReporter.typeError(_function.location(), "Internal functions cannot be payable."); if (_function.isDeclaredConst()) m_errorReporter.typeError(_function.location(), "Functions cannot be constant and payable at the same time."); diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index ebc8bd480..1d68231ea 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -162,7 +162,7 @@ FunctionDefinition const* ContractDefinition::fallbackFunction() const { for (ContractDefinition const* contract: annotation().linearizedBaseContracts) for (FunctionDefinition const* f: contract->definedFunctions()) - if (f->name().empty()) + if (f->isFallback()) return f; return nullptr; } diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index e4656f72c..3e97286bc 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -589,6 +589,7 @@ public: virtual void accept(ASTConstVisitor& _visitor) const override; bool isConstructor() const { return m_isConstructor; } + bool isFallback() const { return name().empty(); } bool isDeclaredConst() const { return m_isDeclaredConst; } bool isPayable() const { return m_isPayable; } std::vector> const& modifiers() const { return m_functionModifiers; } @@ -596,9 +597,9 @@ public: Block const& body() const { solAssert(m_body, ""); return *m_body; } virtual bool isVisibleInContract() const override { - return Declaration::isVisibleInContract() && !isConstructor() && !name().empty(); + return Declaration::isVisibleInContract() && !isConstructor() && !isFallback(); } - virtual bool isPartOfExternalInterface() const override { return isPublic() && !m_isConstructor && !name().empty(); } + virtual bool isPartOfExternalInterface() const override { return isPublic() && !isConstructor() && !isFallback(); } /// @returns the external signature of the function /// That consists of the name of the function followed by the types of the diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 02983a825..9689b700c 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -935,7 +935,7 @@ Json::Value CompilerStack::gasEstimates(string const& _contractName) const for (auto const& it: contract.definedFunctions()) { /// Exclude externally visible functions, constructor and the fallback function - if (it->isPartOfExternalInterface() || it->isConstructor() || it->name().empty()) + if (it->isPartOfExternalInterface() || it->isConstructor() || it->isFallback()) continue; size_t entry = functionEntryPoint(_contractName, *it); @@ -943,12 +943,14 @@ Json::Value CompilerStack::gasEstimates(string const& _contractName) const if (entry > 0) gas = GasEstimator::functionalEstimation(*items, entry, *it); + /// TODO: This could move into a method shared with externalSignature() FunctionType type(*it); string sig = it->name() + "("; auto paramTypes = type.parameterTypes(); for (auto it = paramTypes.begin(); it != paramTypes.end(); ++it) sig += (*it)->toString() + (it + 1 == paramTypes.end() ? "" : ","); sig += ")"; + internalFunctions[sig] = gasToJson(gas); } From d4e44ecb46df58ff21690c28098d20e3c5dfe307 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 27 Jul 2017 20:56:29 +0100 Subject: [PATCH 2/3] Disallow externalSignature for fallback functions --- libsolidity/ast/Types.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 84e4a0777..3f8da5018 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2524,6 +2524,7 @@ bool FunctionType::isBareCall() const string FunctionType::externalSignature() const { solAssert(m_declaration != nullptr, "External signature of function needs declaration"); + solAssert(!m_declaration->name().empty(), "Fallback function has no signature."); bool _inLibrary = dynamic_cast(*m_declaration->scope()).isLibrary(); From 7d37eba4ba5e11a59bd201fde91bef7510510b0f Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Thu, 29 Jun 2017 09:48:53 +0100 Subject: [PATCH 3/3] Remove the need of jumping out of the fallback --- Changelog.md | 1 + libsolidity/codegen/ContractCompiler.cpp | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Changelog.md b/Changelog.md index 18fd00eaa..cedb58591 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Features: * C API (``jsonCompiler``): Export the ``license`` method. + * Code Generator: Optimise the fallback function, by removing a useless jump. * Inline Assembly: Show useful error message if trying to access calldata variables. * Inline Assembly: Support variable declaration without initial value (defaults to 0). * Metadata: Only include files which were used to compile the given contract. diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index cad388dfd..fd0998d49 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -267,18 +267,13 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac m_context << notFound; if (fallback) { - m_context.setStackOffset(0); if (!fallback->isPayable()) appendCallValueCheck(); - // Return tag is used to jump out of the function. - eth::AssemblyItem returnTag = m_context.pushNewTag(); - fallback->accept(*this); - m_context << returnTag; + solAssert(fallback->isFallback(), ""); solAssert(FunctionType(*fallback).parameterTypes().empty(), ""); solAssert(FunctionType(*fallback).returnParameterTypes().empty(), ""); - // Return tag gets consumed. - m_context.adjustStackOffset(-1); + fallback->accept(*this); m_context << Instruction::STOP; } else @@ -536,7 +531,8 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) m_context.adjustStackOffset(-(int)c_returnValuesSize); - if (!_function.isConstructor()) + /// The constructor and the fallback function doesn't to jump out. + if (!_function.isConstructor() && !_function.isFallback()) m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction); return false; }