[PATCH] Add `-verify-ignore-unexpected` option to ignore unexpected diagnostics in VerifyDiagnosticsConsumer
Justin Bogner
mail at justinbogner.com
Fri Jun 12 14:26:33 PDT 2015
Eric Fiselier <eric at efcs.ca> writes:
> Responding to @bogner's comments that he sent via email.
>
>
> ================
> Comment at: include/clang/Basic/DiagnosticOptions.h:37
> @@ -27,1 +36,3 @@
> +};
> +
> /// \brief Options for controlling the compiler diagnostics engine.
> ----------------
>> 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.
This is a little lame, sure. We could always use some templates and
SFINAE to reduce the boilerplate if we start using this for more things
though - ie, adding an enable_if'd operator<< to raw_ostream for enums
where we just want to print the underlying type.
I don't think this is really necessary at this point though. Just an idea.
> 2. `if (EnumValue & Enum::Value)` doesn't work. You need the explicit
> bool cast which is ugly.
This one's harder to improve - you either get type safety or convenient
bools, I guess.
I personally like the enum class version you have here better, but I
could see the argument for just using a raw enum if you prefer that.
Either way's fine. Feel free to commit once you've addressed the last
couple of cosmetic issues below.
>
> ================
> Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:750
> @@ -747,1 +749,3 @@
>
> + DiagnosticLevelMask const IgnoredUnexpectedLevels =
> + Diags.getDiagnosticOptions().VerifyIgnoreUnexpected;
> ----------------
>> 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.
Alright - I think the prevalent style in LLVM is "const DiagnosticLevelMask"
rather than "DiagnosticLevelMask const" though. Please write it that way
for consistency.
> http://reviews.llvm.org/D10138
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
Eric Fiselier <eric at efcs.ca> writes:
> Address bogner's comments.
>
>
> http://reviews.llvm.org/D10138
>
> Files:
> include/clang/Basic/DiagnosticOptions.def
> include/clang/Basic/DiagnosticOptions.h
> include/clang/Driver/CC1Options.td
> lib/Basic/CMakeLists.txt
> lib/Basic/DiagnosticOptions.cpp
> 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,10 @@
> 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
> Index: include/clang/Basic/DiagnosticOptions.h
> ===================================================================
> --- include/clang/Basic/DiagnosticOptions.h
> +++ include/clang/Basic/DiagnosticOptions.h
> @@ -24,6 +24,35 @@
> 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) {
> + return static_cast<DiagnosticLevelMask>(~static_cast<unsigned>(M));
> +}
It doesn't make a difference here since it'll probably never change, but
I prefer to use std::underlying_type for these, like:
inline DiagnosticLevelMask operator~(DiagnosticLevelMask M) {
using UT = std::underlying_type<DiagnosticLevelMask>::type;
return static_cast<DiagnosticLevelMask>(~static_cast<UT>(M));
}
If you disagree, the current way is fine.
> +
> +inline DiagnosticLevelMask operator|(DiagnosticLevelMask LHS,
> + DiagnosticLevelMask RHS) {
> + return static_cast<DiagnosticLevelMask>(
> + static_cast<unsigned>(LHS) | static_cast<unsigned>(RHS));
> +}
> +
> +inline DiagnosticLevelMask operator&(DiagnosticLevelMask LHS,
> + DiagnosticLevelMask RHS) {
> + return static_cast<DiagnosticLevelMask>(
> + static_cast<unsigned>(LHS) & static_cast<unsigned>(RHS));
> +}
> +
> +raw_ostream& operator<<(raw_ostream& Out, DiagnosticLevelMask M);
> +
> /// \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/Basic/CMakeLists.txt
> ===================================================================
> --- lib/Basic/CMakeLists.txt
> +++ lib/Basic/CMakeLists.txt
> @@ -61,6 +61,7 @@
> CharInfo.cpp
> Diagnostic.cpp
> DiagnosticIDs.cpp
> + DiagnosticOptions.cpp
> FileManager.cpp
> FileSystemStatCache.cpp
> IdentifierTable.cpp
> Index: lib/Basic/DiagnosticOptions.cpp
> ===================================================================
> --- /dev/null
> +++ lib/Basic/DiagnosticOptions.cpp
> @@ -0,0 +1,23 @@
> +//===--- 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) {
> + return Out << static_cast<unsigned>(M);
> +}
> +
> +} // end namespace clang
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -321,6 +321,29 @@
> 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) {
> @@ -750,11 +773,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 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);
> 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 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;
> }
> @@ -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 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);
> Index: test/Frontend/verify-ignore-unexpected.c
> ===================================================================
> --- /dev/null
> +++ test/Frontend/verify-ignore-unexpected.c
> @@ -0,0 +1,81 @@
> +// 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 at -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
More information about the cfe-commits
mailing list