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

[Nikolai Kosjar via Phabricator via cfe-commits]

nik requested changes to this revision.
nik added a comment.
This revision now requires changes to proceed.

Generates build errors here:

In file included from /d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:4:0:
/d2/llvm/qtc/source/tools/clang/include/clang-c/Index.h:1332:62: warning: comma at end of enumerator list [-Wpedantic]
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 'tokens_spelling_at_range':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 'print_completion_result':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2365:3: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
At top level:
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2257:13: warning: 'tokens_spelling_at_range' defined but not used [-Wunused-function]



================
Comment at: include/clang-c/Index.h:4753
+ *
+ * \returns The token starting with the given location or NULL if no such tokens
+ * exist. The returned pointer must be freed with clang_disposeTokens before the
----------------
s/tokens/token


================
Comment at: include/clang-c/Index.h:5256
 /**
+ * Retrieve the number of fix-its for the given completion string.
+ */
----------------
s/string/index


================
Comment at: include/clang-c/Index.h:5264
+ * FixIts that *must* be applied before inserting the text for the
+ * corresponding completion item.
+ *
----------------
* Please use proper documentation style, e.g. make use of "\brief", "\param", "\returns". Currently one has to somewhat guess that the returned string is related to the replacement_range parameter.

* Is this a behavior change now? What happens to old code not aware of this function?


================
Comment at: include/clang-c/Index.h:5266
+ *
+ * Completion items with non-empty fixits will not be returned by default, they
+ * should be explicitly requested by setting CXCodeComplete_IncludeFixIts. For
----------------
Please avoid double negation.


================
Comment at: include/clang-c/Index.h:5268
+ * should be explicitly requested by setting CXCodeComplete_IncludeFixIts. For
+ * the editors to be able to compute position of the cursor for the completion
+ * item itself, the following conditions are guaranteed to hold for RemoveRange
----------------
s/editors/clients/


================
Comment at: include/clang-c/Index.h:5269
+ * the editors to be able to compute position of the cursor for the completion
+ * item itself, the following conditions are guaranteed to hold for RemoveRange
+ * of the stored fixits:
----------------
Do you mean replacement_range instead of RemoveRange?


================
Comment at: include/clang-c/Index.h:5284
+ *      vec_ptr.^  // completion returns an item 'push_back', replacing '.'
+ *      with '->' vec_ptr->^ // completion returns an item 'release',
+ *      replacing '->' with '.'
----------------
The vec_ptr-> example should go to its own line.


================
Comment at: include/clang-c/Index.h:5326
+   * Whether to include completion items with small
+   * fix-its, e.g. change '.' to '->' on member access, etc.
+   */
----------------
Currently there is not more, so I would avoid the "etc.".


================
Comment at: test/Index/complete-arrow-dot.cpp:20
+
+// CHECK-NOT: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it: {10:7-10:8} to "->")
+// CHECK-NOT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it: {10:7-10:8} to "->")
----------------
That's pretty verbose, consider to use wildcards/regexes instead just checking for "requires fix-it:".


================
Comment at: tools/c-index-test/c-index-test.c:2478
     completionOptions |= CXCodeComplete_SkipPreamble;
+  if (getenv("CINDEXTEST_COMPLETION_INCLUDE_CORRECTIONS"))
+    completionOptions |= CXCodeComplete_IncludeFixIts;
----------------
Keep terminology of the API - s/CORRECTIONS/FIXITS


https://reviews.llvm.org/D46862