Update documentation for clang tool refactoring.

docs/clang_tool_refactoring.md

Index: docs/clang_tool_refactoring.md
diff --git a/docs/clang_tool_refactoring.md b/docs/clang_tool_refactoring.md
index 8c23aa0957c2c7194bc7c74b92f7686f3fad75d6..e9788c2cdc05504a2760bc028d8f1b3956a81b5b 100644
--- a/docs/clang_tool_refactoring.md
+++ b/docs/clang_tool_refactoring.md
@@ -2,99 +2,171 @@
 
 [TOC]
 
+## Introduction
+
+Clang tools can help with global refactorings of Chromium code. Clang tools can
+take advantage of clang's AST to perform refactorings that would be impossible
+with a traditional find-and-replace regexp:
+
+*   Constructing `scoped_ptr<T>` from `NULL`: <https://crbug.com/173286>
+*   Implicit conversions of `scoped_refptr<T>` to `T*`: <https://crbug.com/110610>
+*   Rename everything in Blink to follow Chromium style: <https://crbug.com/563793>
+
 ## Caveats
 
-*   The current workflow requires git.
-*   This doesn't work on Windows... yet. I'm hoping to have a proof-of-concept
-    working on Windows as well ~~in a month~~ several centuries from now.
+An invocation of the clang tool runs on one build config. Code that only
+compiles on one platform or code that is guarded by a set of compile-time flags
+can be problematic. Performing a global refactoring typically requires running
+the tool once in each build config with code that needs to be updated.
+
+Other minor issues:
+
+*   Requires a git checkout.
+*   Requires [some hacks to run on Windows](https://codereview.chromium.org/718873004).
 
 ## Prerequisites
 
-Everything needed should be in a default Chromium checkout using gclient.
-`third_party/llvm-build/Release+Asserts/bin` should be in your `$PATH`.
+A Chromium checkout created with `fetch` should have everything needed.
 
-## Writing the Tool
+For convenience, add `third_party/llvm-build/Release+Asserts/bin` to `$PATH`.

[danakj, 2016/01/11 21:53:13]

Using this tool needs this dir to be at the front of $PATH, but you don't want
that generally as that puts a different "clang" at the front.. I think we should
put this as a local path change in the how-to-run parts. wdyt?

[dcheng, 2016/01/11 22:52:23]

This really only affects someone with multiple checkouts pinned at different
clang revisions: the script still builds the pinned llvm revision, and things
will work with goma (though I think this is somewhat goma unfriendly, as it
might spam the goma logs with compiler hash mismatch warnings).

 
-An example clang tool is being implemented in
-https://codereview.chromium.org/12746010/. Other useful resources might be the
-[basic tutorial for Clang's AST matchers](http://clang.llvm.org/docs/LibASTMatchersTutorial.html)
-or the
-[AST matcher reference](http://clang.llvm.org/docs/LibASTMatchersReference.html).
+## Writing the tool
 
-Build your tool by running the following command (requires cmake version 2.8.10
-or later):
+LLVM uses C++11 and CMake. Source code for Chromium clang tools lives in
+[//tools/clang](https://chromium.googlesource.com/chromium/src/tools/clang/+/master).
+It is generally easiest to use one of the already-written tools as the base for
+writing a new tool.
 
-```shell
-tools/clang/scripts/update.py --force-local-build --without-android \
---tools <tools>
-```
+Chromium clang tools generally follow this pattern:
 
-`<tools>` is a semicolon delimited list of subdirectories in `tools/clang` to
-build. The resulting binary will end up in
-`third_party/llvm-build/Release+Asserts/bin`. For example, to build the Chrome
-plugin and the empty\_string tool, run the following:
+1.  Instantiate a [`clang::ast_matchers::MatchFinder`](
+    http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder.html).
+2.  Call `addMatcher()` to register [`clang::ast_matchers::MatchFinder::MatchCallback`](
+    http://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder_1_1MatchCallback.html) actions to execute when [matching](http://clang.llvm.org/docs/LibASTMatchersReference.html) the AST.
+3.  Create a new `clang::tooling::FrontendActionFactory` from the `MatchFinder`.
+4.  Run the action across the specified files with
+    [`clang::tooling::ClangTool::run`](http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1ClangTool.html#acec91f63b45ac7ee2d6c94cb9c10dab3).
+5.  Serialize generated [`clang::tooling::Replacement`](
+    http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1Replacement.html)s to
+    `stdout`.
 
-```shell
-tools/clang/scripts/update.py --force-local-build --without-android \
---tools plugins empty_string
-```
+Other useful references when writing the tool:
 
-When writing AST matchers, the following can be helpful to see what clang thinks
-the AST is:
+*   [Clang doxygen reference](http://clang.llvm.org/doxygen/index.html)
+*   [Tutorial for building tools using LibTooling and LibASTMatchers](http://clang.llvm.org/docs/LibASTMatchersTutorial.html)
 
-```shell
-clang++ -cc1 -ast-dump foo.cc
+### Edit serialization format
 ```
+==== BEGIN EDITS ====
+r:::path/to/file1:::offset1:::length1:::replacement text
+r:::path/to/file2:::offset2:::length2:::replacement text
 
-## Running the tool
+       ...
 
-First, you'll need to generate the compilation database with the following
-command:
+==== END EDITS ====
+```
 
+The header and footer are required. Each line between the header and footer
+represents one edit. Fields are separated by `:::`, and the first field must
+be `r` (for replacement). In the future, this may be extended to handle header
+insertion/removal. A deletion is an edit with no replacement text.
+
+The edits are applied by [`run_tool.py`](#Running), which understands certain
+conventions:
+
+*   The tool should munge newlines in replacement text to `\0`. The script
+    knows to translate `\0` back to newlines when applying edits.
+*   When removing an element from a 'list' (e.g. function parameters,
+    initializers), the tool should emit a deletion for just the element. The
+    script understands how to extend the deletion to remove commas, etc. as
+    needed.
+
+TODO: Document more about `SourceLocation` and how spelling loc differs from
+expansion loc, etc.
+
+### Why not RefactoringTool?
+While clang has a [`clang::tooling::RefactoringTool`](
+http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1RefactoringTool.html) to
+automatically apply the generated replacements and save the results, it doesn't
+work well for Chromium:
+
+*   Clang tools run actions serially, so runtime scales poorly to tens of
+    thousands of files.
+*   A parsing error in any file (quite common in NaCl source) prevents any of
+    the generated replacements from being applied.
+
+## Building
+Synopsis:
 ```shell
-cd $HOME/src/chrome/src
-ninja -C out/Debug -t compdb cc cxx objc objcxx > \
-out/Debug/compile_commands.json
+tools/clang/scripts/update.py --force-local-build --without-android \
+  --tools blink_gc_plugin plugins rewrite_to_chrome_style
 ```
-
-This will dump the command lines used to build the C/C++ modules in all of
-Chromium into the resulting file. Then run the following command to run your
-tool across all Chromium code:
-
+Running this command builds the [Oilpan plugin](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/blink_gc_plugin/),
+the [Chrome style
+plugin](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/plugins/),
+and the [Blink to Chrome style rewriter](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/rewrite_to_chrome_style/). Additional arguments to `--tools` should be the name of
+subdirectories in
+[//tools/clang](https://chromium.googlesource.com/chromium/src/+/master/tools/clang).
+Generally, `--tools` should always include `blink_gc_plugin` and `plugins`: otherwise, Chromium won't build.
+
+## Running
+First, build all chromium targets to avoid failures due to missing dependecies
+that are generated as part of the build:
 ```shell
-# Make sure all chromium targets are built to avoid missing generated
-# dependencies
 ninja -C out/Debug
+```
+
+Then run the actual tool:
+```
 tools/clang/scripts/run_tool.py <toolname> \
-<path/to/directory/with/compile_commands.json> <path 1> <path 2> ...
+  --generate-compdb
+  out/Debug <path 1> <path 2> ...
 ```
 
-`<path 1>`, `<path 2>`, etc are optional arguments you use to filter the files
-that will be rewritten. For example, if you only want to run the `empty-string`
-tool on files in `chrome/browser/extensions` and `sync`, you'd do something like:
+`--generate-compdb` can be omitted if the compile DB was already generated and
+the list of build flags and source files has not changed since generation.
+
+`<path 1>`, `<path 2>`, etc are optional arguments to filter the files to run
+the tool across. This is helpful when sharding global refactorings into smaller
+chunks. For example, the following command will run the `empty_string` tool
+across just the files in `//base`:
 
 ```shell
-tools/clang/scripts/run_tool.py empty_string out/Debug \
-chrome/browser/extensions sync
+tools/clang/scripts/run_tool.py empty_string  \
+  --generated-compdb \
+  out/Debug base
 ```
 
-## Limitations
-
-Since the compile database is generated by ninja, that means that files that
-aren't compiled on that platform won't be processed. That means if you want to
-apply a change across all Chromium platforms, you'll have to run the tool once
-on each platform.
+## Debugging
+Dumping the AST for a file:
+```shell
+clang++ -cc1 -ast-dump foo.cc
+```
 
-## Testing
+Using `clang-query` to dynamically test matchers (requires checking out
+and building [clang-tools-extras](https://github.com/llvm-mirror/clang-tools-extra)):
+```shell
+clang-query -p path/to/compdb base/memory/ref_counted.cc
+```
 
-`test_tool.py` is the test harness for running tests. To use it, simply run:
+`printf` debugging:
+```c++
+  clang::Decl* decl = result.Nodes.getNodeAs<clang::Decl>("decl");
+  decl->dumpColor();
+  clang::Stmt* stmt = result.Nodes.getNodeAs<clang::Stmt>("stmt");
+  stmt->dumpColor();
+```
+By default, the script hides the output of the tool. The easiest way to change
+that is to `return 1` from `main()`.

[danakj, 2016/01/11 21:53:13]

Where is the main() function you refer to?

[dcheng, 2016/01/11 22:52:23]

I've updated this to clarify.

 
+## Testing
+Synposis:
 ```shell
 test_tool.py <tool name>
 ```
 
-Note that name of the built tool and the subdirectory it lives in at
-`tools/clang` must match. What the test harness does is find all files that
-match the pattern `*-original.cc` in your tool's tests subdirectory. It then
-runs the tool across those files and compares it to the expected result, stored
-in `*-expected.cc`
+The name of the tool binary and the subdirectory for the tool in
+`//tools/clang` must match. The test runner finds all files that match the
+pattern `//tools/clang/<tool name>/tests/*-original.cc`, runs the tool across
+those files, and compared it to the `*-expected.cc` version. If there is a
+mismatch, the result is saved in `*-actual.cc`.