[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

Wed Nov 30 04:39:43 PST 2022

aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:552-553
   bit LateParsed = 0;
+  // Set to true for attributes that should participate in ODR hashing.
+  bit IsODRHashable = 0;
   // Set to false to prevent an attribute from being propagated from a template
----------------
vsapsai wrote:
> aaron.ballman wrote:
> > Do we want to change the default for any of the derived classes? e.g., should something that is a `TypeAttr` or `DeclOrTypeAttr` default to being ODR hashable because they impact the type system?
> > 
> > Also, can we expand the comment somewhat to help folks understand the circumstances under which they should set that to 1?
> At this point I don't know what the default value should be, for that I'll need to see how many attributes should be hashable. This change is mostly to prove the direction is viable and reasonable. And TypeAttr, DeclOrTypeAttr is a good starting point to decide which attributes should be hashed.
> 
> Good idea with expanding the comment. Though I need some time to prepare non-abhorrent explanation (gosh, docs are harder than code).
Okay, I'm fine being conservative until we figure out what the right default is.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138859