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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
