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

Improve error message on attempt to use instance member in static context #63301

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

valeriyvan
Copy link
Contributor

Resolves #62909

test/Constraints/dynamic_lookup.swift Outdated Show resolved Hide resolved
@@ -1335,7 +1335,7 @@ ERROR(member_shadows_global_function,none,
(DeclNameRef, DescriptiveDeclKind, DescriptiveDeclKind, DeclName, DeclName))

ERROR(instance_member_use_on_type,none,
"instance member %1 cannot be used on type %0; "
"instance member %1 of type %0 cannot be used in static context; "
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that "in static context" brings any clarify because everybody has a different idea what "static context" means, but I'm biased. @hborla WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m probably biased too, but I read this as "you cannot use this at all in a static function". Regrettably, the best alternative I can currently think of is instance member cannot be used on type object.

Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis Jan 30, 2023

Choose a reason for hiding this comment

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

However, maybe it does make sense to say "static context" specifically when self. is implicit, keep the current wording for T., and say "metatype" for meta..

Copy link
Contributor

Choose a reason for hiding this comment

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

"you cannot use this at all in a static function".

This is exactly what I'm afraid of happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guys, how should we proceed with this fix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The primary goal is to tailor the message for implicit accesses on self specifically. The current message reads well when applied to an explicit access on a type identifier as in T.member(). As for how exactly we want to tailor it, we need to come up with a better idea than what was voiced in the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a good start would be to limit this diagnostic to situations when self. is implicit (that's what "in static context" means for a lot of people IMHO).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Putting in my two cents: instance member %1 cannot be used on static 'self'

@valeriyvan valeriyvan force-pushed the InstanceMemberCannotBeUsedOnType branch from 9f895aa to 41b8173 Compare January 31, 2023 15:33
@tysun
Copy link

tysun commented Apr 30, 2023

@AnthonyLatsis @xedin

That alternative error message, instance member %1 cannot be used on static 'self', is a bit confusing. It indicates that there's a static 'self' you're trying to use. Swift doesn't have a concept of a static 'self' as self refers to an instance of the type, while static properties and methods belong to the type itself, not an instance.

Perhaps a more accurate alternative error message could be: Instance member %1 cannot be accessed directly from a static method of the same type. I think this error message makes it clear that you are trying to access an instance member from a static method, which is not allowed in Swift.

@AnthonyLatsis
Copy link
Collaborator

Swift doesn't have a concept of a static 'self' as self refers to an instance of the type

Not really. In a static context, self refers to Self.self (this is usually just the parent type object or metatype), not an instance. We can definitely do better than the alternative I suggested though.

Perhaps a more accurate alternative error message could be: Instance member %1 cannot be accessed directly from a static method of the same type

I think this message has the same problem as the synonymous cannot be used in static context discussed earlier. It can easily be interpreted as "you cannot access this inside a static method".

@xavgru12
Copy link

Swift doesn't have a concept of a static 'self' as self refers to an instance of the type

Not really. In a static context, self refers to Self.self (this is usually just the parent type object or metatype), not an instance. We can definitely do better than the alternative I suggested though.

Perhaps a more accurate alternative error message could be: Instance member %1 cannot be accessed directly from a static method of the same type

I think this message has the same problem as the synonymous cannot be used in static context discussed earlier. It can easily be interpreted as "you cannot access this inside a static method".

What @tysun said really nails it down for me. What do others think?

Instance member %1 cannot be accessed directly from a static method of the same type

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.

"Instance member cannot be used on type" diagnostic message could be more specific
5 participants