usaxena95 added inline comments.
================
Comment at: clang-tools-extra/clangd/quality/CompletionModelCodegen.py:39
+ '''Returns the header guard for the generated header.'''
+ return "GENERATED_DECISION_FOREST_MODEL_{}_H".format(filename.upper())
+
----------------
adamcz wrote:
> Why GENERATED_DECISON_FOREST_MODEL instead of output_dir, to be consistent
> with header guards for other files? Doesn't matter much for generated code,
> but if someone opens this in vim they'll see warnings.
The output_dir is the absolute path and not a project relative path.
I tried to stick with a special prefix for header guard as done in other
Generated headers (e.g. protos)
If someone opens this in vim, there would many other warnings that they would
see like "unused_label" ;)
I don't think that would be a concern since it would be opened for inspection
and not for editing.
================
Comment at: clang-tools-extra/clangd/quality/CompletionModelCodegen.py:177
+
+ code += " friend float Evaluate(const {}&);\n".format(cpp_class.name)
+ code += "};\n"
----------------
adamcz wrote:
> Is there a reason to make this a friend free-function instead of static
> method on the Example class? The problem is that you now end up with
> clang::clangd::Evaluate, so if we every re-use this code gen for another
> model we'll have a name collision.
The class name ("Example" currently) would be different for a different model
and therefore there would be another overload for `Evaluate(const
AnotherClass&)` even if the namespaces are same (`clang::clangd`).
================
Comment at: clang-tools-extra/clangd/unittests/model/CategoricalFeature.h:1
+namespace ns1 {
+namespace ns2 {
----------------
adamcz wrote:
> Can we rename this directory? quality/model makes some sense (although it
> would be better to include something about code completion there), but
> unittests/model is not very descriptive - what model?
>
> How about unittests/decision_forest_model/ or something like that? Or go with
> the Input/TEST_NAME pattern.
You are right "quality" wasn't indicative of code completion here but we
decided to be consistent with the current naming. The current heuristics for
the ranking are in Quality.h and Quality.cpp ;-)
changed the dir name in unittests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83814/new/
https://reviews.llvm.org/D83814
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
