-
Notifications
You must be signed in to change notification settings - Fork 396
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 can skip null value store check #7184
Add can skip null value store check #7184
Conversation
@hzongaro May I ask you to review this change? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your changes look good.
One thing I notice is that the non-helper that's used for checking for stores of null references to null-restricted arrays is called <nonNullableArrayNullStoreCheck>
. Of course, the terminology that's being used in the downstream project has changed over time. I think the names of skip...
, safeToSkip...
and CanSkip...
should be consistent with the name of the non-helper, and they should either all use nonNullableArrayNullStoreCheck
or all use nullValueStoreCheckForNullRestrictedArray
.
If you decide to go use with NullRestricted
rather than nonNullable
, it might be good to trim the length a bit - maybe something like nullRestrictedArrNullStoreCheck
.
Good point! I'll update the name to |
9bd959a
to
0bc229a
Compare
Add `CanSkipNonNullableArrayNullStoreCheck` for recognized method. Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
0bc229a
to
aebf7dd
Compare
@hzongaro The comment is addressed. Ready for another review. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!
Jenkins build all |
x86-64 macOS test failure:
Linux riscv64 failure:
It looks to me an infrastructure issue. |
Jenkins build riscv |
Except for known problems, builds were all successful. Merging. |
Add
CanSkipNonNullableArrayNullStoreCheck
for recognized method.This change is used by the change in: