rtrieu added inline comments.
Herald added a subscriber: JDevlieghere.
================
Comment at: include/clang/AST/CHashVisitor.h:72-79
+ template<typename T>
+ void addData(const llvm::iterator_range<T> &x) {
+ addData(std::distance(x.begin(), x.end()));
+ }
+ template<typename T>
+ void addData(const llvm::ArrayRef<T> &x) {
+ addData(x.size());
----------------
Should these also be storing the hashes of the elements in addition to storing
the size?
================
Comment at: include/clang/AST/CHashVisitor.h:211
+
+ bool VisitTypeDecl(TypeDecl *D) {
+ // If we would hash the resulting type for a typedef, we
----------------
I thought the idea was to have all the code in the *Collectors.td but here you
have a bunch of Visit* functions for AST nodes. This seems counter to your
point of having them generated.
================
Comment at: include/clang/AST/CHashVisitor.h:212-213
+ bool VisitTypeDecl(TypeDecl *D) {
+ // If we would hash the resulting type for a typedef, we
+ // would get into an endless recursion.
+ if (!isa<TypedefNameDecl>(D) && !isa<RecordDecl>(D) && !isa<EnumDecl>(D)) {
----------------
I don't see this example in the test.
================
Comment at: include/clang/AST/CHashVisitor.h:226
+ }
+ if (isa<VarDecl>(ValDecl)) {
+ /* We emulate TraverseDecl here for VarDecl, because we
----------------
Can't you query the hash for the Decl instead of doing work here?
================
Comment at: include/clang/AST/CHashVisitor.h:253
+ bool VisitValueDecl(ValueDecl *D) {
+ /* Field Declarations can induce recursions */
+ if (isa<FieldDecl>(D)) {
----------------
Is this possible recursion included in your test?
================
Comment at: include/clang/AST/CHashVisitor.h:255
+ if (isa<FieldDecl>(D)) {
+ addData(std::string(D->getType().getAsString()));
+ } else {
----------------
Shouldn't this be handled in the VisitFieldDecl function?
================
Comment at: include/clang/AST/DeclDataCollectors.td:5-7
+ // Every Declaration gets a tag field in the hash stream. It is
+ // hashed to add additional randomness to the hash
+ addData(llvm::hash_value(S->getKind()));
----------------
Why is this randomness needed?
================
Comment at: include/clang/AST/DeclDataCollectors.td:9
+
+ // CrossRef
+ addData(S->hasAttrs());
----------------
What do you mean by "CrossRef"?
================
Comment at: include/clang/AST/DeclDataCollectors.td:60-61
+ code Code = [{
+ /* Not every enum has a init expression. Therefore,
+ we extract the actual enum value from it. */
+ addData(S->getInitVal().getExtValue());
----------------
Does that mean you consider:
```
enum E { E1, E2, E3 };
```
to be equivalent to:
```
enum E { E1 = 0, E2 = 1, E3 = 2 };
```
?
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
