Skip to content

Cache functions by name. #11470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2021
+26 −3
Merged

Cache functions by name. #11470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions libsolidity/ast/AST.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,17 @@ FunctionDefinition const* ContractDefinition::nextConstructor(ContractDefinition
return nullptr;
}

multimap<std::string, FunctionDefinition const*> const& ContractDefinition::definedFunctionsByName() const
{
return m_definedFunctionsByName.init([&]{
std::multimap<std::string, FunctionDefinition const*> result;
for (FunctionDefinition const* fun: filteredNodes<FunctionDefinition>(m_subNodes))
result.insert({fun->name(), fun});
return result;
});
}


TypeNameAnnotation& TypeName::annotation() const
{
return initAnnotation<TypeNameAnnotation>();
Expand Down Expand Up @@ -397,6 +408,7 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual(
) const
{
solAssert(!isConstructor(), "");
solAssert(!name().empty(), "");

// If we are not doing super-lookup and the function is not virtual, we can stop here.
if (_searchStart == nullptr && !virtualSemantics())
Expand All @@ -416,10 +428,8 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual(
else
foundSearchStart = true;

for (FunctionDefinition const* function: c->definedFunctions())
for (FunctionDefinition const* function: c->definedFunctions(name()))
if (
function->name() == name() &&
!function->isConstructor() &&
// With super lookup analysis guarantees that there is an implemented function in the chain.
// With virtual lookup there are valid cases where returning an unimplemented one is fine.
(function->isImplemented() || _searchStart == nullptr) &&
Expand Down
13 changes: 13 additions & 0 deletions libsolidity/ast/AST.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@

#include <json/json.h>

#include <range/v3/view/subrange.hpp>
#include <range/v3/view/map.hpp>

#include <memory>
#include <optional>
#include <string>
Expand Down Expand Up @@ -497,6 +500,13 @@ class ContractDefinition: public Declaration, public StructurallyDocumented, pub
std::vector<VariableDeclaration const*> stateVariables() const { return filteredNodes<VariableDeclaration>(m_subNodes); }
std::vector<ModifierDefinition const*> functionModifiers() const { return filteredNodes<ModifierDefinition>(m_subNodes); }
std::vector<FunctionDefinition const*> definedFunctions() const { return filteredNodes<FunctionDefinition>(m_subNodes); }
/// @returns a view<FunctionDefinition const*> of all functions
/// defined in this contract of the given name (excluding inherited functions).
auto definedFunctions(std::string const& _name) const
{
auto&& [b, e] = definedFunctionsByName().equal_range(_name);
return ranges::subrange<decltype(b)>(b, e) | ranges::views::values;
}
Copy link

@leonardoalt leonardoalt Jun 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekpyron is that you

Copy link
Collaborator

@ekpyron ekpyron Jun 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what we wrote about in the chat earlier :-).
I think it has to be

return ranges::subrange(b, e) | ranges::views::values;

to work.

std::vector<EventDefinition const*> events() const { return filteredNodes<EventDefinition>(m_subNodes); }
std::vector<EventDefinition const*> const& interfaceEvents() const;
/// @returns all errors defined in this contract or any base contract
Expand Down Expand Up @@ -546,13 +556,16 @@ class ContractDefinition: public Declaration, public StructurallyDocumented, pub
FunctionDefinition const* nextConstructor(ContractDefinition const& _mostDerivedContract) const;

private:
std::multimap<std::string, FunctionDefinition const*> const& definedFunctionsByName() const;

std::vector<ASTPointer<InheritanceSpecifier>> m_baseContracts;
std::vector<ASTPointer<ASTNode>> m_subNodes;
ContractKind m_contractKind;
bool m_abstract{false};

util::LazyInit<std::vector<std::pair<util::FixedHash<4>, FunctionTypePointer>>> m_interfaceFunctionList[2];
util::LazyInit<std::vector<EventDefinition const*>> m_interfaceEvents;
util::LazyInit<std::multimap<std::string, FunctionDefinition const*>> m_definedFunctionsByName;
};

/**
Expand Down