In D34329#1213667, @sylvestre.ledru wrote:

@arphaman @johannes Is that normal that clang-diff isn't installed by cmake? (like clang-format?)

use generic concat

In D40782#960474, @hfinkel wrote:

In D40782#958027, @johannes wrote:

I use a different approach now, WDYT?

I'm not sure I like this. Before it was clear: !strconcat is for strings, !codeconcat was for code. Having !strconcat also work on code, and return something which isn't a string, seems unfortunate. And we also have !listconcat. If we want to unify things, I think it would make sense to add !concat (which works on all of those things). [If you do this, then also support !strconcat and !listconcat as aliases to be nice to out-of-tree backends].

I use a different approach now, WDYT?

remove codeconcat, make strconcat return code if all arguments are

add documentation + test

In D40731#943564, @stettberger wrote:

For my changes to StmtDataCollectors: I tried to leave the already existing entries in StmtDataCollector.td untouched and the test-cases still work OK. If there is something somebody could break by changing these files it would be good to have a test case for it.

So you can define a category for cHash and add let cHash = [{ .. }] or let cHash = TypeIIClone; if you just want to reuse it.
With this patch you can also use !codeconcat to append some code: https://reviews.llvm.org/D40782

adhere to the column limit

I think it would be quite nice if we manage to consolidate the implementation of hashing mechanisms.

update

update

update

use raw source code of owned tokens instead

update

update

use CXXOperatorCallExpr::isPrefixOp() to determine whether it's infix or postfix
directly traverse statement children instead of copying

@teemperor ok for you? did phabricator make you a blocking reviewer because of the affected code, or did I do that somehow?

undo visibility change

In D37383#858905, @teemperor wrote:

@arphaman I suggested tablegen in D36664 because I remembered we had some CMake sanity check about not having an *.inc in our include dir: https://github.com/llvm-mirror/clang/blob/master/CMakeLists.txt#L266 Not sure if it actually fires for our case, but it looks like it does.

fix by adding an option in RecursiveASTVisitor

fixes

This is currently broken, if a user provides a TraverseClassTemplateDecl, then the same method in this class will not be called, I think I will add a flag (probably not user visible) in RecursiveASTVisitor.h to switch the order for templates

Yeah, this seems to be the best solution for this. I think I ran into the same issue when working on the StmtDataCollector - basically there can only be two Traverse*, one in the Derived class and the other one needs to do all the magic with walking the specialisation hierarchy.

use a specialized getStmtChildren to fix the order for CXXOperatorCallExpr

split to ASTDiff/ASTPatch

detect prefix/postfix from number of arguments

The previous version didn't call TraverseDecl of the derived class, this is fixed now.
The public getDerived.TraverseStmt() does not accept a DataRecursionQueue, so this also could not be used (I think)
I used the wrapper TraverseStmtBase, which should behave exactly as the method that originally traverses CXXOperatorCallExpr

do call derived TraverseStmt for children of CXXOperatorCallExpr

use RecursiveASTVisitor::TraverseStmt

use rewriter to patch a third AST

test ordering, class template

In D37001#852442, @arphaman wrote:

Can you remove getNodeValue now or do you still need it?

use LexicallyOrderedRecursiveASTVisitor

use LexicallyOrderedRecursiveASTVisitor

provide a test
need to update the maxsize parameter to -s=200 here because the test file is
expected to fit within this size

In D36989#848835, @jgravelle-google wrote:

If you have more stop-after options it'd probably be simpler to leave it as one field. I'll just rename it to "stop-diff-after" to avoid the name collision.

casing

some refactoring

Oh wait I get linker errors with BUILD_SHARED_LIBS=1

Looks good, thanks!
You can remove StopAfterTopDown from the ComparisonOptions struct definition

This has already landed.

only use Enums as namespace prefix in C++11

accept a commit range

In D36664#841758, @teemperor wrote:

Very well done, I really like this patch! I added a few remarks mostly about the comments that need some small adjusting.

I'm wondering what would be a nice way of creating a StmtDataCollector that is faster but only works for single translation units (e.g. it only hashes the pointer values of decls instead of their qualified name)? The use case here would be Stmt::Profile and the CloneDetector which could be set to a single-TU-mode in the CloneChecker. For Stmt::Profile it would ensure we don't degrade performance from the current version, for the CloneChecker we probably get a reasonable performance boost (as the hashing is currently the last remaining bottle neck).

@arphaman Any suggestions who could review/approve the additions to AST/?

add

• fix comments
• add unittest for the new node kinds
• make addData() take a const reference

In D36686#840827, @arphaman wrote:

It might be useful to have both source and destination in a different revision. Maybe something like -src-git-rev and -dst-git-rev?