sammccall added inline comments.

================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4220
+  )cpp";
+  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
----------------
aaron.ballman wrote:
> FWIW, this will miss `omp::sequence` and `omp::directive`, but that's not the 
> end of the world. May be worth a fixme comment in case we want to solve it 
> someday.
Well, those
 - don't themselves have Attr nodes or AttrKinds, so the API couldn't query for 
them
 - don't actually have any documentation as far as I can tell
So I'm not sure there's much to fix.

---

The ParsedAttr-kind vs Attr-kind distinction, and the fact that many attributes 
don't leave any AST nodes at all, mean this method doesn't seem "universal". 
But the goal is to show docs in clangd when hovering on an attribute (which we 
only support for those with AST nodes).

If we want to do something different (like show documenation while 
code-completing) we'll probably have to add a second ParsedAttrKind-based API.


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4242
+  llvm::StringRef clang::Attr::getDocumentation(clang::attr::Kind K) {
+    assert(K < llvm::array_lengthof(AttrDoc));
+    return AttrDoc[K];
----------------
aaron.ballman wrote:
> Hmmmm, I am not 100% certain this assertion is correct -- the user may have 
> plugin attributes, which I believe are given a "kind" that's larger than the 
> last-known builtin attribute kind.
Thanks, changed it to an if() instead.
(I don't think it's particularly pressing to allow them to be documented)


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