ilya-biryukov added a comment.
A few go-by comments from me, Haojian should have better context here!
================
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!
----------------
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.
================
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:
> 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)
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:674
llvm::StringMap<unsigned> Identifiers;
- lex(Content, Style, [&](const clang::Token &Tok) {
+ lex(Content, Style, [&](const clang::Token &Tok, const Position) {
switch (Tok.getKind()) {
----------------
Should this be `const Position&` or `Position`?
================
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
----------------
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.
================
Comment at: clang-tools-extra/clangd/SourceCode.h:270
+ /// It will be “a::b” for both carrot locations.
+ std::string CurrentNamespace;
+ /// Offsets into the code marking eligible points to insert a function
----------------
Do we ever name anonymous namespace here? How do we represent them?
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
