Skip to content

Commit 50e0a25

Browse files
Add support for -Xclang -ast-dump to facilitate debugging. (#706)
Add documentation for the feature, but first, split out a "developer guide" from CONTRIBUTING.md to have a good place to put the documentation and make some other minor revisions.
1 parent 5f70035 commit 50e0a25

File tree

4 files changed

+219
-125
lines changed

4 files changed

+219
-125
lines changed

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

Lines changed: 15 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -49,128 +49,18 @@ working, so you may have to wait for us to address 5C-specific
4949
problems arising from your 3C pull request and/or we may ask you to
5050
make specific changes to your pull request to accommodate 5C's code.
5151

52-
## Testing
53-
54-
3C has a regression test suite located in `clang/test/3C`. At the
55-
appropriate time during development of a pull request, please run it
56-
and correct any failures. (For example, it may not make sense to run
57-
it on a draft pull request containing an unfinished demonstration of
58-
an idea.) The easiest way to run it is to run the following in your
59-
build directory:
60-
61-
```
62-
ninja check-3c
63-
```
64-
65-
This command will build everything needed that hasn't already been
66-
built, run the test suite, report success or failure (exit 0 or 1, so
67-
you can use it in scripts), and display some information about any
68-
failures, which may or may not be enough for you to understand what
69-
went wrong.
70-
71-
For deeper troubleshooting, run the following in your build directory
72-
to build all dependencies of the test suite:
73-
74-
```
75-
ninja check-3c-deps
76-
```
77-
78-
Then run the following in the `clang/test/3C` directory:
79-
80-
```
81-
llvm-lit -vv TEST.c
82-
```
83-
84-
where `TEST.c` is the path of the test you want to run (you can also
85-
specify more than one test). This assumes you've put the `bin`
86-
subdirectory of your build directory on your `$PATH` or arranged some
87-
other means of running `llvm-lit` from there. The first `-v` makes
88-
`llvm-lit` display the stdout and stderr of failed tests; the second
89-
makes it display the `RUN` commands as they execute so you can tell
90-
which one failed.
91-
92-
Every `.c` file under `clang/test/3C` is a test file. There are a few
93-
in subdirectories, so `*.c` will not pick up all of them; instead you
94-
can use `llvm-lit -vv .` to specify all test files under the current
95-
directory.
96-
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-
149-
## Coding guidelines
150-
151-
Please follow [LLVM coding
152-
standards](https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)
153-
in your code. Specifically:
154-
155-
* The maximum length of a line: 80 chars
156-
157-
* All comments should start with a Capital letter.
158-
159-
* All Local variables, including fields and parameters, should start
160-
with a Capital letter (similar to PascalCase). Short names are
161-
preferred.
162-
163-
* A space between the conditional keyword and `(` i.e., `if (`,
164-
`while (`, ``for (` etc.
165-
166-
* Space after the type name, i.e., `Type *K` _not_ `Type* K`.
167-
168-
* Space before and after `:` in iterators, i.e., `for (auto &k : List)`
169-
170-
Our goal is that all files should be formatted with `clang-format` and
171-
pass `clang-tidy` ([more information](clang-tidy.md)), and nonempty
172-
files should have a final newline (surprisingly, `clang-format` cannot
173-
enforce this). However, until we have better automation, we decided it
174-
isn't reasonable to require contributors to manually run these tools
175-
and fix style nits in each change; instead, we periodically run the
176-
tools on the entire 3C codebase.
52+
At the appropriate time during development of a pull request, please
53+
run the [regression tests](development.md#regression-tests) and
54+
correct any failures. (For example, it may not make sense to do this
55+
on a draft pull request containing an unfinished demonstration of an
56+
idea.) All regression tests must pass (or be disabled if appropriate)
57+
before your pull request can be merged. If you're changing behavior
58+
(as opposed to just cleaning up the code), we'll typically require you
59+
to add or update enough tests to exercise the important behavior
60+
changes (i.e., those tests fail before your code change and pass after
61+
it). If there's a concern that your change might affect other cases
62+
that are not adequately tested yet, we may ask you to add tests for
63+
those cases as well.
64+
65+
See the [developer's guide](development.md) for additional information
66+
that may be helpful as you work on 3C.

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

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# 3C Developer's Guide
2+
3+
This file collects information that is important or useful to know
4+
when developing 3C as well as some standards we generally expect the
5+
code to follow. (Some other standards are covered in the [contribution
6+
guide](CONTRIBUTING.md)). If you could use help with something that's
7+
not documented here, feel free to ask.
8+
9+
## Regression tests
10+
11+
3C has a regression test suite located in `clang/test/3C`. The easiest
12+
way to run it is to run the following in your build directory:
13+
14+
```
15+
ninja check-3c
16+
```
17+
18+
This command will build everything needed that hasn't already been
19+
built, run the test suite, report success or failure (exit 0 or 1, so
20+
you can use it in scripts), and display some information about any
21+
failures, which may or may not be enough for you to understand what
22+
went wrong.
23+
24+
For deeper troubleshooting, run the following in your build directory
25+
to build all dependencies of the test suite:
26+
27+
```
28+
ninja check-3c-deps
29+
```
30+
31+
Then run the following in the `clang/test/3C` directory:
32+
33+
```
34+
llvm-lit -vv TEST.c
35+
```
36+
37+
where `TEST.c` is the path of the test you want to run (you can also
38+
specify more than one test). This assumes you've put the `bin`
39+
subdirectory of your build directory on your `$PATH` or arranged some
40+
other means of running `llvm-lit` from there. The first `-v` makes
41+
`llvm-lit` display the stdout and stderr of failed tests; the second
42+
makes it display the `RUN` commands as they execute so you can tell
43+
which one failed.
44+
45+
Every `.c` file under `clang/test/3C` is a test file. There are a few
46+
in subdirectories, so `*.c` will not pick up all of them; instead you
47+
can use `llvm-lit -vv .` to specify all test files under the current
48+
directory.
49+
50+
### Diagnostic verification
51+
52+
3C supports the standard Clang diagnostic verifier
53+
([`VerifyDiagnosticConsumer`](https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details))
54+
for testing errors and warnings reported by 3C via its main `DiagnosticsEngine`.
55+
(Some 3C errors and warnings are reported via other means and cannot be tested
56+
this way; the best solution we have for them right now is to `grep` the stderr
57+
of 3C.) Diagnostic verification can be enabled via the usual `-Xclang -verify`
58+
compiler option; other diagnostic verification options (`-Xclang
59+
-verify=PREFIX`, etc.) should also work as normal. These must be passed as
60+
_compiler_ options, not `3c` options; for example, if you are using `--` on the
61+
`3c` command line, these options must be passed _after_ the `--`.
62+
63+
Some notes about diagnostic verification in the context of 3C:
64+
65+
* Parsing of the source files uses some of the compiler logic and thus may
66+
generate compiler warnings, just as if you ran `clang` on the code. These are
67+
sent to the diagnostic verifier along with diagnostics generated by 3C's
68+
analysis. If you find it distracting to have to include the compiler warnings
69+
in the set of expected diagnostics for a test, you can turn them off via the
70+
`-Wno-everything` compiler option (which does not affect diagnostics generated
71+
by 3C's analysis).
72+
73+
* The `3c` tool works in several passes, where each pass runs on all translation
74+
units: first `3c` parses the source files, then it runs several passes of
75+
analysis. If a pass encounters at least one error, `3c` exits at the end of
76+
that pass. Diagnostic verification does not change the _point_ at which `3c`
77+
exits, but it changes the exit _code_ to indicate the result of verification
78+
rather than the presence of errors. The verification includes the diagnostics
79+
from all passes up to the point at which `3c` exits (i.e., the same
80+
diagnostics that would be displayed if verification were not used). However,
81+
an error that doesn't go via the main `DiagnosticsEngine` will cause an
82+
unsuccessful exit code regardless of diagnostic verification. (This is
83+
typically the behavior you want for a test.)
84+
85+
* Diagnostic verification is independent for each translation unit, so in tests
86+
with multiple translation units, you'll have to be careful that preprocessing
87+
of each translation unit sees the correct set of `expected-*` directives for
88+
the diagnostics generated for that translation unit (or an
89+
`expected-no-diagnostics` directive if that translation unit generates no
90+
diagnostics, even if other translation units do generate diagnostics). Be
91+
warned that since which translation unit generated a given diagnostic isn't
92+
visible to a normal user, we don't put much work into coming up with sensible
93+
rules for this, but it should at least be deterministic for testing.
94+
95+
Note that some 3C tests use diagnostic verification on calls to `clang` rather
96+
than `3c`, so if you see `expected-*` directives in a test, you can look at the
97+
`RUN` commands to see which command has `-Xclang -verify` and is using the
98+
directives. If you want to verify diagnostics of more than one `RUN` command in
99+
the same test, you can use different directive prefixes (`-Xclang
100+
-verify=PREFIX`).
101+
102+
## Debugging
103+
104+
### AST dumping
105+
106+
`3c` honors the usual `-Xclang -ast-dump` compiler option to dump the
107+
AST to stdout before running its analysis. (This must be passed as a
108+
compiler option, not a `3c` option, e.g., _after_ the `--` on the
109+
command line if applicable.) The dump includes the AST node memory
110+
addresses (e.g., `VarDecl 0x5569e5ef3988 ...`), so if you see in the
111+
debugger that (for example) a `Decl *` variable in 3C has the value
112+
`0x5569e5ef3988`, you can look for that memory address in the AST dump
113+
to quickly see what AST node it is (modulo pointer adjustments for
114+
multiple inheritance; adding a debugger watch with a cast to the
115+
pointer type used in the AST dump can help with this).
116+
117+
## Coding guidelines
118+
119+
Please follow [LLVM coding
120+
standards](https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)
121+
in your code. Specifically:
122+
123+
* The maximum length of a line: 80 chars
124+
125+
* All comments should start with a Capital letter.
126+
127+
* All Local variables, including fields and parameters, should start
128+
with a Capital letter (similar to PascalCase). Short names are
129+
preferred.
130+
131+
* A space between the conditional keyword and `(` i.e., `if (`,
132+
`while (`, ``for (` etc.
133+
134+
* Space after the type name, i.e., `Type *K` _not_ `Type* K`.
135+
136+
* Space before and after `:` in iterators, i.e., `for (auto &k : List)`
137+
138+
Our goal is that all files should be formatted with `clang-format` and
139+
pass `clang-tidy` ([more information](clang-tidy.md)), and nonempty
140+
files should have a final newline (surprisingly, `clang-format` cannot
141+
enforce this). However, until we have better automation, we decided it
142+
isn't reasonable to require contributors to manually run these tools
143+
and fix style nits in each change; instead, we periodically run the
144+
tools on the entire 3C codebase.

clang/lib/3C/3C.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "clang/3C/ConstraintBuilder.h"
1717
#include "clang/3C/IntermediateToolHook.h"
1818
#include "clang/3C/RewriteUtils.h"
19+
#include "clang/Frontend/ASTConsumers.h"
1920
#include "clang/Frontend/VerifyDiagnosticConsumer.h"
2021
#include "clang/Tooling/ArgumentsAdjusters.h"
2122
#include "llvm/Support/TargetSelect.h"
@@ -251,9 +252,57 @@ class _3CASTBuilderAction : public ToolAction {
251252
if (!AST)
252253
return false;
253254

255+
handleExtraProgramAction(Invocation->getFrontendOpts(),
256+
AST->getASTContext());
257+
254258
ASTs.push_back(std::move(AST));
255259
return true;
256260
}
261+
262+
private:
263+
void handleExtraProgramAction(FrontendOptions &Opts,
264+
ASTContext &C) {
265+
// The Opts.ProgramAction field is normally used only by `clang -cc1` to
266+
// select a FrontendAction (see CreateFrontendBaseAction in
267+
// ExecuteCompilerInvocation.cpp) and is ignored by LibTooling tools, which
268+
// perform a custom FrontendAction. But we want to support at least AST
269+
// dumping (as an addition to 3C's normal workflow) since it's useful for
270+
// debugging 3C, and we prefer to honor the standard `-Xclang -ast-dump`
271+
// option rather than define our own tool-level option like clang-check
272+
// does. We could add support for more `-ast-*` options here if desired.
273+
switch (Opts.ProgramAction) {
274+
case frontend::ParseSyntaxOnly:
275+
// Nothing extra to do.
276+
break;
277+
case frontend::ASTDump: {
278+
// Code copied from ASTDumpAction::CreateASTConsumer since we don't have a
279+
// good way to actually use ASTDumpAction from here. :/
280+
//
281+
// XXX: Maybe we'd prefer to output this somewhere other than stdout to
282+
// separate it from the updated main file written to stdout? This doesn't
283+
// look trivial because ASTPrinter requires ownership of the output
284+
// stream, and it probably isn't important for the intended debugging use
285+
// case.
286+
std::unique_ptr<ASTConsumer> Dumper =
287+
CreateASTDumper(nullptr /*Dump to stdout.*/, Opts.ASTDumpFilter,
288+
Opts.ASTDumpDecls, Opts.ASTDumpAll,
289+
Opts.ASTDumpLookups, Opts.ASTDumpDeclTypes,
290+
Opts.ASTDumpFormat);
291+
// In principle, we should call all the ASTConsumer methods the same way
292+
// the normal AST parsing process would, but there isn't an obvious way to
293+
// do that when using ASTUnit. Instead, we rely on the assumption
294+
// (apparently valid as of this writing) that the only ASTConsumer method
295+
// that has a nonempty implementation in ASTPrinter is
296+
// HandleTranslationUnit, and we just call HandleTranslationUnit manually.
297+
Dumper->HandleTranslationUnit(C);
298+
break;
299+
}
300+
default:
301+
llvm::errs() << "Warning: The requested ProgramAction is not implemented "
302+
"by 3C and will be ignored.\n";
303+
break;
304+
}
305+
}
257306
};
258307

259308
void dumpConstraintOutputJson(const std::string &PostfixStr,

clang/test/3C/ast_dump.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: rm -rf %t*
2+
3+
// Turn off color in the AST dump so that we can more easily match it
4+
// (https://stackoverflow.com/questions/32447542/how-do-i-get-clang-to-dump-the-ast-without-color).
5+
// RUN: 3c -base-dir=%S %s -- -Xclang -ast-dump -fno-color-diagnostics | FileCheck -match-full-lines %s
6+
7+
// RUN: 3c -base-dir=%S %s -- -Xclang -ast-list 2>%t.stderr
8+
// RUN: grep 'The requested ProgramAction is not implemented by 3C' %t.stderr
9+
10+
int *p;
11+
// CHECK: `-VarDecl {{.*}} p 'int *'

0 commit comments

Comments
 (0)