[llvm] fd94103 - Fix PR46880: Fail CHECK-NOT with undefined variable
Thomas Preud'homme via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 20 06:42:31 PDT 2021
Author: Thomas Preud'homme
Date: 2021-04-20T14:42:46+01:00
New Revision: fd941036bf2494cfbd2749f2f179733cbe9e2eb4
URL: https://github.com/llvm/llvm-project/commit/fd941036bf2494cfbd2749f2f179733cbe9e2eb4
DIFF: https://github.com/llvm/llvm-project/commit/fd941036bf2494cfbd2749f2f179733cbe9e2eb4.diff
LOG: Fix PR46880: Fail CHECK-NOT with undefined variable
Currently a CHECK-NOT directive succeeds whenever the corresponding
match fails. However match can fail due to an error rather than a lack
of match, for instance if a variable is undefined. This commit makes match
error a failure for CHECK-NOT.
Reviewed By: jdenny
Differential Revision: https://reviews.llvm.org/D86222
Added:
Modified:
llvm/lib/FileCheck/FileCheck.cpp
llvm/lib/FileCheck/FileCheckImpl.h
llvm/test/FileCheck/dump-input/annotations.txt
llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
llvm/test/FileCheck/numeric-expression.txt
llvm/test/FileCheck/var-scope.txt
llvm/unittests/FileCheck/FileCheckTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/FileCheck/FileCheck.cpp b/llvm/lib/FileCheck/FileCheck.cpp
index 12a0ae63419f5..950d236717db5 100644
--- a/llvm/lib/FileCheck/FileCheck.cpp
+++ b/llvm/lib/FileCheck/FileCheck.cpp
@@ -531,7 +531,7 @@ Expected<std::unique_ptr<NumericVariableUse>> Pattern::parseNumericVariableUse(
// we get below is null, it means no such variable was defined before. When
// that happens, we create a dummy variable so that parsing can continue. All
// uses of undefined variables, whether string or numeric, are then diagnosed
- // in printSubstitutions() after failing to match.
+ // in printNoMatch() after failing to match.
auto VarTableIter = Context->GlobalNumericVariableTable.find(Name);
NumericVariable *NumericVariable;
if (VarTableIter != Context->GlobalNumericVariableTable.end())
@@ -1250,6 +1250,7 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
// Substitute all string variables and expressions whose values are only
// now known. Use of string variables defined on the same line are handled
// by back-references.
+ Error Errs = Error::success();
for (const auto &Substitution : Substitutions) {
// Substitute and check for failure (e.g. use of undefined variable).
Expected<std::string> Value = Substitution->getResult();
@@ -1257,13 +1258,20 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
// Convert to an ErrorDiagnostic to get location information. This is
// done here rather than printMatch/printNoMatch since now we know which
// substitution block caused the overflow.
- Error Err =
- handleErrors(Value.takeError(), [&](const OverflowError &E) {
- return ErrorDiagnostic::get(SM, Substitution->getFromString(),
- "unable to substitute variable or "
- "numeric expression: overflow error");
- });
- return std::move(Err);
+ Errs = joinErrors(std::move(Errs),
+ handleErrors(
+ Value.takeError(),
+ [&](const OverflowError &E) {
+ return ErrorDiagnostic::get(
+ SM, Substitution->getFromString(),
+ "unable to substitute variable or "
+ "numeric expression: overflow error");
+ },
+ [&SM](const UndefVarError &E) {
+ return ErrorDiagnostic::get(SM, E.getVarName(),
+ E.message());
+ }));
+ continue;
}
// Plop it into the regex at the adjusted offset.
@@ -1271,6 +1279,8 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
Value->begin(), Value->end());
InsertOffset += Value->size();
}
+ if (Errs)
+ return std::move(Errs);
// Match the newly constructed regex.
RegExToMatch = TmpStr;
@@ -1349,35 +1359,18 @@ void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
for (const auto &Substitution : Substitutions) {
SmallString<256> Msg;
raw_svector_ostream OS(Msg);
- Expected<std::string> MatchedValue = Substitution->getResult();
- // Substitution failed or is not known at match time, print the undefined
- // variables it uses.
+ Expected<std::string> MatchedValue = Substitution->getResult();
+ // Substitution failures are handled in printNoMatch().
if (!MatchedValue) {
- bool UndefSeen = false;
- handleAllErrors(
- MatchedValue.takeError(), [](const NotFoundError &E) {},
- // Handled in printMatch and printNoMatch().
- [](const ErrorDiagnostic &E) {},
- // Handled in match().
- [](const OverflowError &E) {},
- [&](const UndefVarError &E) {
- if (!UndefSeen) {
- OS << "uses undefined variable(s):";
- UndefSeen = true;
- }
- OS << " ";
- E.log(OS);
- });
- if (!OS.tell())
- continue;
- } else {
- // Substitution succeeded. Print substituted value.
- OS << "with \"";
- OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
- OS.write_escaped(*MatchedValue) << "\"";
+ consumeError(MatchedValue.takeError());
+ continue;
}
+ OS << "with \"";
+ OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
+ OS.write_escaped(*MatchedValue) << "\"";
+
// We report only the start of the match/search range to suggest we are
// reporting the substitutions as set at the start of the match/search.
// Indicating a non-zero-length range might instead seem to imply that the
@@ -2140,12 +2133,6 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
if (Diags)
ErrorMsgs.push_back(E.getMessage().str());
},
- // UndefVarError is reported in printSubstitutions below.
- // FIXME: It probably should be handled as a pattern error and actually
- // change the exit status to 1, even if !ExpectedMatch. To do so, we
- // could stop calling printSubstitutions and actually report the error
- // here as we do ErrorDiagnostic above.
- [](const UndefVarError &E) {},
// NotFoundError is why printNoMatch was invoked.
[](const NotFoundError &E) {});
diff --git a/llvm/lib/FileCheck/FileCheckImpl.h b/llvm/lib/FileCheck/FileCheckImpl.h
index d4d891f7c1f0d..29e721e088d2c 100644
--- a/llvm/lib/FileCheck/FileCheckImpl.h
+++ b/llvm/lib/FileCheck/FileCheckImpl.h
@@ -229,8 +229,7 @@ class UndefVarError : public ErrorInfo<UndefVarError> {
/// Print name of variable associated with this error.
void log(raw_ostream &OS) const override {
- OS << "\"";
- OS.write_escaped(VarName) << "\"";
+ OS << "undefined variable: " << VarName;
}
};
@@ -756,8 +755,7 @@ class Pattern {
/// current values of FileCheck numeric variables and is updated if this
/// match defines new numeric values.
MatchResult match(StringRef Buffer, const SourceMgr &SM) const;
- /// Prints the value of successful substitutions or the name of the undefined
- /// string or numeric variables preventing a successful substitution.
+ /// Prints the value of successful substitutions.
void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
SMRange MatchRange, FileCheckDiag::MatchType MatchTy,
std::vector<FileCheckDiag> *Diags) const;
diff --git a/llvm/test/FileCheck/dump-input/annotations.txt b/llvm/test/FileCheck/dump-input/annotations.txt
index 3118ef67e4d60..45bf54698bca0 100644
--- a/llvm/test/FileCheck/dump-input/annotations.txt
+++ b/llvm/test/FileCheck/dump-input/annotations.txt
@@ -692,22 +692,17 @@
; SUBST-POS-NEXT:check:1'0 ^~~~~~~~~~~~~~~~~~~~~
; SUBST-POS-NEXT:check:1'1 with "DEF_MATCH1" equal to "def-match1"
; SUBST-POS-NEXT:check:1'2 with "DEF_MATCH2" equal to "def-match2"
-; SUBST-POS-NEXT:check:2'0 X error: no match found
-; SUBST-POS-NEXT:check:2'1 with "DEF_MATCH1" equal to "def-match1"
-; SUBST-POS-NEXT:check:2'2 uses undefined variable(s): "UNDEF"
+; SUBST-POS-NEXT:check:2'0 X error: match failed for invalid pattern
+; SUBST-POS-NEXT:check:2'1 undefined variable: UNDEF
+; SUBST-POS-NEXT:check:2'2 with "DEF_MATCH1" equal to "def-match1"
; SUBST-POS-NEXT:check:2'3 with "DEF_NOMATCH" equal to "foobar"
-; SUBST-POS-NEXT: 2: def-match1 def-nomatch
+; SUBST-POS-NEXT: 2: def-match1 def-nomatch
; SUBST-POS-NEXT:check:2'0 ~~~~~~~~~~~~~~~~~~~~~~~
-; SUBST-POS-NEXT:check:2'4 ? possible intended match
+; SUBST-POS-NEXT:check:2'4 ? possible intended match
; SUBST-POS-NEXT:>>>>>>
;--------------------------------------------------
; Substitutions: successful and failed negative directives.
-;
-; FIXME: The first CHECK-NOT directive below uses an undefined variable.
-; Because it therefore cannot match regardless of the input, the directive
-; succeeds. However, it should fail in this case. Update the example below
-; once that's fixed.
;--------------------------------------------------
; RUN: echo 'def-match1 def-nomatch' > %t.in
@@ -725,17 +720,17 @@
; RUN: | FileCheck -match-full-lines %s -check-prefix=SUBST-NEG
; SUBST-NEG:<<<<<<
-; SUBST-NEG-NEXT: 1: def-match1 def-nomatch
-; SUBST-NEG-NEXT:not:1'0 X~~~~~~~~~~~~~~~~~~~~~~
-; SUBST-NEG-NEXT:not:1'1 with "DEF_MATCH1" equal to "def-match1"
-; SUBST-NEG-NEXT:not:1'2 uses undefined variable(s): "UNDEF"
+; SUBST-NEG-NEXT: 1: def-match1 def-nomatch
+; SUBST-NEG-NEXT:not:1'0 X~~~~~~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
+; SUBST-NEG-NEXT:not:1'1 undefined variable: UNDEF
+; SUBST-NEG-NEXT:not:1'2 with "DEF_MATCH1" equal to "def-match1"
; SUBST-NEG-NEXT:not:1'3 with "DEF_NOMATCH" equal to "foobar"
-; SUBST-NEG-NEXT: 2: def-match1 def-match2
+; SUBST-NEG-NEXT: 2: def-match1 def-match2
; SUBST-NEG-NEXT:not:1'0 ~~~~~~~~~~~~~~~~~~~~~~
; SUBST-NEG-NEXT:not:2'0 !~~~~~~~~~~~~~~~~~~~~ error: no match expected
; SUBST-NEG-NEXT:not:2'1 with "DEF_MATCH1" equal to "def-match1"
; SUBST-NEG-NEXT:not:2'2 with "DEF_MATCH2" equal to "def-match2"
-; SUBST-NEG-NEXT: 3: END
+; SUBST-NEG-NEXT: 3: END
; SUBST-NEG-NEXT:check:3 ^~~
; SUBST-NEG-NEXT:>>>>>>
diff --git a/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt b/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
index 4513fdc4f8fc6..a40597d90d053 100644
--- a/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
+++ b/llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt
@@ -20,8 +20,8 @@ ERR-VV-EMPTY:
ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
ERR-NEXT:CHECK-NOT: {{.*}}
ERR-NEXT:{{ *}}^
- ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
- ERR-NEXT:10000000000000000
+ ERR-NEXT:{{.*}}: error: undefined variable: UNDEFVAR
+ ERR-NEXT:{{CHECK-NOT: \[\[#0x8000000000000000\+0x8000000000000000\]\] \[\[UNDEFVAR\]\]}}
ERR-NEXT:^
ERR-NOT:{{error|note|remark}}:
@@ -29,7 +29,7 @@ ERR-VV-EMPTY:
DUMP-NEXT: 1: 10000000000000000
DUMP-NEXT:not:1'0 X~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
DUMP-NEXT:not:1'1 unable to substitute variable or numeric expression: overflow error
- DUMP-NEXT:not:1'2 uses undefined variable(s): "UNDEFVAR"
+ DUMP-NEXT:not:1'2 undefined variable: UNDEFVAR
DUMP-VV-NEXT: 2:
DUMP-VV-NEXT:eof:1 ^
DUMP-NEXT:>>>>>>
diff --git a/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt b/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
index 6c55b545c031e..577f018e4782f 100644
--- a/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
+++ b/llvm/test/FileCheck/match-time-error-propagation/invalid-expected-pattern.txt
@@ -9,8 +9,8 @@ RUN: echo > %t.in '10000000000000000'
ERR:{{.*}}: error: unable to substitute variable or numeric expression: overflow error
ERR-NEXT:CHECK: {{.*}}
ERR-NEXT:{{ *}}^
- ERR-NEXT:<stdin>:1:1: note: uses undefined variable(s): "UNDEFVAR"
- ERR-NEXT:10000000000000000
+ ERR-NEXT:{{.*}}: error: undefined variable: UNDEFVAR
+ ERR-NEXT:{{CHECK: \[\[#0x8000000000000000\+0x8000000000000000\]\] \[\[UNDEFVAR\]\]}}
ERR-NEXT:^
ERR-NOT:{{error|note|remark}}:
@@ -18,7 +18,7 @@ RUN: echo > %t.in '10000000000000000'
DUMP-NEXT: 1: 10000000000000000
DUMP-NEXT:check:1'0 X~~~~~~~~~~~~~~~~~ error: match failed for invalid pattern
DUMP-NEXT:check:1'1 unable to substitute variable or numeric expression: overflow error
-DUMP-NEXT:check:1'2 uses undefined variable(s): "UNDEFVAR"
+DUMP-NEXT:check:1'2 undefined variable: UNDEFVAR
DUMP-NEXT:>>>>>>
;--------------------------------------------------
diff --git a/llvm/test/FileCheck/numeric-expression.txt b/llvm/test/FileCheck/numeric-expression.txt
index 0a7f2f3d2255c..d76552651ef8a 100644
--- a/llvm/test/FileCheck/numeric-expression.txt
+++ b/llvm/test/FileCheck/numeric-expression.txt
@@ -400,15 +400,29 @@ UNDEF VAR USE
UNDEFVAR: 11
UNDEF-USE-LABEL: UNDEF VAR USE
UNDEF-USE-NEXT: UNDEFVAR: [[#UNDEFVAR1+UNDEFVAR2]]
-UNDEF-USE-MSG: numeric-expression.txt:[[#@LINE-1]]:17: error: {{U}}NDEF-USE-NEXT: expected string not found in input
-UNDEF-USE-MSG-NEXT: {{U}}NDEF-USE-NEXT: UNDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
-UNDEF-USE-MSG-NEXT: {{^}} ^{{$}}
-UNDEF-USE-MSG-NEXT: numeric-expression.txt:[[#@LINE-7]]:14: note: scanning from here
-UNDEF-USE-MSG-NEXT: UNDEF VAR USE
-UNDEF-USE-MSG-NEXT: {{^}} ^{{$}}
-UNDEF-USE-MSG-NEXT: numeric-expression.txt:[[#@LINE-10]]:14: note: uses undefined variable(s): "UNDEFVAR1" "UNDEFVAR2"
-UNDEF-USE-MSG-NEXT: UNDEF VAR USE
-UNDEF-USE-MSG-NEXT: {{^}} ^{{$}}
+UNDEF-USE-MSG: numeric-expression.txt:[[#@LINE-1]]:30: error: undefined variable: UNDEFVAR1
+UNDEF-USE-MSG-NEXT: {{U}}NDEF-USE-NEXT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG-NEXT: {{^}} ^{{$}}
+UNDEF-USE-MSG-NEXT: numeric-expression.txt:[[#@LINE-4]]:40: error: undefined variable: UNDEFVAR2
+UNDEF-USE-MSG-NEXT: {{U}}NDEF-USE-NEXT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG-NEXT: {{^}} ^{{$}}
+
+; Numeric expression in negative directive using undefined variables.
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck --check-prefix UNDEF-USE2 --input-file %s %s 2>&1 \
+RUN: | FileCheck --strict-whitespace --check-prefix UNDEF-USE-MSG2 %s
+
+CHECK NOT UNDEF VAR USE
+END MARKER
+UNDEF-USE2-LABEL: CHECK NOT UNDEF VAR USE
+UNDEF-USE2-NOT: UNDEFVAR: [[#UNDEFVAR1+UNDEFVAR2]]
+UNDEF-USE2: END MARKER
+UNDEF-USE-MSG2: numeric-expression.txt:[[#@LINE-2]]:30: error: undefined variable: UNDEFVAR1
+UNDEF-USE-MSG2-NEXT: {{U}}NDEF-USE2-NOT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG2-NEXT: {{^}} ^{{$}}
+UNDEF-USE-MSG2-NEXT: numeric-expression.txt:[[#@LINE-5]]:40: error: undefined variable: UNDEFVAR2
+UNDEF-USE-MSG2-NEXT: {{U}}NDEF-USE2-NOT: {{U}}NDEFVAR: {{\[\[#UNDEFVAR1\+UNDEFVAR2\]\]}}
+UNDEF-USE-MSG2-NEXT: {{^}} ^{{$}}
; Numeric expression with unsupported operator.
RUN: %ProtectFileCheckOutput \
diff --git a/llvm/test/FileCheck/var-scope.txt b/llvm/test/FileCheck/var-scope.txt
index e45610b138ef8..9b3ea0e95d143 100644
--- a/llvm/test/FileCheck/var-scope.txt
+++ b/llvm/test/FileCheck/var-scope.txt
@@ -7,11 +7,11 @@ RUN: FileCheck --check-prefixes CHECK,LOCAL3,GLOBAL --input-file %s %s
RUN: FileCheck --check-prefixes CHECK,GLOBAL --enable-var-scope --input-file %s %s
RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL1 --enable-var-scope --input-file %s %s 2>&1 \
-RUN: | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL %s
+RUN: | FileCheck --check-prefix ERRUNDEFLOCAL %s
RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL2 --enable-var-scope --input-file %s %s 2>&1 \
-RUN: | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCNUM %s
+RUN: | FileCheck --check-prefix ERRUNDEFLOCNUM %s
RUN: %ProtectFileCheckOutput not FileCheck --check-prefixes CHECK,LOCAL3 --enable-var-scope --input-file %s %s 2>&1 \
-RUN: | FileCheck --check-prefixes ERRUNDEF,ERRUNDEFLOCAL,ERRUNDEFLOCNUM %s
+RUN: | FileCheck --check-prefixes ERRUNDEFLOCAL,ERRUNDEFLOCNUM %s
local1
global1
@@ -33,6 +33,5 @@ LOCAL2: local[[#LOCNUM+2]]
LOCAL3: [[LOCAL]][[#LOCNUM+2]]
GLOBAL: [[$GLOBAL]][[#$GLOBNUM+2]]
-ERRUNDEF: expected string not found in input
-ERRUNDEFLOCAL: uses undefined variable(s): "LOCAL"
-ERRUNDEFLOCNUM: uses undefined variable(s): "LOCNUM"
+ERRUNDEFLOCAL: undefined variable: LOCAL
+ERRUNDEFLOCNUM: undefined variable: LOCNUM
diff --git a/llvm/unittests/FileCheck/FileCheckTest.cpp b/llvm/unittests/FileCheck/FileCheckTest.cpp
index 59befc0efb8a5..60ada3836a6a1 100644
--- a/llvm/unittests/FileCheck/FileCheckTest.cpp
+++ b/llvm/unittests/FileCheck/FileCheckTest.cpp
@@ -1445,8 +1445,10 @@ TEST_F(FileCheckTest, Match) {
Succeeded());
Tester.initNextPattern();
// Match with substitution failure.
- ASSERT_FALSE(Tester.parsePattern("[[#UNKNOWN]]"));
- expectUndefErrors({"UNKNOWN"}, Tester.match("FOO").takeError());
+ ASSERT_FALSE(Tester.parsePattern("[[#UNKNOWN1+UNKNOWN2]]"));
+ expectSameErrors<ErrorDiagnostic>(
+ {"undefined variable: UNKNOWN1", "undefined variable: UNKNOWN2"},
+ Tester.match("FOO").takeError());
Tester.initNextPattern();
// Check that @LINE matches the later (given the calls to initNextPattern())
// line number.
More information about the llvm-commits
mailing list