Closed Bug 1316696 Opened 3 years ago Closed 3 years ago

Eliminate ns{Fixed,}{C,}StringRepr from the rust bindings

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: Nika, Assigned: Nika)

References

Details

Attachments

(1 file)

Now that rust 1.13 is stable drop flags are dead \o/ - this means that we can now implement Drop on #[repr(C)] types, meaning that we can hide the nsStringRepr details and directly allow using nsString<'static> in #[repr(C)] structs in place of C++'s `nsString`.
This is possible due to Rust 1.13 eliminating the Drop Flags.

I am not confident whether or not Rust 1.13 (which is only recently stable) is avaliable in infra yet. If it is not, this patch will have to be blocked on that.

MozReview-Commit-ID: AMwAUq82vMo
Attachment #8809944 - Flags: review?(nfroyd)
Summary: Allow rust's nsString bindings to be used in FFI → Eliminate ns{Fixed,}{C,}StringRepr from the rust bindings
(In reply to Michael Layzell [:mystor] [:mrl] from comment #1)
> I am not confident whether or not Rust 1.13 (which is only recently stable)
> is avaliable in infra yet. If it is not, this patch will have to be blocked
> on that.

We not only need it to be in infra, but we also need to update configure to require Rust 1.13.
Depends on: 1316751
We can't require 1.13 on all platforms, it has ARM codegen issues.
(In reply to Mike Hommey [:glandium] from comment #3)
> We can't require 1.13 on all platforms, it has ARM codegen issues.

This is a good point - we won't be able to land this patch until after a 1.13.1
Attachment #8809944 - Flags: review?(nfroyd) → review+
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b368ec57dc57
Eliminate ns{Fixed,}{C,}StringRepr from the rust bindings, r=froydnj
https://hg.mozilla.org/mozilla-central/rev/b368ec57dc57
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1320425
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f50ae3e4924d028b4d7488833999fd7012782727 because of bug  1320425, and rustc 1.13 not being required yet.

We can reland this when we are comfortable with requiring a minimum rustc=1.13 in order to build firefox.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
backout also from central
https://hg.mozilla.org/mozilla-central/rev/f50ae3e4924d
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e259fa04f68
Eliminate ns{Fixed,}{C,}StringRepr from the rust bindings, r=froydnj
https://hg.mozilla.org/mozilla-central/rev/4e259fa04f68
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/56b122662f91
Eliminate ns{Fixed,}{C,}StringRepr from the rust bindings, r=froydnj for being the wrong commit backed out a=backout
Backout by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/f2bccdc9fe7e
Backed out changeset 4e259fa04f68 for asserts in DocAccessibleParent.cpp a=backout
Depends on: 1339218
You need to log in before you can comment on or make changes to this bug.