This is an archive of the discontinued LLVM Phabricator instance.

[sancov] Switch to OptTable from llvm::cl
ClosedPublic

Authored by avillega on Jul 12 2023, 2:14 PM.

Details

Summary

Switch the parse of command line options from llvm::cl to OptTable.

The motivation for this change is to continue adding llvm based tools
to the llvm driver multicall. For more information about the proposal
and motivation, please see https://discourse.llvm.org/t/rfc-llvm-busybox-proposal/58494

Diff Detail

Event Timeline

avillega created this revision.Jul 12 2023, 2:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 2:14 PM
avillega requested review of this revision.Jul 12 2023, 2:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 2:14 PM
leonardchan added inline comments.
llvm/tools/sancov/Opts.td
51

use_default_ignorelist to match the original flag name

llvm/utils/gn/secondary/llvm/tools/sancov/BUILD.gn
1 ↗(On Diff #541256)

I believe the gn sub-build is also peripheral tier and actually updated by a script maintained by @thakis, so BUILD.gn changes shouldn't be included here.

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
4675 ↗(On Diff #541256)

I believe the bazel build is part of the peripheral tier and maintained by someone else who would make the necessary bazel changes. You only need to update the cmake files. Alternatively, if you want to land this also, I think we'd need to find a bazel reviewer to make sure this is correct.

avillega updated this revision to Diff 541270.Jul 17 2023, 4:57 PM

Review comments and rebase.

avillega marked an inline comment as done.Jul 17 2023, 4:58 PM

I've removed the Bazer and gn files.

avillega marked 2 inline comments as done.Jul 17 2023, 4:58 PM
avillega added inline comments.Jul 18 2023, 9:46 AM
llvm/tools/sancov/Opts.td
51

Good catch, thanks.

leonardchan accepted this revision.Jul 19 2023, 2:41 PM

LGTM but probably give it a day before landing to give others time to respond

llvm/tools/sancov/sancov.cpp
61

This anonymous namespace might not be necessary since it's already in an anonymous namespace.

This revision is now accepted and ready to land.Jul 19 2023, 2:41 PM
This revision was landed with ongoing or failed builds.Jul 25 2023, 5:07 PM
This revision was automatically updated to reflect the committed changes.