ilya-biryukov added inline comments.
================
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+ AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
----------------
sammccall wrote:
> ioeric wrote:
> > Could you explain why `AllDeclsInMainFile` is necessary? I think it might
> > still be fine to boost score for symbols with a declaration in the main
> > file even if it has redecls in other files (e.g. function fwd in headers).
> Agree. I think the better signal is (any) decl in main file.
My intuition was that it does not make any sense to functions if there
definitions are in the same cpp file, because it does not give any useful
signals on whether those should be preferably called in the same file.
Also, some defs may not be visible to the compiler at the point of completion
and, therefore, won't get a boost, even if they are in the same file. This is
inconsistent.
E.g.,
```
// === foo.h
class Foo {
int foo();
int bar();
int baz();
int test();
};
int glob_foo();
int glob_bar();
int glob_baz();
// === foo.cpp
#include "foo.h"
static some_local_func() {}
Foo::foo() {
};
int ::glob_foo() {
}
Foo::test() {
^
// out of all functions declared in headers, only foo and global_foo will
get a boost here. That does not make much sense, since:
// - glob_bar() and Foo::bar() are also declared in the same file, but
compiler hasn't seen them yet, so they won't get a boost.
// - glob_baz() and Foo::baz() are not declared in the same file, but they
are so close to `bar` in the header,
// and it doesn't seem to make sense to rank them differently.
}
Foo::bar() {
}
int ::glob_bar() {
}
```
Why upranking decls **only** in the current file is still useful?
- Those are usually helpers that are very relevant to the file. Especially
true for small files.
- As a side-effect, we'll uprank local vars and parameters (they can't have
decls in other files :-)), that seems useful. Maybe such implicit effects are
not desirable, though.
I should've definitely documented this better. If we decide this change is
useful, I'll add more docs.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
