jansvoboda11 added a comment.

That's great to hear, thanks.



================
Comment at: clang/include/clang/Driver/Options.td:371
+  // TODO: Assert that the flags have different value.
+  // TODO: Assert that only one of the flags can be implied.
+
----------------
Does TableGen support some kind of assertion mechanism?


================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:226
   ASSERT_THAT(GeneratedArgs,
-              Contains(StrEq("-fno-experimental-new-pass-manager")));
+              Not(Contains(StrEq("-fno-experimental-new-pass-manager"))));
   ASSERT_THAT(GeneratedArgs,
----------------
dexonsmith wrote:
> Can you clarify why this was dropped? Was it previously emitted due to a 
> limitation in the implementation, or are we no longer supporting options that 
> always get emitted for clarity?
This option was the only one using the old infrastructure 
(`BooleanMarshalledFFlag`).
It was set up to always generate the flag, even when it wasn't necessary (this 
flag sets the keypath to its default value).

I think we should aim to generate only command line arguments that are 
necessary to preserve the original invocation semantics.
I imagine this will be useful when debugging: one won't need to scan hundreds 
of flags that don't have any effect on CompilerInvocation.


================
Comment at: llvm/include/llvm/Option/OptParser.td:176
   : MarshallingInfoFlag<keypath, defaultvalue> {
-  bit ShouldAlwaysEmit = 1;
-  code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
----------------
Removal of this property causes the disappearance of 
`-fno-experimental-new-pass-manager` from list of generated arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to