ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/Features.inc.in:7
#define CLANGD_TIDY_CHECKS @CLANGD_TIDY_CHECKS@
+#define CLANGD_DECISION_FOREST @CLANGD_DECISION_FOREST@
----------------
sammccall wrote:
> we **could** include this in the feature-string too (Feature.cpp) but I think
> it's probably more noise than signal.
Maybe include only when it's disabled? I've added this to the commit, PTAL.
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:215
+ clEnumValN(CodeCompleteOptions::Heuristics, "heuristics",
+ "Use hueristics to rank code completion items")),
+#ifdef CLANGD_DECISION_FOREST
----------------
sammccall wrote:
> hmm, while reordering these: hueristics -> heuristics? :-)
I'm not sure how the reordering happened in the first place, TBH 🤔
I kept it and fixed the typo, but also let me know if I should keep the old
order instead.
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:217
+#ifdef CLANGD_DECISION_FOREST
init(CodeCompleteOptions().RankingModel),
+#else
----------------
sammccall wrote:
> CodeCompleteOptions().RankingModel is also the one used in unit tests.
> I think the default value in the struct should be ifdef'd, or if we
> **really** want to avoid pp-conditionals in header files, we could have an
> `Auto` value which acts sa the platform-default.
I have moved it to the `.cpp` file. I think that avoiding conditionals in the
preprocessor is a good thing definitely.
I also considered putting the constant into `DecisionForest.cpp` to reduce the
number of files that use `#if CLANGD_DECISION_FOREST`.
But the downside is that definition of the constant gets harder to find, so I
opted for keeping it in this file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139107/new/
https://reviews.llvm.org/D139107
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
