Skip to content

Conversation

GagaLP
Copy link
Contributor

@GagaLP GagaLP commented Sep 7, 2023

This implements the out-of-bounds check for host side accessors.

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

Check-perf-impact results: (9e900a306dc9f17e4a27439205a7680c)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Contributor

@PeterTh PeterTh left a comment

Choose a reason for hiding this comment

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

Looks good, just 2 minor remarks.

Copy link
Contributor

@PeterTh PeterTh left a comment

Choose a reason for hiding this comment

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

LGTM

@fknorr fknorr added this to the 0.5.0 milestone Sep 8, 2023
Copy link
Member

@psalz psalz left a comment

Choose a reason for hiding this comment

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

Good stuff!

@GagaLP GagaLP force-pushed the host_accessor branch 3 times, most recently from 4b206a9 to c1f0fb1 Compare September 12, 2023 12:10
@github-actions
Copy link

Check-perf-impact results: (7a518c63eb0655471fea6a4902fc0af7)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Member

@psalz psalz left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@fknorr fknorr left a comment

Choose a reason for hiding this comment

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

A couple of small remarks:

@github-actions
Copy link

Check-perf-impact results: (5a19ced85f862a00d0114dd241122462)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

Copy link
Contributor

@fknorr fknorr left a comment

Choose a reason for hiding this comment

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

LGTM!

@GagaLP GagaLP merged commit 41257df into master Sep 21, 2023
@fknorr fknorr deleted the host_accessor branch September 21, 2023 12:04
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