Skip to content

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jun 12, 2024

I believe we should return after the SDR for the error case, instead of invoking it a second time with Error::success().

I have no idea how to test this (or if that's even possible), I just found this through a static analyser warning.

I believe we should return after the SDR for the error case, instead
of invoking it a second time with Error::success().

I have no idea how to test this (or if that's even possible), I
just found this through a static analyser warning.
@nikic nikic requested a review from lhames June 12, 2024 13:56
Copy link
Contributor

@lhames lhames left a comment

Choose a reason for hiding this comment

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

LGTM. Nice catch. Thanks @nikic!

With some work we could write a unit test to test this, but I don't have the bandwidth at the moment and I don't think it's worth holding up this fix for.

@nikic nikic merged commit 092dbfa into llvm:main Jun 13, 2024
@nikic nikic deleted the execution-engine-error-return branch June 13, 2024 06:24
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