ioeric added inline comments.
================
Comment at: clangd/index/Index.h:430
///
- /// The global scope is "", a top level scope is "foo::", etc.
+ /// The global scope is "", a top level scope is "foo::", etc. "*" is
+ /// wildcard.
----------------
sammccall wrote:
> I'm not a big fan of this magic value, it seems too easy to forget to handle.
> Especially since we have a lot of existing code that touches this interface,
> and we may forget to check some of it.
>
> I suggest making this a separate boolean field `AnyScope`, with a comment
> that scopes explicitly listed will be ranked higher.
> We can probably also remove the "If this is empty" special case from `Scopes`
> now too, as the old behavior can be achieved by setting `Scopes = {};
> AnyScope = true;`
sounds good.
> We can probably also remove the "If this is empty" special case from Scopes
> now too, as the old behavior can be achieved by setting Scopes = {}; AnyScope
> = true;
I think this is a good idea. Unfortunately, there seem to be many tests that
rely on the old behavior. I'll add a FIXME and do it in followup patch.
================
Comment at: clangd/index/Index.h:432
+ /// wildcard.
+ /// FIXME: support assigning different weight to each scope.
std::vector<std::string> Scopes;
----------------
sammccall wrote:
> May not want a heavyweight API with explicit weights...
>
> I think what we really **need** here is the ability to weight
> `clang::clangd::` > `clang::` > `` when you're in the scope of namespace
> clangd.
>
> We could just say something like "the primary scope should come first" and do
> some FileDistance-like penalizing of the others...
Changed the wording of `FIXME`.
> I think what we really need here is the ability to weight clang::clangd:: >
> clang:: > `` when you're in the scope of namespace clangd.
It's unclear what this would mean for scopes from `using-namespace` directives.
But we can revisit when we get there.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
