Skip to content

Sema: Reword diagnostics to say 'without a type annotation' instead of 'without more context' #66498

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

Merged

Conversation

slavapestov
Copy link
Contributor

No description provided.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Member

@hborla hborla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!

@@ -275,10 +275,10 @@ ERROR(no_candidates_match_argument_type,none,
(StringRef, Type, unsigned))

ERROR(cannot_infer_closure_parameter_type,none,
"unable to infer type of a closure parameter %0 in the current context",
"unable to infer type of a closure parameter %0 without a type annotation",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe unable to infer type for a closure parameter %0; please provide a type annotation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to change the first clause, I would write cannot infer type of closure parameter %0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that sounds good and aligns with other diagnostics we have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am confused here but I thought we settled on cannot infer type of closure parameter %0; please provide a type annotation. A type annotation means that the type-checker can just use the type without having to infer anything.

Copy link
Member

@hborla hborla Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think we settled on that. I was specifically suggesting changing the first clause and leaving what Slava added, i.e. cannot infer type of closure parameter %0 without a type annotation.

Copy link
Contributor

@xedin xedin Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I have with that phrasing is that the type annotation suppresses inference, so asking for annotation to help inference don’t make much sense to me personally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without a type annotation and please provide a type annotation both suggest adding a type annotation, but the former is more concise. I'm generally not a big fan of of using semi-colons in diagnostics, because it encourages making the second clause longer which makes it harder to actually get to the actionable bit of information (e.g. the line annotation is often truncated in IDEs so you have to click to expand it)

In any case, we can always file issues for further improvements because these changes are great first issues for folks. This is already a major improvement over the previous "context" phrasing.

Copy link
Contributor

@xedin xedin Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this phrasing makes it sound like the compiler would attempt to infer the type for the parameter although it would just take the type as is which might give wrong impression to the users. But I am not a native speaker, so I might be wrong here.

@slavapestov slavapestov force-pushed the reword-diagnostics-in-current-context branch from 24642e0 to 1957bd6 Compare June 9, 2023 23:43
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants