Skip to content

fix arrow operator#11

Merged
camio merged 1 commit intobemanproject:mainfrom
camio:fix-arrow-operator
Oct 18, 2024
Merged

fix arrow operator#11
camio merged 1 commit intobemanproject:mainfrom
camio:fix-arrow-operator

Conversation

@camio
Copy link
Copy Markdown
Member

@camio camio commented Oct 18, 2024

Also, added a test illustrating the issue.

Depends on #8.

@camio
Copy link
Copy Markdown
Member Author

camio commented Oct 18, 2024

This is a draft until #8 gets merged.

Copy link
Copy Markdown
Member

@steve-downey steve-downey left a comment

Choose a reason for hiding this comment

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

I'll bump up coverage testing on my todo list.

{
return addressof(value);
return std::addressof(value);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Uninstantiated broken code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, exactly.

Also, added a test illustrating the issue.
@camio camio force-pushed the fix-arrow-operator branch from ee32d50 to 5d46888 Compare October 18, 2024 15:10
@camio camio marked this pull request as ready for review October 18, 2024 15:11
@camio camio assigned camio and steve-downey and unassigned camio Oct 18, 2024
@camio camio requested a review from steve-downey October 18, 2024 15:12
Copy link
Copy Markdown
Member

@steve-downey steve-downey left a comment

Choose a reason for hiding this comment

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

+1

@camio camio merged commit 7433749 into bemanproject:main Oct 18, 2024
@camio camio deleted the fix-arrow-operator branch October 18, 2024 17:02
Copy link
Copy Markdown
Member

@neatudarius neatudarius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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.

3 participants