Stream: git-cranelift

Topic: cranelift / PR #1392 Add null pointer check for SimpleJit...

GitHub (Feb 14 2020 at 04:24):

timClicks opened PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [ ] This has been discussed in issue #798

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 14 2020 at 04:24):

timClicks edited PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [x] This has been discussed in issue #798

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 14 2020 at 04:24):

timClicks edited PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [x] This has been discussed in issue #798

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [x] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 14 2020 at 04:24):

timClicks edited PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [x] This has been discussed in issue #798

• [x] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [x] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 14 2020 at 04:29):

timClicks updated PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [x] This has been discussed in issue #798

• [x] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [x] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 24 2020 at 09:31):

bnjbvr requested bnjbvr for a review on PR #1392.

GitHub (Feb 24 2020 at 09:33):

bnjbvr submitted PR Review.

GitHub (Feb 24 2020 at 09:33):

bnjbvr created PR Review Comment:

nit: can you tweak this doc line to tell something like "Panics whenever any symbol pointer is NULL."?

GitHub (Feb 24 2020 at 09:33):

bnjbvr submitted PR Review.

GitHub (Feb 24 2020 at 09:41):

bjorn3 submitted PR Review.

GitHub (Feb 24 2020 at 09:41):

bjorn3 created PR Review Comment:

Please add an expected panic message: https://doc.rust-lang.org/reference/attributes/testing.html?highlight=should_panic#the-should_panic-attribute

GitHub (Feb 24 2020 at 09:41):

bjorn3 submitted PR Review.

GitHub (Feb 24 2020 at 09:45):

timClicks submitted PR Review.

GitHub (Feb 24 2020 at 09:45):

timClicks created PR Review Comment:

Will do :)

GitHub (Feb 24 2020 at 09:45):

timClicks submitted PR Review.

GitHub (Feb 24 2020 at 09:45):

timClicks created PR Review Comment:

Okay, sure thing.

GitHub (Feb 29 2020 at 09:55):

timClicks updated PR #1392 from mark-symbols-unsafe to master:

This commit marks provides the SimpleJitBuilder.symbol() and SimpleJitBuilder.symbols() methods as unsafe, as markers to those embedding the crate. It also includes a null pointer assert! to abort illegal use of the methods.

As a driveby, the commit rewrites symbols() in terms of symbol(), removing some code duplication.

• [x] This has been discussed in issue #798

• [x] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [x] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so and/or ping
  bnjbvr. The list of suggested reviewers on the right can help you.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Jun 26 2020 at 13:19):

tschneidereit closed without merge PR #1392.

Last updated: Dec 23 2024 at 13:07 UTC