[PATCH] D21047: [clang] Documentation fixes for LibASTMatchersTutorial

[Simon Whittaker via cfe-commits]

simon.f.whittaker created this revision.
simon.f.whittaker added reviewers: silvas, bogner.
simon.f.whittaker added a subscriber: cfe-commits.

This patch fixes Bug 25583 as well as cleaning up some other problems in the LibASTMatchersTutorial.

I've updated the tutorial to use:

'./configure.py —bootstrap' rather than 'bootstrap.py' in the ninja buildstep
'ninja check-llvm' and 'ninja check-clang' rather than 'ninja check' and 'ninja clang-test' respectively
Clarified that no output will be produced with the first iteration of the program (Step 1: Create a ClangTool)
Cleaned up some English usage in the section on hasCondition(binaryOperator
Fixed the problem in the original bug report:
'Context->getSourceManager().isFromMainFile(FS->getForLoc()' should be 'Context->getSourceManager().isInMainFile(FS->getForLoc()'

The only change I'm unsure about is changing:

'ninja install' to 'sudo ninja install' in the installation step. If I follow the instructions step-by-step then no special install directory is set and the default is /usr/local on OS X which requires sudo. As we've used sudo in similar circumstances above this then it makes sense to me.

This is my first contribution of any kind to Clang / LLVM (or even open source) so hope I have everything correct.

Thanks,

Simon

http://reviews.llvm.org/D21047

Files:
  docs/LibASTMatchersTutorial.rst

Index: docs/LibASTMatchersTutorial.rst
===================================================================
--- docs/LibASTMatchersTutorial.rst
+++ docs/LibASTMatchersTutorial.rst
@@ -40,7 +40,7 @@
       git clone https://github.com/martine/ninja.git
       cd ninja
       git checkout release
-      ./bootstrap.py
+      ./configure.py —bootstrap
       sudo cp ninja /usr/bin/
 
       cd ~/clang-llvm
@@ -59,9 +59,9 @@
       mkdir build && cd build
       cmake -G Ninja ../llvm -DLLVM_BUILD_TESTS=ON  # Enable tests; default is off.
       ninja
-      ninja check       # Test LLVM only.
-      ninja clang-test  # Test Clang only.
-      ninja install
+      ninja check-llvm   # Test LLVM only.
+      ninja check-clang  # Test Clang only.
+      sudo ninja install
 
 And we're live.
 
@@ -176,6 +176,8 @@
 them from a compilation database - there just aren't any options needed
 right now.
 
+As there are no syntax errors in the example no output will be produced.
+
 Intermezzo: Learn AST matcher basics
 ====================================
 
@@ -404,10 +406,10 @@
         hasLHS(declRefExpr(to(varDecl(hasType(isInteger()))))),
         hasRHS(expr(hasType(isInteger())))))
 
-Why? Because it doesn't work. Of the three loops provided in
+However this doesn't work, of the three loops provided in
 ``test-files/simple.cpp``, zero of them have a matching condition. A
 quick look at the AST dump of the first for loop, produced by the
-previous iteration of loop-convert, shows us the answer:
+previous iteration of loop-convert, shows us why:
 
 ::
 
@@ -498,7 +500,7 @@
         ASTContext *Context = Result.Context;
         const ForStmt *FS = Result.Nodes.getStmtAs<ForStmt>("forLoop");
         // We do not want to convert header files!
-        if (!FS || !Context->getSourceManager().isFromMainFile(FS->getForLoc()))
+        if (!FS || !Context->getSourceManager().isInMainFile(FS->getForLoc()))
           return;
         const VarDecl *IncVar = Result.Nodes.getNodeAs<VarDecl>("incVarName");
         const VarDecl *CondVar = Result.Nodes.getNodeAs<VarDecl>("condVarName");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21047.59808.patch
Type: text/x-patch
Size: 2105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160607/a470d6b2/attachment.bin>