Page MenuHomePhabricator

[clang-diff] Use data collectors for node comparison
Needs ReviewPublic

Authored by johannes on Aug 22 2017, 2:15 AM.

Details

Reviewers
arphaman

Event Timeline

johannes created this revision.Aug 22 2017, 2:15 AM
arphaman edited edge metadata.Aug 25 2017, 5:55 AM

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

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

It is only used in the client, I think it makes sense to move it there, or to remove it altogether.
I think I'd keep it in the client for now, to disambiguate things with different names in the tests.

arphaman added inline comments.Aug 29 2017, 8:57 AM
lib/Tooling/ASTDiff/ASTDiff.cpp
537

I didn't realize that you're including files from within lib. That's not ideal. You should add a pre-commit that moves the *.inc files over to include/AST (and marks them as textual in Clang's module.modulemap). Then you should include them using the clang/AST path.

johannes added inline comments.Sep 1 2017, 12:28 AM
lib/Tooling/ASTDiff/ASTDiff.cpp
537

I tried moving StmtDataCollectors.inc to and adding
+ textual header "AST/StmtDataCollectors.inc"
in the modulemap, then I get some CMake errors, not sure why.

It might make sense to tablegen it like the other .inc files.

johannes updated this revision to Diff 121647.Nov 5 2017, 11:20 AM

use raw source code of owned tokens instead