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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits