Page MenuHomePhabricator

Automatically add include-what-you-use for when building in tree
Needs ReviewPublic

Authored by zturner on Apr 4 2017, 7:11 PM.

Details

Summary

It's a little bit annoying to have to manually edit files and then deal with git thinking that you've got untracked files, and thusly deleting them when you run git clean -fd. Although this is not an officially sanctioned LLVM tool, there seems to be little harm in having this logic here and it makes life easier for anyone who wants to use this.

Diff Detail

Event Timeline

zturner created this revision.Apr 4 2017, 7:11 PM
kimgr edited edge metadata.Apr 5 2017, 12:34 PM

As an include-what-you-use maintainer, I would very much welcome this. I've been too shy to suggest it myself ;-).

Not really sure who to add as a reviewer here, so + a few random people.

BTW, kimgr@, is there any particular reason you haven't tried to upstream the tool? Maybe even as not a standalone tool but as a set of checks in clang-tidy.

kimgr added a comment.Apr 6 2017, 12:51 PM

BTW, kimgr@, is there any particular reason you haven't tried to upstream the tool?
Maybe even as not a standalone tool but as a set of checks in clang-tidy.

Lack of time, mostly. IWYU was written before there was libtooling, and follows Google style, so we've felt it would be an odd bird in clang-tools-extra, for example.

rnk added inline comments.Apr 6 2017, 5:45 PM
clang/tools/.gitignore
12

LLVM doesn't currently have any .gitignore files for other projects, and they don't show up in git status. What's the difference here?

clang/tools/CMakeLists.txt
35

Maybe we should do llvm_add_implicit_projects(CLANG) here instead?

Or do we not want clang/tools to be a project extension point? Would IWYU build fine if we added it to llvm/projects or llvm/tools? Maybe we should just recommend that.

beanz added inline comments.Apr 7 2017, 11:54 AM
clang/tools/.gitignore
12

We actually should put this into the clang/.gitignore instead of a separate .gitignore in the tools directory. That is how we've done things in LLVM & Clang before.

clang/tools/CMakeLists.txt
35

Either llvm_add_implicit_projects(CLANG) or add_llvm_external_project(include-what-you-use include-what-you-use).

The former case would make this a generic extension point, which I think is good, the later would add just this project without needing to wrap it in an if.

I would prefer going the implicit route because I actually have a distaste for having code living in-tree that is specifically for supporting out-of-tree code.

kimgr added inline comments.Apr 8 2017, 1:14 AM
clang/tools/CMakeLists.txt
35

Would IWYU build fine if we added it to llvm/projects or llvm/tools? Maybe we should just recommend that.

IWYU has all dependencies explicitly listed to support out-of-tree builds too, so I think it could be made to work with some include/lib path setup. But it would feel weird layering-wise, since it depends so heavily on clang.

As for llvm_add_implicit_projects vs add_llvm_external_project, I'm guessing it doesn't make a difference for IWYU's CMake system? Mild preference for llvm_add_implicit_projects to keep mention of include-what-you-use out of Clang until we can form a coherent story about upstreaming.

kimgr added a comment.Feb 17 2018, 1:25 AM

Ping! It would be nice to get some traction on this.

This can be closed, IWYU no longer officially supports in-tree builds.