HomePhabricator

[Support][CommandLine] Delete unused llvm::cl::ParseEnvrironmentOptions

Authored by MaskRay on Jul 31 2020, 10:46 AM.

Description

[Support][CommandLine] Delete unused llvm:🆑:ParseEnvrironmentOptions

The function was added in 2003. It is not used and can be emulated with ParseCommandLineOptions.

Details

Committed
MaskRayJul 31 2020, 10:48 AM
Parents
rG93fd8dbdc250: [PowerPC] Add Vector String Isolate instruction definitions and MC Tests
Branches
Unknown
Tags
Unknown

Event Timeline

@MaskRay why was it necessary to delete this? It is used by downstream projects (like Julia). I agree that it isn't too much hassle to implement, but every tool that uses LLVM as a library will have to write a version themselves.

MaskRay added a comment.EditedOct 18 2020, 9:11 PM

@MaskRay why was it necessary to delete this? It is used by downstream projects (like Julia). I agree that it isn't too much hassle to implement, but every tool that uses LLVM as a library will have to write a version themselves.

It was a maintenance burden for very little benefit. Julia can just use ParseCommandLineOptions.

These cl::opt registered options are not user-facing options. Embedding them in applications need prefixes such as -plugin-opt= or -mllvm. They should be used directly anyway.

Yeah, I basically copy-and-pasted the original implementation, since we are indeed parsing this from the environment. https://github.com/JuliaLang/julia/pull/38092/commits/72c2136dfad7a535dba563c0f37f3c70da5558a2

It was a maintenance burden for very little benefit.

Not sure that I agree with that statement. The code as is hasn't changed in a long while and this moves the maintenance from one location to the different consumers. Just because there is no in-tree consumer doesn't meant something is unused.

MaskRay added a comment.EditedOct 18 2020, 10:22 PM

Yeah, I basically copy-and-pasted the original implementation, since we are indeed parsing this from the environment. https://github.com/JuliaLang/julia/pull/38092/commits/72c2136dfad7a535dba563c0f37f3c70da5558a2

It was a maintenance burden for very little benefit.

Not sure that I agree with that statement. The code as is hasn't changed in a long while and this moves the maintenance from one location to the different consumers. Just because there is no in-tree consumer doesn't meant something is unused.

Please see the edited comment before your reply. It is questionable for a downstream project to parse and interpret LLVM internal options directly. It should at least to have a -mllvm or --plugin-opt= prefix.

I also don't agree that "every tool that uses LLVM as a library will have to write a version themselves."

It is questionable for a downstream project to parse and interpret LLVM internal options directly. It should at least to have a -mllvm or --plugin-opt= prefix.

Absolutely. Which is why Julia doesn't pass options to LLVM through command line flags, but by uses an environment variable. This is mostly used for things like -debug-pass.

Searching for this usage pattern with I found it in evmjit as well.