Eric Fiselier <[email protected]> writes: > Hi bogner, grosser, alexfh, > > 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`.
A couple minor comments below, and some of the formatting's a little funny - could you clang-format this please? > http://reviews.llvm.org/D10138 > > Files: > include/clang/Basic/DiagnosticOptions.def > include/clang/Basic/DiagnosticOptions.h > include/clang/Driver/CC1Options.td > lib/Frontend/CompilerInvocation.cpp > lib/Frontend/VerifyDiagnosticConsumer.cpp > test/Frontend/verify-ignore-unexpected.c > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > > Index: include/clang/Basic/DiagnosticOptions.def > =================================================================== > --- include/clang/Basic/DiagnosticOptions.def > +++ include/clang/Basic/DiagnosticOptions.def > @@ -69,7 +69,7 @@ > DIAGOPT(VerifyDiagnostics, 1, 0) /// Check that diagnostics match the > expected > /// diagnostics, indicated by markers in the > /// input source file. > - > +DIAGOPT(VerifyIgnoreUnexpected, 4, 0) Add a doc comment, please. > 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 > Index: include/clang/Basic/DiagnosticOptions.h > =================================================================== > --- include/clang/Basic/DiagnosticOptions.h > +++ include/clang/Basic/DiagnosticOptions.h > @@ -24,6 +24,17 @@ > Ovl_Best ///< Show just the "best" overload candidates. > }; > > +typedef unsigned DiagnosticLevelMask; > + > +enum { > + DLM_None = 0, > + DLM_Note = 1 << 0, > + DLM_Remark = 1 << 1, > + DLM_Warning = 1 << 2, > + DLM_Error = 1 << 3, > + DLM_All = DLM_Note | DLM_Remark | DLM_Warning | DLM_Error > +}; Better to use `enum class DiagnosticLevelMask`. You'll need to define operator| and operator|=, but those are trivial with std::underlying_type. > + > /// \brief Options for controlling the compiler diagnostics engine. > class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{ > public: > Index: include/clang/Driver/CC1Options.td > =================================================================== > --- include/clang/Driver/CC1Options.td > +++ include/clang/Driver/CC1Options.td > @@ -301,6 +301,10 @@ > 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">; > > Index: lib/Frontend/CompilerInvocation.cpp > =================================================================== > --- lib/Frontend/CompilerInvocation.cpp > +++ lib/Frontend/CompilerInvocation.cpp > @@ -337,6 +337,30 @@ > } > } > > +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", DLM_Note) > + .Case("remark", DLM_Remark) > + .Case("warning", DLM_Warning) > + .Case("error", DLM_Error) > + .Default(DLM_None); > + if (PM == DLM_None) { > + Success = false; > + if (Diags) > + Diags->Report(diag::err_drv_invalid_value) << FlagName << Level; > + } > + else > + M |= PM; No need for else here - None is 0. > + } > + return Success; > +} > + > static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind > IK, > DiagnosticsEngine &Diags, > const TargetOptions &TargetOpts) { > @@ -752,11 +776,18 @@ > << 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 VerifyIgnoreMask = DLM_None; > + Success &= parseDiagnosticLevelMask("-verify-ignore-unexpected=", > + Args.getAllArgValues(OPT_verify_ignore_unexpected_EQ), > + Diags, VerifyIgnoreMask); > + if (Args.hasArg(OPT_verify_ignore_unexpected)) > + VerifyIgnoreMask = DLM_All; > + Opts.VerifyIgnoreUnexpected = VerifyIgnoreMask; > 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); > Index: lib/Frontend/VerifyDiagnosticConsumer.cpp > =================================================================== > --- lib/Frontend/VerifyDiagnosticConsumer.cpp > +++ lib/Frontend/VerifyDiagnosticConsumer.cpp > @@ -691,7 +691,8 @@ > 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); > > @@ -727,7 +728,8 @@ > } > // 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; > } > > @@ -745,21 +747,28 @@ > // Seen \ Expected - set seen but not expected > unsigned NumProblems = 0; > > + DiagnosticLevelMask const IgnoredUnexpectedLevels = > + Diags.getDiagnosticOptions().VerifyIgnoreUnexpected; Why bother making this const? Also, a name like DiagMask is probably clearer. > + > // 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(), > + DLM_Error & IgnoredUnexpectedLevels); > > // 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(), > + DLM_Warning & IgnoredUnexpectedLevels); > > // 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(), > + DLM_Remark & IgnoredUnexpectedLevels); > > // 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(), > + DLM_Note & IgnoredUnexpectedLevels); > > return NumProblems; > } > @@ -854,12 +863,20 @@ > // 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")); > + DiagnosticLevelMask const ShowUnexpected = > + ~Diags.getDiagnosticOptions().VerifyIgnoreUnexpected; Same here. > + if (DLM_Error & ShowUnexpected) > + NumErrors += PrintUnexpected(Diags, nullptr, Buffer->err_begin(), > + Buffer->err_end(), "error"); > + if (DLM_Warning & ShowUnexpected) > + NumErrors += PrintUnexpected(Diags, nullptr, Buffer->warn_begin(), > + Buffer->warn_end(), "warn"); > + if (DLM_Remark & ShowUnexpected) > + NumErrors += PrintUnexpected(Diags, nullptr, Buffer->remark_begin(), > + Buffer->remark_end(), "remark"); > + if (DLM_Note & ShowUnexpected) > + NumErrors += PrintUnexpected(Diags, nullptr, Buffer->note_begin(), > + Buffer->note_end(), "note"); > } > > Diags.setClient(CurClient, Owner.release() != nullptr); > Index: test/Frontend/verify-ignore-unexpected.c > =================================================================== > --- /dev/null > +++ test/Frontend/verify-ignore-unexpected.c > @@ -0,0 +1,72 @@ > + > +// 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=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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
