1128578 - Add MOZ_OVERRIDE annotations in TableTicker.h

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Daniel Holbert [:dholbert]]

Starting with bug 1127498's mozilla-central changeset 29ac4f012129, I can't build with clang 3.6 and --enable-warnings-as-errors, due to a few of missing MOZ_OVERRIDES:
{
 0:01.29 In file included from /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform-linux.cc:73:
 0:01.29 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/TableTicker.h:161:16: error: 'Tick' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 0:01.29   virtual void Tick(TickSample* sample);
 0:01.29                ^
 0:01.29 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform.h:303:16: note: overridden virtual function is here
 0:01.29   virtual void Tick(TickSample* sample) = 0;
 0:01.29                ^
 0:01.29 In file included from /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform-linux.cc:73:
 0:01.29 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/TableTicker.h:164:24: error: 'GetBacktrace' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 0:01.29   virtual SyncProfile* GetBacktrace();
 0:01.29                        ^
 0:01.29 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform.h:306:24: note: overridden virtual function is here
 0:01.29   virtual SyncProfile* GetBacktrace() = 0;
 0:01.30                        ^
 0:01.30 In file included from /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform-linux.cc:73:
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/TableTicker.h:167:16: error: 'RequestSave' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 0:01.30   virtual void RequestSave()
 0:01.30                ^
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform.h:309:16: note: overridden virtual function is here
 0:01.30   virtual void RequestSave() = 0;
 0:01.30                ^
 0:01.30 In file included from /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform-linux.cc:73:
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/TableTicker.h:177:16: error: 'HandleSaveRequest' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 0:01.30   virtual void HandleSaveRequest();
 0:01.30                ^
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform.h:311:16: note: overridden virtual function is here
 0:01.30   virtual void HandleSaveRequest() = 0;
 0:01.30                ^
 0:01.30 In file included from /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform-linux.cc:73:
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/TableTicker.h:206:8: error: 'ProfileThreads' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 0:01.30   bool ProfileThreads() const { return mProfileThreads; }
 0:01.30        ^
 0:01.30 /scratch/work/builds/mozilla-inbound/mozilla/tools/profiler/platform.h:329:16: note: overridden virtual function is here
 0:01.30   virtual bool ProfileThreads() const = 0;
 0:01.30                ^
}

(From briefly poking at hg blame, it looks like these have been missing since well before that bug; but strangely, clang only started warning about them with that landing. Meh.)

Filing this bug to add MOZ_OVERRIDE & fix these warnings.

Comment 1

[Markus Stange [:mstange]]

Are you already preparing a patch or should I write one?

Comment 2

[Daniel Holbert [:dholbert]]

Thanks -- I just landed, taking advantage of blanket r+ that ehsan granted me in bug 1126447. :)

pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/64db189b540a

Comment 3

[Markus Stange [:mstange]]

Thanks!

Comment 4

[Daniel Holbert [:dholbert]]

Side note: I was briefly worried about this one...
>   bool ProfileThreads() const { return mProfileThreads; }
...since it's not explicitly marked as 'virtual' and is in the middle of a bunch of other non-overriding methods -- so it looked like maybe it wasn't intended to override.

However, it looks like it *is* indeed supposed to override -- that function declaration was added[1] in the same changeset that added the overridden function in the parent[2].  So it looks like it's an intentional override there, rather than something that slipped in accidentally from a name collision.
[1] http://hg.mozilla.org/mozilla-central/rev/f2e44e02f874#l9.81
[2] http://hg.mozilla.org/mozilla-central/rev/f2e44e02f874#l14.137

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/64db189b540a