[PATCH] D107703: [AST][clangd] Expose documentation of Attrs on hover.
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 12 00:10:56 PDT 2021
kadircet accepted this revision.
kadircet added a comment.
thanks, LG from my side as well, just a couple of clarification questions regarding tablegen.
================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2389
+ HI.Documentation = Attr::getDocumentation(attr::NonNull).str();
+ ASSERT_THAT(HI.Documentation, testing::HasSubstr("parameters"));
}},
----------------
nit: maybe `ASSERT_FALSE(HI.Documentation.empty())` ? (I actually think the test in AST/AttrTest is enough and we can drop this assertion).
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4237
+ static const llvm::StringRef AttrDoc[] = {
+ #define ATTR(NAME) AttrDoc_##NAME,
+ #include "clang/Basic/AttrList.inc"
----------------
i am not well-versed in tablegen, so sorry if i am being dense here, butt what happens to the attributes we skipped above (e.g. the ones without a `ASTNode` flag)?
Maybe we should be emitting a `... AttrDoc_X[] = "";` for all attributes, no matter what?
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4231
+ // Only look at the first documentation if there are several.
+ // (As of now, only one attribute has multiple documentation entries).
+ break;
----------------
not sure if this comment will stay useful.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107703/new/
https://reviews.llvm.org/D107703
More information about the cfe-commits
mailing list