[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added inline comments.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:704
+                        CXAvailabilityKind Availability,
+                        const std::vector<FullFixItHint> &Corrections)
       : Allocator(Allocator), CCTUInfo(CCTUInfo), Priority(Priority),
----------------
ilya-biryukov wrote:
> yvvan wrote:
> > ilya-biryukov wrote:
> > > Maybe accept the vector by value instead of const reference to allow the clients to `std::move` the argument and avoid copies?
> > but if it's accepted by value - it's one copy already by default
> > 
> > Instead I can add one more constructor with rvalue reference.
> If it's accepted by value, it's copy by default for l-values, but callers have an option to `std::move` explicitly.
> Having an r-value ref overload prevents from calling with l-values (e.g. local variables), i.e. requires either an explicitly copy to a temporary or `std::move`.
> 
> In general, I would still suggest passing by-value, unless it's particularly important to prohibit extra copies (e.g. for performance reasons, but I don't think it's the case here).
Ok, it was always a bit unclear what compiler does in such cases but I've found other references that it's actually a move in that case.

So it's changed to pass-by-value in the next diff update.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:1040
 
+  /// \brief Whether to try dot to arrow correction if arrow operator can be applied.
+  bool includeCorrections() const {
----------------
ilya-biryukov wrote:
> Mention other corrections are possible in this comment too?
Thanks!


https://reviews.llvm.org/D41537