[PATCH] D40731: Integrate CHash into CLang

Johannes Altmanninger via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 4 06:43:53 PST 2017


johannes added a comment.

In https://reviews.llvm.org/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.


Yeah, the tests are not comprehensive unfortunately

> However, some of the collectors are not node local, which seems a little bit inconsequent to me. For example, DeclStmt includes information about the its Decl Children. In my opinion every entry in the *DataCollector.td should include only information that can be directly retrieved from this particular AST node. Cross-referencing between nodes should be done on the user side of the collectors (as it is done in CHashVisitor.h, which follows pointers in a cross-tree manner). With this separation of concerns it was quite easy for me to base CHash of the TableGen'ed  headers. Furthermore, a purely syntactical hash (only the children, no cross-tree dependencies) could be derived easily.

Good point, I think we can move the code for DeclStmt to CloneDetection.cpp

> @johannes For the different types of information that should be included for one AST node (e.g., Type II clones): We could use a hierachy of "What should be considered here". But we should clearly define what is the definition of equivalency for that level. For example, if you wake me up at night  I could not define you the difference between a type I and a type II clone.

Yeah, it's difficult to clearly define that though. The thing I noticed is that the function name is included for PredefinedExpr, but Type II clones do not care about values, but only structure. I think most of your additions should be fine , @teemperor could confirm them.


Repository:
  rC Clang

https://reviews.llvm.org/D40731





More information about the cfe-commits mailing list