kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:98
+ ValidValues.push_back(Name);
+ if (!Result && *Input == Name)
+ Result = Value;
----------------
should we assert on multiple matches of the same name ?
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:196
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
+ constexpr static llvm::SourceMgr::DiagKind Warning =
+ llvm::SourceMgr::DK_Warning;
----------------
nit: I know this isn't what the patch is about, but.... what about making these
lambdas/functions of Out.diag with first parameter bound to DK_Error or
DK_Warning? (or having them at an anonymous namespace instead of a member)
================
Comment at: clang-tools-extra/clangd/ConfigFragment.h:124
+
+ /// Controls how clangd understands code outside the current file.
+ struct IndexBlock {
----------------
i think saying "outside the current file" might not be the best description.
Maybe rather "Controls clangd's understanding for entities(symbols, refs,
relations) in the codebase" ?
================
Comment at: clang-tools-extra/clangd/ConfigFragment.h:129
+ /// Legal values are "Build" or "Skip".
+ llvm::Optional<Located<std::string>> Background;
+ };
----------------
why not store the enum in here directly and generate diagnostics during the
parsing stage?
we seem to be doing this for syntactic errors, e.g. `expected a dictionary` and
getting an unexpected enum value sounds similar to that?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83768/new/
https://reviews.llvm.org/D83768
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
