This is an archive of the discontinued LLVM Phabricator instance.

gn build: Add build files for most clang-tools-extra unit tests
ClosedPublic

Authored by thakis on Mar 30 2019, 5:08 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Mar 30 2019, 5:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2019, 5:08 PM
mbonadei accepted this revision.Mar 31 2019, 5:44 AM

LGTM, just a small style comment.

llvm/utils/gn/secondary/clang-tools-extra/unittests/clang-apply-replacements/BUILD.gn
3 ↗(On Diff #192995)

Why don't we use output_name here as well? It seems to be available for executables (https://gn.googlesource.com/gn/+/master/docs/reference.md#target-declarations-executable_declare-an-executable-target-variables). The current style seems to be lisp-case for target names and have output_name matching the cmake build.

This revision is now accepted and ready to land.Mar 31 2019, 5:44 AM
thakis marked an inline comment as done.Mar 31 2019, 9:46 AM

Thanks!

llvm/utils/gn/secondary/clang-tools-extra/unittests/clang-apply-replacements/BUILD.gn
3 ↗(On Diff #192995)

That's a good question. phosek asked about this too here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20181231/614211.html

I don't think there's a really good reason for this. I don't remember why I did it this way.

Trying to come up with a justification after the fact: A weak reason is that for static libraries it doesn't really matter what they're called on disk (other than to llvm-config's lit tests, which is why the GN build matches the cmake build -- else I wouldn't set the output name there), but for executables you need to know the name of the executable to run it, and it's maybe nice if the target has the same name as the executable for that reason.

This revision was automatically updated to reflect the committed changes.