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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D138859#3954975 <https://reviews.llvm.org/D138859#3954975>, @erichkeane wrote:

> The hash there isn't the problem, its that you're adding a field to Attr.td that isn't serialized in ASTWriter/ASTReader.

That's a good point. Sorry I haven't realized it at first and thanks for your patience. So, `IsODRHashable` is a property of the attribute kind, not the attribute instance. Unfortunately, I don't know what is the appropriate way to fix FIXME below (and how urgent it is).

  // FIXME: These are properties of the attribute kind, not state for this
  // instance of the attribute.
  ...
  unsigned IsODRHashable : 1;

Anyway, big chunk of attribute deserialization happens in "AttrPCHRead.inc" and for AMDGPUFlatWorkGroupSizeAttr (non-trivial example) we have

  case attr::AMDGPUFlatWorkGroupSize: {
    bool isInherited = Record.readInt();
    bool isImplicit = Record.readInt();
    bool isPackExpansion = Record.readInt();
    Expr * min = Record.readExpr();
    Expr * max = Record.readExpr();
    New = new (Context) AMDGPUFlatWorkGroupSizeAttr(Context, Info, min, max);
    cast<InheritableAttr>(New)->setInherited(isInherited);
    New->setImplicit(isImplicit);
    New->setPackExpansion(isPackExpansion);
    break;
  }

which calls the generated constructor

  AMDGPUFlatWorkGroupSizeAttr::AMDGPUFlatWorkGroupSizeAttr(ASTContext &Ctx, const AttributeCommonInfo &CommonInfo
                , Expr * Min
                , Expr * Max
               )
    : InheritableAttr(Ctx, CommonInfo, attr::AMDGPUFlatWorkGroupSize, false, false, false)
                , min(Min)
                , max(Max)
    {
  }

where `false, false, false` part corresponds to `bool IsLateParsed, bool IsODRHashable, bool InheritEvenIfAlreadyPresent`. So `IsODRHashable` is never serialized/deserialized and compiler knows what the value should be. It can be a problem if one clang version writes a .pcm file and another version reads it because we can change if an attribute is hashable over time. But as far as I know, we don't support such cross-version scenario already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138859