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

Reply via email to