Skip to content
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 function to tell whether a TypedArray underlying buffer is shared #438

Merged
merged 1 commit into from Sep 6, 2018

Conversation

@ferjm
Copy link
Member

ferjm commented Sep 6, 2018

This change is Reviewable

@ferjm
Copy link
Member Author

ferjm commented Sep 6, 2018

r? @jdm

@jdm
jdm approved these changes Sep 6, 2018
///
/// Return a boolean flag which denotes whether the underlying buffer
/// is a SharedArrayBuffer.
pub unsafe fn is_shared(&self) -> bool {

This comment has been minimized.

@jdm

jdm Sep 6, 2018

Member

This does not need to be unsafe.

@jdm
Copy link
Member

jdm commented Sep 6, 2018

r=me with the unsafe removed.

@ferjm ferjm force-pushed the ferjm:issharedarraybuffer branch from 6b66c97 to 5791825 Sep 6, 2018
@ferjm
Copy link
Member Author

ferjm commented Sep 6, 2018

I knew I added it for a reason :)

❯ cargo build
warning: src/jsglue.cpp:586:5: warning: delete called on non-final 'JS::OwningCompileOptions' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
warning:     delete static_cast<JS::OwningCompileOptions *>(aOpts);
warning:     ^
warning: 1 warning generated.
   Compiling mozjs v0.9.0 (file:///Users/ferjm/dev/mozilla/rust-mozjs)
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
   --> src/typedarray.rs:214:9
    |
214 |         JS_GetTypedArraySharedness(self.object.as_raw())
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
    |
    = note: consult the function's documentation for information on how to avoid undefined behavior

error: aborting due to previous error

For more information about this error, try `rustc --explain E0133`.
The following warnings were emitted during compilation:

warning: src/jsglue.cpp:586:5: warning: delete called on non-final 'JS::OwningCompileOptions' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
warning:     delete static_cast<JS::OwningCompileOptions *>(aOpts);
warning:     ^
warning: 1 warning generated.

error: Could not compile `mozjs`.

To learn more, run the command again with --verbose.
@ferjm ferjm force-pushed the ferjm:issharedarraybuffer branch from 5791825 to 33fb3c9 Sep 6, 2018
@jdm
Copy link
Member

jdm commented Sep 6, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Sep 6, 2018

📌 Commit 33fb3c9 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Sep 6, 2018

Testing commit 33fb3c9 with merge 3efd159...

bors-servo added a commit that referenced this pull request Sep 6, 2018
Add function to tell whether a TypedArray underlying buffer is shared

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/438)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: jdm
Pushing 3efd159 to master...

@bors-servo bors-servo merged commit 33fb3c9 into servo:master Sep 6, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@ferjm ferjm deleted the ferjm:issharedarraybuffer branch Sep 7, 2018
@ferjm
Copy link
Member Author

ferjm commented Sep 7, 2018

@jdm could you publish a new release with this change, please?

@jdm
Copy link
Member

jdm commented Sep 7, 2018

Done.

@ferjm
Copy link
Member Author

ferjm commented Sep 7, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.