[stdlib] Explicitly mark Float80 unavailable, when it is #13305
Conversation
and cause identical `#if`s to not be nested
Dante-Broggi
changed the title
There was a problem hiding this comment.
Hi @Dante-Broggi, welcome to Swift and thanks for looking at this; a few minor questions to ask...
| @@ -1519,7 +1523,7 @@ extension ${Self} { | |||
| % srcBits = src_type.bits | |||
| % That = src_type.stdlib_name | |||
|
|
|||
| % if srcBits == 80: | |||
| % if (srcBits == 80) and (bits != 80): | |||
There was a problem hiding this comment.
I don't think that this piece of the change does anything; what is your goal here?
There was a problem hiding this comment.
I was just syntactically annoyed that the existing implementation has:
#if !os(windows) && (arch(i386) || arch(x86_64))
//code
#if !os(windows) && (arch(i386) || arch(x86_64))
//code
#endif
#endif
There was a problem hiding this comment.
Hmmm, ok. Generally best to skip this sort of change-that-doesn't-really-change-anything, but it is a modest improvement and you've already done it, so let's take it.
| @@ -1721,6 +1725,18 @@ extension ${Self} { | |||
| } | |||
|
|
|||
| % if bits == 80: | |||
| #else | |||
| ${SelfDocComment} | |||
There was a problem hiding this comment.
Please fix-up the indentation here. Also, @moiseev, do we want @_transparent, etc on a dummy object like this?
| @@ -1354,7 +1354,9 @@ extension ${Self} : _ExpressibleByBuiltinIntegerLiteral, ExpressibleByIntegerLit | |||
| } | |||
| } | |||
|
|
|||
| % if bits != 80: | |||
There was a problem hiding this comment.
What's the goal of adding the new check here?
There was a problem hiding this comment.
I was just syntactically annoyed that the existing gyb implementation outputs:
#if !os(windows) && (arch(i386) || arch(x86_64))
//code
#if !os(windows) && (arch(i386) || arch(x86_64))
//code
#endif
#endif
There was a problem hiding this comment.
It's difficult to see what's going on here in GitHub diff form, I'll need to look at this more carefully.
|
|
||
| ${SelfDocComment} | ||
| @_fixed_layout | ||
| @available(*, unavailable, message: "available only #if !os(Windows) && (arch(i386) || arch(x86_64))") |
There was a problem hiding this comment.
I think we can probably make the error message a little more approachable; something like "Float80 is only available on non-Windows x86 targets." What do you think?
There was a problem hiding this comment.
I really had no idea of how (arch(i386) || arch(x86_64)) would map to reader-friendly text, so I just put the code one would need to verify Float80 was actually available.
Putting your suggestion in.
| @@ -1519,7 +1523,7 @@ extension ${Self} { | |||
| % srcBits = src_type.bits | |||
| % That = src_type.stdlib_name | |||
|
|
|||
| % if srcBits == 80: | |||
| % if (srcBits == 80) and (bits != 80): | |||
There was a problem hiding this comment.
Hmmm, ok. Generally best to skip this sort of change-that-doesn't-really-change-anything, but it is a modest improvement and you've already done it, so let's take it.
| @@ -1354,7 +1354,9 @@ extension ${Self} : _ExpressibleByBuiltinIntegerLiteral, ExpressibleByIntegerLit | |||
| } | |||
| } | |||
|
|
|||
| % if bits != 80: | |||
There was a problem hiding this comment.
It's difficult to see what's going on here in GitHub diff form, I'll need to look at this more carefully.
|
@swift-ci Please smoke test and merge |
|
Thanks @Dante-Broggi! |
This PR causes a Float80 type to be explicitly marked unavailable, instead of being simply absent.
It also adds extra conditionals to not generate
#ifs within an#ifchecking the same condition.Resolves SR-6009.
I think I did everything, but this is my first contribution to swift.