[clang][diagnostics] Add '-Wundef-prefix' warning option

[zixuw]

Summary

Add an -Wundef-prefix=<arg1>,<arg2>... option, which is similar to -Wundef, but only give warnings for undefined macros with the given prefixes.

[ributzka]

clang/include/clang/Driver/Options.td

475

We should provide a description that doesn't mention that this is an alias of -Wundef-prefix, because the alias is an implementation detail.

clang/lib/Lex/PPExpressions.cpp

262

What happens when you use -Werror=undef and -Wundef-prefix=<arg>?

[arphaman]

clang/include/clang/Basic/DiagnosticGroups.td

108

It seems like you're not using the UndefinedPrefix group in this patch. Is that intentional, or was this left in the patch by mistake?

[arphaman]

Can you still modify the state of -Wundef with #pragma clang diagnostic? Could you add a test to cover it.

[zixuw]

clang/include/clang/Basic/DiagnosticGroups.td

108

This is intentional to map both Wundef and Wundef-prefix to the same warning. And since Wundef is changed to an alias, the Undefined subgroup is used to keep existing outputs/behaviors consistent (to make -Werror=undef work, and to keep the [-Wundef] option name in the diagnostic message).

[zixuw]

Update test case for #pragma clang diagnostic
Update test case to verify that the state of -Wundef can still be modified using #pragma clang diagnostic

[zixuw]

clang/lib/Lex/PPExpressions.cpp

262

Then we only get errors for undef-prefix=<arg>. Yes this needs to be fixed.
Perhaps a better way to handle the -Werror=undef implication of -Wundef.

[arphaman]

clang/include/clang/Basic/DiagnosticGroups.td

108

Got it, thanks!

clang/lib/Lex/PPExpressions.cpp

262

We should try to fix this in this PR then. Can you check the severity of the diagnostic, and if it's an error always emit it here?

[zixuw]

clang/lib/Lex/PPExpressions.cpp

262

That is an option. But then we lose the ability to intentionally turning just the prefixes into errors, like -Wundef-prefix=FOO -Werror or -Wundef-prefix=BAR -Werror=undef-prefix

[zixuw]

• Refine test cases to check combinations of 'Wundef' and 'Wundef-prefix', and with/without 'Werror';
• Fix issues with '-Werror=undef' by explicitly looking for the option.

[zixuw]

• Remove implementation details from the help text of 'Wundef';
• Hide help text for 'Wundef-prefix' and 'Wundef'.

[zixuw]

Abort the design of making 'Wundef' an alias to 'Wundef-prefix' because it
depends on the alias expansion to work, which adds an empty string to 'UndefPrefixes'
to do the trick. However, any other way to enable 'Wundef', for example, via 'Werror=undef'
or '#pragma clang diagnostic', will not work and cannot be handled easily.

Instead, just suppress 'Wundef-prefix' when 'Wundef' is enabled.

[arphaman]

LGTM.

[ributzka]

LGTM

clang/include/clang/Basic/DiagnosticGroups.td

 def FloatZeroConversion : DiagGroup<"float-zero-conversion">;
 def FloatConversion :
   DiagGroup<"float-conversion", [FloatOverflowConversion,
                                  FloatZeroConversion]>;
 
 def FrameAddress : DiagGroup<"frame-address">;
 def DoublePromotion : DiagGroup<"double-promotion">;
 def EnumTooLarge : DiagGroup<"enum-too-large">;
+def Undefined : DiagGroup<"undef">;
+def UndefinedPrefix : DiagGroup<"undef-prefix", [Undefined]>;
 def UnsupportedNan : DiagGroup<"unsupported-nan">;
 def UnsupportedAbs : DiagGroup<"unsupported-abs">;
 def UnsupportedCB : DiagGroup<"unsupported-cb">;
 def UnsupportedGPOpt : DiagGroup<"unsupported-gpopt">;
 def UnsupportedTargetOpt : DiagGroup<"unsupported-target-opt">;
 def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">;
 def NullConversion : DiagGroup<"null-conversion">;
 def ImplicitConversionFloatingPointToBool :

