[PATCH] Add new tool clang-highlight to clang/tools/extra

[Daniel Jasper]

================
Comment at: clang-highlight/Fuzzy/FuzzyAST.h:122
@@ +121,3 @@
+  llvm::SmallVector<AnnotatedTokenRef, 1> NameSegments;
+  llvm::Optional<std::shared_ptr<TemplateArguments> > TemplateArgs;
+
----------------
kapf wrote:
> djasper wrote:
> > I am quite strongly against the use of shared_ptr here. I understand that ownership in such a tree is somewhat difficult. How about doing what Clang itself does (storing all elements in an ASTContext and not worrying about ownership at all)?
> I thought not using an ASTContext would be simpler, but it might not be simpler in the end. The two alternatives are ASTContext or a clone method. I agree that ASTContext would be the cleanest solution.
I don't mean ASTContext specifically, but create a storage object which simply owns all objects. Then you'll only need to handle pointers later.

================
Comment at: clang-highlight/Fuzzy/FuzzyAST.h:124
@@ +123,3 @@
+
+  void reown(ASTElement *Ref) {
+    for (auto &N : NameSegments)
----------------
kapf wrote:
> djasper wrote:
> > As above, I think we should try to simplify ownership and just put all the nodes into a common context.
> This is a different ownership. Every token points to the AST and every AST node "owns" a list of tokens. When a QualifiedID is parsed, it isn't decided yet whether it'll be part of a Type or a CallExpr. This reown handles the ast reference and cannot be simplified by an ASTContext.
I don't think you need this kind of ownership consideration if you follow the advise above.

================
Comment at: clang-highlight/Fuzzy/FuzzyParser.cpp:24
@@ +23,3 @@
+
+  void skipWhitespaces() {
+    for (;;) {
----------------
kapf wrote:
> djasper wrote:
> > Why not just use Lexer::SetKeepWhitespaceMode?
> Because this also skips comments which need to be highlighted.
Even if you also use SetCommentRetentionState()?

================
Comment at: clang-highlight/Fuzzy/FuzzyParser.cpp:109
@@ +108,3 @@
+
+static std::unique_ptr<Expr> parseExpr(TokenFilter &TF, int Precedence = 1,
+                                       bool StopAtGreater = false);
----------------
kapf wrote:
> djasper wrote:
> > Why end the unnamed namespace above and make this and the following functions all static?
> I followed the LLVM guideline: http://llvm.org/docs/CodingStandards.html#anonymous-namespaces
> I don't quite agree with the reasoning, so I'm happy to change this.
No. Coding guidelines are the law ;-)..

http://reviews.llvm.org/D4725

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/