Skip to content

Commit

Permalink
[VerifyDiagnosticConsumer] support -verify=<prefixes>
Browse files Browse the repository at this point in the history
This mimics FileCheck's --check-prefixes option.

The default prefix is "expected". That is, "-verify" is equivalent to
"-verify=expected".

The goal is to permit exercising a single test suite source file with different
compiler options producing different sets of diagnostics.  While cpp can be
combined with the existing -verify to accomplish the same goal, source is often
easier to maintain when it's not cluttered with preprocessor directives or
duplicate passages of code. For example, this patch also rewrites some existing
clang tests to demonstrate the benefit of this feature.

Patch by Joel E. Denny, thanks!

Differential Revision: https://reviews.llvm.org/D39694

llvm-svn: 320908
  • Loading branch information
Hal Finkel committed Dec 16, 2017
1 parent fd563a0 commit 05e4648
Show file tree
Hide file tree
Showing 10 changed files with 315 additions and 554 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
Expand Up @@ -338,4 +338,8 @@ def warn_drv_msvc_not_found : Warning<
def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
"option '-ffine-grained-bitfield-accesses' cannot be enabled together with a sanitizer; flag ignored">,
InGroup<OptionIgnored>;

def note_drv_verify_prefix_spelling : Note<
"-verify prefixes must start with a letter and contain only alphanumeric"
" characters, hyphens, and underscores">;
}
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticOptions.h
Expand Up @@ -100,6 +100,10 @@ class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{
/// prefixes removed.
std::vector<std::string> Remarks;

/// The prefixes for comment directives sought by -verify ("expected" by
/// default).
std::vector<std::string> VerifyPrefixes;

public:
// Define accessors/mutators for diagnostic options of enumeration type.
#define DIAGOPT(Name, Bits, Default)
Expand Down
6 changes: 5 additions & 1 deletion clang/include/clang/Driver/CC1Options.td
Expand Up @@ -400,8 +400,12 @@ def fcaret_diagnostics_max_lines :
HelpText<"Set the maximum number of source lines to show in a caret diagnostic">;
def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"<N>">,
HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
def verify_EQ : CommaJoined<["-"], "verify=">,
MetaVarName<"<prefixes>">,
HelpText<"Verify diagnostic output using comment directives that start with"
" prefixes in the comma-separated sequence <prefixes>">;
def verify : Flag<["-"], "verify">,
HelpText<"Verify diagnostic output using comment directives">;
HelpText<"Equivalent to -verify=expected">;
def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
HelpText<"Ignore unexpected diagnostic messages">;
def verify_ignore_unexpected_EQ : CommaJoined<["-"], "verify-ignore-unexpected=">,
Expand Down
33 changes: 32 additions & 1 deletion clang/lib/Frontend/CompilerInvocation.cpp
Expand Up @@ -1071,6 +1071,26 @@ static bool parseShowColorsArgs(const ArgList &Args, bool DefaultColor) {
llvm::sys::Process::StandardErrHasColors());
}

static bool checkVerifyPrefixes(const std::vector<std::string> &VerifyPrefixes,
DiagnosticsEngine *Diags) {
bool Success = true;
for (const auto &Prefix : VerifyPrefixes) {
// Every prefix must start with a letter and contain only alphanumeric
// characters, hyphens, and underscores.
auto BadChar = std::find_if(Prefix.begin(), Prefix.end(),
[](char C){return !isAlphanumeric(C)
&& C != '-' && C != '_';});
if (BadChar != Prefix.end() || !isLetter(Prefix[0])) {
Success = false;
if (Diags) {
Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
Diags->Report(diag::note_drv_verify_prefix_spelling);
}
}
}
return Success;
}

bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
DiagnosticsEngine *Diags,
bool DefaultDiagColor, bool DefaultShowOpt) {
Expand Down Expand Up @@ -1158,7 +1178,18 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
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);
Opts.VerifyDiagnostics = Args.hasArg(OPT_verify) || Args.hasArg(OPT_verify_EQ);
Opts.VerifyPrefixes = Args.getAllArgValues(OPT_verify_EQ);
if (Args.hasArg(OPT_verify))
Opts.VerifyPrefixes.push_back("expected");
// Keep VerifyPrefixes in its original order for the sake of diagnostics, and
// then sort it to prepare for fast lookup using std::binary_search.
if (!checkVerifyPrefixes(Opts.VerifyPrefixes, Diags)) {
Opts.VerifyDiagnostics = false;
Success = false;
}
else
std::sort(Opts.VerifyPrefixes.begin(), Opts.VerifyPrefixes.end());
DiagnosticLevelMask DiagMask = DiagnosticLevelMask::None;
Success &= parseDiagnosticLevelMask("-verify-ignore-unexpected=",
Args.getAllArgValues(OPT_verify_ignore_unexpected_EQ),
Expand Down
130 changes: 90 additions & 40 deletions clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
Expand Up @@ -229,22 +229,52 @@ class ParseHelper
return true;
}

