Page MenuHomePhabricator

[clang] Documentation fixes for LibASTMatchersTutorial
AbandonedPublic

Authored by silvas on Jun 6 2016, 5:27 PM.

Details

Summary

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

Diff Detail

Event Timeline

simon.f.whittaker retitled this revision from to [clang] Documentation fixes for LibASTMatchersTutorial.
simon.f.whittaker updated this object.
simon.f.whittaker added reviewers: silvas, bogner.
simon.f.whittaker added a subscriber: cfe-commits.

Added Manuel Klimek at Sean's suggestion.

silvas accepted this revision.Jun 6 2016, 5:44 PM
silvas edited edge metadata.

Overall this LGTM, but let Manuel or one of the other libTooling folks sign off.

docs/LibASTMatchersTutorial.rst
46

This part is a bit out of date. Ninja is pretty easy to obtain (e.g. from its releases page on github) and CMake has had Ninja support built-in for a while now.

I guess this change doesn't hurt, but (probably in a separate patch) you may want to revamp this whole section on ninja and cmake.

This revision is now accepted and ready to land.Jun 6 2016, 5:44 PM
silvas resigned from this revision.Mar 25 2020, 6:32 PM
This revision now requires review to proceed.Mar 25 2020, 6:32 PM
silvas commandeered this revision.Mar 25 2020, 6:32 PM
silvas edited reviewers, added: simon.f.whittaker; removed: silvas.
silvas abandoned this revision.Mar 25 2020, 6:37 PM