clang/include/clang/Basic/DiagnosticLexKinds.td

def ext_pp_redef_builtin_macro : ExtWarn<"redefining builtin macro">,
   InGroup<BuiltinMacroRedefined>;
 def pp_disabled_macro_expansion : Warning<
   "disabled expansion of recursive macro">, DefaultIgnore,
   InGroup<DiagGroup<"disabled-macro-expansion">>;
 def pp_macro_not_used : Warning<"macro is not used">, DefaultIgnore,
   InGroup<DiagGroup<"unused-macros">>;
 def warn_pp_undef_identifier : Warning<
   "%0 is not defined, evaluates to 0">,
-  InGroup<DiagGroup<"undef">>, DefaultIgnore;
+  InGroup<Undefined>, DefaultIgnore;
 def warn_pp_ambiguous_macro : Warning<
   "ambiguous expansion of macro %0">, InGroup<AmbiguousMacro>;
 def note_pp_ambiguous_macro_chosen : Note<
   "expanding this definition of %0">;
 def note_pp_ambiguous_macro_other : Note<
   "other definition of %0">;
 def warn_pp_macro_hides_keyword : Extension<
   "keyword is hidden by macro definition">, InGroup<KeywordAsMacro>;

clang/include/clang/Basic/DiagnosticOptions.h

public:
 
   /// The file to serialize diagnostics to (non-appending).
   std::string DiagnosticSerializationFile;
 
   /// The list of -W... options used to alter the diagnostic mappings, with the
   /// prefixes removed.
   std::vector<std::string> Warnings;
 
+  /// The list of prefixes from -Wundef-prefix=... used to generate warnings
+  /// for undefined macros.
+  std::vector<std::string> UndefPrefixes;
+
   /// The list of -R... options used to alter the diagnostic mappings, with the
   /// prefixes removed.
   std::vector<std::string> Remarks;
 
   /// The prefixes for comment directives sought by -verify ("expected" by
   /// default).
   std::vector<std::string> VerifyPrefixes;
 

clang/include/clang/Driver/Options.td

 // FIXME: This is broken; these should not be Joined arguments.
 def Wno_nonportable_cfstrings : Joined<["-"], "Wno-nonportable-cfstrings">, Group<W_Group>,
   Flags<[CC1Option]>;
 def Wnonportable_cfstrings : Joined<["-"], "Wnonportable-cfstrings">, Group<W_Group>,
   Flags<[CC1Option]>;
 def Wp_COMMA : CommaJoined<["-"], "Wp,">,
   HelpText<"Pass the comma separated arguments in <arg> to the preprocessor">,
   MetaVarName<"<arg>">, Group<Preprocessor_Group>;
+def Wundef_prefix_EQ : Joined<["-"], "Wundef-prefix=">, Group<W_value_Group>,
+  Flags<[CC1Option, CoreOption]>, MetaVarName<"<arg>">,
+  HelpText<"Enable warnings for undefined macros with a prefix in the comma separated list <arg>">;
+// Rely on the implementation that a Flag alias for a Joined option is provided a default argument
+// of an empty string (""). So we have -Wundef => -Wundef-prefix=""
+def Wundef: Flag<["-"], "Wundef">, Group<W_Group>, Flags<[CC1Option, CoreOption]>,
+  Alias<Wundef_prefix_EQ>,
+  HelpText<"Aliased to '-Wundef-prefix=\"\"', enable warnings for undefined macros">;
 def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
 def Wno_write_strings : Flag<["-"], "Wno-write-strings">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
 def W_Joined : Joined<["-"], "W">, Group<W_Group>, Flags<[CC1Option, CoreOption]>,
   MetaVarName<"<warning>">, HelpText<"Enable the specified warning">;
 def Xanalyzer : Separate<["-"], "Xanalyzer">,
   HelpText<"Pass <arg> to the static analyzer">, MetaVarName<"<arg>">,
   Group<StaticAnalyzer_Group>;
 def Xarch__ : JoinedAndSeparate<["-"], "Xarch_">, Flags<[DriverOption]>;