// Return true if string literal is found.
// When true, P marks begin-position of S in content.
bool Search(StringRef S, bool EnsureStartOfWord = false) {
// Return true if string literal S is matched in content.
// When true, P marks begin-position of the match, and calling Advance sets C
// to end-position of the match.
// If S is the empty string, then search for any letter instead (makes sense
// with FinishDirectiveToken=true).
// If EnsureStartOfWord, then skip matches that don't start a new word.
// If FinishDirectiveToken, then assume the match is the start of a comment
// directive for -verify, and extend the match to include the entire first
// token of that directive.
bool Search(StringRef S, bool EnsureStartOfWord = false,
bool FinishDirectiveToken = false) {
do {
P = std::search(C, End, S.begin(), S.end());
PEnd = P + S.size();
if (!S.empty()) {
P = std::search(C, End, S.begin(), S.end());
PEnd = P + S.size();
}
else {
P = C;
while (P != End && !isLetter(*P))
++P;
PEnd = P + 1;
}
if (P == End)
break;
if (!EnsureStartOfWord
// Check if string literal starts a new word.
|| P == Begin || isWhitespace(P[-1])
// Or it could be preceded by the start of a comment.
|| (P > (Begin + 1) && (P[-1] == '/' || P[-1] == '*')
&& P[-2] == '/'))
return true;
// Otherwise, skip and search again.
// If not start of word but required, skip and search again.
if (EnsureStartOfWord
// Check if string literal starts a new word.
&& !(P == Begin || isWhitespace(P[-1])
// Or it could be preceded by the start of a comment.
|| (P > (Begin + 1) && (P[-1] == '/' || P[-1] == '*')
&& P[-2] == '/')))
continue;
if (FinishDirectiveToken) {
while (PEnd != End && (isAlphanumeric(*PEnd)
|| *PEnd == '-' || *PEnd == '_'))
++PEnd;
// Put back trailing digits and hyphens to be parsed later as a count
// or count range. Because -verify prefixes must start with letters,
// we know the actual directive we found starts with a letter, so
// we won't put back the entire directive word and thus record an empty
// string.
assert(isLetter(*P) && "-verify prefix must start with a letter");
while (isDigit(PEnd[-1]) || PEnd[-1] == '-')
--PEnd;
}
return true;
} while (Advance());
return false;
}
Expand Down Expand Up @@ -314,37 +344,68 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
// A single comment may contain multiple directives.
bool FoundDirective = false;
for (ParseHelper PH(S); !PH.Done();) {
// Search for token: expected
if (!PH.Search("expected", true))
// Search for the initial directive token.
// If one prefix, save time by searching only for its directives.
// Otherwise, search for any potential directive token and check it later.
const auto &Prefixes = Diags.getDiagnosticOptions().VerifyPrefixes;
if (!(Prefixes.size() == 1 ? PH.Search(*Prefixes.begin(), true, true)
: PH.Search("", true, true)))
break;
PH.Advance();

// Next token: -
if (!PH.Next("-"))
continue;
PH.Advance();
// Default directive kind.
bool RegexKind = false;
const char* KindStr = "string";

// Parse the initial directive token in reverse so we can easily determine
// its exact actual prefix. If we were to parse it from the front instead,
// it would be harder to determine where the prefix ends because there
// might be multiple matching -verify prefixes because some might prefix
// others.
StringRef DToken(PH.P, PH.C - PH.P);

// Next token: { error | warning | note }
// Regex in initial directive token: -re
if (DToken.endswith("-re")) {
RegexKind = true;
KindStr = "regex";
DToken = DToken.substr(0, DToken.size()-3);
}

// Type in initial directive token: -{error|warning|note|no-diagnostics}
DirectiveList *DL = nullptr;
if (PH.Next("error"))
bool NoDiag = false;
StringRef DType;
if (DToken.endswith(DType="-error"))
DL = ED ? &ED->Errors : nullptr;
else if (PH.Next("warning"))
else if (DToken.endswith(DType="-warning"))
DL = ED ? &ED->Warnings : nullptr;
else if (PH.Next("remark"))
else if (DToken.endswith(DType="-remark"))
DL = ED ? &ED->Remarks : nullptr;
else if (PH.Next("note"))
else if (DToken.endswith(DType="-note"))
DL = ED ? &ED->Notes : nullptr;
else if (PH.Next("no-diagnostics")) {
else if (DToken.endswith(DType="-no-diagnostics")) {
NoDiag = true;
if (RegexKind)
continue;
}
else
continue;
DToken = DToken.substr(0, DToken.size()-DType.size());

// What's left in DToken is the actual prefix. That might not be a -verify
// prefix even if there is only one -verify prefix (for example, the full
// DToken is foo-bar-warning, but foo is the only -verify prefix).
if (!std::binary_search(Prefixes.begin(), Prefixes.end(), DToken))
continue;

if (NoDiag) {
if (Status == VerifyDiagnosticConsumer::HasOtherExpectedDirectives)
Diags.Report(Pos, diag::err_verify_invalid_no_diags)
<< /*IsExpectedNoDiagnostics=*/true;
else
Status = VerifyDiagnosticConsumer::HasExpectedNoDiagnostics;
continue;
} else
continue;
PH.Advance();

}
if (Status == VerifyDiagnosticConsumer::HasExpectedNoDiagnostics) {
Diags.Report(Pos, diag::err_verify_invalid_no_diags)
<< /*IsExpectedNoDiagnostics=*/false;
Expand All @@ -357,17 +418,6 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
if (!DL)
return true;

// Default directive kind.
bool RegexKind = false;
const char* KindStr = "string";

// Next optional token: -
if (PH.Next("-re")) {
PH.Advance();
RegexKind = true;
KindStr = "regex";
}

// Next optional token: @
SourceLocation ExpectedLoc;
bool MatchAnyLine = false;
Expand Down
6 changes: 4 additions & 2 deletions clang/test/Frontend/diagnostics-order.c
Expand Up @@ -2,9 +2,11 @@
// Previously, these diagnostics were grouped by diagnostic level with all
// notes last.
//
// RUN: not %clang_cc1 -O999 -std=bogus %s 2> %t
// RUN: not %clang_cc1 -O999 -std=bogus -verify=-foo %s 2> %t
// RUN: FileCheck < %t %s
//
// CHECK: warning: optimization level '-O999' is not supported
// CHECK: error: invalid value '-foo' in '-verify='
// CHECK-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// CHECK-NEXT: warning: optimization level '-O999' is not supported
// CHECK-NEXT: error: invalid value 'bogus' in '-std=bogus'
// CHECK-NEXT: note: use {{.*}} for {{.*}} standard
118 changes: 118 additions & 0 deletions clang/test/Frontend/verify-prefixes.c
@@ -0,0 +1,118 @@
#if GC
# define GCONST const
#else
# define GCONST
#endif

// gconst-note@8 {{variable 'glb' declared const here}}
GCONST int glb = 5;


// Check various correct prefix spellings and combinations.
//
// RUN: %clang_cc1 -DGC -verify=gconst %s
// RUN: %clang_cc1 -Wcast-qual -DLC -verify=lconst %s
// RUN: %clang_cc1 -DSC -verify=expected %s
// RUN: %clang_cc1 -DSC -verify %s
// RUN: %clang_cc1 -DSC -verify -verify %s
// RUN: %clang_cc1 -verify=nconst %s
// RUN: %clang_cc1 -verify=n-const %s
// RUN: %clang_cc1 -verify=n_const %s
// RUN: %clang_cc1 -verify=NConst %s
// RUN: %clang_cc1 -verify=NConst2 %s
// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -verify=gconst,lconst %s
// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -DSC -verify=gconst,lconst,expected %s
// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -verify=gconst -verify=lconst %s
// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -DSC -verify=gconst,lconst -verify %s
// RUN: %clang_cc1 -DGC -DSC -verify -verify=gconst -verify %s
//
// Duplicate prefixes.
// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -verify=gconst,lconst,gconst %s
// RUN: %clang_cc1 -DGC -verify=gconst -verify=gconst,gconst %s
// RUN: %clang_cc1 -DSC -verify=expected -verify=expected %s
// RUN: %clang_cc1 -DSC -verify -verify=expected %s
//
// Various tortured cases: multiple directives with different prefixes per
// line, prefixes used as comments, prefixes prefixing prefixes, and prefixes
// with special suffixes.
// RUN: %clang_cc1 -Wcast-qual -DLC -verify=foo %s
// RUN: %clang_cc1 -DSC -verify=bar %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo,bar %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=bar,foo %s
// RUN: %clang_cc1 -DSC -verify=foo-bar %s
// RUN: %clang_cc1 -Wcast-qual -DLC -verify=bar-foo %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo,foo-bar %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo-bar,foo %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=bar,bar-foo %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=bar-foo,bar %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo-bar,bar-foo %s
// RUN: %clang_cc1 -DSC -verify=foo-warning %s
// RUN: %clang_cc1 -Wcast-qual -DLC -verify=bar-warning-re %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo,foo-warning %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=foo-warning,foo %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=bar,bar-warning-re %s
// RUN: %clang_cc1 -Wcast-qual -DLC -DSC -verify=bar-warning-re,bar %s


// Check invalid prefixes. Check that there's no additional output, which
// might indicate that diagnostic verification became enabled even though it
// was requested incorrectly. Check that prefixes are reported in command-line
// order.
//
// RUN: not %clang_cc1 -verify=5abc,-xy,foo,_k -verify='#a,b$' %s 2> %t
// RUN: FileCheck --check-prefixes=ERR %s < %t
//
// ERR-NOT: {{.}}
// ERR: error: invalid value '5abc' in '-verify='
// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// ERR-NEXT: error: invalid value '-xy' in '-verify='
// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// ERR-NEXT: error: invalid value '_k' in '-verify='
// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// ERR-NEXT: error: invalid value '#a' in '-verify='
// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// ERR-NEXT: error: invalid value 'b$' in '-verify='
// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
// ERR-NOT: {{.}}


// Check that our test code actually has expected diagnostics when there's no
// -verify.
//
// RUN: not %clang_cc1 -Wcast-qual -DGC -DLC -DSC %s 2> %t
// RUN: FileCheck --check-prefix=ALL %s < %t
//
// ALL: cannot assign to variable 'glb' with const-qualified type 'const int'
// ALL: variable 'glb' declared const here
// ALL: cast from 'const int *' to 'int *' drops const qualifier
// ALL: initializing 'int *' with an expression of type 'const int *' discards qualifiers


#if LC
# define LCONST const
#else
# define LCONST
#endif

#if SC
# define SCONST const
#else
# define SCONST
#endif

void foo() {
LCONST int loc = 5;
SCONST static int sta = 5;
// We don't actually expect 1-2 occurrences of this error. We're just
// checking the parsing.
glb = 6; // gconst-error1-2 {{cannot assign to variable 'glb' with const-qualified type 'const int'}}
*(int*)(&loc) = 6; // lconst-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
; // Code, comments, and many directives with different prefixes per line, including cases where some prefixes (foo and bar) prefix others (such as foo-bar and bar-foo), such that some prefixes appear as normal comments and some have special suffixes (-warning and -re): foo-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} foo-bar-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}} foo-warning-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}} bar-warning-re-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} bar-foo-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} bar-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
int *p = &sta; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
}

// nconst-no-diagnostics
// n-const-no-diagnostics
// n_const-no-diagnostics
// NConst-no-diagnostics
// NConst2-no-diagnostics

0 comments on commit 05e4648

Please sign in to comment.