kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/StdSymbolMap.inc:958
SYMBOL(remainder, std::, <cmath>)
-SYMBOL(remove, std::, <cstdio>)
SYMBOL(remove_all_extents, std::, <type_traits>)
----------------
sammccall wrote:
> Is the stdio version so uncommon we are willing to be wrong about it? For
> `move` we omit it from here and consider it case-by-case, I'd have imagined
> doing the same here
>
> (BTW at some point we may want to extend this to list alternative headers for
> ambiguous symbols)
>Is the stdio version so uncommon we are willing to be wrong about it?
Waiting for permissions on a big codebase with lots of third_party code to
perform some analysis.
> For move we omit it from here and consider it case-by-case, I'd have
> imagined doing the same here
`std::move` is different, both occurrences are marked as variants. We don't
really drop it during post-processing saying there are multiple options, it
just never makes it to the final list.
```
move<>() (algorithm) (since C++11)
move<>() (utility) (since C++11)
```
I am not sure how we ended up with this ignore the `variant`ed symbols behavior.
> (BTW at some point we may want to extend this to list alternative headers for
> ambiguous symbols)
I agree, there is some room for improvement, like also listing different
overloads with expected number of parameters and language versions.
It's unclear whether we need such detail today though.
================
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
----------------
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.
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
