[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

@aaron.ballman We will need to do something like this in general, but I'm not sure it's going to be as easy as just inheriting the attribute in the general case. What do you think?



================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3540
+    Attr *IA = Previous->getAttr<MSInheritanceAttr>();
+    D->addAttr(IA);
+  } else if (!Previous->hasAttr<MSInheritanceAttr>() &&
----------------
I think it would be more consistent to clone the attribute here (and mark it as inherited) rather than attaching the same attribute to multiple declarations.


================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3544
+    Attr *IA = D->getAttr<MSInheritanceAttr>();
+    Previous->addAttr(IA);
+  }
----------------
I think we should only propagate attributes forwards along redeclaration chains. Is this single-step-backwards propagation necessary?


================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3709
+  // it needs to be added to all the declarations in the redeclarable chain.
+  mergeInheritableAttributes(D, Previous);
 }
----------------
Please add a FIXME to do this is general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83174