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

[SR-15176] Fixit for override func that should be override var #57499

Open
CodaFi opened this issue Sep 9, 2021 · 29 comments
Open

[SR-15176] Fixit for override func that should be override var #57499

CodaFi opened this issue Sep 9, 2021 · 29 comments
Assignees
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation fix-its Feature: diagnostic fix-its good first issue Good for newcomers improvement override Feature: Overriding in classes swift 5.9

Comments

@CodaFi
Copy link
Member

CodaFi commented Sep 9, 2021

Previous ID SR-15176
Radar rdar://28337871
Original Reporter @CodaFi
Type Improvement
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, DiagnosticsQoI, StarterBug
Assignee jiaren wang (JIRA)
Priority Medium

md5: b17661d61829d04b65f822d78b07388a

Issue Description:

Consider

class C {
  var canBecomeFirstResponder: Bool

  init() {}
}

class D: C {
    override func canBecomeFirstResponder() -> Bool {
    }
}

We should add a special case to diagnoseGeneralOverrideFailure that tries to see if it can correct no-args FuncDecls to VarDecls and vice-versa.

@swift-ci
Copy link
Collaborator

swift-ci commented Sep 9, 2021

Comment by jiaren wang (JIRA)

I would like give a shot on this. Could I get some hint?

@LucianoPAlmeida
Copy link
Collaborator

If I understand it correctly, the idea it here

diags.diagnose(decl, diag::override_multiple_decls_base,
somehow try to detect cases where you have a property trying to override a no-args function on base class or vice versa and try to provide a fix-it to provide a fix it for making one into another. For example, the fix-it offered will make the func in the example be

class D: C {
   - override func canBecomeFirstResponder() -> Bool {
   - }
   + override var canBecomeFirstResponder: Bool {
   + }
}

Since the override attempt have the same name and the same is a no-args function that produce the same result as the base class property (Bool).

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@danielfalbo
Copy link

Swift Mentorship program mentee here, trying to contribute for the first time. Will dive deeper and work on this 😁

@RAJAGOPALAN-GANGADHARAN
Copy link
Contributor

Hello @AnthonyLatsis - is this issue still open, can I give it a shot? Thanks!

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Oct 16, 2022

You’re welcome to! Make sure it still reproduces though. Want we to assign you?

@danielfalbo
Copy link

danielfalbo commented Oct 16, 2022

I got busier and did not complete it yet, but I did make progress on this and was planning on opening the PR soon.

https://github.com/danielfalbo/swift/tree/fixit-override-func-that-should-be-override-var

@danielfalbo
Copy link

Update: I don’t think I’ll keep working on this anytime soon, I’m unassigning myself

@danielfalbo danielfalbo removed their assignment Oct 28, 2022
@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 18, 2023

Let's try this!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 19, 2023

Hey @LucianoPAlmeida @AnthonyLatsis , I managed to find the error call site:
File --> /lib/Sema/TypeCheckDeclPrimary.cpp

(Case 1: When the method is overridden instead of the property)

FD->diagnose(diag::method_does_not_override, isClassContext)
                .highlight(OA->getLocation());

(Case 2: When the property is overridden instead of the method)

VD->diagnose(diag::property_does_not_override, isClassContext)
                .highlight(OA->getLocation());

So I think fixing the actual issue remains. I will try to do it!

@AnthonyLatsis AnthonyLatsis added override Feature: Overriding in classes fix-its Feature: diagnostic fix-its swift 5.9 labels Feb 19, 2023
@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 20, 2023

@AnthonyLatsis I have a little off-topic issue, I have come across this earlier too, what I did was to delete my workspace and again create it, but is there any other solution to this?

<unknown>:0: error: compiled module was created by an older version of the compiler; rebuild 'Swift' and try again:

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 20, 2023

PS: Rebuilding fixed the problem!

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Feb 20, 2023

is there any other solution to this?

This means the binary module format version associated with the just-built compiler is different than the one that was used to serialize the Standard Library (the Swift module). Most likely, you pulled in the latest changes and rebuilt just the compiler (swift-frontend) rather than everything, including the Standard Library. Re-creating the workspace is almost never the answer, because the workspace or the generated Xcode projects are not responsible for building anything — the Ninja build is.

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 20, 2023

Yes, also one more thing, I am suddenly encountering problems with syntax colouring and type hints not working properly in Xcode files while I am making changes, I tried clearing ~/Library/Developer/Xcode/DerivedData folder too, which could potentially re-index the files, but this isn't still working...

(Some of the text is white, while some of them green, and some are showing few function type hints, not all)

@AnthonyLatsis
Copy link
Collaborator

I have always seen these functional issues come up randomly when navigating the code base in Xcode. They usually get worse the older and more outdated the generated project becomes. Re-generating the projects after every checkout is probably the right answer, but I wouldn’t know for certain: I am too lazy to always keep the generated projects in sync, and tend to fall back to searching or spawning tabs whenever these issues show up.

@Rajveer100
Copy link
Contributor

Also, are there any docs related to these specific types of files, meaning, learning about the Types, Decl, etc, to grab the pointers for exactly what I am looking for, or do I have to manually learn and figure out?

@Rajveer100
Copy link
Contributor

Like in this issue, I will have to find a match for the function parameter and its declaration in the parent class, to correctly identify whether this override can be fixed...same goes for the property...

@LucianoPAlmeida
Copy link
Collaborator

Like in this issue, I will have to find a match for the function parameter and its declaration in the parent class, to correctly identify whether this override can be fixed...same goes for the property...

There is a little bit of info here

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 21, 2023

Hey thanks a lot @LucianoPAlmeida! Looks like Swift Forums is a good place for asking such doubts!

@LucianoPAlmeida
Copy link
Collaborator

There is a compiler category on the forums where you can ask more general questions about development in the compiler.
But if those are related to the issue is fine to keep the discussion here or on PRs =]

