andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 1 | # Clang Tool Refactoring |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 2 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 3 | [TOC] |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 4 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 5 | ## Introduction |
| 6 | |
| 7 | Clang tools can help with global refactorings of Chromium code. Clang tools can |
| 8 | take advantage of clang's AST to perform refactorings that would be impossible |
| 9 | with a traditional find-and-replace regexp: |
| 10 | |
| 11 | * Constructing `scoped_ptr<T>` from `NULL`: <https://crbug.com/173286> |
| 12 | * Implicit conversions of `scoped_refptr<T>` to `T*`: <https://crbug.com/110610> |
| 13 | * Rename everything in Blink to follow Chromium style: <https://crbug.com/563793> |
jdoerrie | 8c5b825 | 2017-10-14 06:28:58 | [diff] [blame] | 14 | * Clean up of deprecated `base::Value` APIs: <https://crbug.com/581865> |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 15 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 16 | ## Caveats |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 17 | |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 18 | * Invocations of a clang tool runs on on only one build config at a time. For |
| 19 | example, running the tool across a `target_os="win"` build won't update code |
| 20 | that is guarded by `OS_POSIX`. Performing a global refactoring will often |
| 21 | require running the tool once for each build config. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 22 | |
| 23 | ## Prerequisites |
| 24 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 25 | A Chromium checkout created with `fetch` should have everything needed. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 26 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 27 | For convenience, add `third_party/llvm-build/Release+Asserts/bin` to `$PATH`. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 28 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 29 | ## Writing the tool |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 30 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 31 | LLVM uses C++11 and CMake. Source code for Chromium clang tools lives in |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 32 | [//tools/clang]. It is generally easiest to use one of the already-written tools |
| 33 | as the base for writing a new tool. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 34 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 35 | Chromium clang tools generally follow this pattern: |
| 36 | |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 37 | 1. Instantiate a |
| 38 | [`clang::ast_matchers::MatchFinder`][clang-docs-match-finder]. |
| 39 | 2. Call `addMatcher()` to register |
| 40 | [`clang::ast_matchers::MatchFinder::MatchCallback`][clang-docs-match-callback] |
| 41 | actions to execute when [matching][matcher-reference] the AST. |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 42 | 3. Create a new `clang::tooling::FrontendActionFactory` from the `MatchFinder`. |
| 43 | 4. Run the action across the specified files with |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 44 | [`clang::tooling::ClangTool::run`][clang-docs-clang-tool-run]. |
| 45 | 5. Serialize generated [`clang::tooling::Replacement`][clang-docs-replacement]s |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 46 | to `stdout`. |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 47 | |
| 48 | Other useful references when writing the tool: |
| 49 | |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 50 | * [Clang doxygen reference][clang-docs] |
| 51 | * [Tutorial for building tools using LibTooling and |
| 52 | LibASTMatchers][clang-tooling-tutorial] |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 53 | |
| 54 | ### Edit serialization format |
| 55 | ``` |
| 56 | ==== BEGIN EDITS ==== |
Lukasz Anforowicz | e4d0e92f | 2020-05-06 20:47:12 | [diff] [blame] | 57 | r:::path/to/file/to/edit:::offset1:::length1:::replacement text |
| 58 | r:::path/to/file/to/edit:::offset2:::length2:::replacement text |
| 59 | r:::path/to/file2/to/edit:::offset3:::length3:::replacement text |
| 60 | include-user-header:::path/to/file2/to/edit:::-1:::-1:::header/file/to/include.h |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 61 | |
| 62 | ... |
| 63 | |
| 64 | ==== END EDITS ==== |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 65 | ``` |
| 66 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 67 | The header and footer are required. Each line between the header and footer |
| 68 | represents one edit. Fields are separated by `:::`, and the first field must |
Lukasz Anforowicz | e4d0e92f | 2020-05-06 20:47:12 | [diff] [blame] | 69 | be `r` (for replacement) or `include-user-header`. |
| 70 | A deletion is an edit with no replacement text. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 71 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 72 | The edits are applied by [`apply_edits.py`](#Running), which understands certain |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 73 | conventions: |
| 74 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 75 | * The clang tool should munge newlines in replacement text to `\0`. The script |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 76 | knows to translate `\0` back to newlines when applying edits. |
| 77 | * When removing an element from a 'list' (e.g. function parameters, |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 78 | initializers), the clang tool should emit a deletion for just the element. |
| 79 | The script understands how to extend the deletion to remove commas, etc. as |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 80 | needed. |
| 81 | |
| 82 | TODO: Document more about `SourceLocation` and how spelling loc differs from |
| 83 | expansion loc, etc. |
| 84 | |
| 85 | ### Why not RefactoringTool? |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 86 | While clang has a [`clang::tooling::RefactoringTool`](http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1RefactoringTool.html) |
| 87 | to automatically apply the generated replacements and save the results, it |
| 88 | doesn't work well for Chromium: |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 89 | |
Daniel Cheng | 82f80d6 | 2017-05-18 05:39:38 | [diff] [blame] | 90 | * Clang tools run actions serially, so run time scales poorly to tens of |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 91 | thousands of files. |
| 92 | * A parsing error in any file (quite common in NaCl source) prevents any of |
| 93 | the generated replacements from being applied. |
| 94 | |
| 95 | ## Building |
| 96 | Synopsis: |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 97 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 98 | ```shell |
Hans Wennborg | d1eec55 | 2019-05-02 14:59:07 | [diff] [blame] | 99 | tools/clang/scripts/build.py --bootstrap --without-android \ |
dcheng | f239071 | 2017-01-05 06:41:45 | [diff] [blame] | 100 | --extra-tools rewrite_to_chrome_style |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 101 | ``` |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 102 | |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 103 | Running this command builds the [Oilpan plugin][//tools/clang/blink_gc_plugin], |
| 104 | the [Chrome style plugin][//tools/clang/plugins], and the [Blink to Chrome style |
| 105 | rewriter][//tools/clang/rewrite_to_chrome_style]. Additional arguments to |
| 106 | `--extra-tools` should be the name of subdirectories in [//tools/clang]. |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 107 | |
danakj | 30d0f8c9 | 2016-01-28 00:26:33 | [diff] [blame] | 108 | It is important to use --bootstrap as there appear to be [bugs](https://crbug.com/580745) |
| 109 | in the clang library this script produces if you build it with gcc, which is the default. |
| 110 | |
vabr | 9ed3f43 | 2017-06-09 07:30:42 | [diff] [blame] | 111 | Once clang is bootsrapped, incremental builds can be done by invoking `ninja` in |
| 112 | the `third_party/llvm-build/Release+Asserts` directory. In particular, |
| 113 | recompiling solely the tool you are writing can be accomplished by executing |
| 114 | `ninja rewrite_to_chrome_style` (replace `rewrite_to_chrome_style` with your |
| 115 | tool's name). |
| 116 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 117 | ## Running |
qyearsley | c0dc6f4 | 2016-12-02 22:13:39 | [diff] [blame] | 118 | First, build all Chromium targets to avoid failures due to missing dependencies |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 119 | that are generated as part of the build: |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 120 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 121 | ```shell |
danakj | ca6b31b5 | 2016-12-22 22:05:53 | [diff] [blame] | 122 | ninja -C out/Debug # For non-Windows |
| 123 | ninja -d keeprsp -C out/Debug # For Windows |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 124 | |
| 125 | # experimental alternative: |
Yannic Bonenberger | 748bb0d | 2018-07-02 21:10:06 | [diff] [blame] | 126 | $gen_targets = $(ninja -C out/Debug -t targets all \ |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 127 | | grep '^gen/[^: ]*\.[ch][pc]*:' \ |
Yannic Bonenberger | 748bb0d | 2018-07-02 21:10:06 | [diff] [blame] | 128 | | cut -f 1 -d :) |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 129 | ninja -C out/Debug $gen_targets |
danakj | ca6b31b5 | 2016-12-22 22:05:53 | [diff] [blame] | 130 | ``` |
| 131 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 132 | Then run the actual clang tool to generate a list of edits: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 133 | |
| 134 | ```shell |
Daniel Cheng | 51c5530 | 2017-05-04 00:39:16 | [diff] [blame] | 135 | tools/clang/scripts/run_tool.py --tool <path to tool> \ |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 136 | --generate-compdb |
Daniel Cheng | 51c5530 | 2017-05-04 00:39:16 | [diff] [blame] | 137 | -p out/Debug <path 1> <path 2> ... >/tmp/list-of-edits.debug |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 138 | ``` |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 139 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 140 | `--generate-compdb` can be omitted if the compile DB was already generated and |
| 141 | the list of build flags and source files has not changed since generation. |
| 142 | |
| 143 | `<path 1>`, `<path 2>`, etc are optional arguments to filter the files to run |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 144 | the tool against. This is helpful when sharding global refactorings into smaller |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 145 | chunks. For example, the following command will run the `empty_string` tool |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 146 | against just the `.c`, `.cc`, `.cpp`, `.m`, `.mm` files in `//net`. Note that |
| 147 | the filtering is not applied to the *output* of the tool - the tool can emit |
| 148 | edits that apply to files outside of `//cc` (i.e. edits that apply to headers |
| 149 | from `//base` that got included by source files in `//cc`). |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 150 | |
| 151 | ```shell |
Daniel Cheng | 51c5530 | 2017-05-04 00:39:16 | [diff] [blame] | 152 | tools/clang/scripts/run_tool.py --tool empty_string \ |
Yannic Bonenberger | 748bb0d | 2018-07-02 21:10:06 | [diff] [blame] | 153 | --generate-compdb \ |
Daniel Cheng | 51c5530 | 2017-05-04 00:39:16 | [diff] [blame] | 154 | -p out/Debug net >/tmp/list-of-edits.debug |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 155 | ``` |
| 156 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 157 | Note that some header files might only be included from generated files (e.g. |
| 158 | from only from some `.cpp` files under out/Debug/gen). To make sure that |
| 159 | contents of such header files are processed by the clang tool, the clang tool |
| 160 | needs to be run against the generated files. The only way to accomplish this |
| 161 | today is to pass `--all` switch to `run_tool.py` - this will run the clang tool |
| 162 | against all the sources from the compilation database. |
| 163 | |
| 164 | Finally, apply the edits as follows: |
| 165 | |
| 166 | ```shell |
| 167 | cat /tmp/list-of-edits.debug \ |
| 168 | | tools/clang/scripts/extract_edits.py \ |
Daniel Cheng | 51c5530 | 2017-05-04 00:39:16 | [diff] [blame] | 169 | | tools/clang/scripts/apply_edits.py -p out/Debug <path 1> <path 2> ... |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 170 | ``` |
| 171 | |
| 172 | The apply_edits.py tool will only apply edits to files actually under control of |
| 173 | `git`. `<path 1>`, `<path 2>`, etc are optional arguments to further filter the |
| 174 | files that the edits are applied to. Note that semantics of these filters is |
| 175 | distinctly different from the arguments of `run_tool.py` filters - one set of |
| 176 | filters controls which files are edited, the other set of filters controls which |
| 177 | files the clang tool is run against. |
| 178 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 179 | ## Debugging |
| 180 | Dumping the AST for a file: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 181 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 182 | ```shell |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 183 | clang++ -Xclang -ast-dump -std=c++14 foo.cc | less -R |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 184 | ``` |
| 185 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 186 | Using `clang-query` to dynamically test matchers (requires checking out |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 187 | and building [clang-tools-extra][]): |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 188 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 189 | ```shell |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 190 | clang-query -p path/to/compdb base/memory/ref_counted.cc |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 191 | ``` |
| 192 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 193 | `printf` debugging: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 194 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 195 | ```c++ |
| 196 | clang::Decl* decl = result.Nodes.getNodeAs<clang::Decl>("decl"); |
| 197 | decl->dumpColor(); |
| 198 | clang::Stmt* stmt = result.Nodes.getNodeAs<clang::Stmt>("stmt"); |
| 199 | stmt->dumpColor(); |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 200 | ``` |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 201 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 202 | By default, the script hides the output of the tool. The easiest way to change |
| 203 | that is to `return 1` from the `main()` function of the clang tool. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 204 | |
| 205 | ## Testing |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 206 | Synposis: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 207 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 208 | ```shell |
Ramin Halavati | eb3f807a | 2017-08-02 05:31:31 | [diff] [blame] | 209 | tools/clang/scripts/test_tool.py <tool name> [--apply-edits] |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 210 | ``` |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 211 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 212 | The name of the tool binary and the subdirectory for the tool in |
| 213 | `//tools/clang` must match. The test runner finds all files that match the |
Ramin Halavati | eb3f807a | 2017-08-02 05:31:31 | [diff] [blame] | 214 | pattern `//tools/clang/<tool name>/tests/*-original.cc`, and runs the tool |
| 215 | across those files. |
| 216 | If `--apply-edits` switch is presented, tool outputs are applied to respective |
| 217 | files and compared to the `*-expected.cc` version. If there is a mismatch, the |
| 218 | result is saved in `*-actual.cc`. |
| 219 | When `--apply-edits` switch is not presented, tool outputs are compared to |
| 220 | `*-expected.txt` and if different, the result is saved in `*-actual.txt`. Note |
| 221 | that in this case, only one test file is expected. |
Daniel Cheng | 69413be | 2017-10-24 01:23:57 | [diff] [blame] | 222 | |
| 223 | [//tools/clang]: https://chromium.googlesource.com/chromium/src/+/master/tools/clang/ |
| 224 | [clang-docs-match-finder]: http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder.html |
| 225 | [clang-docs-match-callback]: http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder_1_1MatchCallback.html |
| 226 | [matcher-reference]: http://clang.llvm.org/docs/LibASTMatchersReference.html |
| 227 | [clang-docs-clang-tool-run]: http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1ClangTool.html#acec91f63b45ac7ee2d6c94cb9c10dab3 |
| 228 | [clang-docs-replacement]: http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1Replacement.html |
| 229 | [clang-docs]: http://clang.llvm.org/doxygen/index.html |
| 230 | [clang-tooling-tutorial]: http://clang.llvm.org/docs/LibASTMatchersTutorial.html |
| 231 | [//tools/clang/blink_gc_plugin]: https://chromium.googlesource.com/chromium/src/+/master/tools/clang/blink_gc_plugin/ |
| 232 | [//tools/clang/plugins]: https://chromium.googlesource.com/chromium/src/+/master/tools/clang/plugins/ |
| 233 | [//tools/clang/rewrite_to_chrome_style]: https://chromium.googlesource.com/chromium/src/+/master/tools/clang/rewrite_to_chrome_style/ |
| 234 | [clang-tools-extra]: (https://github.com/llvm-mirror/clang-tools-extra) |