Skip to content

Commit

Permalink
Migrate the difftypes* tests to the diagnostic verifier.
Browse files Browse the repository at this point in the history
This feels a little cleaner than manually scanning the stderr and lets
us check the diagnostics more precisely, for what that's worth. We
couldn't do this before #488 both because 3C exited before the
diagnostics could be verified and because 3C didn't support custom
verify prefixes. #488 solved the first problem, and this PR solves the
second.
  • Loading branch information
mattmccutchen-cci committed Jun 14, 2021
1 parent 54924ac commit 7e813d1
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 15 deletions.
18 changes: 17 additions & 1 deletion clang/lib/3C/3C.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ class _3CDiagnosticConsumer : public DiagnosticConsumer {
public:
_3CDiagnosticConsumer(DiagnosticsEngine &Engine)
: UnderlyingConsumer(Engine.takeClient()) {
// This code currently only supports the default LibTooling setup in which
// the ClangTool has no global DiagnosticConsumer, so
// CompilerInstance::createDiagnostics creates one owned by the
// DiagnosticsEngine. If we needed to support the case in which the
// DiagnosticConsumer isn't owned, we could use an "UnderlyingConsumerOwner"
// pattern like VerifyDiagnosticConsumer does.
assert(UnderlyingConsumer);
}

Expand Down Expand Up @@ -153,7 +159,17 @@ class _3CDiagnosticConsumer : public DiagnosticConsumer {
return UnderlyingConsumer->getNumErrors() == 0;
}

~_3CDiagnosticConsumer() { assert(CurrentState == S_Done); }
~_3CDiagnosticConsumer() {
// We considered asserting that the state is S_Done here, but if
// ASTUnit::LoadFromCompilerInvocation fails and returns null, the
// _3CDiagnosticConsumer may be destructed without reaching S_Done. However,
// even without such an assertion, we're still well protected against
// forgetting to finish verification and missing an error because if the
// ASTUnit is null, _3CInterface::parseASTs will record a "non-diagnostic
// error", while if the ASTUnit is non-null, it will get added to
// _3CInterface::ASTs and determineExitCode will call finish3CAnalysis. (And
// the _3CInterface destructor enforces that determineExitCode is called.)
}
};

// Based on LibTooling's ASTBuilderAction but does several custom things that we
Expand Down
14 changes: 10 additions & 4 deletions clang/test/3C/difftypes1a.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
//RUN: rm -rf %t*
//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- 2>%t.stderr
//RUN: grep -q "merging failed" %t.stderr
// Since the RUN commands in difftypes1a.c and difftypes1b.c process the two
// files in different orders and the location where the error is reported
// depends on the order, we need to use a different diagnostic verification
// prefix (and set of corresponding comments) for each RUN command. Verification
// is per translation unit, so the translation unit with no diagnostic needs
// `expected-no-diagnostics`.

// The desired behavior in this case is to fail, so other checks are omitted
//RUN: rm -rf %t*
//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- -Xclang -verify=ab-expected

// ab-expected-no-diagnostics
// ba-expected-error@+1 {{merging failed for 'foo'}}
_Ptr<int> foo(int, char);
14 changes: 10 additions & 4 deletions clang/test/3C/difftypes1b.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
//RUN: rm -rf %t*
//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1a.c -- 2>%t.stderr
//RUN: grep -q "merging failed" %t.stderr
// Since the RUN commands in difftypes1a.c and difftypes1b.c process the two
// files in different orders and the location where the error is reported
// depends on the order, we need to use a different diagnostic verification
// prefix (and set of corresponding comments) for each RUN command. Verification
// is per translation unit, so the translation unit with no diagnostic needs
// `expected-no-diagnostics`.

// The desired behavior in this case is to fail, so other checks are omitted
//RUN: rm -rf %t*
//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1a.c -- -Xclang -verify=ba-expected

// ba-expected-no-diagnostics
// ab-expected-error@+1 {{merging failed for 'foo'}}
int *foo(int, char *);
12 changes: 10 additions & 2 deletions clang/test/3C/difftypes2a.c
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
// Since the RUN commands in difftypes2a.c and difftypes2b.c process the two
// files in different orders and the location where the error is reported
// depends on the order, we need to use a different diagnostic verification
// prefix (and set of corresponding comments) for each RUN command. Verification
// is per translation unit, so the translation unit with no diagnostic needs
// `expected-no-diagnostics`.

// RUN: rm -rf %t*
// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2b.c -- 2>%t.stderr
// RUN: grep -q "merging failed" %t.stderr
// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2b.c -- -Xclang -verify=ab-expected

// The desired behavior in this case is to fail, so other checks are omitted

// Test no body vs body

// ab-expected-no-diagnostics
// ba-expected-error@+1 {{merging failed for 'foo'}}
void foo(char *x);
14 changes: 10 additions & 4 deletions clang/test/3C/difftypes2b.c
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
// RUN: rm -rf %t*
// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2a.c -- 2>%t.stderr
// RUN: grep -q "merging failed" %t.stderr
// Since the RUN commands in difftypes2a.c and difftypes2b.c process the two
// files in different orders and the location where the error is reported
// depends on the order, we need to use a different diagnostic verification
// prefix (and set of corresponding comments) for each RUN command. Verification
// is per translation unit, so the translation unit with no diagnostic needs
// `expected-no-diagnostics`.

// The desired behavior in this case is to fail, so other checks are omitted
// RUN: rm -rf %t*
// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2a.c -- -Xclang -verify=ba-expected

// Test body vs no body

// ba-expected-no-diagnostics
// ab-expected-error@+1 {{merging failed for 'foo'}}
void foo(char **y) {
// this is the body
}

0 comments on commit 7e813d1

Please sign in to comment.