Add `-verify-ignore-unexpected` option to ignore unexpected diagnostics in VerifyDiagnosticsConsumer

[EricWF]

Summary

The goal of this patch is to make -verify easier to use when testing libc++. The notes attached to compile error diagnostics are numerous and relatively unstable when they reference libc++ header internals. This patch allows libc++ to write stable compilation failure tests by allowing unexpected diagnostic messages to be ignored where they are not relevant.

This patch adds a new CC1 flag called -verify-ignore-unexpected. -verify-ignore-unexpected tells VerifyDiagnosticsConsumer to ignore *all* unexpected diagnostic messages. -verify-ignore-unexpected=<LevelList> can be used to only ignore certain diagnostic levels. <LevelList> is a comma separated list of diagnostic levels to ignore. The supported levels are note, remark, warning and error.

[EricWF]

Responding to @bogner's comments that he sent via email.

include/clang/Basic/DiagnosticOptions.h

37

Better to use enum class DiagnosticLevelMask. You'll need to define
operator| and operator|=, but those are trivial with std::underlying_type.

I would add operator&(Enum, Enum), operator|(Enum, Enum) and `~operator(

I would prefer to use a enum class but there are a couple of problems:

1. You need to define a raw_ostream operator<<(...) function. This is required by the ENUM_DIAGOPT macro.
2. if (EnumValue & Enum::Value) doesn't work. You need the explicit bool cast which is ugly.

lib/Frontend/VerifyDiagnosticConsumer.cpp

750

Why bother making this const? Also, a name like DiagMask is probably clearer.

I like to make things const so I can't accidentally change it. I'm happy to change the name though.

[EricWF]

Address @bogner's comments.

[EricWF]

Address @bogners comments.

[EricWF]

Accepting with @bogner's permission.

include/clang/Basic/DiagnosticOptions.h

 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include <string>
+#include <type_traits>
 #include <vector>
 
 namespace clang {

   Ovl_Best  ///< Show just the "best" overload candidates.
 };
 
+/// \brief A bitmask representing the diagnostic levels used by
+/// VerifyDiagnosticConsumer.
+enum class DiagnosticLevelMask : unsigned {
+  None    = 0,
+  Note    = 1 << 0,
+  Remark  = 1 << 1,
+  Warning = 1 << 2,
+  Error   = 1 << 3,
+  All     = Note | Remark | Warning | Error
+};
+
+inline DiagnosticLevelMask operator~(DiagnosticLevelMask M) {
+  using UT = std::underlying_type<DiagnosticLevelMask>::type;
+  return static_cast<DiagnosticLevelMask>(~static_cast<UT>(M));
+}
+
+inline DiagnosticLevelMask operator|(DiagnosticLevelMask LHS,
+                                     DiagnosticLevelMask RHS) {
+  using UT = std::underlying_type<DiagnosticLevelMask>::type;
+  return static_cast<DiagnosticLevelMask>(
+    static_cast<UT>(LHS) | static_cast<UT>(RHS));
+}
+
+inline DiagnosticLevelMask operator&(DiagnosticLevelMask LHS,
+                                     DiagnosticLevelMask RHS) {
+  using UT = std::underlying_type<DiagnosticLevelMask>::type;
+  return static_cast<DiagnosticLevelMask>(
+    static_cast<UT>(LHS) & static_cast<UT>(RHS));
+}
+
+raw_ostream& operator<<(raw_ostream& Out, DiagnosticLevelMask M);
+
 /// \brief Options for controlling the compiler diagnostics engine.
 class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{
 public:

include/clang/Basic/DiagnosticOptions.def

 DIAGOPT(VerifyDiagnostics, 1, 0) /// Check that diagnostics match the expected
                                  /// diagnostics, indicated by markers in the
                                  /// input source file.
-
+ENUM_DIAGOPT(VerifyIgnoreUnexpected, DiagnosticLevelMask, 4,
+             DiagnosticLevelMask::None) /// Ignore unexpected diagnostics of
+                                        /// the specified levels when using
+                                        /// -verify.
 DIAGOPT(ElideType, 1, 0)         /// Elide identical types in template diffing
 DIAGOPT(ShowTemplateTree, 1, 0)  /// Print a template tree when diffing
 DIAGOPT(CLFallbackMode, 1, 0)    /// Format for clang-cl fallback mode

include/clang/Driver/CC1Options.td

   HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
 def verify : Flag<["-"], "verify">,
   HelpText<"Verify diagnostic output using comment directives">;
+def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
+  HelpText<"Ignore unexpected diagnostic messages">;
+def verify_ignore_unexpected_EQ : CommaJoined<["-"], "verify-ignore-unexpected=">,
+  HelpText<"Ignore unexpected diagnostic messages">;
 def Wno_rewrite_macros : Flag<["-"], "Wno-rewrite-macros">,
   HelpText<"Silence ObjC rewriting warnings">;
 

lib/Basic/CMakeLists.txt

   CharInfo.cpp
   Diagnostic.cpp
   DiagnosticIDs.cpp
+  DiagnosticOptions.cpp
   FileManager.cpp
   FileSystemStatCache.cpp
   IdentifierTable.cpp

lib/Basic/DiagnosticOptions.cpp

+//===--- DiagnosticOptions.cpp - C Language Family Diagnostic Handling ----===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file implements the DiagnosticOptions related interfaces.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/DiagnosticOptions.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+
+raw_ostream& operator<<(raw_ostream& Out, DiagnosticLevelMask M) {
+  using UT = std::underlying_type<DiagnosticLevelMask>::type;
+  return Out << static_cast<UT>(M);
+}
+
+} // end namespace clang

lib/Frontend/CompilerInvocation.cpp

   return Pattern;
 }
 
+static bool parseDiagnosticLevelMask(StringRef FlagName,
+                                     const std::vector<std::string> &Levels,
+                                     DiagnosticsEngine *Diags,
+                                     DiagnosticLevelMask &M) {
+  bool Success = true;
+  for (const auto &Level : Levels) {
+    DiagnosticLevelMask const PM =
+      llvm::StringSwitch<DiagnosticLevelMask>(Level)
+        .Case("note",    DiagnosticLevelMask::Note)
+        .Case("remark",  DiagnosticLevelMask::Remark)
+        .Case("warning", DiagnosticLevelMask::Warning)
+        .Case("error",   DiagnosticLevelMask::Error)
+        .Default(DiagnosticLevelMask::None);
+    if (PM == DiagnosticLevelMask::None) {
+      Success = false;
+      if (Diags)
+        Diags->Report(diag::err_drv_invalid_value) << FlagName << Level;
+    }
+    M = M | PM;
+  }
+  return Success;
+}
+
 static void parseSanitizerKinds(StringRef FlagName,
                                 const std::vector<std::string> &Sanitizers,
                                 DiagnosticsEngine &Diags, SanitizerSet &S) {

       << Args.getLastArg(OPT_fdiagnostics_format)->getAsString(Args)
       << Format;
   }
-  
   Opts.ShowSourceRanges = Args.hasArg(OPT_fdiagnostics_print_source_range_info);
   Opts.ShowParseableFixits = Args.hasArg(OPT_fdiagnostics_parseable_fixits);
   Opts.ShowPresumedLoc = !Args.hasArg(OPT_fno_diagnostics_use_presumed_location);
   Opts.VerifyDiagnostics = Args.hasArg(OPT_verify);
+  DiagnosticLevelMask DiagMask = DiagnosticLevelMask::None;
+  Success &= parseDiagnosticLevelMask("-verify-ignore-unexpected=",
+    Args.getAllArgValues(OPT_verify_ignore_unexpected_EQ),
+    Diags, DiagMask);
+  if (Args.hasArg(OPT_verify_ignore_unexpected))
+    DiagMask = DiagnosticLevelMask::All;
+  Opts.setVerifyIgnoreUnexpected(DiagMask);
   Opts.ElideType = !Args.hasArg(OPT_fno_elide_type);
   Opts.ShowTemplateTree = Args.hasArg(OPT_fdiagnostics_show_template_tree);
   Opts.ErrorLimit = getLastArgIntValue(Args, OPT_ferror_limit, 0, Diags);

lib/Frontend/VerifyDiagnosticConsumer.cpp

                            const char *Label,
                            DirectiveList &Left,
                            const_diag_iterator d2_begin,
-                           const_diag_iterator d2_end) {
+                           const_diag_iterator d2_end,
+                           bool IgnoreUnexpected) {
   std::vector<Directive *> LeftOnly;
   DiagList Right(d2_begin, d2_end);
 

   }
   // Now all that's left in Right are those that were not matched.
   unsigned num = PrintExpected(Diags, SourceMgr, LeftOnly, Label);
-  num += PrintUnexpected(Diags, &SourceMgr, Right.begin(), Right.end(), Label);
+  if (!IgnoreUnexpected)
+    num += PrintUnexpected(Diags, &SourceMgr, Right.begin(), Right.end(), Label);
   return num;
 }
 

   //   Seen \ Expected - set seen but not expected
   unsigned NumProblems = 0;
 
+  const DiagnosticLevelMask DiagMask =
+    Diags.getDiagnosticOptions().getVerifyIgnoreUnexpected();
+
   // See if there are error mismatches.
   NumProblems += CheckLists(Diags, SourceMgr, "error", ED.Errors,
-                            Buffer.err_begin(), Buffer.err_end());
+                            Buffer.err_begin(), Buffer.err_end(),
+                            bool(DiagnosticLevelMask::Error & DiagMask));
 
   // See if there are warning mismatches.
   NumProblems += CheckLists(Diags, SourceMgr, "warning", ED.Warnings,
-                            Buffer.warn_begin(), Buffer.warn_end());
+                            Buffer.warn_begin(), Buffer.warn_end(),
+                            bool(DiagnosticLevelMask::Warning & DiagMask));
 
   // See if there are remark mismatches.
   NumProblems += CheckLists(Diags, SourceMgr, "remark", ED.Remarks,
-                            Buffer.remark_begin(), Buffer.remark_end());
+                            Buffer.remark_begin(), Buffer.remark_end(),
+                            bool(DiagnosticLevelMask::Remark & DiagMask));
 
   // See if there are note mismatches.
   NumProblems += CheckLists(Diags, SourceMgr, "note", ED.Notes,
-                            Buffer.note_begin(), Buffer.note_end());
+                            Buffer.note_begin(), Buffer.note_end(),
+                            bool(DiagnosticLevelMask::Note & DiagMask));
 
   return NumProblems;
 }

     // Check that the expected diagnostics occurred.
     NumErrors += CheckResults(Diags, *SrcManager, *Buffer, ED);
   } else {
-    NumErrors += (PrintUnexpected(Diags, nullptr, Buffer->err_begin(),
-                                  Buffer->err_end(), "error") +
-                  PrintUnexpected(Diags, nullptr, Buffer->warn_begin(),
-                                  Buffer->warn_end(), "warn") +
-                  PrintUnexpected(Diags, nullptr, Buffer->note_begin(),
-                                  Buffer->note_end(), "note"));
+    const DiagnosticLevelMask DiagMask =
+        ~Diags.getDiagnosticOptions().getVerifyIgnoreUnexpected();
+    if (bool(DiagnosticLevelMask::Error & DiagMask))
+      NumErrors += PrintUnexpected(Diags, nullptr, Buffer->err_begin(),
+                                   Buffer->err_end(), "error");
+    if (bool(DiagnosticLevelMask::Warning & DiagMask))
+      NumErrors += PrintUnexpected(Diags, nullptr, Buffer->warn_begin(),
+                                   Buffer->warn_end(), "warn");
+    if (bool(DiagnosticLevelMask::Remark & DiagMask))
+      NumErrors += PrintUnexpected(Diags, nullptr, Buffer->remark_begin(),
+                                   Buffer->remark_end(), "remark");
+    if (bool(DiagnosticLevelMask::Note & DiagMask))
+      NumErrors += PrintUnexpected(Diags, nullptr, Buffer->note_begin(),
+                                   Buffer->note_end(), "note");
   }
 
   Diags.setClient(CurClient, Owner.release() != nullptr);

test/Frontend/verify-ignore-unexpected.c

+// RUN: not %clang_cc1 -DTEST_SWITCH -verify-ignore-unexpected=remark,aoeu,note -verify %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-BAD-SWITCH %s
+#ifdef TEST_SWITCH
+// expected-no-diagnostics
+#endif
+// CHECK-BAD-SWITCH: error: 'error' diagnostics seen but not expected:
+// CHECK-BAD-SWITCH-NEXT: (frontend): invalid value 'aoeu' in '-verify-ignore-unexpected='
+
+// RUN: %clang_cc1 -DTEST1 -verify %s
+// RUN: %clang_cc1 -DTEST1 -verify -verify-ignore-unexpected %s
+#ifdef TEST1
+#warning MyWarning1
+    // expected-warning@-1 {{MyWarning1}}
+int x; // expected-note {{previous definition is here}}
+float x; // expected-error {{redefinition of 'x'}}
+#endif
+
+// RUN: not %clang_cc1 -DTEST2 -verify %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-UNEXP %s
+// RUN: not %clang_cc1 -DTEST2 -verify -verify-ignore-unexpected= %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-UNEXP %s
+// RUN: not %clang_cc1 -DTEST2 -verify -verify-ignore-unexpected=note %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-NOTE %s
+// RUN: not %clang_cc1 -DTEST2 -verify -verify-ignore-unexpected=warning %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-WARN %s
+// RUN: not %clang_cc1 -DTEST2 -verify -verify-ignore-unexpected=error %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-ERR %s
+#ifdef TEST2
+#warning MyWarning2
+int x;
+float x;
+#endif
+// CHECK-UNEXP: no expected directives found
+// CHECK-UNEXP-NEXT: 'error' diagnostics seen but not expected
+// CHECK-UNEXP-NEXT: Line {{[0-9]+}}: redefinition of 'x'
+// CHECK-UNEXP-NEXT: 'warning' diagnostics seen but not expected
+// CHECK-UNEXP-NEXT: Line {{[0-9]+}}: MyWarning2
+// CHECK-UNEXP-NEXT: 'note' diagnostics seen but not expected
+// CHECK-UNEXP-NEXT: Line {{[0-9]+}}: previous definition is here
+// CHECK-UNEXP-NEXT: 4 errors generated.
+
+// CHECK-NOTE: no expected directives found
+// CHECK-NOTE-NEXT: 'error' diagnostics seen but not expected
+// CHECK-NOTE-NEXT: Line {{[0-9]+}}: redefinition of 'x'
+// CHECK-NOTE-NEXT: 'warning' diagnostics seen but not expected
+// CHECK-NOTE-NEXT: Line {{[0-9]+}}: MyWarning2
+// CHECK-NOTE-NEXT: 3 errors generated.
+
+// CHECK-WARN: no expected directives found
+// CHECK-WARN-NEXT: 'error' diagnostics seen but not expected
+// CHECK-WARN-NEXT: Line {{[0-9]+}}: redefinition of 'x'
+// CHECK-WARN-NEXT: 'note' diagnostics seen but not expected
+// CHECK-WARN-NEXT: Line {{[0-9]+}}: previous definition is here
+// CHECK-WARN-NEXT: 3 errors generated.
+
+// CHECK-ERR: no expected directives found
+// CHECK-ERR-NEXT: 'warning' diagnostics seen but not expected
+// CHECK-ERR-NEXT: Line {{[0-9]+}}: MyWarning2
+// CHECK-ERR-NEXT: 'note' diagnostics seen but not expected
+// CHECK-ERR-NEXT: Line {{[0-9]+}}: previous definition is here
+// CHECK-ERR-NEXT: 3 errors generated.
+
+// RUN: not %clang_cc1 -DTEST3 -verify -verify-ignore-unexpected %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-EXP %s
+#ifdef TEST3
+// expected-error {{test3}}
+#endif
+// CHECK-EXP: 'error' diagnostics expected but not seen
+// CHECK-EXP-NEXT: Line {{[0-9]+}}: test3
+
+// RUN: not %clang_cc1 -DTEST4 -verify -verify-ignore-unexpected %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-NOEXP %s
+// RUN: not %clang_cc1 -DTEST4 -verify -verify-ignore-unexpected=warning,error,note %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=CHECK-NOEXP %s
+#ifdef TEST4
+#warning MyWarning4
+int x;
+float x;
+#endif
+// CHECK-NOEXP: error: no expected directives found
+// CHECK-NOEXP-NEXT: 1 error generated