yvvan added inline comments.

================
Comment at: include/clang-c/Index.h:5278
+  /**
+   * \brief Whether to try dot to arrow correction if arrow operator can be 
applied.
+   */
----------------
ilya-biryukov wrote:
> This implies that "dot to arrow" is the only available correction. Maybe 
> rephrase to mention that others are possible in theory?
> E.g.
> ```Whether to include completion items with corrections (small fix-its), e.g. 
> change '.' to '->' on member access, etc.```
thanks, forgot to change that one


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:415
+/// diagnostic.
+class FullFixItHint : public FixItHint {
+public:
----------------
ilya-biryukov wrote:
> Why do we need this wrapper?
> It seems that storing `SourceManager` and `LangOpts` in each fix-it is 
> clearly confusing (they are the same for all diags in the file).
> All clients that want to access the fix-it should have a reference to an 
> existing `SourceManager`, right?
My first version took source manager and language options from 
CXCodeCompleteResults (which is in fact AllocatedCXCodeCompleteResults) so I 
needed to pass it as an extra parameter which looks kind of ugly,

The single CXCodeCompleteResult does not have them. But I can leave 
CXCodeCompleteResults pointer as a parameter to avoid this wrapper.

And since it's a libclang design part it probably should be fixed/workarounded 
there.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:577
+  /// \brief For this completion result correction is required.
+  std::vector<FullFixItHint> Corrections;
+
----------------
ilya-biryukov wrote:
> Storing fix-its in `CodeCompletionString` seems like to be against its 
> original intention, i.e. now it depends on `SourceLocation`s, which require 
> `SourceManager`, etc.
> Is there a way to get to a `CodeCompletionResult` from `libclang`? Storing 
> fix-its there would be ideal.
CodeCompletionString is an abstraction used by libclang to store everything. So 
every completion detail getter in libclang works through it.
 
CXCompletionResult stores only cursor kind and a pointer to that string.


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


https://reviews.llvm.org/D41537



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to