This is left over from the first implementation of protocol extensions, where they never got to satisfy requirements and thus were "final" in that sense. We should probably remove it at this point, which will require a small language proposal.

Comment by Brian (JIRA)

I looked into a fix for this but it's being a bit tricky. The desired error is generated in `lib/Sema/TypeCheckAttr.cpp` guarded behind `!D->getDeclContext()->getAsProtocolExtensionContext()`, so this seemed like a pretty easy fix. But, when I removed that condition and added some test cases, it would still not generate any error message. Given this test case:

protocol P {
final func bar()
}
extension P {
final func foo() {}
}

It turns out that `FD->getAttrs()` is empty in the extension, and populated in the initial protocol declaration when `TypeChecker::checkDeclAttributes` is invoked. I suspected that the parser was handling a special case, but `parseNewDeclAttribute` is adding the decl in both cases. I grepped in the dark for a few trying to figure out where the decl was being removed but didn't have any luck. It would be nice to be able to dump the representation of the SILModule so I could hone in on where the decl was being dropped, but `dump(false)` was failing on a symbol not existing. Any tricks here that might help?

I don't understand what you're trying to do. Either we parse but ignore the 'final' attribute (a problem in Parse), or we parse and then mark it invalid without diagnosing (a problem in Sema, under TypeChecker somewhere), or possibly both. This all happens well before SIL.

Comment by Brian (JIRA)

I'm trying to figure out what is causing the FinalDecl to not appear in the attributes inside `TypeChecker::checkDeclAttributes`. I was trying to dump the AST between parseIntoSourceFile and performTypeChecking to debug things, and was dumping the SILModule(which was obviously the wrong thing to dump) and the SF.Decl , both of which reported symbol not found errors.

But I think I figured out how to keep debugging, I realized that most decl's do dump fine, so I grab the address in the parse phase and am able to see the representation transform from

{{(func_decl "foo()" final
  (parameter_list
    (parameter "self"))
  (parameter_list)
  (brace_stmt))

to

(func_decl "foo()" interface type='<Self where Self : F> (Self) -> () -> ()' access=internal
  (parameter_list
    (parameter "self" type='Self' interface type='Self'))
  (parameter_list)
  (brace_stmt))
}}

I should be able to zero in on where final is being dropped from here.

Comment by Brian (JIRA)

Turns out it was getting removed in `AttributeEarlyChecker`. I have a branch (KingOfBrian@247b109) that now generates an error in Swift 3, a warning in Swift 4 and adds fixits for all of the 'final not supported' errors.

Should I open a PR or does the SE proposal need to get approved first? I'd gladly write one up but I'm not sure I understand the design criteria of the initial protocol implementation to explain the why sufficiently.

Comment by Brian (JIRA)

Hey @belkadan, I wrote a draft of a SE proposal for this bug: https://gist.github.com/KingOfBrian/6f20c566114ac0ef54c8092d80e54ee7. I believe it should be included in Phase 1 since it impacts source compatibility. I'm not sure if this is right so I thought I'd get a set of eyes on it before sending an email to the SE mailing list.

I'm aware this is probably pretty low priority, but thought I'd try to push it forward.

Thanks!

In the original protocol model of Swift, a developer could use the final keyword when declaring a function in a protocol extension to ensure the function could not be overridden.

It didn't actually affect behavior at all then either. We just originally required it to signify that there was no dynamic dispatch going on. Once we started allowing protocol extension methods to fulfill requirements, it became more confusing than useful.

I don't think we even shipped a release of Swift with required-final on protocol extension methods. I think this was Swift 2.0 WWDC beta vs. Swift 2.0 GM.

Comment by Brian (JIRA)

Interesting! I started using swift in the 2.2 days, so I was just trying to make up an origin story that made sense. I'll update the proposal if SE wants a proposal, but Slava was thinking it was more of a bug against the specification than a change in the specification. I pushed a PR up with a fix: #8010

I did see the PR. :-) I'd like a core team member's blessing that this wouldn't have to go through swift-evolution. @DougGregor, @jckarter?

I didn't know we still allowed this. We should probably put it through evolution since it would be a source break from 3.0 at this point IMO.

Comment by Brian (JIRA)

I sent the proposal here to the list yesterday and got some good feedback from Erica on the language. Glad to incorporate that and a more accurate historical representation. Should I close the PR for now?

I think it's fine to leave it in limbo. We actually have a label for things pending -evolution discussion, though I don't remember what it is offhand.

We should probably require `override` here, but IMO it's a swift-evolution topic. (And I'm glad to see the swift-evolution PR on this)

@swift-ci create

KingOfBrian (JIRA User), now that SE-0164 has been accepted, do you have that pull request? 🙂

Comment by Brian (JIRA)

Yea, working on tests now. The core functionality is there, it's just upgrading a warning to an error in Swift 4 mode.

Comment by Brian (JIRA)

Resolved by #8890