[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

493

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

265

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

[arphaman]

clang/include/clang/Basic/DiagnosticGroups.td

108 ↗

(On Diff #266979)

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 ↗

(On Diff #266979)

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

265

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 ↗

(On Diff #266979)

Got it, thanks!

clang/lib/Lex/PPExpressions.cpp

265

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

265

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/DiagnosticLexKinds.td

 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;
+def warn_pp_undef_prefix : Warning<
+  "%0 is not defined, evaluates to 0">,
+  InGroup<DiagGroup<"undef-prefix">>, 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 : CommaJoined<["-"], "Wundef-prefix=">, Group<W_value_Group>,
+  Flags<[CC1Option, CoreOption, HelpHidden]>, MetaVarName<"<arg>">,
+  HelpText<"Enable warnings for undefined macros with a prefix in the comma separated list <arg>">;
 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]>;
 def Xarch_host : Separate<["-"], "Xarch_host">, Flags<[DriverOption]>,
   HelpText<"Pass <arg> to the CUDA/HIP host compilation">, MetaVarName<"<arg>">;
 def Xarch_device : Separate<["-"], "Xarch_device">, Flags<[DriverOption]>,
   HelpText<"Pass <arg> to the CUDA/HIP device compilation">, MetaVarName<"<arg>">;
 def Xassembler : Separate<["-"], "Xassembler">,

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);
+
+  Opts.UndefPrefixes = Args.getAllArgValues(OPT_Wundef_prefix_EQ);
+
   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

 // which parses and evaluates integer constant expressions for #if directives.
 //
 //===----------------------------------------------------------------------===//
 //
 // FIXME: implement testing for #assert's.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include <cassert>
 
 using namespace clang;
 
 namespace {
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) {
           PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
+
+          const DiagnosticsEngine &DiagEngine = PP.getDiagnostics();
+          // If 'Wundef' is enabled, do not emit 'undef-prefix' diagnostics.
+          if (DiagEngine.isIgnored(diag::warn_pp_undef_identifier,
+                                   PeekTok.getLocation())) {
+            const std::vector<std::string> UndefPrefixes =
+                DiagEngine.getDiagnosticOptions().UndefPrefixes;
+            const StringRef IdentifierName = II->getName();
+            if (llvm::any_of(UndefPrefixes,
+                             [&IdentifierName](const std::string &Prefix) {
+                               return IdentifierName.startswith(Prefix);
+                             }))
+              PP.Diag(PeekTok, diag::warn_pp_undef_prefix)
+                  << AddFlagValue{llvm::join(UndefPrefixes, ",")} << 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.c

+// RUN: %clang_cc1 %s -Eonly -Wundef -verify=undef
+// RUN: %clang_cc1 %s -Eonly -Wundef-prefix=A,BC -verify=undef-prefix
+// RUN: %clang_cc1 %s -Eonly -Wundef -Wundef-prefix=A,BC -verify=both
+// RUN: %clang_cc1 %s -Eonly -Werror=undef -verify=undef-error
+// RUN: %clang_cc1 %s -Eonly -Werror=undef-prefix -Wundef-prefix=A,BC -verify=undef-prefix-error
+// RUN: %clang_cc1 %s -Eonly -Werror=undef -Wundef-prefix=A,BC -verify=both-error
+
+extern int x;
+
+#if AB // #1
+#endif
+// undef-warning@#1 {{'AB' is not defined, evaluates to 0}}
+// undef-prefix-warning@#1 {{'AB' is not defined, evaluates to 0}}
+// both-warning@#1 {{'AB' is not defined, evaluates to 0}}
+// undef-error-error@#1 {{'AB' is not defined, evaluates to 0}}
+// undef-prefix-error-error@#1 {{'AB' is not defined, evaluates to 0}}
+// both-error-error@#1 {{'AB' is not defined, evaluates to 0}}
+
+#if B // #2
+#endif
+// undef-warning@#2 {{'B' is not defined, evaluates to 0}}
+// no warning for undef-prefix
+// both-warning@#2 {{'B' is not defined, evaluates to 0}}
+// undef-error-error@#2 {{'B' is not defined, evaluates to 0}}
+// no error for undef-prefix
+// both-error-error@#2 {{'B' is not defined, evaluates to 0}}
+
+#define BC 0
+#if BC // no warning/error
+#endif
+
+#undef BC
+#if BC // #3
+#endif
+// undef-warning@#3 {{'BC' is not defined, evaluates to 0}}
+// undef-prefix-warning@#3 {{'BC' is not defined, evaluates to 0}}
+// both-warning@#3 {{'BC' is not defined, evaluates to 0}}
+// undef-error-error@#3 {{'BC' is not defined, evaluates to 0}}
+// undef-prefix-error-error@#3 {{'BC' is not defined, evaluates to 0}}
+// both-error-error@#3 {{'BC' is not defined, evaluates to 0}}
+
+// Test that #pragma-enabled 'Wundef' can override 'Wundef-prefix'
+#pragma clang diagnostic error "-Wundef"
+
+#if C // #4
+#endif
+// undef-error@#4 {{'C' is not defined, evaluates to 0}}
+// undef-prefix-error@#4 {{'C' is not defined, evaluates to 0}}
+// both-error@#4 {{'C' is not defined, evaluates to 0}}
+// undef-error-error@#4 {{'C' is not defined, evaluates to 0}}
+// undef-prefix-error-error@#4 {{'C' is not defined, evaluates to 0}}
+// both-error-error@#4 {{'C' is not defined, evaluates to 0}}