Skip to content

Commit b3d9d9f

Browse files
chrisethchriseth
committed
Changes after review.
1 parent f0c9a0d commit b3d9d9f

File tree

10 files changed

+60
-36
lines changed

10 files changed

+60
-36
lines changed

‎libyul/backends/evm/AsmCodeGen.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ void CodeGenerator::assemble(
5252
builtinContext,
5353
_optimizeStackAllocation,
5454
_identifierAccessCodeGen,
55-
_useNamedLabelsForFunctions
55+
_useNamedLabelsForFunctions ?
56+
CodeTransform::UseNamedLabels::YesAndForceUnique :
57+
CodeTransform::UseNamedLabels::Never
5658
);
5759
transform(_parsedData);
5860
if (!transform.stackErrors().empty())

‎libyul/backends/evm/EVMCodeTransform.cpp‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ CodeTransform::CodeTransform(
5252
EVMDialect const& _dialect,
5353
BuiltinContext& _builtinContext,
5454
ExternalIdentifierAccess::CodeGenerator _identifierAccessCodeGen,
55-
bool _useNamedLabelsForFunctions,
55+
UseNamedLabels _useNamedLabelsForFunctions,
5656
shared_ptr<Context> _context,
5757
vector<TypedName> _delayedReturnVariables,
5858
optional<AbstractAssembly::LabelID> _functionExitLabel
@@ -405,8 +405,12 @@ void CodeTransform::operator()(FunctionDefinition const& _function)
405405
if (!m_allowStackOpt)
406406
subTransform.setupReturnVariablesAndFunctionExit();
407407

408+
subTransform.m_assignedNamedLabels = move(m_assignedNamedLabels);
409+
408410
subTransform(_function.body);
409411

412+
m_assignedNamedLabels = move(subTransform.m_assignedNamedLabels);
413+
410414
m_assembly.setSourceLocation(originLocationOf(_function));
411415
if (!subTransform.m_stackErrors.empty())
412416
{
@@ -585,8 +589,16 @@ void CodeTransform::createFunctionEntryID(FunctionDefinition const& _function)
585589
if (_function.debugData)
586590
astID = _function.debugData->astID;
587591

592+
bool nameAlreadySeen = !m_assignedNamedLabels.insert(_function.name).second;
593+
594+
if (m_useNamedLabelsForFunctions == UseNamedLabels::YesAndForceUnique)
595+
yulAssert(!nameAlreadySeen);
596+
588597
m_context->functionEntryIDs[&scopeFunction] =
589-
m_useNamedLabelsForFunctions ?
598+
(
599+
m_useNamedLabelsForFunctions != UseNamedLabels::Never &&
600+
!nameAlreadySeen
601+
) ?
590602
m_assembly.namedLabel(
591603
_function.name.str(),
592604
_function.parameters.size(),

‎libyul/backends/evm/EVMCodeTransform.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ struct CodeTransformContext
6464
class CodeTransform
6565
{
6666
public:
67+
/// Use named labels for functions 1) Yes and check that the names are unique
68+
/// 2) For none of the functions 3) for the first function of each name.
69+
enum class UseNamedLabels { YesAndForceUnique, Never, ForFirstFunctionOfEachName };
70+
6771
/// Create the code transformer.
6872
/// @param _identifierAccessCodeGen used to generate code for identifiers external to the inline assembly
6973
/// As a side-effect of its construction, translates the Yul code and appends it to the
@@ -78,7 +82,7 @@ class CodeTransform
7882
BuiltinContext& _builtinContext,
7983
bool _allowStackOpt = false,
8084
ExternalIdentifierAccess::CodeGenerator const& _identifierAccessCodeGen = {},
81-
bool _useNamedLabelsForFunctions = false
85+
UseNamedLabels _useNamedLabelsForFunctions = UseNamedLabels::Never
8286
): CodeTransform(
8387
_assembly,
8488
_analysisInfo,
@@ -108,7 +112,7 @@ class CodeTransform
108112
EVMDialect const& _dialect,
109113
BuiltinContext& _builtinContext,
110114
ExternalIdentifierAccess::CodeGenerator _identifierAccessCodeGen,
111-
bool _useNamedLabelsForFunctions,
115+
UseNamedLabels _useNamedLabelsForFunctions,
112116
std::shared_ptr<Context> _context,
113117
std::vector<TypedName> _delayedReturnVariables,
114118
std::optional<AbstractAssembly::LabelID> _functionExitLabel
@@ -193,7 +197,8 @@ class CodeTransform
193197
EVMDialect const& m_dialect;
194198
BuiltinContext& m_builtinContext;
195199
bool const m_allowStackOpt = true;
196-
bool const m_useNamedLabelsForFunctions = false;
200+
UseNamedLabels const m_useNamedLabelsForFunctions = UseNamedLabels::Never;
201+
std::set<YulString> m_assignedNamedLabels;
197202
ExternalIdentifierAccess::CodeGenerator m_identifierAccessCodeGen;
198203
std::shared_ptr<Context> m_context;
199204

‎libyul/backends/evm/EVMObjectCompiler.cpp‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,6 @@
3030
using namespace solidity::yul;
3131
using namespace std;
3232

33-
namespace
34-
{
35-
class AreFunctionNamesUnique: ASTWalker
36-
{
37-
public:
38-
static bool check(Block& _block)
39-
{
40-
AreFunctionNamesUnique instance;
41-
instance(_block);
42-
return instance.m_unique;
43-
}
44-
private:
45-
using ASTWalker::operator();
46-
void operator()(FunctionDefinition const& _functionDefinition) override
47-
{
48-
ASTWalker::operator()(_functionDefinition);
49-
if (!m_functionNames.insert(_functionDefinition.name).second)
50-
m_unique = false;
51-
}
52-
bool m_unique = true;
53-
std::set<YulString> m_functionNames;
54-
};
55-
}
56-
5733
void EVMObjectCompiler::compile(Object& _object, AbstractAssembly& _assembly, EVMDialect const& _dialect, bool _optimize)
5834
{
5935
EVMObjectCompiler compiler(_assembly, _dialect);
@@ -86,10 +62,6 @@ void EVMObjectCompiler::run(Object& _object, bool _optimize)
8662

8763
yulAssert(_object.analysisInfo, "No analysis info.");
8864
yulAssert(_object.code, "No code.");
89-
90-
// We cannot use named labels if the function names are not unique.
91-
bool useNamedLabelsForFunctions = AreFunctionNamesUnique::check(*_object.code);
92-
9365
// We do not catch and re-throw the stack too deep exception here because it is a YulException,
9466
// which should be native to this part of the code.
9567
CodeTransform transform{
@@ -100,7 +72,7 @@ void EVMObjectCompiler::run(Object& _object, bool _optimize)
10072
context,
10173
_optimize,
10274
{},
103-
useNamedLabelsForFunctions
75+
CodeTransform::UseNamedLabels::ForFirstFunctionOfEachName
10476
};
10577
transform(*_object.code);
10678
if (!transform.stackErrors().empty())

‎libyul/optimiser/Disambiguator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class Disambiguator: public ASTCopier
5555
void enterScope(Block const& _block) override;
5656
void leaveScope(Block const& _block) override;
5757
void enterFunction(FunctionDefinition const& _function) override;
58-
void leaveFunction (FunctionDefinition const& _function) override;
58+
void leaveFunction(FunctionDefinition const& _function) override;
5959
YulString translateIdentifier(YulString _name) override;
6060

6161
void enterScopeInternal(Scope& _scope);

‎test/cmdlineTests/inline_assembly_function_name_clash/args‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--experimental-via-ir --combined-json function-debug-runtime

‎test/cmdlineTests/inline_assembly_function_name_clash/err‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
2+
--> inline_assembly_function_name_clash/input.sol
3+
4+
Warning: Source file does not specify required compiler version!
5+
--> inline_assembly_function_name_clash/input.sol

‎test/cmdlineTests/inline_assembly_function_name_clash/input.sol‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
contract C {
2+
function f() public pure returns (uint r) {
3+
assembly { function f() -> x { x := 1 } r := f() }
4+
}
5+
function g() public pure returns (uint r) {
6+
assembly { function f() -> x { x := 2 } r := f() }
7+
}
8+
}
9+
// ====
10+
// compileViaYul: also
11+
// ----
12+
// f() -> 1
13+
// g() -> 2

‎test/cmdlineTests/inline_assembly_function_name_clash/output‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"contracts":{"inline_assembly_function_name_clash/input.sol:C":{"function-debug-runtime":{"abi_decode_tuple_":{"entryPoint":216,"parameterSlots":2,"returnSlots":0},"abi_encode_t_uint256_to_t_uint256_fromStack":{"entryPoint":250,"parameterSlots":2,"returnSlots":0},"abi_encode_tuple_t_uint256__to_t_uint256__fromStack":{"entryPoint":265,"parameterSlots":2,"returnSlots":1},"allocate_unbounded":{"entryPoint":196,"parameterSlots":0,"returnSlots":1},"cleanup_t_uint256":{"entryPoint":240,"parameterSlots":1,"returnSlots":1},"fun_f_7":{"entryPoint":302,"id":7,"parameterSlots":0,"returnSlots":1},"fun_g_14":{"entryPoint":343,"id":14,"parameterSlots":0,"returnSlots":1},"revert_error_42b3090547df1d2001c96683413b8cf91c1b902ef5e3cb8d9f6f304cf7446f74":{"entryPoint":292,"parameterSlots":0,"returnSlots":0},"revert_error_ca66f745a3ce8ff40e2ccaf1ad45db7774001b90d25810abd9040049be7bf4bb":{"entryPoint":206,"parameterSlots":0,"returnSlots":0},"revert_error_dbdddcbe895c83990c08b3492a0e83918d802a52331272ac6fdb6a7c4aea3b1b":{"entryPoint":211,"parameterSlots":0,"returnSlots":0},"shift_right_224_unsigned":{"entryPoint":183,"parameterSlots":1,"returnSlots":1},"usr$f":{"entryPoint":320,"parameterSlots":0,"returnSlots":1},"zero_value_for_split_t_uint256":{"entryPoint":297,"parameterSlots":0,"returnSlots":1}}}},"version": "<VERSION REMOVED>"}

‎test/libsolidity/semanticTests/inlineAssembly/function_name_clash.sol‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
contract C {
2+
function f() public pure returns (uint r) {
3+
assembly { function f() -> x { x := 1 } r := f() }
4+
}
5+
function g() public pure returns (uint r) {
6+
assembly { function f() -> x { x := 2 } r := f() }
7+
}
8+
}
9+
// ====
10+
// compileViaYul: also
11+
// ----
12+
// f() -> 1
13+
// g() -> 2

0 commit comments

Comments
 (0)