clang/lib/Frontend/CompilerInvocation.cpp

bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   if (Opts.TabStop == 0 || Opts.TabStop > DiagnosticOptions::MaxTabStop) {
     Opts.TabStop = DiagnosticOptions::DefaultTabStop;
     if (Diags)
       Diags->Report(diag::warn_ignoring_ftabstop_value)
       << Opts.TabStop << DiagnosticOptions::DefaultTabStop;
   }
   Opts.MessageLength =
       getLastArgIntValue(Args, OPT_fmessage_length_EQ, 0, Diags);
+
+  for (const auto *A : Args.filtered(OPT_Wundef_prefix_EQ)) {
+    SmallVector<StringRef, 2> Prefixes;
+    // Keep empty strings to allow '-Wundef' (aliased to '-Wundef-prefix=""')
+    // to generate diagnostics for all undefined macros.
+    StringRef{A->getValue()}.split(Prefixes, ",");
+    for (const auto &P : Prefixes)
+      Opts.UndefPrefixes.push_back(P.str());
+  }
+
   addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings);
   addDiagnosticArgs(Args, OPT_R_Group, OPT_R_value_Group, Opts.Remarks);
 
   return Success;
 }
 
 static void ParseFileSystemArgs(FileSystemOptions &Opts, ArgList &Args) {
   Opts.WorkingDir = std::string(Args.getLastArgValue(OPT_working_directory));

clang/lib/Lex/PPExpressions.cpp

if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
       // Handle "defined X" and "defined(X)".
       if (II->isStr("defined"))
         return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
 
       if (!II->isCPlusPlusOperatorKeyword()) {
         // If this identifier isn't 'defined' or one of the special
         // preprocessor keywords and it wasn't macro expanded, it turns
         // into a simple 0
-        if (ValueLive)
+        if (ValueLive) {
+          const auto &UndefPrefixes =
+              PP.getDiagnostics().getDiagnosticOptions().UndefPrefixes;
+          const auto &IdentifierName = II->getName();
+          // Check whether UndefPrefixes is empty for standalone
+          // "-Werror=undef" to work.
+          // "-Werror=undef" implies "-Wundef" but does not add an empty
+          // string to UndefPrefixes as an explicit "-Wundef" does.
+          if (UndefPrefixes.empty() ||
+              llvm::any_of(UndefPrefixes,
+                           [&IdentifierName](const auto &Prefix) {
+                             return IdentifierName.startswith(Prefix);
+                           }))
             PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
+        }
         Result.Val = 0;
         Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
         Result.setIdentifier(II);
         Result.setRange(PeekTok.getLocation());
         DT.IncludedUndefinedIds = true;
         PP.LexNonComment(PeekTok);
         return false;
       }

clang/test/Preprocessor/warn-macro-undef-prefix.c

+// RUN: %clang_cc1 %s -Eonly -Wundef-prefix=A,BC -verify
+
+extern int x;
+
+#if A // expected-warning {{'A' is not defined, evaluates to 0}}
+#endif
+
+#if A1 // expected-warning {{'A1' is not defined, evaluates to 0}}
+#endif
+
+#if B // no warning
+#endif
+
+#define BC 0
+#if BC // no warning
+#endif
+
+#undef BC
+#if BC // expected-warning {{'BC' is not defined, evaluates to 0}}
+#endif
+
+#if BC1 // expected-warning {{'BC1' is not defined, evaluates to 0}}
+#endif

clang/test/Preprocessor/warn-macro-undef.c

+// RUN: %clang_cc1 %s -Eonly -Wundef -verify
+// RUN: %clang_cc1 %s -Eonly -Wundef -Wundef-prefix=A -verify
+
+extern int x;
+
+#if A // expected-warning {{'A' is not defined, evaluates to 0}}
+#endif
+
+#pragma clang diagnostic ignored "-Wundef"
+
+#if B // no warning
+#endif
+
+#pragma clang diagnostic warning "-Wundef"
+
+#if B // expected-warning {{'B' is not defined, evaluates to 0}}
+#endif