sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/AST.cpp:434 + for (AttributedTypeLoc ATL = *ATLPtr; !ATL.isNull(); + ATL = ATL.getNextTypeLoc().getAs<AttributedTypeLoc>()) + Result.push_back(ATL.getAttr()); ---------------- hokein wrote: > this looks like not safe, `getNextTypeLoc()` may return a null TypeLoc. I don't think it can - an AttributedType always modifies a real type. I've added an assert. ================ Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:233 + }; + ASSERT_THAT(DeclAttrs("X"), Each(implicitAttr())); + ASSERT_THAT(DeclAttrs("Y"), Contains(attrKind(attr::WarnUnusedResult))); ---------------- hokein wrote: > sorry, I'm not familiar with attributes, what is an implicit attr? It is > unclear to me why there is an attr for `class X`, the source code doesn't > have any attribute label for X (the same question for f and a) Right, implicit attributes are when there's nothing written in the source ,but something else modifies the semantics in a way that clang authors decided to model as an attribute (e.g. because semantics match that of an explicit attribute). I'm not familiar with many examples either, but a couple: - when targeting windows, top-level classes appear to have an implicit "type visibility" attribute that I guess models the difference between default unix/windows symbol visibility. - Aaron Ballman gave an example of `[[interrupt(...)]]` which also adds an implicit `[[used]]` attribute. The windows example was why I assert there are no explicit attributes, instead of that there are none at all. I've added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89785/new/ https://reviews.llvm.org/D89785 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits