[PATCH] D135472: [ODRHash] Hash attributes on declarations.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.


================
Comment at: clang/lib/AST/ODRHash.cpp:479-480
+
+  llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
+                [](const Attr *A) { return !A->isImplicit(); });
+}
----------------
I'm not sure `isImplicit` is the best indicator of attributes to check, so suggestions in this area are welcome. I think we can start strict and relax some of the checks if needed.

If people have strong opinions that some attributes shouldn't be ignored, we can add them to the tests to avoid regressions. Personally, I believe that alignment and packed attributes should never be silently ignored.


================
Comment at: clang/test/Modules/odr_hash.cpp:3640
 
+namespace Attributes {
+#if defined(FIRST)
----------------
As we land hashing for C and Objective-C, we can move these tests to their own file. But for now I think it makes sense to keep everything in odr_hash.cpp. Though I don't have a strong preference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135472/new/

https://reviews.llvm.org/D135472