sammccall added a comment.
In D114724#3163579 <https://reviews.llvm.org/D114724#3163579>, @kadircet wrote:
> for whatever reason I remembered the std::remove to have been referenced
> ~17k. looks like these two are much closer. I don't think it makes sense to
> prefer one or the other here.
Agree, for StdSymbolMap.inc this means neither version of remove should be
there.
> I suggest moving forward with a mechanism that enables accepting certain
> variants and accepting both unnamed and algorithm variants for std::remove,
> hence it being rejected as an ambiguous symbol in the end. That way we won't
> have a special case to handle once we start including multiple options for a
> symbol.
> WDYT?
In practice we don't actually need the ability to accept specific variants,
just variants in general.
(Maybe we'll need it later, but maybe not.)
================
Comment at: clang-tools-extra/clangd/CSymbolMap.inc:701
SYMBOL(remainderl, None, <math.h>)
-SYMBOL(remove, None, <stdio.h>)
SYMBOL(remquo, None, <math.h>)
----------------
hmm, I would expect this to still be in CSymbolMap, it's not ambiguous there.
================
Comment at: clang-tools-extra/clangd/include-mapping/cppreference_parser.py:138
# FIXME: use these as a fallback rather than ignoring entirely.
- if variant:
+ if variant != (namespace+symbol_name in variants_to_accept):
continue
----------------
kadircet wrote:
> sammccall wrote:
> > This logic seems a bit weird and non-general: it allows us to accept any
> > variant and reject any non-variant, but it doesn't allow us to accept
> > a*specific* variant (they are named) and doesn't allow us to accept both
> > (which currently would lead to the symbol being dropped as ambiguous).
> I was trying to keep the change to a minimum. Happy to go with a solution
> that accepts a dictionary in the form of `(FQN, list_of_accepted_variants)`,
> where the list can have `""` variant, or instead of a list we can also have a
> glob, but I don't think it'll provide much value considering the irregularity
> of std header names.
I'd just change this to `variant or (ns + name in variants to accept)`
That way we accept both versions of move, and end up dropping it as ambiguous.
================
Comment at: clang-tools-extra/clangd/include-mapping/cppreference_parser.py:164
+ # those symbols.
+ variants_to_accept = set(["std::remove"])
symbols = []
----------------
we should have a comment on the symbol
# std::remove<> template has variant "algorithm"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114724/new/
https://reviews.llvm.org/D114724
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
