Skip to content

Commit 76b6266

Browse files
Fix the diagnostic verifier, plus a few other fixes. (#598)
* Reverse name of 3c -disccty option to reflect what it actually does. By default, 3c adds the -f3c-tool compiler option to disable the Checked C type checker. 3c's -disccty option (apparently an abbreviation for "disable Checked C type checker") made 3c _not_ pass -f3c-tool, hence leaving the type checker _enabled_. 3C's internal flag variable, DisableCCTypeChecker, was also backwards, though interestingly, the `3c -help` text was correct ("Do not disable checked c type checker"). This commit renames -disccty to -enccty and renames DisableCCTypeChecker to EnableCCTypeChecker. We take the opportunity to further improve the `3c -help` text too. * Completely remove the old `3c -verify` option. Now that 3C only runs one compiler "invocation" per translation unit, with the new diagnostic verifier implementation, callers will be able to just use the `-Xclang -verify` compiler option. And given that we may want to use other diagnostic verifier options (e.g., `-verify=PREFIX` or `-verify-ignore-unexpected`), let's standardize on using the original compiler options rather than trying to define `3c` options wrapping all of them. * Switch from ClangTool::buildASTs to a custom _3CASTBuilderAction. Replace the existing ArgumentsAdjusters with modifications to the option data structures in _3CASTBuilderAction. More will be added to _3CASTBuilderAction in the subsequent commits. Fixes #536. * Canonicalize -I directory paths in _3CASTBuilderAction. This works per translation unit, so hopefully it is a complete and correct fix to #515 that benefits all callers of 3C. Remove the previous crude workaround from convert_project. Fixes #515. * Make diagnostic verification work after the redesign of #488. Re-enable diagnostic verification in regression tests where it still succeeds. It remains disabled in some tests in which failures were introduced while it was disabled; #609 is for that. Fixes #503. * Migrate the difftypes* tests to the diagnostic verifier. 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. * Document diagnostic verification in 3C's CONTRIBUTING.md.
1 parent e549c91 commit 76b6266

19 files changed

+432
-247
lines changed

clang/docs/checkedc/3C/CONTRIBUTING.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,58 @@ in subdirectories, so `*.c` will not pick up all of them; instead you
9494
can use `llvm-lit -vv .` to specify all test files under the current
9595
directory.
9696

97+
### Diagnostic verification
98+
99+
3C supports the standard Clang diagnostic verifier
100+
([`VerifyDiagnosticConsumer`](https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details))
101+
for testing errors and warnings reported by 3C via its main `DiagnosticsEngine`.
102+
(Some 3C errors and warnings are reported via other means and cannot be tested
103+
this way; the best solution we have for them right now is to `grep` the stderr
104+
of 3C.) Diagnostic verification can be enabled via the usual `-Xclang -verify`
105+
compiler option; other diagnostic verification options (`-Xclang
106+
-verify=PREFIX`, etc.) should also work as normal. These must be passed as
107+
_compiler_ options, not `3c` options; for example, if you are using `--` on the
108+
`3c` command line, these options must be passed _after_ the `--`.
109+
110+
Some notes about diagnostic verification in the context of 3C:
111+
112+
* Parsing of the source files uses some of the compiler logic and thus may
113+
generate compiler warnings, just as if you ran `clang` on the code. These are
114+
sent to the diagnostic verifier along with diagnostics generated by 3C's
115+
analysis. If you find it distracting to have to include the compiler warnings
116+
in the set of expected diagnostics for a test, you can turn them off via the
117+
`-Wno-everything` compiler option (which does not affect diagnostics generated
118+
by 3C's analysis).
119+
120+
* The `3c` tool works in several passes, where each pass runs on all translation
121+
units: first `3c` parses the source files, then it runs several passes of
122+
analysis. If a pass encounters at least one error, `3c` exits at the end of
123+
that pass. Diagnostic verification does not change the _point_ at which `3c`
124+
exits, but it changes the exit _code_ to indicate the result of verification
125+
rather than the presence of errors. The verification includes the diagnostics
126+
from all passes up to the point at which `3c` exits (i.e., the same
127+
diagnostics that would be displayed if verification were not used). However,
128+
an error that doesn't go via the main `DiagnosticsEngine` will cause an
129+
unsuccessful exit code regardless of diagnostic verification. (This is
130+
typically the behavior you want for a test.)
131+
132+
* Diagnostic verification is independent for each translation unit, so in tests
133+
with multiple translation units, you'll have to be careful that preprocessing
134+
of each translation unit sees the correct set of `expected-*` directives for
135+
the diagnostics generated for that translation unit (or an
136+
`expected-no-diagnostics` directive if that translation unit generates no
137+
diagnostics, even if other translation units do generate diagnostics). Be
138+
warned that since which translation unit generated a given diagnostic isn't
139+
visible to a normal user, we don't put much work into coming up with sensible
140+
rules for this, but it should at least be deterministic for testing.
141+
142+
Note that some 3C tests use diagnostic verification on calls to `clang` rather
143+
than `3c`, so if you see `expected-*` directives in a test, you can look at the
144+
`RUN` commands to see which command has `-Xclang -verify` and is using the
145+
directives. If you want to verify diagnostics of more than one `RUN` command in
146+
the same test, you can use different directive prefixes (`-Xclang
147+
-verify=PREFIX`).
148+
97149
## Coding guidelines
98150

99151
Please follow [LLVM coding

clang/include/clang/3C/3C.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct _3COptions {
5757

5858
bool AddCheckedRegions;
5959

60-
bool DisableCCTypeChecker;
60+
bool EnableCCTypeChecker;
6161

6262
bool WarnRootCause;
6363

@@ -68,11 +68,6 @@ struct _3COptions {
6868
bool ForceItypes;
6969
#endif
7070

71-
// Currently applies only to the rewriting phase (because it is the only phase
72-
// that generates diagnostics, except for the declaration merging diagnostics
73-
// that are currently fatal) and uses the default "expected" prefix.
74-
bool VerifyDiagnosticOutput;
75-
7671
bool DumpUnwritableChanges;
7772
bool AllowUnwritableChanges;
7873

@@ -104,6 +99,8 @@ class _3CInterface {
10499
const std::vector<std::string> &SourceFileList,
105100
clang::tooling::CompilationDatabase *CompDB);
106101

102+
~_3CInterface();
103+
107104
// Call clang to provide the data
108105
bool parseASTs();
109106

@@ -141,10 +138,25 @@ class _3CInterface {
141138
// Dump all stats related to performance.
142139
bool dumpStats();
143140

141+
// Determine the exit code that the `3c` tool should exit with after the last
142+
// 3C stage, considering diagnostic verification. Must be called exactly once
143+
// before the _3CInterface is destructed (unless construction failed).
144+
int determineExitCode();
145+
144146
private:
145147
_3CInterface(const struct _3COptions &CCopt,
146148
const std::vector<std::string> &SourceFileList,
147-
clang::tooling::CompilationDatabase *CompDB, bool &Failed);
149+
clang::tooling::CompilationDatabase *CompDB);
150+
151+
bool ConstructionFailed = false;
152+
bool DeterminedExitCode = false;
153+
154+
bool HadNonDiagnosticError = false;
155+
156+
// Determine whether 3C can continue to the next stage of processing. Checks
157+
// HadNonDiagnosticError and error diagnostics but ignores diagnostic
158+
// verification.
159+
bool isSuccessfulSoFar();
148160

149161
// saved ASTs
150162
std::vector< std::unique_ptr< ASTUnit >> ASTs;

0 commit comments

Comments
 (0)