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.

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

Add DebugData to AssemblyItem. #14841

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Feb 10, 2024

No description provided.

libevmasm/AssemblyItem.h Outdated Show resolved Hide resolved
libyul/AST.h Outdated Show resolved Hide resolved
@aarlt aarlt force-pushed the add_debugdata_to_assemblyitem branch from 0a11f83 to 3c4b64e Compare February 11, 2024 01:06
libyul/AST.h Outdated Show resolved Hide resolved
libyul/AST.h Outdated Show resolved Hide resolved
libyul/AST.h Outdated Show resolved Hide resolved
@aarlt aarlt force-pushed the add_debugdata_to_assemblyitem branch 3 times, most recently from 617c950 to 7657bba Compare February 13, 2024 22:39
@aarlt aarlt self-assigned this Feb 13, 2024
@aarlt aarlt marked this pull request as ready for review February 13, 2024 22:41
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

I'd call this almost done short of some very minor tweaks - nice that it just seems to work so far without too many complications!

libevmasm/AssemblyItem.h Outdated Show resolved Hide resolved
libevmasm/CommonSubexpressionEliminator.cpp Outdated Show resolved Hide resolved
libevmasm/ControlFlowGraph.cpp Outdated Show resolved Hide resolved
libyul/AST.h Outdated Show resolved Hide resolved
libyul/AsmParser.h Outdated Show resolved Hide resolved
libyul/AST.h Outdated Show resolved Hide resolved
@aarlt aarlt force-pushed the add_debugdata_to_assemblyitem branch from 2c7d4ab to 3d9857c Compare February 14, 2024 16:19
libevmasm/AssemblyItem.h Outdated Show resolved Hide resolved
@ethereum ethereum deleted a comment from stackenbotten Feb 14, 2024
@aarlt aarlt force-pushed the add_debugdata_to_assemblyitem branch from fe5d789 to 6bd3e72 Compare February 14, 2024 20:05
return m_debugData->nativeLocation;
}

void setDebugData(langutil::DebugData::ConstPtr const& _debugData) { m_debugData = _debugData; }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void setDebugData(langutil::DebugData::ConstPtr const& _debugData) { m_debugData = _debugData; }
void setDebugData(langutil::DebugData::ConstPtr const& _debugData)
{
solAssert(_debugData);
m_debugData = _debugData;
}

if we want to guarantee that there's always a valid one there, we also need to do it here.

Copy link
Member

@ekpyron ekpyron Feb 15, 2024

Choose a reason for hiding this comment

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

If you want, while you're at it: passing the pointer by value and moving it is, I think, a bit more idiomatic for shared pointers... but also doesn't matter too much (and it's also not consistently done one way or the other on develop either)

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right! I change that only where it is possible to use std::move - e.g. if there is exactly one other place where this pointer can be moved. I let it be a constant reference, when the pointer is used to set multiple things.

Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Ok, we somehow lost the change for #14841 (comment) (although it's also not exactly super vital) - and apart from that
#14841 (comment)
is the only thing missing I can still find.

@ekpyron ekpyron merged commit 172f3cf into develop Feb 19, 2024
66 of 68 checks passed
@ekpyron ekpyron deleted the add_debugdata_to_assemblyitem branch February 19, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants