[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3544
+    const auto *IA =
+        dyn_cast<MSInheritanceAttr>(Previous->getAttr<MSInheritanceAttr>());
+
----------------
aaron.ballman wrote:
> Don't do `hasAttr` followed by `getAttr` (that duplicates work); also, you don't need to `dyn_cast<>` the call to `getAttr`. How about:
> ```
> const auto *IA = Previous->getAttr<MSInheritanceAttr>();
> if (IA && Previous->hasAttr<MSInheritanceAttr>()) {
>   ...
> }
> ```
I'm really sorry but I had a think-o in my suggestion. What I really meant was:
```
const auto *IA = Previous->getAttr<MSInheritanceAttr>();
if (IA && !D->hasAttr<MSInheritanceAttr>()) {
  ...
}
```
That said, I'm surprised you're not getting a test failure thanks to implementing my mistake. Were the tests failing for you, or are we missing test coverage?


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

https://reviews.llvm.org/D83174