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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Could you upload the patch with full context?



================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:704
+                        CXAvailabilityKind Availability,
+                        const std::vector<FullFixItHint> &Corrections)
       : Allocator(Allocator), CCTUInfo(CCTUInfo), Priority(Priority),
----------------
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).


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:600
+  /// \brief Check if this completion requires some correction.
+  const std::vector<FixItHint> &getCorrections() const { return Corrections; }
+
----------------
NIT: return `llvm::ArrayRef<FixItHint>` instead. 


================
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 {
----------------
Mention other corrections are possible in this comment too?


================
Comment at: include/clang/Sema/CodeCompleteOptions.h:43
+  /// Show also results after dot to arrow correction if arrow operator can be applied.
+  unsigned IncludeCorrections : 1;
+
----------------
Mention other corrections are possible in this comment too?




https://reviews.llvm.org/D41537