[PATCH] D48266: [Driver] Add -fno-digraphs

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 9 14:27:08 PDT 2018


rsmith added inline comments.


================
Comment at: include/clang/Driver/Options.td:1337-1340
+def fdigraphs : Flag<["-"], "fdigraphs">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Enable alternative token representations '<:', ':>', '<%', '%>', '%:' (default)">;
+def fno_digraphs : Flag<["-"], "fno-digraphs">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Disallow alternative token representations '<:', ':>', '<%', '%>', '%:'">;
----------------
It'd make sense to also list `%:%:` here, particularly because it is controlled by this flag and isn't technically a digraph.


================
Comment at: lib/Frontend/CompilerInvocation.cpp:2181
 
+  if (const Arg* A = Args.getLastArg(OPT_fdigraphs, OPT_fno_digraphs)) {
+    // Prevent the user from enabling or disabling digraphs when they are not supported.
----------------
` *` not `* `, please.


================
Comment at: lib/Frontend/CompilerInvocation.cpp:2183-2185
+    if (!Opts.Digraphs && LangStdArg)
+      Diags.Report(diag::err_drv_argument_not_allowed_with)
+          << A->getSpelling() << LangStdArg->getAsString(Args);
----------------
Do we have any languages that disable digraphs by default? This won't work in such cases.

And actually... there doesn't seem to be a good reason to disallow enabling digraphs in C89-like modes, so maybe we should just remove the diagnostic for this case entirely? We generally don't prevent the user from arbitrarily combining language features.


Repository:
  rC Clang

https://reviews.llvm.org/D48266





More information about the cfe-commits mailing list