[Driver] Add -fno-digraphs

[jtbandes]

Summary

Add a flag -fno-digraphs to disable digraphs in the lexer, similar to -fno-operator-names which disables alternative names for C++ operators.

[rsmith]

include/clang/Driver/Options.td

1337–1338 ↗

(On Diff #151655)

In the driver, we generally want a -ffoo option matching -fno-foo. That is, the driver (but not -cc1) should support a matching -fdigraphs option to undo the effect of -fno-digraphs, unless there's a good reason not to do so.

[jtbandes]

Added -fdigraphs. I kept it as a cc1 option because it seemed awkward to "check whether the last arg was -fno-digraphs and pass only that arg to cc1" (if just directly forwarding all args, there would be an unrecognized argument error if it's not a cc1 option).

[jtbandes]

include/clang/Driver/Options.td

1337–1338 ↗

(On Diff #151655)

Didn't find a great place to put this option; I'm not sure what this file's organization scheme is supposed to be.

[rsmith]

lib/Driver/ToolChains/Clang.cpp

3973 ↗

(On Diff #151771)

Use Args.hasFlag to determine if -fdigraphs or -fno-digraphs was specified last, then CmdArgs.push_back("-fno-digraphs") if digraphs are disabled. (There are lots of examples of that in this file that you can follow.)

What should -fdigraphs do under -std=c89? I think produing a "this flag is not compatible with that flag" diagnostic would make sense for that case.

[jtbandes]

Added an error when language standard doesn't support digraphs.

Still keeping -fdigraphs as a cc1 option because then I can distinguish explicitly-enabled/disabled from the absence of a flag. I can also check whether digraphs are supported using the LangStandard/LangOpts in the CompilerInstance rather than hard-coding an incompatibility with -std=c89.

[jtbandes]

Friendly ping!

[rsmith]

include/clang/Driver/Options.td

1337–1340 ↗

(On Diff #151858)

It'd make sense to also list %:%: here, particularly because it is controlled by this flag and isn't technically a digraph.

lib/Frontend/CompilerInvocation.cpp

2181 ↗

(On Diff #151858)

* not * , please.

2183–2185 ↗

(On Diff #151858)

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.

[jtbandes]

• remove diagnostic; fix formatting

[jtbandes]

If I understood your comment correctly, you meant to remove the diagnostic completely (regardless of whether -fdigraphs or -fno-digraphs is given, and regardless of whether digraphs were enabled for the selected language)? I've done that, and added a couple -std=c89 invocations to the test cases.

[jtbandes]

• include %:%: in help text

[jtbandes]

Ping again 😇

[rsmith]

Looks good with one cleanup.

lib/Frontend/CompilerInvocation.cpp

2177–2178 ↗

(On Diff #155116)

Simplify this to Opts.Digraphs = Args.hasFlag(OPT_fdigraphs, OPT_fno_diagraphs, Opts.Digraphs);

[jtbandes]

This comment was removed by jtbandes.

cfe/trunk/include/clang/Driver/Options.td

 def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Do not include fixit information in diagnostics">;
 def fno_diagnostics_show_hotness : Flag<["-"], "fno-diagnostics-show-hotness">, Group<f_Group>;
 def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, Group<f_Group>;
 def fno_diagnostics_show_note_include_stack : Flag<["-"], "fno-diagnostics-show-note-include-stack">,
     Flags<[CC1Option]>, Group<f_Group>;
+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 '<:', ':>', '<%', '%>', '%:', '%:%:'">;
 def fno_declspec : Flag<["-"], "fno-declspec">, Group<f_clang_Group>,
   HelpText<"Disallow __declspec as a keyword">, Flags<[CC1Option]>;
 def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, Group<f_Group>,
   HelpText<"Disallow '$' in identifiers">, Flags<[CC1Option]>;
 def fno_elide_constructors : Flag<["-"], "fno-elide-constructors">, Group<f_Group>,
   HelpText<"Disable C++ copy constructor elision">, Flags<[CC1Option]>;
 def fno_eliminate_unused_debug_symbols : Flag<["-"], "fno-eliminate-unused-debug-symbols">, Group<f_Group>;
 def fno_exceptions : Flag<["-"], "fno-exceptions">, Group<f_Group>;

cfe/trunk/lib/Driver/ToolChains/Clang.cpp

#endif
 
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
+  Args.AddLastArg(CmdArgs, options::OPT_fdigraphs, options::OPT_fno_digraphs);
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
                   options::OPT_fno_emulated_tls);
 
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa<PreprocessJobAction>(JA) || Output.getType() != types::TY_PP_Asm)
     Args.AddLastArg(CmdArgs, options::OPT_fzvector);
 

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
   // We abuse '-f[no-]gnu-keywords' to force overriding all GNU-extension
   // keywords. This behavior is provided by GCC's poorly named '-fasm' flag,
   // while a subset (the non-C++ GNU keywords) is provided by GCC's
   // '-fgnu-keywords'. Clang conflates the two for simplicity under the single
   // name, as it doesn't seem a useful distinction.
   Opts.GNUKeywords = Args.hasFlag(OPT_fgnu_keywords, OPT_fno_gnu_keywords,
                                   Opts.GNUKeywords);
 
+  Opts.Digraphs = Args.hasFlag(OPT_fdigraphs, OPT_fno_digraphs, Opts.Digraphs);
+
   if (Args.hasArg(OPT_fno_operator_names))
     Opts.CXXOperatorNames = 0;
 
   if (Args.hasArg(OPT_fcuda_is_device))
     Opts.CUDAIsDevice = 1;
 
   if (Args.hasArg(OPT_fcuda_allow_variadic_functions))
     Opts.CUDAAllowVariadicFunctions = 1;

cfe/trunk/test/Lexer/digraph.c

-// RUN: %clang_cc1 -fsyntax-only -verify -ffreestanding %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -DDIGRAPHS=1 -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -DDIGRAPHS=1 -fno-digraphs -fdigraphs -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -fno-digraphs -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -fdigraphs -fno-digraphs -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -std=c89 -DDIGRAPHS=1 -fdigraphs -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -std=c89 -fno-digraphs -fsyntax-only -verify -ffreestanding %s
+
+#if DIGRAPHS
 
+// expected-no-diagnostics
 %:include <stdint.h>
 
     %:ifndef BUFSIZE
      %:define BUFSIZE  512
     %:endif
 
     void copy(char d<::>, const char s<::>, int len)
     <%
         while (len-- >= 0)
         <%
             d<:len:> = s<:len:>;
         %>
     %>
+#else
+
+// expected-error@+1 {{expected identifier or '('}}
+%:include <stdint.h>
+;
+// expected-error@+1 {{expected ')'}} expected-note@+1{{to match this '('}}
+void copy(char d<::>);
+
+// expected-error@+1 {{expected function body}}
+void copy() <% %>
+
+#endif