Last Comment Bug 764616 - platform.h: In constructor ‘Sampler::Sampler(int, bool)’: warning: ‘Sampler::active_’ will be initialized after ... Atomic32 Sampler::paused_’ ... when initialized here [-Wreorder]
: platform.h: In constructor ‘Sampler::Sampler(int, bool)’: warning: ‘Sampler::...
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
Depends on:
Blocks: buildwarning 750989
  Show dependency treegraph
 
Reported: 2012-06-13 15:16 PDT by Daniel Holbert [:dholbert]
Modified: 2012-06-14 02:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.91 KB, patch)
2012-06-13 15:51 PDT, Daniel Holbert [:dholbert]
bgirard: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-06-13 15:16:43 PDT
../../../mozilla/tools/profiler/platform.h: In constructor ‘Sampler::Sampler(int, bool)’:
../../../mozilla/tools/profiler/platform.h:237:12: warning: ‘Sampler::active_’ will be initialized after [-Wreorder]
../../../mozilla/tools/profiler/platform.h:236:12: warning:   ‘Atomic32 Sampler::paused_’ [-Wreorder]
../../../mozilla/tools/profiler/platform-linux.cc:182:1: warning:   when initialized here [-Wreorder]

Looks like http://hg.mozilla.org/mozilla-central/rev/fdea12fb0063 added "paused_" to the constructor init lists in the wrong order. (after active_, instead of before active_)
Comment 1 Daniel Holbert [:dholbert] 2012-06-13 15:51:59 PDT
Created attachment 632926 [details] [diff] [review]
fix
Comment 2 Benoit Girard (:BenWa) 2012-06-13 15:59:24 PDT
Comment on attachment 632926 [details] [diff] [review]
fix

It pains me that compiler complain about this.
Comment 3 Daniel Holbert [:dholbert] 2012-06-13 16:26:59 PDT
Yeah. :) (It really only matters when the init list is invoking actual constructors and functions (instead of just initializing a boolean value) -- then, it's more important so that it's clear which functions actually get invoked first.)

Thanks for the review! Pushed to m-i:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/669274e1fcf7
Comment 4 :Ehsan Akhgari 2012-06-13 18:18:13 PDT
(In reply to Daniel Holbert [:dholbert] from comment #3)
> Yeah. :) (It really only matters when the init list is invoking actual
> constructors and functions (instead of just initializing a boolean value) --
> then, it's more important so that it's clear which functions actually get
> invoked first.)

IIRC even in that case the compiler will generate code to initialize the members in the order they're declared in the class.  This warning is only useful to tell the developer that the order in which they initialize class members in the definition of the constructor actually doesn't matter.  I also agree that warning this for POD types is a waste of everybody's time, and for the case where the initialization has side effects, the compiler can probably pick a better warning message!
Comment 5 Daniel Holbert [:dholbert] 2012-06-13 22:01:31 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #4)
> IIRC even in that case the compiler will generate code to initialize the
> members in the order they're declared in the class.

(Correct -- that's what I meant, sorry if it wasn't clear.)
Comment 6 Benoit Girard (:BenWa) 2012-06-13 22:10:44 PDT
Thanks for the clarification ^-^.
Comment 7 Ed Morley [:emorley] 2012-06-14 02:46:01 PDT
https://hg.mozilla.org/mozilla-central/rev/669274e1fcf7

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