swift icon indicating copy to clipboard operation
swift copied to clipboard

Diagnostic for use of plain protocol name in type position should suggest `some` or `any`

Open hborla opened this issue 1 year ago • 15 comments

The following code is invalid:

protocol P {
  associatedtype A
}

func generic(value: P) {}

The compiler currently produces the error message Use of protocol 'P' as a type must be written 'any P' with a fix-it to insert the any keyword.

However, a different (and often better!) fix here is to use the some keyword. The error message should be re-worded to prompt the programmer to consider any or some, and there should be two notes, each with a fix-it to insert any or some, respectively.

hborla avatar Sep 01 '23 21:09 hborla

@hborla sir, what is meant here, can you please elaborate, so that I can contribute accordingly.

Dhandeep10 avatar Sep 09 '23 19:09 Dhandeep10

I’m currently working on fixing this bug in diagnostics as part of the swift mentorship program

taevonlewis avatar Sep 09 '23 22:09 taevonlewis

I too want to contribute and get started, I am new to open source, so want to learn and explore this.

Dhandeep10 avatar Sep 10 '23 10:09 Dhandeep10

@taevonlewis are you still working on this issue?

saehejkang avatar Oct 09 '23 01:10 saehejkang

@saehejkang Yes, I am.

taevonlewis avatar Oct 09 '23 15:10 taevonlewis

Hi @taevonlewis, are you still working on this?

li3zhen1 avatar Dec 28 '23 21:12 li3zhen1

@AnthonyLatsis can this be reassigned due to its lack of activity?

EDIT

Even if this issue is still being worked on, I found some great notes on Diagnostics.md to help me wrap my head around how fix-itsand diagnostics are created/tested. Just had a question to test my knowlede with the current fix-it.

protocol P {
  associatedtype A
}
func generic(value: P) {} // expected-error {{use of protocol 'P' as a type must be written 'any P'}}
                        // expected-note@-1 {{Replace 'P' with 'any P'}} {{21-21= any P}}

Is this close to how the verifier should be written? This is not passing the -verify flag when I run it locally, so would love to know what it should be.

Let's say I get it to pass verification, but I change the fix-it to actually now be some P instead of any P. I understand the verification will fail, but will the console output what it should be below (show the diff).

saehejkang avatar May 01 '24 00:05 saehejkang

-verify mode diagnoses both grammatical issues and mismatches — with details on actual vs. expected diagnostics (fix-its).

AnthonyLatsis avatar May 01 '24 12:05 AnthonyLatsis

@taevonlewis Are you fine with passing this over to Saehej?

AnthonyLatsis avatar May 01 '24 12:05 AnthonyLatsis

Yes, they can take over this issue

taevonlewis avatar May 01 '24 12:05 taevonlewis

with details on actual vs. expected diagnostics (fix-its).

With the -verify flag added to the swift-frontend build arguments, I get this output (no details on actual), just an error that the note is not produced.

/Documents/xcode/SwiftCompiler/SwiftCompiler/main.swift:71:28: error: expected note not produced
                        // expected-note@-1 {{Replace 'P' with 'any P'}} {{21-21=any P}}
~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

saehejkang avatar May 01 '24 17:05 saehejkang

Right, there is no actual note emitted on the specified line (and at all), so there is nothing to compare your expectation with, just the fact that it was not fulfilled.

AnthonyLatsis avatar May 01 '24 17:05 AnthonyLatsis

there is no actual note emitted on the specified line

Screenshot 2024-05-01 at 10 36 58 AM

Is the fix-it not an expected note? Maybe I wrote one of these wrong

// expected-error {{use of protocol 'P' as a type must be written 'any P'}}
// expected-note@-1 {{Replace 'P' with 'any P'}} {{21-21= any P}}

saehejkang avatar May 01 '24 17:05 saehejkang

We want to make updates to this diagnostic I found below. I was thinking to add a new variable Type and pass in the 'some P' to the error, but I had some questions about where that comes from.

ERROR(existential_requires_any,none, "use of %select{protocol |}2%0 as a type must be written %1", 
(Type, Type, bool))

On this line in TypeCheckType the diagnostic is used and passes in the Type returned by getDeclaredExistentialType(). Correct me if I am wrong, but I believe that is the value 'any P'. The ExistentialType is spelled with the keyword any as it says so here in Types.

My thought process was to look for something similar that uses the keyword some and I landed on looking into OpaqueTypes. Is there something similar to getDeclaredExistentialType() that pretty much fetches the value 'some P'? I delved deeper into OpaqueTypes and feel I need use something like this .

saehejkang avatar May 02 '24 00:05 saehejkang

Is the fix-it not an expected note?

A fix-it by itself is not a diagnostic message. It is a text substitution descriptor (a range + a replacement string) that is attached to and emitted with a diagnostic message. Custom fix-it descriptions are implemented by attaching the fix-it to a note rather than the primary warning/error. The fix-it on the screenshot is attached to an error; its description was autogenerated by Xcode.


I suggest hardcoding the some keyword instead of attempting to construct a semantic type that will print as some P. An opaque parameter type — the alternative we want to suggest — is no more than syntactic sugar for the declaration and use of a generic parameter, and does not have a separate syntax-preserving representation in the type system. OpaqueTypeArchetypeType is for opaque result types.

AnthonyLatsis avatar May 02 '24 14:05 AnthonyLatsis

@hborla Do you reckon we should also suggest both options in positions where some X is an opaque result type? Also, we should probably refrain from suggesting some if the parent declaration is ABI-public and library evolution is enabled. That could cause an ABI break.

AnthonyLatsis avatar Aug 15 '24 17:08 AnthonyLatsis