Last Comment Bug 756965 - mozilla::ThreadLocal only allows pointers
: mozilla::ThreadLocal only allows pointers
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
:
Mentors:
Depends on:
Blocks: 756439 753119
  Show dependency treegraph
 
Reported: 2012-05-20 23:40 PDT by Mike Hommey [:glandium]
Modified: 2012-05-23 08:06 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Allow mozilla::ThreadLocal to store integer types smaller than, or as large as, a pointer (3.50 KB, patch)
2012-05-20 23:41 PDT, Mike Hommey [:glandium]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-05-20 23:40:24 PDT
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.
Comment 1 Mike Hommey [:glandium] 2012-05-20 23:41:40 PDT
Created attachment 625561 [details] [diff] [review]
Allow mozilla::ThreadLocal to store integer types smaller than, or as large as, a pointer
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-21 09:33:01 PDT
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.
Comment 4 Ed Morley [:emorley] 2012-05-23 08:06:58 PDT
https://hg.mozilla.org/mozilla-central/rev/b3570567bd5d

Note You need to log in before you can comment on or make changes to this bug.