Closed Bug 1444964 Opened 2 years ago Closed 2 years ago

Add #includes, 'using', etc. to let accessible/base build in non-unified-build mode

Categories

(Core :: Disability Access APIs, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

Details

Attachments

(3 files)

Bug 1444633 prompted me to get the rest of accessible/base to build successfully in non-unified mode.

(This bug is one in a series of occasioanl "fix latent missing-#include/declaration/namespace compile errors which are currently being hidden by unified builds" bugs.  This is worth doing periodically, because otherwise these issues are just landmines that get discovered when we add new .cpp files, make another change that reshuffles the unification, or adjust #includes in seemingly-unrelated files.)

Patches coming up.
Here's the diagnostic patch (not intended for landing) to disable unified builds and flush out all brokenness here.
Attached file raw build output
Here's the raw build output, from "./mach build accessible/base -j100" after applying the previous diagnostic patch.

I added a chunk at the beginning with per-filename filtering for the files touched in this patch, for convenience.

(I'm hoping this helps in review, so if you're wondering why a particular file is getting a particular include/namespace/etc, this will tell you why it needs it.)
(In reply to Daniel Holbert [:dholbert] from comment #2)
> Created attachment 8958152 [details]
> raw build output
> [...]
> (I'm hoping this helps in review, so if you're wondering why a particular
> file is getting a particular include/namespace/etc, this will tell you why
> it needs it.)

(Note that this log isn't *quite* comprehensive, for this patch -- there are a few errors that I'm fixing here that aren't in this log, because they only show up as a second-order error after another error has been fixed. E.g. After EventTree.h gets the forward-decl for "NotificationController" (to fix an error in this log), *then* the compiler start warning about EventTree.cpp not having a complete definition for that type, for its Controller()->... usages (fixed in this patch, but error not shown in this log).)

Anyway, let me know if you have questions about any particular tweak here -- but this is all pretty trivial so I'll try to avoid overkill on pre-justifying all of the changes. :)
Comment on attachment 8958157 [details]
Bug 1444964: Add needed #includes and namespaces to fix non-unified build bustage in accessible/base.

https://reviewboard.mozilla.org/r/227102/#review232998

this looks reasonable, no conerns, I would try to avoid to add more includes into headers, but that would require more -inl files
Attachment #8958157 - Flags: review?(surkov.alexander) → review+
Comment on attachment 8958157 [details]
Bug 1444964: Add needed #includes and namespaces to fix non-unified build bustage in accessible/base.

https://reviewboard.mozilla.org/r/227102/#review232998

Thanks!

(Agreed in spirit RE worthiness of avoiding unnecessary #includes in headers.  And at this point, these added ones are indeed all necessary.  We could refactor to spin off -inl files to make them unnecessary, but that's a bit more involved.)
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89c65fe897e5
Add needed #includes and namespaces to fix non-unified build bustage in accessible/base. r=surkov
https://hg.mozilla.org/mozilla-central/rev/89c65fe897e5
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.