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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
