Closed Bug 1366247 Opened 3 years ago Closed 3 years ago

stylo: Add sugar for already_AddRefed

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: xidorn, Assigned: xidorn)

Details

Attachments

(1 file)

No description provided.
I don't change methods of RefPtr because binding functions cannot return already_AddRefed, and thus changing that only complicates existing code.
Comment on attachment 8869425 [details]
Bug 1366247 - Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer.

https://reviewboard.mozilla.org/r/141064/#review144696

::: servo/components/style/gecko_string_cache/mod.rs:353
(Diff revision 1)
> +        debug_assert!(!ptr.is_null());
> +        unsafe { Atom(WeakAtom::new(ptr)) }
> +    }
> +}
> +
> +impl Into<already_AddRefed<nsIAtom>> for Atom {

usually better to impl from than into (from implies into, into doesn't imply from). coherence should let you do it.
Attachment #8869425 - Flags: review?(manishearth) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dd3a293cf053
Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer. r=manishearth
Backout by philringnalda@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b5d15ecde93a
Backed out changeset dd3a293cf053 for making Stylo build, but not run
So per emilio, this is not going to work, because already_AddRefed has destructor, which would make the two sides disagree on how arguments are passed.

I guess we still want something to represent addrefed pointer somehow...
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.