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