[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

Thank you for the update. A few questions:

- Can you add a test with macro expansion? Would we get two inlay hints right after the spelling location? e.g.,

  #define Decl_Record \
    struct Widget {  \
      [very long definition that reach `EndDefinitionCommentMinLines`]. \
      struct SubWidget { \
        [another chunk of members.] \
      // not ending with a bracket here!
  
  //  but appears here:
  //          v                       v
  Decl_Record } // struct SubWidget ; } // struct Widget (Hmm...) ;



- I was thinking if we should add some test cases for C. (I have learned that it is allowed to write a declaration in an expression while C++ doesn't.) For example,

  int main(void) {
    _Alignof(struct S {
      [long definition]
    } // Is it suitable to hint here? What do you think?
    );
  }



================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:285-286
+  bool VisitEnumDecl(EnumDecl *D) {
+    if (Cfg.InlayHints.EndDefinitionComments &&
+        D->isThisDeclarationADefinition()) {
+      StringRef DeclPrefix;
----------------
nit: bail out early?



================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:797
+      // differently.
+      assert(Label.empty());
+      Label += printName(AST, D);
----------------
nit: `assert(Label.empty() && "Label should be empty with FunctionDecl")`. might be helpful for debugging.


================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1716
+
+    // No hint becaus trailing ';' is only allowed for class/struct/union/enum
+    void foo() {
----------------
typo: `becaus` -> `because`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150635/new/

https://reviews.llvm.org/D150635