[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: unittests/clangd/QualityTests.cpp:129
+  Test.Code = R"cpp(
+    #include "foo.h"
+    int ::test_func_in_header_and_cpp() {
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > this is not needed, the `#include` is implicit in TestTU
> > > 
> > > (and so you don't need to specify HeaderFilename either)
> > Done.
> > 
> > I did not expect the `TestTU` to do that, actually. Is this implicit `#include` something we want?
> > Maybe we should just have a default convention for naming the headers instead and the default `Code` value that only includes the header?
> > E.g. say that a file `test_header.h` is provided by TestTU and let the tests specify `#include "test_header.h"` if needed. WDYT?
> Missed this - definitely making sure there's a #include in the cc file that lines up to the name/location of the header file is one of the main things I wanted this helper to do. Maybe the documentation can be more clear?
SG, I'd definitely mention that the file is included and the way it's done, i.e. via `-include` flag passed to clang.


================
Comment at: unittests/clangd/QualityTests.cpp:139
+  WithProximity.ProximityScore = 0.2;
+  EXPECT_LT(Default.evaluate(), WithProximity.evaluate());
 }
----------------
sammccall wrote:
> nit: can you EXPECT_GT and reverse the order? (default is on the right in all tests above, even the positive signals for symbolquality)
I always use `<` and `<=` in all the code I write. That gives the "natural" order of the arguments. (argument to the left is less)
So it's completely opposite for me: I find the `Default.evaluate()` on the left side to be more readable.

Feel free to change it, though. Just wanted to point out this style can be more or less readable based on the personal preferences.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46943