Skip to content

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Feb 10, 2025

It is undefined behaviour to construct a pointer that is out-of-bounds, not just to use it.

Cherry-pick of commit in mimalloc-bench:
daanx/mimalloc-bench@b517ae3

It is undefined behaviour to construct a pointer that is out-of-bounds,
not just to use it.

Commited in mimalloc-bench:
daanx/mimalloc-bench@b517ae3
@fmayer fmayer requested a review from vonosmas February 10, 2025 23:38
{
register precision w;
register cacheType *kludge = pcache + size; /* for shitty compilers */
register cacheType *kludge; /* for shitty compilers */
Copy link

Choose a reason for hiding this comment

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

I realize you are just modifying an existing line of code w/ the comment but I don't believe this comment lines up w/ our CoC and so I think a drive by edit is called for here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a cherry-pick of daanx/mimalloc-bench@b517ae3

Copy link
Contributor

Choose a reason for hiding this comment

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

You can put it into description or title

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did?

Copy link
Contributor

@vitalybuka vitalybuka Feb 11, 2025

Choose a reason for hiding this comment

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

Thanks, cherry-pick makes reader to expect an exact copy.

Copy link
Contributor

Choose a reason for hiding this comment

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

MultiSource is a bit of an odd one in that we're pulling in source from other projects; we usually try to keep modifications to that source small so that we don't run into more painful merges later. In that regard, the comment is "fine" to leave in because it's not our source. That said, the comment is certainly inappropriate and we don't update sources particularly often, so a merge conflict would not be a deal-breaker here.

I think a drive-by edit would be fine, I also think leaving the comment wouldn't be the end of the world.

@vitalybuka vitalybuka requested a review from shafik February 11, 2025 00:36
@fmayer fmayer merged commit 9391cd9 into llvm:main Feb 12, 2025
1 check passed
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.

4 participants