Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[.]Type completion for (any P). shows it will produce an existential metatype #65843

Open
AnthonyLatsis opened this issue May 10, 2023 · 32 comments · May be fixed by #73163
Open

[.]Type completion for (any P). shows it will produce an existential metatype #65843

AnthonyLatsis opened this issue May 10, 2023 · 32 comments · May be fixed by #73163
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. code completion Area → source tooling: code completion existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values good first issue Good for newcomers metatypes Feature → types: Metatypes source tooling Area: IDE support, SourceKit, and other source tooling swift 5.9 types Feature: types unexpected behavior Bug: Unexpected behavior or incorrect output

Comments

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented May 10, 2023

Test case:

// RUN: %empty-directory(%t)
// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t
protocol P {}

(any P).#^COMPLETE?check=META^# 
// META: Keyword/CurrNominal:  Type[#(any P).Type#]; name=Type

This test will fail, because the current completion is Type[#any P.Type#]; name=Type. Notice how it thinks the completion will produce an existential metatype any P.Type, whereas the actual result is the singleton metatype (any P).Type.

Note
any syntax was proposed in SE-0335.

Environment

@AnthonyLatsis AnthonyLatsis added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. code completion Area → source tooling: code completion source tooling Area: IDE support, SourceKit, and other source tooling existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values metatypes Feature → types: Metatypes swift 5.9 unexpected behavior Bug: Unexpected behavior or incorrect output types Feature: types labels May 10, 2023
@AnthonyLatsis
Copy link
Collaborator Author

@Rajveer100 This one is somewhat less involved if you’re interested.

@Rajveer100
Copy link
Contributor

So do you suggest working on this, before #65699?

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented May 11, 2023

Yeah, I think that’s a good idea if this is your first time working on code completion.

@Rajveer100
Copy link
Contributor

error: 'clang/Driver/Options.inc' file not found

@Rajveer100
Copy link
Contributor

@AnthonyLatsis What could be the cause for this?

@AnthonyLatsis
Copy link
Collaborator Author

Stale build artifacts, for example. Rule no. 1 (assuming your checkout revisions are in sync): try a clean build.

@Rajveer100
Copy link
Contributor

Nope, still doesn't work, my local repo is in-fact synced and I also tried doing a clean rebuild, same error.

@AnthonyLatsis
Copy link
Collaborator Author

I can't tell much about what went wrong without more context. Can you share the steps you took originally and after my suggestion?

@Rajveer100
Copy link
Contributor

Rajveer100 commented May 18, 2023

I mean the only thing I did is change my target scheme to swift-ide-test, as you said to test for code completion.

@AnthonyLatsis
Copy link
Collaborator Author

Ah, so you're getting this error in Xcode, not as a result of invoking build-script. Does it build fine if you switch back to swift-frontend? Could you have selected a generated swift-ide-test target/scheme instead of manually creating your own, as you did for swift-frontend per the guide?

@Rajveer100
Copy link
Contributor

Oh yeah, I used the existing one!

@Rajveer100
Copy link
Contributor

Rajveer100 commented May 18, 2023

So I need to create a new scheme with the target as swift-ide-test? Any other configurations that's needed like it was in swift-frontend?

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented May 18, 2023

You need to follow the exact same steps, but for the swift-ide-test executable target. Keep in mind that these steps are opt-in and you can always build/run on the command line.

@Rajveer100
Copy link
Contributor

Now it builds successfully and does run with the following output:

action required
OVERVIEW: Swift IDE Test

USAGE: swift-ide-test [options] [input files...]

...

@Rajveer100
Copy link
Contributor

Rajveer100 commented May 24, 2023

@AnthonyLatsis I added the following as args... in the scheme for swift-ide-test, what would be the source file that's required as per the log message:

ARGS: swift-ide-test --code-completion --code-completion-token="#^COMPLETE?check=META^#"

LOG: source file required

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented May 24, 2023

Good job puzzling out how to output completions for a single token! As in the Swift frontend, this tool requires a path to an input source file to read from: -source-filename <path to file>. Also, -code-completion-token expects a token name/id like COMPLETE, not the token itself.

@Rajveer100
Copy link
Contributor

Got this:

found code completion token COMPLETE at offset 951
Begin completions, 3 items
Keyword[self]/CurrNominal:          self[#(any P).Type#]; name=self
Keyword/CurrNominal:                Protocol[#(any P).Type#]; name=Protocol
Keyword/CurrNominal:                Type[#any P.Type#]; name=Type
End completions
LookedupTypeNames: ['swift_ide_test.P']

@Rajveer100
Copy link
Contributor

Rajveer100 commented May 26, 2023

I am aware of existential meta types, but could you differentiate it from singleton meta types and also brief me out the log message which I obtained above. Regarding the files, I suppose CompletionLookup::getTypeCompletions and CompletionLookup::getPostfixKeywordCompletions seems to be the place to debug.

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented May 30, 2023

The singleton metatype is simply the metatype of a given type T written as T.Type (e.g. Int.Type, (any P).Type). The only possible instance of a Swift metatype is the type object T.self, hence singleton. An existential metatype is not really a metatype, but is called as such because it can be assigned type objects. If any P is a box that wraps a value of dynamic type that conforms to P, then any P.Type, aka an existential metatype, is a box that wraps a value of dynamic metatype whose instance type conforms to P. So if Int conforms to P, then any P can wrap 0, and any P.Type can wrap Int.self.


The places to debug should be correct.

@Rajveer100
Copy link
Contributor

Rajveer100 commented May 30, 2023

Yeah that gave me a good idea! Let me try to debug!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Jun 11, 2023

@AnthonyLatsis
Under CompletionLookup::getPostfixKeywordCompletions, we convert the ParsedExpr to a type of AnyMetatypeType,
which is where the check for isExistentialType() succeeds and the following snippet probably causes it:

// Just below "Protocol" keyword addition
addKeyword("Type", ExistentialMetatypeType::get(instanceTy),
                   SemanticContextKind::CurrentNominal);

Is my intuition right?

@Rajveer100
Copy link
Contributor

Rajveer100 commented Mar 9, 2024

Do we want an extra bracket () surrounded for the BaseType before calling lookupVisibleMemberDecls ?

@Rajveer100
Copy link
Contributor

@AnthonyLatsis
Am I on the right track here?

@AnthonyLatsis
Copy link
Collaborator Author

which is where the check for isExistentialType() succeeds and the following snippet probably causes it

Yes, this very much looks like the cause. You have to understand that any P.Type and (any P).Type are totally different types semantically. It is not that one type simply has an additional layer of parentheses as in Int vs. (Int):

  • base.TypeMetatypeType(base) — a regular, singleton, metatype type.
  • any base.TypeExistentialMetatypeType(base) — an existential metatype type.

Hence (any P).TypeMetatypeType(any P) and any P.TypeExistentialMetatypeType(P).

@Rajveer100
Copy link
Contributor

In this function:

inline bool CanType::isExistentialTypeImpl(CanType type) {
  return isa<ProtocolType>(type) ||
         isa<ProtocolCompositionType>(type) ||
         isa<ExistentialType>(type) ||
         isa<ParameterizedProtocolType>(type);
}

Why are Protocols considered as isExistentialTypeImpl?

@AnthonyLatsis
Copy link
Collaborator Author

For no good reason. That this has not been known to cause problems is a slightly worrying fluke. Before a separate representation for existential types was adopted in the model, this function worked with what is now known as a constraint type — protocol and protocol composition types. Then (apparently during the transition) someone added the isa<ExistentialType>(type) term . It might have been an intentional technical debt.

@Rajveer100
Copy link
Contributor

Rajveer100 commented Apr 16, 2024

At the moment I feel there could be the following possibilities:

  • Either the problem lies in the check for existential type.
  • Or when we get the expression by using getAs.

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented Apr 17, 2024

The current logic assumes that a postfix Type completion will necessarily produce an existential metatype if the instance type of the parsed type expression isAnyExistentialType():

https://github.com/apple/swift/blob/532a7de23e361b7367b168fa76c1082f69b681d0/lib/IDE/CompletionLookup.cpp#L2437-L2445

This is true for any P but false for (any P): the parentheses do not affect the canonical type of the expression, but they do change the outcome of appending Type.

@Rajveer100
Copy link
Contributor

If the canonical type remains the same shouldn't this check succeed:

!instanceTy->getCanonicalType()->is<MetatypeType>()

@AnthonyLatsis
Copy link
Collaborator Author

I’m not catching on to the causal relationship, but yes, the instance type is not a metatype in either of the above cases.

Rajveer100 added a commit to Rajveer100/swift that referenced this issue Apr 20, 2024
…n metatype instead of existential metatype.

Resolves swiftlang#65843
@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented Apr 21, 2024

Let me give the explanation another shot.

  • (any P).Type (the singleton metatype of existential type any P) ≠ any P.Type = P.Type (an existential metatype constrained to protocol P).
  • For P., the correct Type completion is Type[#any P.Type#] because any P.Type is the modern way of spelling P.Type = P. + Type.
  • Ditto any P.. In this case, the result is exactly any P.Type.
  • For (any P)., the correct Type completion is Type[#(any P).Type#] because (any P). + Type = (any P).Type.

If you feel unconfident around any/metatypes, you may want to put a pause on this and do some research on them.

@Jamezzzb
Copy link
Contributor

Jamezzzb commented Apr 22, 2024

Also from the SE-0335 .md: Spell the existential metatype as any P.Type, and the protocol metatype as (any P).Type.

Reiterating part of the above: any P and (any P) are the same but any P.Type and (any P).Type are different. Which is what the current completion logic seems to be missing out on.

Rajveer100 added a commit to Rajveer100/swift that referenced this issue Apr 22, 2024
…n metatype instead of existential metatype.

Resolves swiftlang#65843
Rajveer100 added a commit to Rajveer100/swift that referenced this issue Apr 24, 2024
…n metatype instead of existential metatype.

Resolves swiftlang#65843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. code completion Area → source tooling: code completion existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values good first issue Good for newcomers metatypes Feature → types: Metatypes source tooling Area: IDE support, SourceKit, and other source tooling swift 5.9 types Feature: types unexpected behavior Bug: Unexpected behavior or incorrect output
Projects
None yet
3 participants