ilya-biryukov added inline comments.
================
Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar
c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz
c#>
+}
----------------
arphaman wrote:
> Sorry, I see the issue now. However, I don't think that we'd like to change
> the signature for a function like this, as we'd still prefer to show `func
> (foo::type, ns::bar, ns::baz);` on this line.
>
> In Xcode we actually avoid the problem with `std::vector<>`s that you've
> pointed out entirely by using `value_type`. I'll check what our solution does.
>
> Btw, maybe using things like `value_type` is completely wrong (with or
> without the qualifier)? If we have `std::vector<int>` shouldn't we rather
> show `push_back(int _Value)`, rather than the `value_type`? Perhaps this kind
> of change should be discussed with a wider community somehow to find out
> what's best for all users.
Using `int _Value` may be good in case of `vector<int>`, but it would probably
loose typedefs, i.e. `vector<int32>` would still have `int`, which may be
undesirable.
Also, for `vector<vector<int>>`, the type inside `push_back` would probably end
up being `vector<int, std::allocator<int>>`.
As for the current vs new behavior, I can totally see why you want to see more
context in completion items.
I would argue that the current code does not do a great job there, as it only
adds qualifiers to unqualified references. But the context may be missing for
qualified references as well.
For example, in the following code:
```
template <class T, class Alloc = std::allocator<T>>
struct vector {
typedef T value_type;
typename value_type::some_type foo();
value_type bar()
};
```
Completion item for `vector<X>::bar` will have return type `vector<X,
std::allocator<X>>::value_type`. However, completion item for `vector<X,
std::allocator<X>>::foo` will have return type `value_type::some_type` (note
that no `vector<X, std::allocator<X>>` qualifier was added).
I would also question the intent of the current implementation. The following
line suggest that adding those qualifers was not intended in the first place:
```
Policy.SuppressUnwrittenScope = true;
```
But that's just a wild guess, I may be completely wrong.
That being said, I don't think there is one right preference, different people
may prefer different options. We can surely discuss that in a wider community,
though.
Would you be open to adding an option for that in completion and keeping the
current behavior as a default?
https://reviews.llvm.org/D38538
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
