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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Important: please upload the patch with full context diff



================
Comment at: include/clang-c/Index.h:5278
+  /**
+   * \brief Whether to try dot to arrow correction if arrow operator can be applied.
+   */
----------------
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.```


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:415
+/// diagnostic.
+class FullFixItHint : public FixItHint {
+public:
----------------
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?


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:577
+  /// \brief For this completion result correction is required.
+  std::vector<FullFixItHint> Corrections;
+
----------------
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.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:704
+                        CXAvailabilityKind Availability,
+                        const std::vector<FullFixItHint> &Corrections)
       : Allocator(Allocator), CCTUInfo(CCTUInfo), Priority(Priority),
----------------
Maybe accept the vector by value instead of const reference to allow the clients to `std::move` the argument and avoid copies?


https://reviews.llvm.org/D41537