kadircet marked 5 inline comments as done.
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:655
+lex(llvm::StringRef Code, const format::FormatStyle &Style,
+ llvm::function_ref<void(const clang::Token &, const Position)> A) {
// FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Yay! Thanks! Less templates, more type-checking!
> >
> > Could you rename 'A' to something more specific, though?
> > Not that there is no nice type name to guide the readers, `A` feels a bit
> > confusing.
> why `const Position` and not `const Position&`?
> top-level const in function parameters is weird, it's basically ignored
> (except at the definition site)
dropping the const
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:996
+ std::vector<std::string> Enclosing = {""};
+ // FIXME: In addition to namespaces try to generate events for function
+ // definitions as well. One might use a closing parantheses(")" followed by
an
----------------
ilya-biryukov wrote:
> If we were to do this, I'd rather try matching brace structure and only
> report `closing brace that is probably end of the top-level decl(function,
> struct, etc)` events.
>
> Everything else seems super hard to get right.
agreed, my suggestion was also just for figuring out when a `{` might start a
definition, and trigger an event at the corresponding `}`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68024/new/
https://reviews.llvm.org/D68024
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
