[PATCH] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.
Chuanqi Xu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 16 23:30:54 PST 2023
ChuanqiXu added inline comments.
================
Comment at: clang/lib/AST/Decl.cpp:4881-4882
+ // For RecordDecl the ODRHash is stored in the remaining 26
+ // bit of RecordDeclBits, adjust the hash to accomodate.
+ setODRHash(Hash.CalculateHash() >> 6);
+ return RecordDeclBits.ODRHash;
----------------
I am curious for the operation. It looks dangerous. How can we be sure that the hash value is still meaningful after remove its lower 6 bits?
================
Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:1550-1551
+bool ODRDiagsEmitter::diagnoseMismatch(const RecordDecl *FirstRecord,
+ const RecordDecl *SecondRecord) const {
+ if (FirstRecord == SecondRecord)
----------------
The implementation of this function looks redundant with other overloads. Of course this is not the problem of the patch. I think we can add a FIXME at least.
================
Comment at: clang/lib/AST/ODRHash.cpp:592-593
+void ODRHash::AddRecordDecl(const RecordDecl *Record) {
+ AddDecl(Record);
+
----------------
For the current implementation, if it makes sense to add an assertion `!isa<CXXRecordDecl>(Record);`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71734/new/
https://reviews.llvm.org/D71734
More information about the cfe-commits
mailing list