Closed
Bug 772807
Opened 12 years ago
Closed 12 years ago
Try "include-what-you-use" on editor/
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: ayg, Assigned: ayg)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
See bug 634839. In theory, it might cut down on compile times. In practice, it seems buggy and it didn't create a big improvement for me. It got down compilation of editor/ on a hot cache with ccache cleared from maybe 26s to 25s. On the other hand, it's possible that it will reduce incremental compile times by removing unnecessary dependencies. And it alphabetizes #includes, which is a nice bonus! And adds comments saying what the files are used for.
For a lot of files it only adds includes, for things that are otherwise only indirectly included. But for instance, in nsEditor.h it converts a lot of includes to forward declarations -- it figured out that things like nsIContent.h didn't have to be included. It got nsHTMLCSSUtils.h down to four includes -- nsCOMPtr, nsTArray, prtypes, and nscore.
Sometimes I don't know what it thinks it's doing -- like including nsAString.h, nsString.h, and nsStringFwd.h in the same file. But that doesn't hurt anything, I guess.
I had to patiently teach it not to include stuff like string-template-def-char.h, but I think I mostly succeeded.
The command I'm using is this, having gotten SVN checkouts as described in bug 634839:
find editor -type f -exec touch {} + && make -k -j16 -C objdir-ff-release/editor/libeditor/base CXX=/tmp/llvm/Release+Asserts/bin/include-what-you-use 2>&1 | tee /tmp/iwyu.out | python /tmp/llvm/tools/clang/tools/include-what-you-use/fix_includes.py --nosafe_headers --comments
I go directory-by-directory because it tends to die. I have to do manual fixups after that, mostly due to <http://code.google.com/p/include-what-you-use/issues/detail?id=74>.
So in conclusion, this was interesting and doesn't hurt anything, but in retrospect, probably was not worth the time. :/
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #641021 -
Flags: review?(ehsan)
Assignee | ||
Comment 2•12 years ago
|
||
I should add -- I add a bunch of iwyu pragma comments in various files outside of editor/. Let me know if you think I should ask the owners of those modules for that. Also, I did look over the include changes for attempts to include private headers instead of public ones, but I might have missed some.
Comment 3•12 years ago
|
||
Comment on attachment 641021 [details] [diff] [review]
Patch
Review of attachment 641021 [details] [diff] [review]:
-----------------------------------------------------------------
The build failed on Windows. Can you please fix that before I review this? The patch is sort of painful to review... ;-)
Attachment #641021 -
Flags: review?(ehsan)
Comment 4•12 years ago
|
||
Oh, and thanks a lot for taking this on! I've been meaning to play with IWYU for such a long time, and you finally beat me to it. :-)
Assignee | ||
Comment 5•12 years ago
|
||
New try, compiled successfully on all platforms: https://tbpl.mozilla.org/?tree=Try&rev=142c891a968b
Attachment #641021 -
Attachment is obsolete: true
Attachment #641855 -
Flags: review?(ehsan)
Comment 6•12 years ago
|
||
Comment on attachment 641855 [details] [diff] [review]
Patch v2
Review of attachment 641855 [details] [diff] [review]:
-----------------------------------------------------------------
Great! I'll land this right now, since this very easily gets bitrotten!
Attachment #641855 -
Flags: review?(ehsan) → review+
Comment 7•12 years ago
|
||
Target Milestone: --- → mozilla16
Comment 8•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 9•12 years ago
|
||
Thanks!
You need to log in
before you can comment on or make changes to this bug.
Description
•