Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
+1 to being able to rename @objc enums. I'm a little surprised we even allow exposing nested types to Objective-C, and am not at all sure the printer is set up to handle that correctly, but the renaming is independently useful.
Jordan
···
On Dec 9, 2015, at 15:18, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
+1 to this idea.
-Chris
···
On Dec 9, 2015, at 3:18 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
Either we just apply it to the One variant and ignore the Two variant, possibly with a warning, or we emit an error.
-Kevin
···
On Wed, Dec 9, 2015, at 06:01 PM, Jordan Rose wrote:
> On Dec 9, 2015, at 15:18, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
>
> Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
>
> Example:
>
> import Foundation
>
> class Foo: NSObject {
> @objc enum Bar: Int {
> case One, Two
> }
> }
>
> This generates the following:
>
> SWIFT_CLASS("_TtC7unnamed3Foo")
> @interface Foo : NSObject
> - (nonnull instancetype)init OBJC_DESIGNATED_INITIALIZER;
> @end
>
> typedef SWIFT_ENUM(NSInteger, Bar) {
> BarOne = 0,
> BarTwo = 1,
> };
>
> I'd like to resolve this by saying @objc(FooBar) but that emits an error.
>
> I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
+1 to being able to rename @objc enums. I'm a little surprised we even allow exposing nested types to Objective-C, and am not at all sure the printer is set up to handle that correctly, but the renaming is independently useful.
Should the generated Obj-C declarations use the swift_name attribute to indicate the Swift type it came from? Proposal SE-0005 generalizes swift_name to apply to any arbitrary C or Obj-C entity, so it will be legal to put on enums.
Labelling it makes it clear to the reader what the Swift equivalent is, but I'm unsure if there's any downsides to doing this.
-Kevin Ballard
···
On Thu, Dec 10, 2015, at 09:45 PM, Chris Lattner wrote:
> On Dec 9, 2015, at 3:18 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
>
> Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
+1 to this idea.
-Chris
>
> Example:
>
> import Foundation
>
> class Foo: NSObject {
> @objc enum Bar: Int {
> case One, Two
> }
> }
>
> This generates the following:
>
> SWIFT_CLASS("_TtC7unnamed3Foo")
> @interface Foo : NSObject
> - (nonnull instancetype)init OBJC_DESIGNATED_INITIALIZER;
> @end
>
> typedef SWIFT_ENUM(NSInteger, Bar) {
> BarOne = 0,
> BarTwo = 1,
> };
>
> I'd like to resolve this by saying @objc(FooBar) but that emits an error.
>
> I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
>
> -Kevin Ballard
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
Either we just apply it to the One variant and ignore the Two variant, possibly with a warning, or we emit an error.
-Kevin
On Wed, Dec 9, 2015, at 06:01 PM, Jordan Rose wrote:
On Dec 9, 2015, at 15:18, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
+1 to being able to rename @objc enums. I'm a little surprised we even allow exposing nested types to Objective-C, and am not at all sure the printer is set up to handle that correctly, but the renaming is independently useful.
I was actually surprised that this didn’t already work; @objc is supposed to be able to rename effectively anything at this point. Given that, and that another core team member has already +1’d, we can skip the evolution process and call this a bug fix. Want to contribute a fix?
I listed one open question:
Should the generated Obj-C declarations use the swift_name attribute to indicate the Swift type it came from? Proposal SE-0005 generalizes swift_name to apply to any arbitrary C or Obj-C entity, so it will be legal to put on enums.
Labelling it makes it clear to the reader what the Swift equivalent is, but I'm unsure if there's any downsides to doing this.
Yes, it makes sense to use the swift_name attribute here. If you’re implementing your proposal, check out lib/PrintAsObjC/PrintAsObjC.cpp and how we use the SWIFT_COMPILE_NAME macro for this in the generated header.
- Doug
···
On Dec 11, 2015, at 1:19 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
-Kevin Ballard
On Thu, Dec 10, 2015, at 09:45 PM, Chris Lattner wrote:
On Dec 9, 2015, at 3:18 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
+1 to this idea.
-Chris
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
Do we have any attributes that are valid on enum cases today? If so, how
do they handle this scenario?
I'm inclined to agree with you that it should be an error. It's
reasonable to assume that an attribute on the declaration applies to all
cases in the declaration, which is fine for most attributes, it just
doesn't work for @objc(name) because it produces a name collision.
I suspect the simplest approach here is to just figure out what the
error message will be if you say
Either we just apply it to the One variant and ignore the Two
variant, possibly with a warning, or we emit an error.
-Kevin
On Wed, Dec 9, 2015, at 06:01 PM, Jordan Rose wrote:
On Dec 9, 2015, at 15:18, Kevin Ballard via swift-evolution <swift- >>>> evolution@swift.org> wrote:
Swift allows for placing @objc on an enum that has an Int raw type
in order to expose it to Obj-C. But it doesn't currently let you
rename the enum when exposing it to Obj-C. This is particularly
problematic when exposing a Swift enum that's nested in a
struct/class, as the nesting resolves ambiguity in Swift but is not
present in Obj-C.
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int { case One,
Two } }
I'd like to resolve this by saying @objc(FooBar) but that emits an
error.
I'm also going to submit a separate proposal saying we should
change the default naming here so the enum is named FooBar, but
these two proposals go hand-in-hand (there are cases where you
might want to rename a root-level enum, to add a prefix for
disambiguation in Obj-C, or you may want to selectively opt out of
the proposed renaming rules by forcing your nested enum to use just
its name in Obj-C).
+1 to being able to rename @objc enums. I'm a little surprised we
even allow exposing nested types to Objective-C, and am not at all
sure the printer is set up to handle that correctly, but the
renaming is independently useful.
I was actually surprised that this didn’t already work; @objc is
supposed to be able to rename effectively anything at this point.
Given that, and that another core team member has already +1’d, we
can skip the evolution process and call this a bug fix. Want to
contribute a fix?
I'd love to! I'll start working on one over the weekend.
I listed one open question:
Should the generated Obj-C declarations use the swift_name attribute
to indicate the Swift type it came from? Proposal SE-0005 generalizes
swift_name to apply to any arbitrary C or Obj-C entity, so it will be
legal to put on enums.
Labelling it makes it clear to the reader what the Swift equivalent
is, but I'm unsure if there's any downsides to doing this.
Yes, it makes sense to use the swift_name attribute here. If you’re
implementing your proposal, check out
lib/PrintAsObjC/PrintAsObjC.cpp and how we use the
SWIFT_COMPILE_NAME macro for this in the generated header.
Will do. My worry with using swift_name is that, at least in the clang
that ships with Xcode 7.2, you can't actually put the swift_name
attribute on an enum. I assume that will change with SE-0005, but I also
assume this hasn't been implemented yet. And even when it is
implemented, emitting it will be a backwards-compatibility hazard if the
generated header needs to work with older clangs. Is there any way to
suppress the error?
I'm also not sure offhand where you actually put the attribute on an
enum, since the enum definition is actually 2 declarations, one for the
typedef and one for the enum itself. Would it go after the `enum`
keyword, in the place where SWIFT_ENUM_EXTRA is?
-Kevin Ballard
···
On Fri, Dec 11, 2015, at 02:39 PM, Douglas Gregor wrote:
On Dec 11, 2015, at 1:19 PM, Kevin Ballard via swift-evolution <swift- >> evolution@swift.org> wrote:
I don't think we have any enum case attributes, but we do allow @objc on properties:
import Foundation
class Test : NSObject { @objc(foo) var foo, bar : Int
}
…it looks like it ends up applying to both of them, and then erroring that you get two of the same (via the selector conflict diagnostics). We can probably make this more principled (both for cases and in general).
Jordan
···
On Dec 10, 2015, at 11:30, Kevin Ballard <kevin@sb.org> wrote:
Do we have any attributes that are valid on enum cases today? If so, how do they handle this scenario?
I'm inclined to agree with you that it should be an error. It's reasonable to assume that an attribute on the declaration applies to all cases in the declaration, which is fine for most attributes, it just doesn't work for @objc(name) because it produces a name collision.
I suspect the simplest approach here is to just figure out what the error message will be if you say
Either we just apply it to the One variant and ignore the Two variant, possibly with a warning, or we emit an error.
-Kevin
On Wed, Dec 9, 2015, at 06:01 PM, Jordan Rose wrote:
On Dec 9, 2015, at 15:18, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Swift allows for placing @objc on an enum that has an Int raw type in order to expose it to Obj-C. But it doesn't currently let you rename the enum when exposing it to Obj-C. This is particularly problematic when exposing a Swift enum that's nested in a struct/class, as the nesting resolves ambiguity in Swift but is not present in Obj-C.
Example:
import Foundation
class Foo: NSObject { @objc enum Bar: Int {
case One, Two
}
}
I'd like to resolve this by saying @objc(FooBar) but that emits an error.
I'm also going to submit a separate proposal saying we should change the default naming here so the enum is named FooBar, but these two proposals go hand-in-hand (there are cases where you might want to rename a root-level enum, to add a prefix for disambiguation in Obj-C, or you may want to selectively opt out of the proposed renaming rules by forcing your nested enum to use just its name in Obj-C).
+1 to being able to rename @objc enums. I'm a little surprised we even allow exposing nested types to Objective-C, and am not at all sure the printer is set up to handle that correctly, but the renaming is independently useful.
I was actually surprised that this didn’t already work; @objc is supposed to be able to rename effectively anything at this point. Given that, and that another core team member has already +1’d, we can skip the evolution process and call this a bug fix. Want to contribute a fix?
I'd love to! I'll start working on one over the weekend.
I listed one open question:
Should the generated Obj-C declarations use the swift_name attribute to indicate the Swift type it came from? Proposal SE-0005 generalizes swift_name to apply to any arbitrary C or Obj-C entity, so it will be legal to put on enums.
Labelling it makes it clear to the reader what the Swift equivalent is, but I'm unsure if there's any downsides to doing this.
Yes, it makes sense to use the swift_name attribute here. If you’re implementing your proposal, check out lib/PrintAsObjC/PrintAsObjC.cpp and how we use the SWIFT_COMPILE_NAME macro for this in the generated header.
Will do. My worry with using swift_name is that, at least in the clang that ships with Xcode 7.2, you can't actually put the swift_name attribute on an enum. I assume that will change with SE-0005, but I also assume this hasn't been implemented yet. And even when it is implemented, emitting it will be a backwards-compatibility hazard if the generated header needs to work with older clangs. Is there any way to suppress the error?
Hrm, that’s a good point. The generalized swift_name support is implemented in upstream Clang, but you’re right that Xcode 7.2 won’t be able to parse it. We can add a “__has_feature” entry for generalized swift_name in Clang and wrap this particular use of swift_name in
if __has_feature(generalized_swift_name)
// ... #endif
I'm also not sure offhand where you actually put the attribute on an enum, since the enum definition is actually 2 declarations, one for the typedef and one for the enum itself. Would it go after the `enum` keyword, in the place where SWIFT_ENUM_EXTRA is?
Yes, that’s correct.
- Doug
···
On Dec 11, 2015, at 9:40 PM, Kevin Ballard <kevin@sb.org> wrote:
On Fri, Dec 11, 2015, at 02:39 PM, Douglas Gregor wrote:
On Dec 11, 2015, at 1:19 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
That works for me. Can I assume this will get added by someone who's
familiar with clang, or should I try and figure out how to do this
myself and submit a patch?
-Kevin Ballard
···
On Fri, Dec 11, 2015, at 10:05 PM, Douglas Gregor wrote:
On Dec 11, 2015, at 9:40 PM, Kevin Ballard <kevin@sb.org> wrote:
Will do. My worry with using swift_name is that, at least in the
clang that ships with Xcode 7.2, you can't actually put the
swift_name attribute on an enum. I assume that will change with SE-
0005, but I also assume this hasn't been implemented yet. And even
when it is implemented, emitting it will be a backwards-compatibility
hazard if the generated header needs to work with older clangs. Is
there any way to suppress the error?
Hrm, that’s a good point. The generalized swift_name support is
implemented in upstream Clang, but you’re right that Xcode 7.2 won’t
be able to parse it. We can add a “__has_feature” entry for
generalized swift_name in Clang and wrap this particular use of
swift_name in
if __has_feature(generalized_swift_name) // ... #endif
When I add a module.modulemap and import the module into the swift
integrated REPL, :print_module shows the enum
enum Bar : Int { init?(rawValue: Int) var rawValue: Int { get {} }
case FooBarBaz case FooBarQux }
(I haven't touched case prefix stripping; presumably it's trying to
strip the Swift name instead of the ObjC name)
But if I actually try and reference the type Bar, it tells me it's an
undeclared type! I tried dumping the importer lookup table with swift-ide-
test, and while I don't actually understand the output format, from
looking at the IDE/dump_swift_lookup_tables.swift test, I get the
impression that the following is wrong (emphasis mine):
<<Bridging header lookup table>> Base name -> entry mappings: Bar:
TU: FooBar * FooBar: * * TU: FooBar, FooBar** * ** FooBarBaz:
FooBar: FooBarBaz FooBarQux: FooBar: FooBarQux
I suspect that using __attribute__((swift_name)) was never tested with
enums, because there's no pre-existing enum macro that supports it.
Should I just file this as a bug and submit my PR as-is? Or do you have
any guidance on what might be going on here?
-Kevin Ballard
···
On Fri, Dec 11, 2015, at 10:05 PM, Douglas Gregor wrote:
Hrm, that’s a good point. The generalized swift_name support is
implemented in upstream Clang, but you’re right that Xcode 7.2 won’t
be able to parse it. We can add a “__has_feature” entry for
generalized swift_name in Clang and wrap this particular use of
swift_name in
if __has_feature(generalized_swift_name) // ... #endif
I'm also not sure offhand where you actually put the attribute on an
enum, since the enum definition is actually 2 declarations, one for
the typedef and one for the enum itself. Would it go after the `enum`
keyword, in the place where SWIFT_ENUM_EXTRA is?
On Dec 11, 2015, at 10:33 PM, Kevin Ballard <kevin@sb.org> wrote:
On Fri, Dec 11, 2015, at 10:05 PM, Douglas Gregor wrote:
On Dec 11, 2015, at 9:40 PM, Kevin Ballard <kevin@sb.org <mailto:kevin@sb.org>> wrote:
Will do. My worry with using swift_name is that, at least in the clang that ships with Xcode 7.2, you can't actually put the swift_name attribute on an enum. I assume that will change with SE-0005, but I also assume this hasn't been implemented yet. And even when it is implemented, emitting it will be a backwards-compatibility hazard if the generated header needs to work with older clangs. Is there any way to suppress the error?
Hrm, that’s a good point. The generalized swift_name support is implemented in upstream Clang, but you’re right that Xcode 7.2 won’t be able to parse it. We can add a “__has_feature” entry for generalized swift_name in Clang and wrap this particular use of swift_name in
if __has_feature(generalized_swift_name)
// ... #endif
That works for me. Can I assume this will get added by someone who's familiar with clang, or should I try and figure out how to do this myself and submit a patch?
When I add a module.modulemap and import the module into the swift integrated REPL, :print_module shows the enum
enum Bar : Int {
init?(rawValue: Int)
var rawValue: Int {
get {}
}
case FooBarBaz
case FooBarQux
}
Nifty.
(I haven't touched case prefix stripping; presumably it's trying to strip the Swift name instead of the ObjC name)
Probably. It’s not actually clear which prefix it should try stripping with.
But if I actually try and reference the type Bar, it tells me it's an undeclared type!
Yeah, there is some known brokenness here: the swift_name attribute doesn’t actually work on global declarations, because the Clang importer currently makes assumptions about the mapping from Swift names to Objective-C names. That’s exactly why I’m introducing the Swift name lookup tables into the Clang importer now, because they make it possible for arbitrary renaming of Objective-C entities to actually work in Swift.
I tried dumping the importer lookup table with swift-ide-test, and while I don't actually understand the output format, from looking at the IDE/dump_swift_lookup_tables.swift test, I get the impression that the following is wrong (emphasis mine):
<<Bridging header lookup table>>
Base name -> entry mappings:
Bar:
TU: FooBar
FooBar:
TU: FooBar, FooBar
Your macro is swift_name’ing the enum but not the typedef, which is why there are entries for both Bar -> FooBar and FooBar -> FooBar. The duplication in the FooBar result is interesting, though: perhaps it’s related to the fact that the enum is first declared without the swift_name attribute?
I suspect that using __attribute__((swift_name)) was never tested with enums, because there's no pre-existing enum macro that supports it. Should I just file this as a bug and submit my PR as-is? Or do you have any guidance on what might be going on here?
It’s fine to submit a PR for this that tests what works (e.g., use FileCheck to verify that the Swift API dumps for the imported enum have the right names) and also have some tests that verify the bad behavior (“cannot find Bar”) with a FIXME to say that the test should succeed. That way, you can get your change in and move the needle forward a little, and when we start turning on the Swift name lookup tables, these tests will help us see that more things are working.
- Doug
···
On Dec 14, 2015, at 6:52 PM, Kevin Ballard <kevin@sb.org> wrote:
(I haven't touched case prefix stripping; presumably it's trying to
strip the Swift name instead of the ObjC name)
Probably. It’s not actually clear which prefix it should try
stripping with.
It seems to me that it should strip the ObjC name, as the enum constants
will presumably be named after the ObjC enum name. I already went ahead
and implemented this earlier today and it works (I added an option to
ClangImporter::Implementation::importFullName called
ImportNameFlags::IgnoreSwiftNameAttr).
But if I actually try and reference the type Bar, it tells me it's an
undeclared type!
Yeah, there is some known brokenness here: the swift_name attribute
doesn’t actually work on global declarations, because the Clang
importer currently makes assumptions about the mapping from Swift
names to Objective-C names. That’s exactly why I’m introducing the
Swift name lookup tables into the Clang importer now, because they
make it possible for arbitrary renaming of Objective-C entities to
actually work in Swift.
Ah ok. I assumed that because there was code in here to look for the
attribute that it was expected to work.
Your macro is swift_name’ing the enum but not the typedef, which is
why there are entries for both Bar -> FooBar and FooBar -> FooBar. The
duplication in the FooBar result is interesting, though: perhaps it’s
related to the fact that the enum is first declared without the
swift_name attribute?
Should I be swift_name'ing the typedef too?
As for the duplication, that actually matches the output from the
IDE/dump_swift_lookup_tables.swift file. Given the following Obj-C
declaration:
The thing that was weird in my case was that it was using the name
FooBar instead of Bar.
It’s fine to submit a PR for this that tests what works (e.g., use
FileCheck to verify that the Swift API dumps for the imported enum
have the right names) and also have some tests that verify the bad
behavior (“cannot find Bar”) with a FIXME to say that the test should
succeed. That way, you can get your change in and move the needle
forward a little, and when we start turning on the Swift name lookup
tables, these tests will help us see that more things are working.
Ok, I'll figure out what tests I can write now for this and I'll submit
the PR soon (probably tomorrow evening).
-Kevin Ballard
···
On Mon, Dec 14, 2015, at 09:34 PM, Douglas Gregor wrote:
On Dec 14, 2015, at 6:52 PM, Kevin Ballard <kevin@sb.org> wrote:
(I haven't touched case prefix stripping; presumably it's trying to strip the Swift name instead of the ObjC name)
Probably. It’s not actually clear which prefix it should try stripping with.
It seems to me that it should strip the ObjC name, as the enum constants will presumably be named after the ObjC enum name. I already went ahead and implemented this earlier today and it works (I added an option to ClangImporter::Implementation::importFullName called ImportNameFlags::IgnoreSwiftNameAttr).
Note that you could grab the name directly from the Clang declaration with a small amount of effort, so it doesn't get translated at all.
But if I actually try and reference the type Bar, it tells me it's an undeclared type!
Yeah, there is some known brokenness here: the swift_name attribute doesn’t actually work on global declarations, because the Clang importer currently makes assumptions about the mapping from Swift names to Objective-C names. That’s exactly why I’m introducing the Swift name lookup tables into the Clang importer now, because they make it possible for arbitrary renaming of Objective-C entities to actually work in Swift.
Ah ok. I assumed that because there was code in here to look for the attribute that it was expected to work.
I'm in the middle of implementing generalized swift_name support. importFullName is about a week old :)
Your macro is swift_name’ing the enum but not the typedef, which is why there are entries for both Bar -> FooBar and FooBar -> FooBar. The duplication in the FooBar result is interesting, though: perhaps it’s related to the fact that the enum is first declared without the swift_name attribute?
Should I be swift_name'ing the typedef too?
IMO, yes. We want both the enum and typedef to have the new name.
As for the duplication, that actually matches the output from the IDE/dump_swift_lookup_tables.swift file. Given the following Obj-C declaration:
The thing that was weird in my case was that it was using the name FooBar instead of Bar.
I expected one FooBar for the typedef. It's the other that surprised me.
It’s fine to submit a PR for this that tests what works (e.g., use FileCheck to verify that the Swift API dumps for the imported enum have the right names) and also have some tests that verify the bad behavior (“cannot find Bar”) with a FIXME to say that the test should succeed. That way, you can get your change in and move the needle forward a little, and when we start turning on the Swift name lookup tables, these tests will help us see that more things are working.
Ok, I'll figure out what tests I can write now for this and I'll submit the PR soon (probably tomorrow evening).
Okay!
- Doug
···
Sent from my iPhone
On Dec 14, 2015, at 10:59 PM, Kevin Ballard <kevin@sb.org> wrote:
On Mon, Dec 14, 2015, at 09:34 PM, Douglas Gregor wrote:
On Dec 14, 2015, at 6:52 PM, Kevin Ballard <kevin@sb.org> wrote: