Closed Bug 1142181 Opened 5 years ago Closed 5 years ago

ProfilerBacktrace.cpp should #include its own .h file first

Categories

(Core :: Gecko Profiler, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: dholbert, Assigned: chiajung)

References

Details

Attachments

(1 file, 2 obsolete files)

As noted in bug 1129249 comment 38, ProfilerBacktrace.h currently is missing a forward-decl for "JSStreamWriter".

We should make ProfilerBacktrace.h be the first #include in its .cpp file (as is convention in gecko), to catch issues like this proactively.
(Right now ProfilerBacktrace.cpp has:

> #include "JSStreamWriter.h"
> #include "ProfilerBacktrace.h"
> #include "SyncProfile.h"

which masks the missing forward-decl, because it sees the JSStreamWriter class-definition before it sees its .h file with the missing forward decl. If we put the "ProfilerBacktrace.h" include first, then we would've hit this error when compiling ProfilerBacktrace.cpp and caught it sooner.)
Assignee: nobody → chung
Attached patch v1 (obsolete) — Splinter Review
Attachment #8576456 - Flags: review?(aklotz)
Blocks: 1129249
Comment on attachment 8576456 [details] [diff] [review]
v1

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

::: tools/profiler/ProfilerBacktrace.cpp
@@ +7,1 @@
>  #include "ProfilerBacktrace.h"

Please add a blank line between the ProfilerBacktrace.h and the JSStreamWriter.h includes.
Attachment #8576456 - Flags: review?(aklotz) → review+
Attached patch v1 final (obsolete) — Splinter Review
Comment addressed
Attachment #8576456 - Attachment is obsolete: true
Attachment #8577819 - Flags: review+
Attached patch v1 final(add r=)Splinter Review
Add reviewer info, 
Try ticket:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=649ee64c216c
Attachment #8577819 - Attachment is obsolete: true
Attachment #8578444 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/56f421903a91
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.