-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Sema] Improve error for unqualified use of static variable in method #85706
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
base: main
Are you sure you want to change the base?
Conversation
xedin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! I left a comment inline regarding the code itself and I think the new wording might not actually make things clearer.
| auto SR = semantic->getSourceRange(); | ||
| if (SR.isValid()) { | ||
| auto CSR = Lexer::getCharSourceRangeFromSourceRange(SM,SR); | ||
| instanceName = SM.extractText(CSR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure whether the diagnostic is any better but regardless of that this is not the way to get the instance reference, it should be recorded as part of recording a fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly we had some previous attempts at re-phrasing this diagnostic too, you might want to look for the discussion older PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly we had some previous attempts at re-phrasing this diagnostic too, you might want to look for the discussion older PRs.
Hi @xedin - I was looking at #48759 which referenced SR-6207 and that was where I got the diagnostic suggestion from user @huonw (below) which is what I implemented.
static member 'cvar' can only be used on the type 'HasStatic', not the instance 'X'
I found another discussion in #12644 in which @jrose-apple was in agreement with Huon's recommendation.
I'm open to suggestions though, I was unable to think of anything better myself so I just went with what was in the original defect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... but regardless of that this is not the way to get the instance reference, it should be recorded as part of recording a fix.
Hi @xedin - If I am reading your suggestion correctly, I should create a fix and record it with the base during constraint solving and then have the emitter read it rather than pulling it from the source text?
Resolves
#48759
Summary
This PR improves the diagnostic emitted when a static member is referenced through an instance.
The previous diagnostic was misleading because it stated that the static member “cannot be used on an instance,” without clarifying the correct usage.
This change updates the diagnostic to explicitly state that static members must be referenced on the type.
Changes made
Original Example (from Issue)
Previous Output
Updated Output