[PATCH] D114645: [CommandLine] Keep option default value unset if no cl::init() is used

Don Hinton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 8 15:29:46 PST 2021


hintonda added inline comments.


================
Comment at: llvm/unittests/Support/CommandLineTest.cpp:1793
+  StackOption<bool> OptA("a", cl::desc("option a"), cl::init(false));
+  StackOption<bool> OptB(cl::init(false),
       "b", cl::desc("option b -- This option turns on option a"),
----------------
yrouban wrote:
> hintonda wrote:
> > Are these additional `cl::init` calls needed? 
> Yes, I found them needed as the test failed otherwise.
> 
> For example, the first cl::init(false) for the option "a" when reset by cl::ResetAllOptionOccurrences() ...
> 
> CommandLine.h:
> ```
>   template <class T,
>             class = std::enable_if_t<std::is_assignable<T &, T>::value>>
>   void setDefaultImpl() {
>     const OptionValue<DataType> &V = this->getDefault();
>     if (V.hasValue())
>       this->setValue(V.getValue());
>   }
> ```
> 
> Without the explicit default value specified //V.hasValue()// is //None// and //setDefaultValueImple<bool>()// does not call //setValue()//. So the option is left not reset at line 1843.
> 
Ah, got it.  

Would it be possible to tweak `cl::ResetAllOptionOccurrences()` instead?  That way users don't have to modify their code.

I'm just worried about breaking code that's been around for a while.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114645/new/

https://reviews.llvm.org/D114645



More information about the llvm-commits mailing list