Closed Bug 1026718 Opened 5 years ago Closed 5 years ago

Make VolatileBuffer not use MOZ_DECLARE_REFCOUNTED_TYPENAME

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(2 files)

The issue here is that mozalloc is not part of libxul, so
MOZ_DECLARE_REFCOUNTED_TYPENAME expands to nothing there.  MSVC seems
to just happily inline these function calls even though they are on a
dllimport class.
Assignee: nobody → ehsan
Blocks: winclang
Attachment #8441631 - Flags: review?(mh+mozilla)
Comment on attachment 8441631 [details] [diff] [review]
Make VolatileBuffer not use MOZ_DECLARE_REFCOUNTED_TYPENAME

Review of attachment 8441631 [details] [diff] [review]:
-----------------------------------------------------------------

I'd rather change MOZ_DECLARE_REFCOUNTED_TYPENAME to expand independently of MOZILLA_INTERNAL_API.
Attachment #8441631 - Flags: review?(mh+mozilla) → review-
Attachment #8442053 - Flags: review?(mh+mozilla)
Comment on attachment 8442053 [details] [diff] [review]
Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it available outside of libxul as well; r=glandium

Review of attachment 8442053 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/RefPtr.h
@@ +166,5 @@
> +// two small inline functions which will hopefully get eliminated by the linker
> +// in non-leak-checking builds.
> +#define MOZ_DECLARE_REFCOUNTED_TYPENAME(T) \
> +  const char* typeName() const { return #T; } \
> +  size_t typeSize() const { return sizeof(*this); }

I think you can reasonably put that under #if defined(DEBUG) || defined(FORCE_BUILD_REFCNT_LOGGING)

That said, I'm not a MFBT peer.
Attachment #8442053 - Flags: review?(mh+mozilla) → review?(jwalden+bmo)
(In reply to Mike Hommey [:glandium] from comment #4)
> Comment on attachment 8442053 [details] [diff] [review]
> Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it
> available outside of libxul as well; r=glandium
> 
> Review of attachment 8442053 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mfbt/RefPtr.h
> @@ +166,5 @@
> > +// two small inline functions which will hopefully get eliminated by the linker
> > +// in non-leak-checking builds.
> > +#define MOZ_DECLARE_REFCOUNTED_TYPENAME(T) \
> > +  const char* typeName() const { return #T; } \
> > +  size_t typeSize() const { return sizeof(*this); }
> 
> I think you can reasonably put that under #if defined(DEBUG) ||
> defined(FORCE_BUILD_REFCNT_LOGGING)

Sure, I could do that.  Shouldn't make a difference either way...
Component: General → MFBT
Comment on attachment 8442053 [details] [diff] [review]
Expand MOZ_DECLARE_REFCOUNTED_TYPENAME unconditioanlly in order to make it available outside of libxul as well; r=glandium

Review of attachment 8442053 [details] [diff] [review]:
-----------------------------------------------------------------

Actually you are, per very ancient discussion from ~2012ish (?) that only made its way into the module page list just recently.  :-)
Attachment #8442053 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/8eeb287c59fd
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.