[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 14 13:02:54 PDT 2020
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
More information about the cfe-commits
mailing list