[PATCH] D82922: [NewPM] Add explicit init value to -enable-new-pm

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks marked an inline comment as done.
aeubanks added inline comments.


================
Comment at: llvm/tools/opt/opt.cpp:75
+static cl::opt<bool> EnableNewPassManager(
+    "enable-new-pm", cl::desc("Enable the new pass manager"), cl::init(false));
 
----------------
MaskRay wrote:
> aeubanks wrote:
> > MaskRay wrote:
> > > aeubanks wrote:
> > > > MaskRay wrote:
> > > > > false is the default value so this appears to be NFC...
> > > > I can more easily change the default value to true and test this way.
> > > > If there is a better way I'd like to know.
> > > Adding/removing `cl::init(true)` is very straightforward. Why is this NFC change needed?
> > It's easier for me to replace "false" with "true" than to type the whole thing out.
> > Also, at some point I want to add a CMake variable which controls this flag's default value (same as the one that controls clang's NPM default behavior), and this is one step to that.
> If adding a cmake variable is like moving from 0 to 1, then this NFC patch seems to me 0 to 0.1
> 
> I wonder why we didn't move directly to 1. This patch added unnecessary churn to the history.
Ok, maybe the CMake argument isn't so strong, but this patch still does make my life just a little easier when testing NPM with check-llvm. I'm not sure where the threshold for a patch's usefulness is for it to be worth a commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82922