Skip to content

Address char signedness differences in TestNSNumber #265

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

Merged
merged 1 commit into from
Feb 25, 2016
Merged

Address char signedness differences in TestNSNumber #265

merged 1 commit into from
Feb 25, 2016

Conversation

hpux735
Copy link
Contributor

@hpux735 hpux735 commented Feb 21, 2016

No description provided.

@phausler
Copy link
Contributor

so this applies to all distros of arm linux? IIRC Android claims signed char, but does ubuntu on arm do the same?

@tienex
Copy link

tienex commented Feb 25, 2016

Yes, Linux/arm defaults to unsigned chars too.

@tienex
Copy link

tienex commented Feb 25, 2016

I mean, on Ubuntu, of course.

@phausler
Copy link
Contributor

it might be a bit dangerous but I wonder if we could use -funsigned-char?

@tienex
Copy link

tienex commented Feb 25, 2016

I'm unsure that is the best strategy, @modocache tried to fix c_layout.sil in swift tests because of the same issue, he could comment if this would work. IMHO Int8 should be really signed and CChar should map to either Int8 or UInt8 depending on the underlying configuration, at the moment on Linux/arm there's no way to use a signed 8-bits data type.

@modocache
Copy link
Contributor

@jckarter commented on swiftlang/swift#1103 that platform-specific checks would be preferred to using -fsigned-char.

@phausler
Copy link
Contributor

hmm more the reason these should not be signed or unsigned and CChar instead (but we cannot change the type since that would constitute an API change from Foundation's perspective)

phausler added a commit that referenced this pull request Feb 25, 2016
Address char signedness differences in TestNSNumber
@phausler phausler merged commit 5dc50a0 into swiftlang:master Feb 25, 2016
@jckarter
Copy link
Contributor

We could float the idea of making CChar a distinct type on swift-evolution. We do that for other target-dependent types like CGFloat.

@hpux735
Copy link
Contributor Author

hpux735 commented Feb 25, 2016

@jckarter I'm sure @tienex and I are interested in doing that!

@tienex
Copy link

tienex commented Feb 26, 2016

@jckarter What's interesting and a bit depressing is that at the SIL level, even CSignedChar is unsigned. I am trying to see why although, apparently, everything is correct and that should work in theory, even Clang generates signext for "signed char" correctly.

@tienex
Copy link

tienex commented Feb 26, 2016

Oh maybe I got it wrong, is SIL still using the typealias defined in CTypes?

@jckarter
Copy link
Contributor

SIL should canonicalize away all typealiases. If CSignedChar is aliased to an unsigned type, that's a bug. Do we do something dumb like typealias CSignedChar = CChar?

@tienex
Copy link

tienex commented Feb 26, 2016

No, that's fine, CSignedChar maps to Int8 as well, like CChar, I got confused because of the BuiltinMappedTypes.def :-)

@hpux735
Copy link
Contributor Author

hpux735 commented Feb 26, 2016

@jckarter @phausler @modocache Yesterday, I submitted an email to swift-evolution to open up this discussion. https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160222/011101.html

atrick pushed a commit to atrick/swift-corelibs-foundation that referenced this pull request Jan 12, 2021
… flags for sourcekit-lsp (swiftlang#265)

* Add serverArguments extension configuration to allow setting flags for sourcekit-lsp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants