awarzynski updated this revision to Diff 360100.
awarzynski added a comment.

Switch from `BoolFOption` to `OptInFFlag`/`OptOutFFlag`

I've refactored `OptInFFlag`/`OptOutFFlag` a tiny bit and created 
specialisations for `clang -cc1` (`OptInCC1FFlag`/`OptOutCC1FFlag`). I have 
added some comments in Options.td to clarify the semantics of 
`OptInFFlag`/`OptOutFFlag`. Does this make sense to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

Files:
  clang/include/clang/Driver/Options.td
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90

Index: flang/test/Driver/driver-help.f90
===================================================================
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -40,7 +40,6 @@
 ! HELP-NEXT:                        Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes          Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
-! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -fopenacc              Enable OpenACC
 ! HELP-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fxor-operator         Enable .XOR. as a synonym of .NEQV.
@@ -68,6 +67,8 @@
 ! HELP-FC1-NEXT: -E                     Only run the preprocessor
 ! HELP-FC1-NEXT: -falternative-parameter-statement
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
+! HELP-FC1-NEXT: -fanalyzed-objects-for-unparse
+! HELP-FC1-NEXT:                        Use the analyzed objects when unparsing
 ! HELP-FC1-NEXT: -fbackslash            Specify that backslash in string introduces an escape character
 ! HELP-FC1-NEXT: -fdebug-dump-all       Dump symbols and the parse tree after the semantic checks
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree-no-sema
Index: flang/test/Driver/driver-help-hidden.f90
===================================================================
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -40,7 +40,6 @@
 ! CHECK-NEXT:                        Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flarge-sizes          Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
-! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! CHECK-NEXT: -fopenacc              Enable OpenACC
 ! CHECK-NEXT: -fopenmp               Parse OpenMP pragmas and generate parallel code.
 ! CHECK-NEXT: -fxor-operator         Enable .XOR. as a synonym of .NEQV.
Index: flang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -277,33 +277,27 @@
     }
   }
 
-  if (const llvm::opt::Arg *arg =
-          args.getLastArg(clang::driver::options::OPT_fimplicit_none,
-              clang::driver::options::OPT_fno_implicit_none)) {
-    opts.features_.Enable(
-        Fortran::common::LanguageFeature::ImplicitNoneTypeAlways,
-        arg->getOption().matches(clang::driver::options::OPT_fimplicit_none));
-  }
-  if (const llvm::opt::Arg *arg =
-          args.getLastArg(clang::driver::options::OPT_fbackslash,
-              clang::driver::options::OPT_fno_backslash)) {
-    opts.features_.Enable(Fortran::common::LanguageFeature::BackslashEscapes,
-        arg->getOption().matches(clang::driver::options::OPT_fbackslash));
-  }
-  if (const llvm::opt::Arg *arg =
-          args.getLastArg(clang::driver::options::OPT_flogical_abbreviations,
-              clang::driver::options::OPT_fno_logical_abbreviations)) {
-    opts.features_.Enable(
-        Fortran::common::LanguageFeature::LogicalAbbreviations,
-        arg->getOption().matches(
-            clang::driver::options::OPT_flogical_abbreviations));
-  }
-  if (const llvm::opt::Arg *arg =
-          args.getLastArg(clang::driver::options::OPT_fxor_operator,
-              clang::driver::options::OPT_fno_xor_operator)) {
-    opts.features_.Enable(Fortran::common::LanguageFeature::XOROperator,
-        arg->getOption().matches(clang::driver::options::OPT_fxor_operator));
-  }
+  // -f{no-}implicit-none
+  opts.features_.Enable(
+      Fortran::common::LanguageFeature::ImplicitNoneTypeAlways,
+      args.hasFlag(clang::driver::options::OPT_fimplicit_none,
+          clang::driver::options::OPT_fno_implicit_none, false));
+
+  // -f{no-}backslash
+  opts.features_.Enable(Fortran::common::LanguageFeature::BackslashEscapes,
+      args.hasFlag(clang::driver::options::OPT_fbackslash,
+          clang::driver::options::OPT_fno_backslash, false));
+
+  // -f{no-}logical-abbreviations
+  opts.features_.Enable(Fortran::common::LanguageFeature::LogicalAbbreviations,
+      args.hasFlag(clang::driver::options::OPT_flogical_abbreviations,
+          clang::driver::options::OPT_fno_logical_abbreviations, false));
+
+  // -f{no-}xor-operator
+  opts.features_.Enable(Fortran::common::LanguageFeature::XOROperator,
+      args.hasFlag(clang::driver::options::OPT_fxor_operator,
+          clang::driver::options::OPT_fno_xor_operator, false));
+
   if (args.hasArg(
           clang::driver::options::OPT_falternative_parameter_statement)) {
     opts.features_.Enable(Fortran::common::LanguageFeature::OldStyleParameter);
@@ -404,11 +398,10 @@
     res.SetModuleFileSuffix(moduleSuffix->getValue());
   }
 
-  // -fno-analyzed-objects-for-unparse
-  if (args.hasArg(
-          clang::driver::options::OPT_fno_analyzed_objects_for_unparse)) {
-    res.SetUseAnalyzedObjectsForUnparse(false);
-  }
+  // -f{no-}analyzed-objects-for-unparse
+  res.SetUseAnalyzedObjectsForUnparse(
+      args.hasFlag(clang::driver::options::OPT_fanalyzed_objects_for_unparse,
+          clang::driver::options::OPT_fno_analyzed_objects_for_unparse, true));
 
   return diags.getNumErrors() == numErrorsBefore;
 }
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -243,6 +243,7 @@
 def clang_ignored_gcc_optimization_f_Group : OptionGroup<
   "<clang_ignored_gcc_optimization_f_Group>">, Group<f_Group>, Flags<[Ignored]>;
 
+class EmptyKPM<string base> : KeyPathAndMacro<"", "", ""> {}
 class DiagnosticOpts<string base>
   : KeyPathAndMacro<"DiagnosticOpts->", base, "DIAG_"> {}
 class LangOpts<string base>
@@ -268,27 +269,37 @@
 class MigratorOpts<string base>
   : KeyPathAndMacro<"MigratorOpts.", base, "MIGRATOR_"> {}
 
-// A boolean option which is opt-in in CC1. The positive option exists in CC1 and
-// Args.hasArg(OPT_ffoo) can be used to check that the flag is enabled.
-// This is useful if the option is usually disabled.
-// Use this only when the option cannot be declared via BoolFOption.
-multiclass OptInFFlag<string name, string pos_prefix, string neg_prefix="",
-                      string help="", list<OptionFlag> flags=[]> {
+// Boolean options which are opt-in (`OptInFFlag`) or opt-out (`OptOutFFlag`).
+// Unlike for e.g. BoolFOption, the opt-in/opt-out semantics are to be
+// implemented in the driver using C++ rather than in this file using TableGen.
+// Here, we simply skip the help text for the `-ffoo` (opt-out) or `-fno_foo`
+// (opt-in`) version so that it's not printed with `<driver> -help`. Inside
+// the driver, you can use
+//  * Args.hasArg(OPT_ffoo) to check that the flag is enabled,
+//  * Args.hasArg(OPT_fno_foo) to check that the flag is disabled,
+// This is useful if the option is usually disabled. Use this only when the
+// option cannot be declared via BoolFOption.
+multiclass OptInFFlag<string name, string help="", list<OptionFlag> flags=[]> {
   def f#NAME : Flag<["-"], "f"#name>, Flags<[CC1Option] # flags>,
-               Group<f_Group>, HelpText<pos_prefix # help>;
+               Group<f_Group>, HelpText<help>;
   def fno_#NAME : Flag<["-"], "fno-"#name>, Flags<flags>,
-                  Group<f_Group>, HelpText<neg_prefix # help>;
+                  Group<f_Group>;
 }
-
-// A boolean option which is opt-out in CC1. The negative option exists in CC1 and
-// Args.hasArg(OPT_fno_foo) can be used to check that the flag is disabled.
-// Use this only when the option cannot be declared via BoolFOption.
-multiclass OptOutFFlag<string name, string pos_prefix, string neg_prefix,
-                       string help="", list<OptionFlag> flags=[]> {
+multiclass OptOutFFlag<string name, string help="", list<OptionFlag> flags=[]> {
   def f#NAME : Flag<["-"], "f"#name>, Flags<flags>,
-               Group<f_Group>, HelpText<pos_prefix # help>;
+               Group<f_Group>;
   def fno_#NAME : Flag<["-"], "fno-"#name>, Flags<[CC1Option] # flags>,
-                  Group<f_Group>, HelpText<neg_prefix # help>;
+                  Group<f_Group>, HelpText<help>;
+}
+
+// Specialisation of OptInFFlag/OptOutFFflag for CC1
+multiclass
+    OptInCC1FFlag<string name, string help = "", list<OptionFlag> flags = []> {
+  defm NAME : OptInFFlag<name, help, [CC1Option] #flags>;
+}
+multiclass
+    OptOutCC1FFlag<string name, string help = "", list<OptionFlag> flags = []> {
+  defm NAME : OptInFFlag<name, help, [CC1Option] #flags>;
 }
 
 // Creates a positive and negative flags where both of them are prefixed with
@@ -1255,7 +1266,7 @@
   CodeGenOpts<"Addrsig">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Emit">, NegFlag<SetFalse, [], "Don't emit">,
   BothFlags<[CoreOption], " an address-significance table">>;
-defm blocks : OptInFFlag<"blocks", "Enable the 'blocks' language feature", "", "", [CoreOption]>;
+defm blocks : OptInCC1FFlag<"blocks", "Enable the 'blocks' language feature", [CoreOption]>;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group<f_Group>;
 defm borland_extensions : BoolFOption<"borland-extensions",
   LangOpts<"Borland">, DefaultFalse,
@@ -1271,7 +1282,7 @@
   Flags<[CC1Option]>, MetaVarName<"<version>">, Values<"<major>.<minor>,latest">,
   HelpText<"Attempt to match the ABI of Clang <version>">;
 def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group<f_Group>;
-defm color_diagnostics : OptInFFlag<"color-diagnostics", "Enable", "Disable", " colors in diagnostics",
+defm color_diagnostics : OptInCC1FFlag<"color-diagnostics", "Enable colors in diagnostics",
   [CoreOption, FlangOption]>;
 def fdiagnostics_color : Flag<["-"], "fdiagnostics-color">, Group<f_Group>,
   Flags<[CoreOption, NoXarchOption]>;
@@ -1389,8 +1400,8 @@
     HelpText<"Do not elide types when printing diagnostics">,
     MarshallingInfoNegativeFlag<DiagnosticOpts<"ElideType">>;
 def feliminate_unused_debug_symbols : Flag<["-"], "feliminate-unused-debug-symbols">, Group<f_Group>;
-defm eliminate_unused_debug_types : OptOutFFlag<"eliminate-unused-debug-types",
-  "Do not emit ", "Emit ", " debug info for defined but unused types">;
+defm eliminate_unused_debug_types : OptOutCC1FFlag<"eliminate-unused-debug-types",
+  "Do not emit debug info for defined but unused types">;
 def femit_all_decls : Flag<["-"], "femit-all-decls">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Emit all declarations, even if unused">,
   MarshallingInfoFlag<LangOpts<"EmitAllDecls">>;
@@ -1489,7 +1500,7 @@
 def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group<f_Group>,
   Flags<[CC1Option]>, MarshallingInfoString<CodeGenOpts<"SymbolPartition">>;
 
-defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap memory profiling">;
+defm memory_profile : OptInCC1FFlag<"memory-profile", "Enable heap memory profiling">;
 def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
     Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<directory>">,
     HelpText<"Enable heap memory profiling and dump results into <directory>">;
@@ -2130,10 +2141,8 @@
   LangOpts<"PCHInstantiateTemplates">, DefaultFalse,
   PosFlag<SetTrue, [], "Instantiate templates already while building a PCH">,
   NegFlag<SetFalse>, BothFlags<[CC1Option, CoreOption]>>;
-defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
-  "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
-defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
-  "debug info for types in an object file built from this PCH and do not generate them elsewhere">;
+defm pch_codegen: OptInCC1FFlag<"pch-codegen", "Generate code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
+defm pch_debuginfo: OptInCC1FFlag<"pch-debuginfo", "Generate debug info for types in an object file built from this PCH and do not generate them elsewhere">;
 
 def fimplicit_module_maps : Flag <["-"], "fimplicit-module-maps">, Group<f_Group>,
   Flags<[NoXarchOption, CC1Option]>,
@@ -4487,28 +4496,19 @@
   HelpText<"Set the default real kind to an 8 byte wide type">;
 def flarge_sizes : Flag<["-"],"flarge-sizes">, Group<f_Group>,
   HelpText<"Use INTEGER(KIND=8) for the result type in size-related intrinsics">;
-def fbackslash : Flag<["-"], "fbackslash">, Group<f_Group>,
-  HelpText<"Specify that backslash in string introduces an escape character">,
-  DocBrief<[{Change the interpretation of backslashes in string literals from
-a single backslash character to "C-style" escape characters.}]>;
-def fno_backslash : Flag<["-"], "fno-backslash">, Group<f_Group>;
-def fxor_operator : Flag<["-"], "fxor-operator">, Group<f_Group>,
-  HelpText<"Enable .XOR. as a synonym of .NEQV.">;
-def fno_xor_operator : Flag<["-"], "fno-xor-operator">, Group<f_Group>;
-def flogical_abbreviations : Flag<["-"], "flogical-abbreviations">, Group<f_Group>,
-  HelpText<"Enable logical abbreviations">;
-def fno_logical_abbreviations : Flag<["-"], "fno-logical-abbreviations">, Group<f_Group>;
-def fimplicit_none : Flag<["-"], "fimplicit-none">, Group<f_Group>,
-  HelpText<"No implicit typing allowed unless overridden by IMPLICIT statements">;
-def fno_implicit_none : Flag<["-"], "fno-implicit-none">, Group<f_Group>;
 def falternative_parameter_statement : Flag<["-"], "falternative-parameter-statement">, Group<f_Group>,
   HelpText<"Enable the old style PARAMETER statement">;
 def fintrinsic_modules_path : Separate<["-"], "fintrinsic-modules-path">,  Group<f_Group>, MetaVarName<"<dir>">,
   HelpText<"Specify where to find the compiled intrinsic modules">,
   DocBrief<[{This option specifies the location of pre-compiled intrinsic modules, 
   if they are not in the default location expected by the compiler.}]>;
+defm backslash : OptInFFlag<"backslash", "Specify that backslash in string introduces an escape character">;
+defm xor_operator : OptInFFlag<"xor-operator", "Enable .XOR. as a synonym of .NEQV.">;
+defm logical_abbreviations : OptInFFlag<"logical-abbreviations", "Enable logical abbreviations">;
+defm implicit_none : OptInFFlag<"implicit-none", "No implicit typing allowed unless overridden by IMPLICIT statements">;
 }
 
+
 def J : JoinedOrSeparate<["-"], "J">,
   Flags<[RenderJoined, FlangOption, FC1Option, FlangOnlyOption]>,
   Group<gfortran_Group>,
@@ -4561,12 +4561,11 @@
 
 def module_suffix : Separate<["-"], "module-suffix">,  Group<f_Group>, MetaVarName<"<suffix>">,
   HelpText<"Use <suffix> as the suffix for module files (the default value is `.mod`)">;
-def fanalyzed_objects_for_unparse : Flag<["-"],
-  "fanalyzed-objects-for-unparse">,  Group<f_Group>;
-def fno_analyzed_objects_for_unparse : Flag<["-"],
-  "fno-analyzed-objects-for-unparse">,  Group<f_Group>,
-  HelpText<"Do not use the analyzed objects when unparsing">;
 
+defm analyzed_objects_for_unparse : BoolFOption<"analyzed-objects-for-unparse", EmptyKPM<"">, DefaultTrue,
+  PosFlag<SetTrue, [], "Use ">,
+  NegFlag<SetFalse, [], "Do not use ">,
+  BothFlags<[], "the analyzed objects when unparsing">>;
 }
 
 //===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to