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> |
| 14 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 15 | ## Caveats |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 16 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 17 | An invocation of the clang tool runs on one build config. Code that only |
| 18 | compiles on one platform or code that is guarded by a set of compile-time flags |
| 19 | can be problematic. Performing a global refactoring typically requires running |
| 20 | the tool once in each build config with code that needs to be updated. |
| 21 | |
| 22 | Other minor issues: |
| 23 | |
| 24 | * Requires a git checkout. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 25 | |
| 26 | ## Prerequisites |
| 27 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 28 | A Chromium checkout created with `fetch` should have everything needed. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 29 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 30 | For convenience, add `third_party/llvm-build/Release+Asserts/bin` to `$PATH`. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 31 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 32 | ## Writing the tool |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 33 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 34 | LLVM uses C++11 and CMake. Source code for Chromium clang tools lives in |
| 35 | [//tools/clang](https://chromium.googlesource.com/chromium/src/tools/clang/+/master). |
| 36 | It is generally easiest to use one of the already-written tools as the base for |
| 37 | writing a new tool. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 38 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 39 | Chromium clang tools generally follow this pattern: |
| 40 | |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 41 | 1. Instantiate a [`clang::ast_matchers::MatchFinder`](http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder.html). |
| 42 | 2. Call `addMatcher()` to register [`clang::ast_matchers::MatchFinder::MatchCallback`](http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder_1_1MatchCallback.html) |
| 43 | actions to execute when [matching](http://clang.llvm.org/docs/LibASTMatchersReference.html) |
| 44 | the AST. |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 45 | 3. Create a new `clang::tooling::FrontendActionFactory` from the `MatchFinder`. |
| 46 | 4. Run the action across the specified files with |
| 47 | [`clang::tooling::ClangTool::run`](http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1ClangTool.html#acec91f63b45ac7ee2d6c94cb9c10dab3). |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 48 | 5. Serialize generated [`clang::tooling::Replacement`](http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1Replacement.html)s |
| 49 | to `stdout`. |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 50 | |
| 51 | Other useful references when writing the tool: |
| 52 | |
| 53 | * [Clang doxygen reference](http://clang.llvm.org/doxygen/index.html) |
| 54 | * [Tutorial for building tools using LibTooling and LibASTMatchers](http://clang.llvm.org/docs/LibASTMatchersTutorial.html) |
| 55 | |
| 56 | ### Edit serialization format |
| 57 | ``` |
| 58 | ==== BEGIN EDITS ==== |
| 59 | r:::path/to/file1:::offset1:::length1:::replacement text |
| 60 | r:::path/to/file2:::offset2:::length2:::replacement text |
| 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 |
| 69 | be `r` (for replacement). In the future, this may be extended to handle header |
| 70 | insertion/removal. 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 | |
| 90 | * Clang tools run actions serially, so runtime scales poorly to tens of |
| 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 |
danakj | 30d0f8c9 | 2016-01-28 00:26:33 | [diff] [blame] | 99 | tools/clang/scripts/update.py --bootstrap --force-local-build --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 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 103 | Running this command builds the [Oilpan plugin](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/blink_gc_plugin/), |
| 104 | the [Chrome style |
| 105 | plugin](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/plugins/), |
dcheng | f239071 | 2017-01-05 06:41:45 | [diff] [blame^] | 106 | and the [Blink to Chrome style rewriter](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/rewrite_to_chrome_style/). Additional arguments to `--extra-tools` should be the name of |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 107 | subdirectories in |
| 108 | [//tools/clang](https://chromium.googlesource.com/chromium/src/+/master/tools/clang). |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 109 | |
danakj | 30d0f8c9 | 2016-01-28 00:26:33 | [diff] [blame] | 110 | It is important to use --bootstrap as there appear to be [bugs](https://crbug.com/580745) |
| 111 | in the clang library this script produces if you build it with gcc, which is the default. |
| 112 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 113 | ## Running |
qyearsley | c0dc6f4 | 2016-12-02 22:13:39 | [diff] [blame] | 114 | First, build all Chromium targets to avoid failures due to missing dependencies |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 115 | that are generated as part of the build: |
danakj | ef9f1fa | 2016-01-16 00:37:28 | [diff] [blame] | 116 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 117 | ```shell |
danakj | ca6b31b5 | 2016-12-22 22:05:53 | [diff] [blame] | 118 | ninja -C out/Debug # For non-Windows |
| 119 | ninja -d keeprsp -C out/Debug # For Windows |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 120 | |
| 121 | # experimental alternative: |
| 122 | $gen_targets = $(ninja -C out/gn -t targets all \ |
| 123 | | grep '^gen/[^: ]*\.[ch][pc]*:' \ |
| 124 | | cut -f 1 -d :`) |
| 125 | ninja -C out/Debug $gen_targets |
danakj | ca6b31b5 | 2016-12-22 22:05:53 | [diff] [blame] | 126 | ``` |
| 127 | |
| 128 | On Windows, generate the compile DB first, and after making any source changes. |
| 129 | Then omit the `--generate-compdb` in later steps. |
| 130 | |
| 131 | ```shell |
| 132 | tools/clang/scripts/generate_win_compdb.py out/Debug |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 133 | ``` |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 134 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 135 | Then run the actual clang tool to generate a list of edits: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 136 | |
| 137 | ```shell |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 138 | tools/clang/scripts/run_tool.py <toolname> \ |
| 139 | --generate-compdb |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 140 | out/Debug <path 1> <path 2> ... >/tmp/list-of-edits.debug |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 141 | ``` |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 142 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 143 | `--generate-compdb` can be omitted if the compile DB was already generated and |
| 144 | the list of build flags and source files has not changed since generation. |
| 145 | |
| 146 | `<path 1>`, `<path 2>`, etc are optional arguments to filter the files to run |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 147 | the tool against. This is helpful when sharding global refactorings into smaller |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 148 | chunks. For example, the following command will run the `empty_string` tool |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 149 | against just the `.c`, `.cc`, `.cpp`, `.m`, `.mm` files in `//net`. Note that |
| 150 | the filtering is not applied to the *output* of the tool - the tool can emit |
| 151 | edits that apply to files outside of `//cc` (i.e. edits that apply to headers |
| 152 | from `//base` that got included by source files in `//cc`). |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 153 | |
| 154 | ```shell |
| 155 | tools/clang/scripts/run_tool.py empty_string \ |
| 156 | --generated-compdb \ |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 157 | out/Debug net >/tmp/list-of-edits.debug |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 158 | ``` |
| 159 | |
lukasza | f9b89e7 | 2016-12-28 19:43:06 | [diff] [blame] | 160 | Note that some header files might only be included from generated files (e.g. |
| 161 | from only from some `.cpp` files under out/Debug/gen). To make sure that |
| 162 | contents of such header files are processed by the clang tool, the clang tool |
| 163 | needs to be run against the generated files. The only way to accomplish this |
| 164 | today is to pass `--all` switch to `run_tool.py` - this will run the clang tool |
| 165 | against all the sources from the compilation database. |
| 166 | |
| 167 | Finally, apply the edits as follows: |
| 168 | |
| 169 | ```shell |
| 170 | cat /tmp/list-of-edits.debug \ |
| 171 | | tools/clang/scripts/extract_edits.py \ |
| 172 | | tools/clang/scripts/apply_edits.py out/Debug <path 1> <path 2> ... |
| 173 | ``` |
| 174 | |
| 175 | The apply_edits.py tool will only apply edits to files actually under control of |
| 176 | `git`. `<path 1>`, `<path 2>`, etc are optional arguments to further filter the |
| 177 | files that the edits are applied to. Note that semantics of these filters is |
| 178 | distinctly different from the arguments of `run_tool.py` filters - one set of |
| 179 | filters controls which files are edited, the other set of filters controls which |
| 180 | files the clang tool is run against. |
| 181 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 182 | ## Debugging |
| 183 | Dumping the AST for a file: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 184 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 185 | ```shell |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 186 | clang++ -cc1 -ast-dump foo.cc |
| 187 | ``` |
| 188 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 189 | Using `clang-query` to dynamically test matchers (requires checking out |
| 190 | and building [clang-tools-extras](https://github.com/llvm-mirror/clang-tools-extra)): |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 191 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 192 | ```shell |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 193 | clang-query -p path/to/compdb base/memory/ref_counted.cc |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 194 | ``` |
| 195 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 196 | `printf` debugging: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 197 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 198 | ```c++ |
| 199 | clang::Decl* decl = result.Nodes.getNodeAs<clang::Decl>("decl"); |
| 200 | decl->dumpColor(); |
| 201 | clang::Stmt* stmt = result.Nodes.getNodeAs<clang::Stmt>("stmt"); |
| 202 | stmt->dumpColor(); |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 203 | ``` |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 204 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 205 | By default, the script hides the output of the tool. The easiest way to change |
| 206 | that is to `return 1` from the `main()` function of the clang tool. |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 207 | |
| 208 | ## Testing |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 209 | Synposis: |
Daniel Cheng | 9ce2a30 | 2016-01-16 01:17:57 | [diff] [blame] | 210 | |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 211 | ```shell |
lukasza | 1333f1b9 | 2016-08-27 00:24:43 | [diff] [blame] | 212 | tools/clang/scripts/test_tool.py <tool name> |
andybons | 3322f76 | 2015-08-24 21:37:09 | [diff] [blame] | 213 | ``` |
andybons | 6eaa0c0d | 2015-08-26 20:12:52 | [diff] [blame] | 214 | |
dcheng | ce2375e | 2016-01-12 01:09:07 | [diff] [blame] | 215 | The name of the tool binary and the subdirectory for the tool in |
| 216 | `//tools/clang` must match. The test runner finds all files that match the |
| 217 | pattern `//tools/clang/<tool name>/tests/*-original.cc`, runs the tool across |
| 218 | those files, and compared it to the `*-expected.cc` version. If there is a |
| 219 | mismatch, the result is saved in `*-actual.cc`. |