[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Thank you!


Repository:
  rL LLVM

https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342897: [python] [tests] Update test_code_completion 
(authored by mgorny, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50171?vs=164871&id=166703#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50171

Files:
  cfe/trunk/bindings/python/tests/cindex/test_code_completion.py


Index: cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
===
--- cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
+++ cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || 
Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available 
|| Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 


Index: cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
===
--- cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
+++ cfe/trunk/bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50171: [python] [tests] Update test_code_completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D50171#1236792, @mgorny wrote:

> @ilya-biryukov, gentle ping. I'd like to patch this for 7.0.0 in Gentoo. Do 
> you think my patch would be good enough, or do you expect to submit something 
> else soonish?


Sorry about the delay.
LGTM to allow committing/cherrypicking. I'll make sure to investigate why the 
change was there and explain/fix this case and update this thread.


https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@ilya-biryukov, gentle ping. I'd like to patch this for 7.0.0 in Gentoo. Do you 
think my patch would be good enough, or do you expect to submit something else 
soonish?


https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Will take a closer look. Thanks for finding the offending revision.


https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

...and unless I'm mistaken, this commit is responsible for both changes (that 
is, I need to apply all of this patch to make tests pass at its point).


https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Apparently the first change changing the completion results is:

  commit a408f8d27bfd5bab55c39ef2a6fff6850be4a351
  Author: Ilya Biryukov 
  Date:   Tue Apr 24 15:48:53 2018
  
  [CodeComplete] Fix completion at the end of keywords
  
  Summary:
  Make completion behave consistently no matter if it is run at the
  start, in the middle or at the end of an identifier that happens to
  be a keyword or a macro name. Since completion is often ran on
  incomplete identifiers, they may turn into keywords by accident.
  
  For example, we should produce same results for all of these
  completion points:
  
  // ^ is completion point.
  ^class
  cla^ss
  class^
  
  Previously clang produced different results for the last case (as if
  the completion point was after a space: `class ^`).
  
  This change also updates some offsets in tests that (unintentionally?)
  relied on the old behavior.
  
  Reviewers: sammccall, bkramer, arphaman, aaron.ballman
  
  Reviewed By: sammccall
  
  Subscribers: cfe-commits
  
  Differential Revision: https://reviews.llvm.org/D45887
  
  git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330717 
91177308-0d34-0410-b5e6-96231b3b80d8


https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 164871.
mgorny added a comment.

Ai, sorry about that. Uploaded the proper diff now. I suppose it's not going to 
make it for 7.0.0 anymore, so it's not a priority. I'll try to bisect it today 
once I finish testing RC3.


https://reviews.llvm.org/D50171

Files:
  bindings/python/tests/cindex/test_code_completion.py


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || 
Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available 
|| Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50171: [python] [tests] Update test_code_completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Also can't explain why `const` and `volatile` have a different priority now.
The `P::` and `Q::` seem to be completely different completion items from `P` 
and `Q` (wildly different priorities suggest they're not the same), can't 
explain what caused the first ones not being added anymore.

Happy to LGTM to unbreak tests, but won't have time to dig up the commits that 
introduced these changes.

PS Could you upload the diff with full context?


Repository:
  rC Clang

https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D50171



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: ilya-biryukov, arphaman, bkramer, gribozavr.

Update expected completions to match output generated by clang-7.0.

Disclaimer: I have no clue if the results are correct. I'm kinda wondering why 
the '::' part is no longer included immediately.

Also PR38417 .


Repository:
  rC Clang

https://reviews.llvm.org/D50171

Files:
  bindings/python/tests/cindex/test_code_completion.py


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || 
Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available 
|| Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits