Default values with UnsafeRawPointers do not compile about xcstrings-tool HOT 4 CLOSED

Hi @kkiermasz 👋 Good catch, thanks for raising this! I haven't actually used %s and %p format specifiers myself so that's why this was missed.

I have however been looking into some improvements around this part of how we generate the defaultValue (String.LocalizationValue type) because of a separate issue with parameter ordering and I think there might be some limitations with the new String.LocalizationValue type compared to using NSLocalizedString(..).

(I should mention that I only added parsing support for %s and %p as I was mirroring what other tools such as R.swift and SwiftGen supported)

The ### hashes are a quick workaround to escape double quotes that may exist inside the value that I construct. In the example from your screenshot ###"Test \###(arg1)"### is the same as "Test \(arg1)".

With that in mind, I think that we can work out the exact behaviour that we want by testing the String.LocalizationValue directly... I'm guessing that the following code doesn't work?

let arg: UnsafeRawPointer = ...
let value: String.LocalizationValue = "Test \(arg)"
dump(value)

I think that this is because the String.LocalizationValue type is much more restrictive compared to the NSLocalizedString(...) API. You can look at String.LocalizationValue.StringInterpolation to get an idea of which custom types it accepts for interpolation:

https://developer.apple.com/documentation/swift/string/localizationvalue/stringinterpolation

From what I can see, there are three overloads of interest to us that accept different values:

1. NSObject
2. String
3. _FormatSpecifiable

It seems that UnsafeRawPointer doesn't fit into any of these overloads, which is why I think that we are seeing this compiler error 😕

@_alwaysEmitIntoClient @_semantics("constant_evaluable") private func formatSpecifier<T>(_ type: T.Type) -> Swift.String {
    switch type {
    case is Int.Type:
        fallthrough
    case is Int64.Type:
        return "%lld"
    case is Int8.Type:
        fallthrough
    case is Int16.Type:
        fallthrough
    case is Int32.Type:
        return "%d"
    case is UInt.Type:
        fallthrough
    case is UInt64.Type:
        return "%llu"
    case is UInt8.Type:
        fallthrough
    case is UInt16.Type:
        fallthrough
    case is UInt32.Type:
        return "%u"
    case is Float.Type:
        return "%f"
    case is CGFloat.Type:
        fallthrough
    case is Double.Type:
        return "%lf"
    default:
        return "%@"
    }
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
public protocol _FormatSpecifiable : Swift.Equatable {
  associatedtype _Arg : Swift.CVarArg
  var _arg: Self._Arg { get }
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Int : Foundation._FormatSpecifiable {
  public var _arg: Swift.Int64 {
    get
  }
  public typealias _Arg = Swift.Int64
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Int8 : Foundation._FormatSpecifiable {
  public var _arg: Swift.Int32 {
    get
  }
  public typealias _Arg = Swift.Int32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Int16 : Foundation._FormatSpecifiable {
  public var _arg: Swift.Int32 {
    get
  }
  public typealias _Arg = Swift.Int32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Int32 : Foundation._FormatSpecifiable {
  public var _arg: Swift.Int32 {
    get
  }
  public typealias _Arg = Swift.Int32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Int64 : Foundation._FormatSpecifiable {
  public var _arg: Swift.Int64 {
    get
  }
  public typealias _Arg = Swift.Int64
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.UInt : Foundation._FormatSpecifiable {
  public var _arg: Swift.UInt64 {
    get
  }
  public typealias _Arg = Swift.UInt64
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.UInt8 : Foundation._FormatSpecifiable {
  public var _arg: Swift.UInt32 {
    get
  }
  public typealias _Arg = Swift.UInt32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.UInt16 : Foundation._FormatSpecifiable {
  public var _arg: Swift.UInt32 {
    get
  }
  public typealias _Arg = Swift.UInt32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.UInt32 : Foundation._FormatSpecifiable {
  public var _arg: Swift.UInt32 {
    get
  }
  public typealias _Arg = Swift.UInt32
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.UInt64 : Foundation._FormatSpecifiable {
  public var _arg: Swift.UInt64 {
    get
  }
  public typealias _Arg = Swift.UInt64
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Float : Foundation._FormatSpecifiable {
  public var _arg: Swift.Float {
    get
  }
  public typealias _Arg = Swift.Float
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension Swift.Double : Foundation._FormatSpecifiable {
  public var _arg: Swift.Double {
    get
  }
  public typealias _Arg = Swift.Double
}
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
extension CoreFoundation.CGFloat : Foundation._FormatSpecifiable {
  public var _arg: CoreFoundation.CGFloat {
    get
  }
  public typealias _Arg = CoreFoundation.CGFloat
}

So, with this in mind, I am not sure how to proceed 😄 I have a few ideas though:

1. Try adding _FormatSpecifiable conformance to UnsafeRawPointer to the target where the code is generated (XCStringsTool can't do this).
2. If you are working with NSObject types, maybe the codegen could accept the NSObject type rather than UnsafeRawPointer?
3. Remove (the broken) support for %s and %p as it isn't supported by String.LocalizationValue (I think this would also help with any attempt that I make to solve #40)
4. In your use case, use %@ in your string and instead format the pointer value prior to passing it into the generated code

   let _arg: UnsafeRawPointer = ...
   let arg = String(format: "%p", _arg)
   let localized = String(localized: .localizable.p(arg))

I hope that this brain-dump helps 😄 As I don't really use this feature myself, I am not sure what the best option is so it would be great to hear your thoughts! Thanks 🙏

from xcstrings-tool.

Hey @liamnichols 👋🏻 ,

wow, thanks for the response 😄

I'm not using them either. I was checking the library condition before adding it to the project 😛 The screenshot is from the test snapshot.

Thanks for the reference, now I see it's not support out-of-the-box :)

Since I'm not a String expert (which is a complex topic), I'm not voting for any of the 4 solutions you mentioned 🫠 I just thought it might be worth mentioning this in the documentation in some "limitations" section, since someone will probably encounter this problem later.

Btw it's a pity that there's no way (or I'm the reason in this case :D) to use one shared catalog across multiple feature modules, as Apple invented (View -> Catalog -> .strings), I'd give it a try.

from xcstrings-tool.

Btw did you notice any performance issues? One SwiftUI view (re)render causes ~2.4k LocalizedStringResource resolve attempts, each taking ~0.00016s. I wonder if that's because the tool (catalogs itself) was not designed to expose resources publicly :/
(you know, one Resources module imported by several feature modules).

When using the old .strings in the same shared module with localizedString(forKey: key, value: value, table: table) the resolve attempt time is pretty much the same, however, it's not called every time swiftui's view rerender.

It's unusable (LocalizedStringResource) with this performance tbh

I know it's not on your side, as I would assume SwiftUI is caching resources data or smth similar, but passing resources to the views was triggering the above issue. As I have some complex views it impacts performance too much. I made a fork and changed the file generation to something similar to SwiftGen. https://github.com/kkiermasz/xcstrings-tool/blob/main/Tests/XCStringsToolTests/__Snapshots__/GenerateTests/testGenerateWithPackageAccessLevel.Localizable.swift

Who's brain-dumping now? Haha

from xcstrings-tool.

I'm not using them either. I was checking the library condition before adding it to the project 😛

Ahh right I see, that's a good strategy 🙂

The screenshot is from the test snapshot.

Ooooh, good spot. I opened #56 to keep on top of that in the future.

Since I'm not a String expert (which is a complex topic), I'm not voting for any of the 4 solutions you mentioned 🫠 I just thought it might be worth mentioning this in the documentation in some "limitations" section, since someone will probably encounter this problem later.

Well now that I've had a think about it, I think that I am going to peruse option 3 and remove direct support for %s and %p to help with the @Sendable work.

I believe that option 4 would then be the viable workaround for anybody that did need to use this kind of formatting... I'll keep this issue open to track that 👍

Btw it's a pity that there's no way (or I'm the reason in this case :D) to use one shared catalog across multiple feature modules, as Apple invented (View -> Catalog -> .strings), I'd give it a try.

I'm not quite sure if I understood what you are trying to achieve here, but does using the public or package access level do what you need here?

Btw did you notice any performance issues?

As for this - not yet. I haven't personally used XCStrings Tool in any complex SwiftUI projects where it has become a concern.

I'm not quite sure of the solution to this though because my understanding is that the purpose of LocalizedStringResource is to defer the resolution of the localized string so that SwiftUI can handle it instead. I think this was primarily to support out-of-process rendering for things like App Intents, but also it lets you pass a custom locale in via the SwiftUI environment and then have the correct language resolved dynamically.

Maybe it isn't common usage though, and in that case, caching the resolved strings might be a better option, but I don't think it's a good default.

I think that it probably needs some more investigation though, so thanks for flagging it.. Perhaps you can open a separate issue to track this? There might be some ways that we can optimise the generated code more, or maybe at least add some best practice documentation 🙏

from xcstrings-tool.