Note: There are a few cases of duplicates in user autocompletion which are being worked on.

mozilla::ThreadLocal only allows pointers

RESOLVED FIXED in mozilla15

Status

()

Core
MFBT
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks: 1 bug)

Trunk
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
There are several cases, including the examples given in ThreadLocal.h, where ThreadLocal may be used with integer types, which is actually not possible with the current implementation (which makes the example code wrong). Also, since the type given to the template is not exactly the one returned by get() and taken by set(), it's not entirely clear when writing a ThreadLocal variable definition what its type actually is.
(Assignee)

Comment 1

5 years ago
Created attachment 625561 [details] [diff] [review]
Allow mozilla::ThreadLocal to store integer types smaller than, or as large as, a pointer
Attachment #625561 - Flags: review?(jwalden+bmo)
Comment on attachment 625561 [details] [diff] [review]
Allow mozilla::ThreadLocal to store integer types smaller than, or as large as, a pointer

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

Seems reasonable.

::: mfbt/ThreadLocal.h
@@ +74,5 @@
>  
> +    typedef union {
> +      void *ptr;
> +      T value;
> +    } helper;

Just |union Helper|, please.  The typedef here is unnecessary, since everything using this is a member of ThreadLocal for name lookup purposes.  And it should be InterCaps-named like everything else.
Attachment #625561 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3570567bd5d
Target Milestone: --- → mozilla15

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/b3570567bd5d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.