Skip to content
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

Jump to bottom

replace YulString with YulName typedef #15083

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

replace YulString with YulName typedef #15083

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion libsolidity/analysis/ReferencesResolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ void ReferencesResolver::resolveInheritDoc(StructuredDocumentation const& _docum
}
}

void ReferencesResolver::validateYulIdentifierName(yul::YulString _name, SourceLocation const& _location)
void ReferencesResolver::validateYulIdentifierName(yul::YulName _name, SourceLocation const& _location)
{
if (util::contains(_name.str(), '.'))
m_errorReporter.declarationError(
Expand Down
3 changes: 2 additions & 1 deletion libsolidity/analysis/ReferencesResolver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <libsolidity/ast/ASTAnnotations.h>
#include <liblangutil/EVMVersion.h>
#include <libyul/optimiser/ASTWalker.h>
#include <libyul/YulName.h>

#include <list>
#include <map>
Expand Down Expand Up @@ -94,7 +95,7 @@ class ReferencesResolver: private ASTConstVisitor, private yul::ASTWalker
void resolveInheritDoc(StructuredDocumentation const& _documentation, StructurallyDocumentedAnnotation& _annotation);

/// Checks if the name contains a '.'.
void validateYulIdentifierName(yul::YulString _name, langutil::SourceLocation const& _location);
void validateYulIdentifierName(yul::YulName _name, langutil::SourceLocation const& _location);

langutil::ErrorReporter& m_errorReporter;
NameAndTypeResolver& m_resolver;
Expand Down
8 changes: 4 additions & 4 deletions libsolidity/codegen/CompilerContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,11 @@ void CompilerContext::appendInlineAssembly(
{
unsigned startStackHeight = stackHeight();

std::set<yul::YulString> externallyUsedIdentifiers;
std::set<yul::YulName> externallyUsedIdentifiers;
for (auto const& fun: _externallyUsedFunctions)
externallyUsedIdentifiers.insert(yul::YulString(fun));
externallyUsedIdentifiers.insert(yul::YulName(fun));
for (auto const& var: _localVariables)
externallyUsedIdentifiers.insert(yul::YulString(var));
externallyUsedIdentifiers.insert(yul::YulName(var));

yul::ExternalIdentifierAccess identifierAccess;
identifierAccess.resolve = [&](
Expand Down Expand Up @@ -532,7 +532,7 @@ void CompilerContext::appendInlineAssembly(
}


void CompilerContext::optimizeYul(yul::Object& _object, yul::EVMDialect const& _dialect, OptimiserSettings const& _optimiserSettings, std::set<yul::YulString> const& _externalIdentifiers)
void CompilerContext::optimizeYul(yul::Object& _object, yul::EVMDialect const& _dialect, OptimiserSettings const& _optimiserSettings, std::set<yul::YulName> const& _externalIdentifiers)
{
#ifdef SOL_OUTPUT_ASM
cout << yul::AsmPrinter(*dialect)(*_object.code) << endl;
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/codegen/CompilerContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class CompilerContext
/// Otherwise returns "revert(0, 0)".
std::string revertReasonIfDebug(std::string const& _message = "");

void optimizeYul(yul::Object& _object, yul::EVMDialect const& _dialect, OptimiserSettings const& _optimiserSetting, std::set<yul::YulString> const& _externalIdentifiers = {});
void optimizeYul(yul::Object& _object, yul::EVMDialect const& _dialect, OptimiserSettings const& _optimiserSetting, std::set<yul::YulName> const& _externalIdentifiers = {});

/// Appends arbitrary data to the end of the bytecode.
void appendToAuxiliaryData(bytes const& _data) { m_asm->appendToAuxiliaryData(_data); }
Expand Down
8 changes: 4 additions & 4 deletions libsolidity/codegen/ir/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ struct CopyTranslate: public yul::ASTCopier
return ASTCopier::operator()(_identifier);
}

yul::YulString translateIdentifier(yul::YulString _name) override
yul::YulName translateIdentifier(yul::YulName _name) override
{
// Strictly, the dialect used by inline assembly (m_dialect) could be different
// from the Yul dialect we are compiling to. So we are assuming here that the builtin
Expand All @@ -85,7 +85,7 @@ struct CopyTranslate: public yul::ASTCopier
if (m_dialect.builtin(_name))
return _name;
else
return yul::YulString{"usr$" + _name.str()};
return yul::YulName{"usr$" + _name.str()};
}

yul::Identifier translate(yul::Identifier const& _identifier) override
Expand Down Expand Up @@ -202,9 +202,9 @@ struct CopyTranslate: public yul::ASTCopier
solAssert(false);

if (isDigit(value.front()))
return yul::Literal{_identifier.debugData, yul::LiteralKind::Number, yul::YulString{value}, {}};
return yul::Literal{_identifier.debugData, yul::LiteralKind::Number, yul::YulName{value}, {}};
else
return yul::Identifier{_identifier.debugData, yul::YulString{value}};
return yul::Identifier{_identifier.debugData, yul::YulName{value}};
}


Expand Down
6 changes: 3 additions & 3 deletions libsolidity/experimental/codegen/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ struct CopyTranslate: public yul::ASTCopier
return ASTCopier::operator()(_identifier);
}

yul::YulString translateIdentifier(yul::YulString _name) override
yul::YulName translateIdentifier(yul::YulName _name) override
{
if (m_dialect.builtin(_name))
return _name;
else
return yul::YulString{"usr$" + _name.str()};
return yul::YulName{"usr$" + _name.str()};
}

yul::Identifier translate(yul::Identifier const& _identifier) override
Expand All @@ -102,7 +102,7 @@ struct CopyTranslate: public yul::ASTCopier
solAssert(type);
solAssert(m_context.env->typeEquals(*type, m_context.analysis.typeSystem().type(PrimitiveType::Word, {})));
std::string value = IRNames::localVariable(*varDecl);
return yul::Identifier{_identifier.debugData, yul::YulString{value}};
return yul::Identifier{_identifier.debugData, yul::YulName{value}};
}

IRGenerationContext const& m_context;
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/formal/SMTEncoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ void SMTEncoder::endVisit(Block const& _block)

bool SMTEncoder::visit(InlineAssembly const& _inlineAsm)
{
/// This is very similar to `yul::Assignments`, except I need to collect `Identifier`s and not just names as `YulString`s.
/// This is very similar to `yul::Assignments`, except I need to collect `Identifier`s and not just names as `YulName`s.
struct AssignedExternalsCollector: public yul::ASTWalker
{
AssignedExternalsCollector(InlineAssembly const& _inlineAsm): externalReferences(_inlineAsm.annotation().externalReferences)
Expand Down
11 changes: 6 additions & 5 deletions libyul/AST.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include <libyul/ASTForward.h>
#include <libyul/YulString.h>
#include <libyul/YulName.h>

#include <liblangutil/DebugData.h>

Expand All @@ -34,16 +35,16 @@
namespace solidity::yul
{

using Type = YulString;
using Type = YulName;

struct TypedName { langutil::DebugData::ConstPtr debugData; YulString name; Type type; };
struct TypedName { langutil::DebugData::ConstPtr debugData; YulName name; Type type; };
using TypedNameList = std::vector<TypedName>;

/// Literal number or string (up to 32 bytes)
enum class LiteralKind { Number, Boolean, String };
struct Literal { langutil::DebugData::ConstPtr debugData; LiteralKind kind; YulString value; Type type; };
struct Literal { langutil::DebugData::ConstPtr debugData; LiteralKind kind; YulName value; Type type; };
Copy link
Member

@cameel cameel May 10, 2024 •
edited

Choose a reason for hiding this comment

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

The PR seems to be just replacing all instances of YulString with YulName. That's not very useful. The idea was to rename only in those cases where YulString is used for identifiers. Those are the cases which we'd want to replace with numeric IDs and we'll do this by switching YulName to be some other type.

Literal is a good example of a situation where this must not happen. If you have a string "xyz", in the code, it must remain as "xyz" in the AST. It can't be replaced with some number that we'll only later remap back to the original value.

I think we don't want this for TypedName either. Types are defined in the dialect so renaming then just in the AST will make us unable to find the right type definition.

BTW, TypeName is probably a legacy thing at this point and maybe we'll remove it eventually. It is used in the typed Yul dialect, which was an intermediate step in the process of translating the code to Ewasm. Ewasm was dropped and I'm not sure if we'll have any other use for it. In the untyped dialect Type will always be just the default type.

As for FunctionDefinition, for that one using YulName seems correct, though I'm surprised it doesn't use a whole Identifier instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was under the impression you'd wanted to get rid of YulString altogether, my bad then.

As for Literal and TypedName: How would replacing a string "xyz" with some identifier and later remapping it be conceptually different from using YulString, which replaces the string "xyz" with a handle (id/hash) to later remap it?

/// External / internal identifier or label reference
struct Identifier { langutil::DebugData::ConstPtr debugData; YulString name; };
struct Identifier { langutil::DebugData::ConstPtr debugData; YulName name; };
/// Assignment ("x := mload(20:u256)", expects push-1-expression on the right hand
/// side and requires x to occupy exactly one stack slot.
///
Expand All @@ -59,7 +60,7 @@ struct VariableDeclaration { langutil::DebugData::ConstPtr debugData; TypedNameL
/// Block that creates a scope (frees declared stack variables)
struct Block { langutil::DebugData::ConstPtr debugData; std::vector<Statement> statements; };
/// Function definition ("function f(a, b) -> (d, e) { ... }")
struct FunctionDefinition { langutil::DebugData::ConstPtr debugData; YulString name; TypedNameList parameters; TypedNameList returnVariables; Block body; };
struct FunctionDefinition { langutil::DebugData::ConstPtr debugData; YulName name; TypedNameList parameters; TypedNameList returnVariables; Block body; };
Copy link
Member

Choose a reason for hiding this comment

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

We may have a small problem here. With this change, user-defined functions will use YulName, while the built-in functions in the dialect will still be identified with YulString. Since the latter by design have fixed names, I'm not sure it's even feasible to use YulName for them. But then FunctionCall node will not be able to refer to them.

/// Conditional execution without "else" part.
struct If { langutil::DebugData::ConstPtr debugData; std::unique_ptr<Expression> condition; Block body; };
/// Switch case or default case
Expand Down
48 changes: 24 additions & 24 deletions libyul/AsmAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ AsmAnalysisInfo AsmAnalyzer::analyzeStrictAssertCorrect(Dialect const& _dialect,
return analysisInfo;
}

std::vector<YulString> AsmAnalyzer::operator()(Literal const& _literal)
std::vector<YulName> AsmAnalyzer::operator()(Literal const& _literal)
{
expectValidType(_literal.type, nativeLocationOf(_literal));
if (_literal.kind == LiteralKind::String && _literal.value.str().size() > 32)
Expand All @@ -122,11 +122,11 @@ std::vector<YulString> AsmAnalyzer::operator()(Literal const& _literal)
return {_literal.type};
}

std::vector<YulString> AsmAnalyzer::operator()(Identifier const& _identifier)
std::vector<YulName> AsmAnalyzer::operator()(Identifier const& _identifier)
{
yulAssert(!_identifier.name.empty(), "");
auto watcher = m_errorReporter.errorWatcher();
YulString type = m_dialect.defaultType;
YulName type = m_dialect.defaultType;

if (m_currentScope->lookup(_identifier.name, GenericVisitor{
[&](Scope::Variable const& _var)
Expand Down Expand Up @@ -180,7 +180,7 @@ std::vector<YulString> AsmAnalyzer::operator()(Identifier const& _identifier)
void AsmAnalyzer::operator()(ExpressionStatement const& _statement)
{
auto watcher = m_errorReporter.errorWatcher();
std::vector<YulString> types = std::visit(*this, _statement.expression);
std::vector<YulName> types = std::visit(*this, _statement.expression);
if (watcher.ok() && !types.empty())
m_errorReporter.typeError(
3083_error,
Expand All @@ -199,7 +199,7 @@ void AsmAnalyzer::operator()(Assignment const& _assignment)
size_t const numVariables = _assignment.variableNames.size();
yulAssert(numVariables >= 1, "");

std::set<YulString> variables;
std::set<YulName> variables;
for (auto const& _variableName: _assignment.variableNames)
if (!variables.insert(_variableName.name).second)
m_errorReporter.declarationError(
Expand All @@ -210,7 +210,7 @@ void AsmAnalyzer::operator()(Assignment const& _assignment)
" occurs multiple times on the left-hand side of the assignment."
);

std::vector<YulString> types = std::visit(*this, *_assignment.value);
std::vector<YulName> types = std::visit(*this, *_assignment.value);

if (types.size() != numVariables)
m_errorReporter.declarationError(
Expand Down Expand Up @@ -249,7 +249,7 @@ void AsmAnalyzer::operator()(VariableDeclaration const& _varDecl)

if (_varDecl.value)
{
std::vector<YulString> types = std::visit(*this, *_varDecl.value);
std::vector<YulName> types = std::visit(*this, *_varDecl.value);
if (types.size() != numVariables)
m_errorReporter.declarationError(
3812_error,
Expand All @@ -265,7 +265,7 @@ void AsmAnalyzer::operator()(VariableDeclaration const& _varDecl)

for (size_t i = 0; i < _varDecl.variables.size(); ++i)
{
YulString givenType = m_dialect.defaultType;
YulName givenType = m_dialect.defaultType;
if (i < types.size())
givenType = types[i];
TypedName const& variable = _varDecl.variables[i];
Expand Down Expand Up @@ -301,12 +301,12 @@ void AsmAnalyzer::operator()(FunctionDefinition const& _funDef)
(*this)(_funDef.body);
}

std::vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
std::vector<YulName> AsmAnalyzer::operator()(FunctionCall const& _funCall)
{
yulAssert(!_funCall.functionName.name.empty(), "");
auto watcher = m_errorReporter.errorWatcher();
std::vector<YulString> const* parameterTypes = nullptr;
std::vector<YulString> const* returnTypes = nullptr;
std::vector<YulName> const* parameterTypes = nullptr;
std::vector<YulName> const* returnTypes = nullptr;
std::vector<std::optional<LiteralKind>> const* literalArguments = nullptr;

if (BuiltinFunction const* f = m_dialect.builtin(_funCall.functionName.name))
Expand Down Expand Up @@ -390,7 +390,7 @@ std::vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
std::to_string(_funCall.arguments.size()) + "."
);

std::vector<YulString> argTypes;
std::vector<YulName> argTypes;
for (size_t i = _funCall.arguments.size(); i > 0; i--)
{
Expression const& arg = _funCall.arguments[i - 1];
Expand Down Expand Up @@ -453,7 +453,7 @@ std::vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
return *returnTypes;
}
else if (returnTypes)
return std::vector<YulString>(returnTypes->size(), m_dialect.defaultType);
return std::vector<YulName>(returnTypes->size(), m_dialect.defaultType);
else
return {};
}
Expand All @@ -476,7 +476,7 @@ void AsmAnalyzer::operator()(Switch const& _switch)
"\"switch\" statement with only a default case."
);

YulString valueType = expectExpression(*_switch.expression);
YulName valueType = expectExpression(*_switch.expression);

std::set<u256> cases;
for (auto const& _case: _switch.cases)
Expand Down Expand Up @@ -541,9 +541,9 @@ void AsmAnalyzer::operator()(Block const& _block)
m_currentScope = previousScope;
}

YulString AsmAnalyzer::expectExpression(Expression const& _expr)
YulName AsmAnalyzer::expectExpression(Expression const& _expr)
{
std::vector<YulString> types = std::visit(*this, _expr);
std::vector<YulName> types = std::visit(*this, _expr);
if (types.size() != 1)
m_errorReporter.typeError(
3950_error,
Expand All @@ -555,7 +555,7 @@ YulString AsmAnalyzer::expectExpression(Expression const& _expr)
return types.empty() ? m_dialect.defaultType : types.front();
}

YulString AsmAnalyzer::expectUnlimitedStringLiteral(Literal const& _literal)
YulName AsmAnalyzer::expectUnlimitedStringLiteral(Literal const& _literal)
{
yulAssert(_literal.kind == LiteralKind::String, "");
yulAssert(m_dialect.validTypeForLiteral(LiteralKind::String, _literal.value, _literal.type), "");
Expand All @@ -565,7 +565,7 @@ YulString AsmAnalyzer::expectUnlimitedStringLiteral(Literal const& _literal)

void AsmAnalyzer::expectBoolExpression(Expression const& _expr)
{
YulString type = expectExpression(_expr);
YulName type = expectExpression(_expr);
if (type != m_dialect.boolType)
m_errorReporter.typeError(
1733_error,
Expand All @@ -578,11 +578,11 @@ void AsmAnalyzer::expectBoolExpression(Expression const& _expr)
);
}

void AsmAnalyzer::checkAssignment(Identifier const& _variable, YulString _valueType)
void AsmAnalyzer::checkAssignment(Identifier const& _variable, YulName _valueType)
{
yulAssert(!_variable.name.empty(), "");
auto watcher = m_errorReporter.errorWatcher();
YulString const* variableType = nullptr;
YulName const* variableType = nullptr;
bool found = false;
if (Scope::Identifier const* var = m_currentScope->lookup(_variable.name))
{
Expand Down Expand Up @@ -641,7 +641,7 @@ Scope& AsmAnalyzer::scope(Block const* _block)
return *scopePtr;
}

void AsmAnalyzer::expectValidIdentifier(YulString _identifier, SourceLocation const& _location)
void AsmAnalyzer::expectValidIdentifier(YulName _identifier, SourceLocation const& _location)
{
// NOTE: the leading dot case is handled by the parser not allowing it.
if (boost::ends_with(_identifier.str(), "."))
Expand All @@ -666,7 +666,7 @@ void AsmAnalyzer::expectValidIdentifier(YulString _identifier, SourceLocation co
);
}

void AsmAnalyzer::expectValidType(YulString _type, SourceLocation const& _location)
void AsmAnalyzer::expectValidType(YulName _type, SourceLocation const& _location)
{
if (!m_dialect.types.count(_type))
m_errorReporter.typeError(
Expand All @@ -676,7 +676,7 @@ void AsmAnalyzer::expectValidType(YulString _type, SourceLocation const& _locati
);
}

void AsmAnalyzer::expectType(YulString _expectedType, YulString _givenType, SourceLocation const& _location)
void AsmAnalyzer::expectType(YulName _expectedType, YulName _givenType, SourceLocation const& _location)
{
if (_expectedType != _givenType)
m_errorReporter.typeError(
Expand All @@ -689,7 +689,7 @@ void AsmAnalyzer::expectType(YulString _expectedType, YulString _givenType, Sour
bool AsmAnalyzer::validateInstructions(std::string const& _instructionIdentifier, langutil::SourceLocation const& _location)
{
// NOTE: This function uses the default EVM version instead of the currently selected one.
auto const builtin = EVMDialect::strictAssemblyForEVM(EVMVersion{}).builtin(YulString(_instructionIdentifier));
auto const builtin = EVMDialect::strictAssemblyForEVM(EVMVersion{}).builtin(YulName(_instructionIdentifier));
if (builtin && builtin->instruction.has_value())
return validateInstructions(builtin->instruction.value(), _location);
else
Expand Down