This is an archive of the discontinued LLVM Phabricator instance.

[clang-scan-deps] Migrate to OptTable
ClosedPublic

Authored by abrachet on Dec 13 2022, 9:11 AM.

Diff Detail

Event Timeline

abrachet created this revision.Dec 13 2022, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2022, 9:11 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
abrachet requested review of this revision.Dec 13 2022, 9:11 AM
abrachet updated this revision to Diff 482535.Dec 13 2022, 10:00 AM
MaskRay added inline comments.Dec 17 2022, 12:01 PM
clang/tools/clang-scan-deps/ClangScanDeps.cpp
71

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

We use static for variables and functions.

Thanks for creating the patch!

clang/tools/clang-scan-deps/Opts.td
8

We generally prefer double-dash. - can be added for legacy reasons to help users migrate.

22

The convention is to have no trailing period for OptTable help messages. You can check some llvm/tools/llvm-* tools.

abrachet updated this revision to Diff 484163.Dec 19 2022, 9:39 PM
abrachet marked 3 inline comments as done.
abrachet added inline comments.
clang/tools/clang-scan-deps/Opts.td
8

Yeah unfortunately, most of the tests use -, so I assume users do as well

MaskRay accepted this revision.Dec 20 2022, 1:12 PM
This revision is now accepted and ready to land.Dec 20 2022, 1:12 PM
abrachet updated this revision to Diff 486012.Jan 3 2023, 9:23 AM
abrachet added a reviewer: thakis.
abrachet added a subscriber: thakis.

Updated bazel and gn builds. @MaskRay would you mind taking a quick look at the bazel changes, +@thakis for the gn changes. I have verified both work

Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 9:23 AM
curl -L 'https://reviews.llvm.org/D139949?download=1' | patch -p0
cd utils/bazel
bazel-5.0.0 build --config=generic_clang @llvm-project//clang

works. The Bazel change LGTM.

utils/bazel/llvm-project-overlay/clang/BUILD.bazel
2220

Move this before :driver

GMNGeoffrey added inline comments.
utils/bazel/llvm-project-overlay/clang/BUILD.bazel
2220

Buildifier sorts these. Why does it need to be before driver? If it's really required, add a comment to prevent buildifier from sorting: http://go/buildifier#do-not-sort

This revision was landed with ongoing or failed builds.Jan 13 2023, 10:12 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2023, 10:12 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
abrachet marked 2 inline comments as done.Jan 13 2023, 10:12 AM
abrachet added inline comments.
utils/bazel/llvm-project-overlay/clang/BUILD.bazel
2220

Move this before :driver

Done in commit

2220

Move this before :driver