Introduce a special DeclBaseName for "deinit" #10965
Conversation
|
@swift-ci Please test |
lib/AST/Decl.cpp
Outdated
There was a problem hiding this comment.
The only reference to the ~> operator I could find was in Policy.swift which stated that it is a workaround for rdar://problem/14011860. Is that still up to date? GenericDispatch.swift seems to test it but it's used nowhere in the standard library.
There was a problem hiding this comment.
Aha, yes, that's gone now that we have protocol extensions. Still, removing it entirely should probably be a separate PR.
There was a problem hiding this comment.
OK, I'll have a look how much of it can be removed later on.
lib/SIL/SILPrinter.cpp
Outdated
There was a problem hiding this comment.
I'm not quite sure if we should escape deinit in SIL if used as a normal identifier. For subscripts we needed to, since otherwise the names of subscript getters/setters could have clashed with the getters/setters of instance variables named "subscript". AFAIK destructors in SIL always have the suffix !destructor so there's no name clash. Should we strive for consistency between the two keywords or simplicity on deinit? See the changes in vtables.swift for an example of how the dotted paths look like.
There was a problem hiding this comment.
Ah, I forgot @swiftix was out on vacation. He'll be back tomorrow.
There was a problem hiding this comment.
I agree with Jordan. Escaping all keywords seems like the right conservative answer.
ahoppen
force-pushed
the
deinit-special-name
branch
from
14f1d5f to
c13bbbe
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
lib/AST/ASTMangler.cpp
Outdated
There was a problem hiding this comment.
I'm 90% sure this is true but try defining a local type inside a destructor to make sure.
There was a problem hiding this comment.
OK, thanks for the reassurance.
include/swift/AST/Decl.h
Outdated
There was a problem hiding this comment.
Do we actually use this one? It's always going to be Id_init.
There was a problem hiding this comment.
I left it in for consistency. You can call getName() on all Decls that are guaranteed to be backed by Identifiers. But I don't think it's actually used. Long term goal would be to remove it once/if Id_init becomes a special name as well.
ahoppen
force-pushed
the
deinit-special-name
branch
from
c13bbbe to
5002fce
Compare
|
The way the PR description is phrased this sounds like some kind of troll. Just sayin' . |
ahoppen
changed the title
deinit
ahoppen
changed the title
deinit
|
@dabrahams That's certainly not what I wanted. Thanks for the feedback. I've updated it and hope it sounds more serious now. |
ahoppen
force-pushed
the
deinit-special-name
branch
2 times, most recently
from
418ff0d to
d18f196
Compare
This name is not used yet
ahoppen
force-pushed
the
deinit-special-name
branch
from
d18f196 to
8a839ed
Compare
|
@jrose-apple Could you have another look through this? I think all the questions have been answered and your comments taken care of. @swift-ci Please test |
|
Build failed |
|
Build failed |
No longer use the known identifier `deinit` for destructors. This allows classes to have functions called `deinit`.
ahoppen
force-pushed
the
deinit-special-name
branch
from
8a839ed to
ebd701c
Compare
|
Ah, I see, I thought by all keywords you meant escaping all keywords that clashed with special decl names. I'll create another PR for it then. @swift-ci Please test and merge |
|
Test once again before merging since old test is already several days old. @swift-ci Please smoke test and merge |
|
swift-ci didn't run. Let's try again @swift-ci Please smoke test and merge |
5 participants
Problem statement
Currently, destructors are represented by the known identifier "deinit", making it impossible to have a function or instance variable named "deinit" in a class. Declaring a function named "deinit" currently results in an error about an invalid redeclaration of
deinitand adding an instance variable named "deinit" crashes the compiler.Solution
Similar to how special
DeclNames were introduced for subscripts to avoid the name collision between functions named "subscript" and proper subscripts, introduce a specialDeclNamefor destructors and remove "deinit" from the list of known identifiers, making the identifier named "deinit" and the name representing the destructor completely distinct and thus resolving the above issue.