Opaque return type cannot be sending

Development Compiler
,
1

Elementary = GitHub - elementary-swift/elementary: A modern and efficient HTML rendering library - inspired by SwiftUI, built for the web.

import Elementary
protocol MyProtocol {
	associatedtype View: HTML
	@HTMLBuilder
	func render() -> sending View
}

struct MyStruct: MyProtocol {
	@HTMLBuilder
	func render() -> sending some HTML {
		HTMLRaw("testing")
	}
}

This results in the following error:

error: 'sending' may only be used on parameters and results
 8 | struct MyStruct: MyProtocol {
 9 |    @HTMLBuilder
10 |    func render() -> sending some HTML {
   |                   `- error: 'sending' may only be used on parameters and results
11 |            HTMLRaw("testing")
12 |    }

Is this an intended limitation or a bug?

3 Likes
2

my intuition is that this is an unintended functionality gap because it works if you return a concrete type, a generic, or an existential. i haven't yet thought of a reason why you would want to intentionally disallow the feature in the opaque result case and not those others, particularly given that generic and existential returns work fine. my guess is that the AST structure produced for an opaque return type is sufficiently different than other return types that it's just not being handled appropriately at the moment.

1 Like
3

In ResultTypeRequest::evaluate, we resolve the result type with TypeResolverContext::FunctionResult, except when its an opaque result type, which makes us take a different code path. We end up in OpaqueResultTypeRequest::evaluate, which uses TypeResolverContext::None instead:

  auto interfaceType =
      TypeResolution::forInterface(opaqueDecl, TypeResolverContext::None,
                                   /*unboundTyOpener*/ nullptr,
                                   /*placeholderHandler*/ nullptr,
                                   /*packElementOpener*/ nullptr)
          .resolveType(repr);

It looks like an oversight.

3 Likes
4

This one looked familiar to me!

1 Like
5

It’s a one-liner fix, if either of you wants to take it on. I didn’t do it because I was too lazy to come up with a test case to demonstrate that the attribute is not simply ignored.

6

you think the type resolver context should be set to FunctionResult unconditionally here?

edit: let's see what CI has to say while coming up with a new test case

1 Like
7

It would be worth investigating what happens with properties and subscripts too. What context is used when there’s no opaque return type?

8

with the unconditional change, Sema/diag_erroneous_iuo.swift failed on this:

func opaqueResult() -> (some Equatable)! { return 1 }
// expected-swift4-warning@-1:40 {{using '!' here is deprecated; this is an error in the Swift 5 language mode}}{{none}}
// expected-swift5-error@-2:40 {{'!' is not allowed here}}{{none}}
// expected-note@-3:40 {{use '?' instead}}{{40-41=?}}

as the warnings are no longer produced. i can't say i have a good sense of what that is trying to enforce, and why these variations do not seem to produce similar diagnostics:

func exIUO() -> (any Equatable)! { 1 }
func genIUO<T: Equatable>() -> T! { T?.none }

with the caveat that i'm not certain about the best way to infer this information, printing things out in various places made it seem like non-opaque returns from computed properties forward to their value's interface type (so presumably use whatever context value is appropriate there; not sure what that is exactly), and subscripts resolve the type repr of their element type and then evaluate that with the FunctionResult context.

i think we may still need some special casing somewhere because the unconditional change makes this valid:

protocol P {}
struct S: P {}
var computedSendingOpaque: sending (some P) { S() } // ✅

but this still is not:

var computedSendingConcrete: sending S { S() } // 🛑

also, the unconditional change appears to affect other types of declarations, like stored properties or local variables and admits sending in invalid positions, like:

let sendOpaque: sending (some Equatable) = 0 // ✅⁉️

though sending does feel like it should be valid on computed properties, that seems like a related, but separate issue to its simply not working at all in legal positions with opaque result types.

setting the context back to None in the case that the originating decl for the opaque result type request is a VarDecl seems to restore the existing diagnostic behavior in these cases, but idk how principled of a solution that is really. perhaps a safer approach would be to change the resolver context to 'function result' when the originating decl is a subscript or abstract function type, and then other cases will perhaps retain their existing (hopefully correct) behavior for now.

edit: actually just restricting to functions/subscripts isn't going to resolve that existing unit test failure with the IUO return value i don't think... so something will have to be done with that.

1 Like
9

I agree and I think it is safe to just update the expectation in this test case.

I did some digging, when resolving the type of a property with a non-opaque return type, we ultimately end up in PatternTypeRequest::evaluate, which uses TypeResolverContext::PatternBindingDecl. So you actually need to use TypeResolverContext::PatternBindingDecl in the VarDecl case, and TypeResolverContext::FunctionResult for functions and subscripts. That covers all possibilities; there are no other declaration kinds with opaque return types.

It's ultimately equivalent though, because it appears that nothing actually checks for TypeResolverContext::PatternBindingDecl, so in fact it behaves like TypeResolverContext::None. Perhaps it could be removed.

1 Like
10

i looked into the history of that specific test expectation, and it looks like its progenitor was originally added in this PR. seems like the intent may have just been to get such a construct to compile successfully, not necessarily that it should produce that particular diagnostic?

anyway, removing those expectations did allow the PR to enable this functionality to pass.