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

Ivan Donchevskii via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 25 03:02:36 PDT 2018


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





More information about the cfe-commits mailing list