Closed
Bug 1464794
Opened 6 years ago
Closed 6 years ago
Remove 'using mozilla::XYZ' from header files
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
Details
Attachments
(1 file, 2 obsolete files)
16.60 KB,
patch
|
anba
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•6 years ago
|
||
- Remove "using mozilla::{Forward,Move}" from js/src/ds/OrderedHashTable.h - Remove "using mozilla::FloatingPoint" from js/src/jit/MacroAssembler.h - Remove "using mozilla::DebugOnly" from js/src/jit/arm/MacroAssembler-arm.h - And then handle the fallout of the removal in lots of places.
Attachment #8981129 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 2•6 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=521a2f405912539382b674ac8ecabc8ed5ccf7f7 Try (with ARM fixed): https://treeherder.mozilla.org/#/jobs?repo=try&revision=35aac30c5c3e0f4203e603c69f4aa6ad9f840a08
Comment 3•6 years ago
|
||
Comment on attachment 8981129 [details] [diff] [review] bug1464794.patch Review of attachment 8981129 [details] [diff] [review]: ----------------------------------------------------------------- Could you add |using mozilla::Move;| and similar to the top of the non-header files, then not make the other changes to those files? Makes for lots smaller patches, and this is how we tend to write things now these days anyway. A quick ack on js/src suggests 446 instances of "using mozilla::", to get a sense of things.
Attachment #8981129 -
Flags: review?(jwalden+bmo) → review+
Comment 4•6 years ago
|
||
(In reply to Jeff Walden [:Waldo] from comment #3) > Could you add |using mozilla::Move;| and similar to the top of the > non-header files, then not make the other changes to those files? Don't we have the same problem with cpp files, due to the unified builds?
Comment 5•6 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] {backlog: 36} from comment #4) > Don't we have the same problem with cpp files, due to the unified builds? Somewhat yes, and that's why unified builds are somewhat stoopid. But that's why we have a non-unified build on treeherder. And the problem is qualitatively different for headers versus .cpp files: headers deliberately infect other code, .cpp do not except incidentally. If we were really serious about that solution, we would remove all |using| declarations at namespace scope and we would fully qualify everything that would otherwise depend on them. I don't see us doing that, and I'm not sure this is the bug to have that argument in anyway.
Assignee | ||
Comment 6•6 years ago
|
||
Updated per review comments, carrying r+.
Attachment #8981129 -
Attachment is obsolete: true
Attachment #8982446 -
Flags: review+
Assignee | ||
Comment 7•6 years ago
|
||
Try: https://hg.mozilla.org/try/rev/66e0deb6926fb5dfdbfdadd82312caf3c190c956
Keywords: checkin-needed
Assignee | ||
Comment 9•6 years ago
|
||
Rebased patch, carrying r+.
Attachment #8982446 -
Attachment is obsolete: true
Attachment #8982996 -
Flags: review+
Assignee | ||
Comment 10•6 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=59678f7799f8642b3c58a72e76cd7855e4f756cd
Keywords: checkin-needed
Comment 11•6 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4cc60824e00d Remove 'using mozilla::*' from header files in js/src. r=Waldo
Keywords: checkin-needed
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4cc60824e00d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•