@Rajveer100
Copy link
Contributor

Cool!

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Feb 22, 2023

actually when I was looking for type hint

What do you mean by "type hint"? Code completion hints?

Also, are there any docs related to these specific types of files, meaning, learning about the Types, Decl, etc, to grab the pointers for exactly what I am looking for, or do I have to manually learn and figure out?

I do not know of any detailed write-ups on the AST design or the parts of semantic analysis that do not involve subtle and complex algorithms among our docs. This is where the inline documentation and comments come in handy. But we do have a list of external resources, many of which I found very helpful back in the day, with many more yet to read. Check out the article by Slava Pestov about how things are represented in the compiler.

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 22, 2023

Thanks! @AnthonyLatsis
I will go through the external resources!

Why was this issue closed and re opened?

@Rajveer100
Copy link
Contributor

I have added the notes in the diagnostic file, I hope the changes are relatively fine:
Commits

@AnthonyLatsis
Copy link
Collaborator

Have you tested them? Please open a pull request if you want your changes to be reviewed.

@Rajveer100
Copy link
Contributor

Coming soon!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 23, 2023

I have one question, regarding the super class inherit, should we consider the case for only one base class or can it be multiple classes, like, for example, class D could inherit from two classes B and C, where the function | property could be from either of them...Maybe an idea would be to limit the number of inherited types to be 2 or 3 considering optimisation?

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 23, 2023

I finally managed to do it! I just need to provide a fixit now!

Here is the output:

error: method does not override any method from its superclass
    override func canBecomeFirstResponder() -> Bool {
    ~~~~~~~~      ^
note: did you mean to override the property 'canBecomeFirstResponder'?
    override func canBecomeFirstResponder() -> Bool {
                  ^

@AnthonyLatsis
Copy link
Collaborator

I have one question, regarding the super class inherit, should we consider the case for only one base class or can it be multiple classes

Swift does not have multiple inheritance.

@Rajveer100
Copy link
Contributor

Oops! My bad! I misinterpreted it with C++ as it has the 'virtual' keyword to handle the diamond problem.

Rajveer100 added a commit to Rajveer100/swift that referenced this issue Mar 23, 2023
Rajveer100 added a commit to Rajveer100/swift that referenced this issue Mar 27, 2023
Rajveer100 added a commit to Rajveer100/swift that referenced this issue Apr 8, 2023
Rajveer100 added a commit to Rajveer100/swift that referenced this issue Apr 11, 2023
Rajveer100 added a commit to Rajveer100/swift that referenced this issue May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation fix-its Feature: diagnostic fix-its good first issue Good for newcomers improvement override Feature: Overriding in classes swift 5.9
Projects
None yet
Development

No branches or pull requests

7 participants