Try #expect(throwing()) or #expect(try throwing())?

Development Swift Testing
1

try #expect(throwing()) or #expect(try throwing())?

It seems like these are often interchangeable… but I am also seeing cases where only try #expect(throwing()) compiles without errors (and #expect(try throwing()) does error). Is there a best practice between these two?

A related question is async expectations:

await #expect(awaiting()) or #expect(await awaiting())?

Is it generally a better idea (less probability for a compiler error) to specify the await outside of the expect?

2 Likes
2

The short answer is to prefer this form as it will compile:

#expect(try f())

The longer, more technically arcane answer follows:

The reason try #expect(f()) fails to compile is because macros like #expect cannot directly "see" their enclosing lexical scope. That is, if you break down the expression try #expect(f()) into a syntax tree, there's a try expression containing a #expect-invoking macro expression, but the macro expression can't see the try expression that contains it.

That means that the macro doesn't know there's a try keyword there. When it performs the macro expansion of f(), it doesn't know it needs to maintain throwing semantics for the expanded form, so it miscompiles. This is not something Swift Testing can solve alone and is a general constraint of macros in Swift.

Note: #expect(try f()) does compile, but because Swift Testing doesn't know what part of the expression try f() might throw (pretend for a moment that it's more complex than a zero-argument free function call), Swift Testing does not try to perform any expansion on it at all.

4 Likes
4

We care greatly about ergonomics, I assure you. As I just explained, Swift Testing can't see the try keyword in that position. This is a general macro limitation.

13 Likes
5

@grynspan Hmm… I'm seeing some confusing reporting when I have a "nested" test inside a confirmation test:

func f() throws -> Bool {
  true
}

@Test func test() async throws {
  await confirmation { onConfirm in
    #expect(try f())
  }
}

That fails to compile with this error:

 8 | @Test func test() async throws {
 9 |   await confirmation { onConfirm in
10 |     #expect(try f())
   +--- macro expansion #expect ----------------------------------------
   |1 | Testing.__checkValue(try f(), expression: .__fromSyntaxNode("try f()"), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
   |  |                      `- error: errors thrown from here are not handled
   +--------------------------------------------------------------------
11 |   }
12 | }

I found one similar issue (#expect does not catch error · Issue #504 · apple/swift-testing · GitHub). Does this sound like the same idea affecting both?

6

Is it due to some language feature that is missing?

7

Yes, I think so. Macros are generally unaware of types or effects they can only reason about the syntax tree.

In this PR Jonathan explained it:

func foo() async -> Int { ... }
#expect(await quux(1, 2, 3, foo() + bar()) > 10)

Because swift-testing can only see the syntax tree (that is, the characters the developer typed into a Swift source file) and not the types or effects of expressions, when presented with the #expect() expression above, it has no way to know that the only part of the expression that needs to be awaited is foo() .

2 Likes
8

Thank you! The PR is very interesting indeed :+1:

1 Like
9

Doug responded to these missing features in the following threads

1 Like
10

You just need to put try on confirmation {} too.

1 Like
11

I'm not so sure we need knowledge of type information in the macro implementation to solve this particular problem.

@grynspan said something along these lines already above, but to underscore it: We only need the expression macro implementation to be able to see that it has try or await keywords before it. That's still just syntax information, not full type information. The macro system already exposes some of the parent syntax nodes of an expression macro, such as any surrounding type declarations, via the MacroExpansionContext.lexicalContext property. The problem is that lexicalContext does not include preceding try or await keyword syntax nodes — it doesn't consider those relevant context to include.

I think the only thing we need is for lexicalContext to begin including those. Then, the macro implementation in swift-testing would be able to see whether those keywords precede #expect and could forward them to the inner expression in its macro expansion. I'm not sure whether it was a principled decision to omit them, or whether it just hasn't come up before.

I just opened a PR on swift-syntax with this suggested improvement. If there's no objection to that, hopefully we can land it and then adopt it in #expect.

2 Likes
12

Is the first one intended to compile? I have working only second option, or in more advanced cases compiler requires me to put await inside and outside, like with specific error handling:

await #expect(throws: SpecificError.self) { try await someCall() }

Which IIRC is intended Swift behaviour on await in general.

On the general topic, despite language allowance on try, I'd prefer in any way to have it inside #expect as it is part of the expectation, which in that way has clearer communication. It is also pairs nicely with #require have outside try as it has different behaviour implication.

1 Like
13

Hmm… that's what I was thinking… but then I I saw a new warning and the same error:

 8 | @Test func test() async throws {
 9 |   try await confirmation { onConfirm in
10 |     #expect(try f())
   +--- macro expansion #expect ----------------------------------------
   |1 | Testing.__checkValue(try f(), expression: .__fromSyntaxNode("try f()"), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
   |  |                      `- error: errors thrown from here are not handled
   +--------------------------------------------------------------------
11 |   }
12 | }

/Users/rick/Desktop/TestingDemo/Tests/TestingDemoTests/TestingDemoTests.swift:9:3: warning: no calls to throwing functions occur within 'try' expression
 7 | 
 8 | @Test func test() async throws {
 9 |   try await confirmation { onConfirm in
   |   `- warning: no calls to throwing functions occur within 'try' expression
10 |     #expect(try f())
11 |   }

Does this look like a legit error? Or should we track this in GitHub for a potential fix in the future?

14

That looks like it may be a parsing error. @Douglas_Gregor can you advise?

2 Likes
15

Here is an example where both versions seem to work (for now):

func f() async -> Bool {
  true
}

@Test func test() async {
  #expect(await f() == true)
  await #expect(f() == true)
}
1 Like
16

There are some expressions that can compile in this context and others that can't no matter what we do. In this case, because f() is the left-hand operand of a binary operator, it is eagerly evaluated (does not need to be placed in a closure) which means that its await keyword can be hoisted.

The inconsistency is the real problem here: it's "spooky action" that this sometimes works and sometimes doesn't.

4 Likes
17

Respectfully, I don't think it's sufficient. In the case of a binary operation where only one side throws an error, it becomes impossible to statically determine which side requires the effect.

@Test func example() throws {
    func foo() -> Int { 123 }
    func bar() throws -> Int { 456 }
    try #expect(foo() != bar())
}

This expands to:

Testing.__checkBinaryOperation(foo(), {
        $0 != $1()
    }, bar(), expression: .__fromBinaryOperation(.__fromSyntaxNode("foo()"), "!=", .__fromSyntaxNode("bar()")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()

In this expansion, the 3rd (rhs) parameter is an autoclosure. Without dynamic type or effect information, it remains impossible to determine which side of the operator requires certain effects.

The lexical context only indicates that somewhere in the expression there is a function call that requires throwing. If bar() throws, this currently fails to compile. (Moving the throws effect over to foo() works fine because __checkBinaryOperation's lhs is a regular parameter). Your pull request that exposes effects – while helpful – won't help the #expect macro's expansion compile in this case, because it's still unaware whether lhs or rhs is throwing.

1 Like
18

We are aware that the syntax tree is insufficient to determine what subexpression has side effects, and Swift Testing has compile-time logic in place to suppress expression expansion if side effects are detected. This logic, of course, doesn't work when try or await occurs before #expect(), since the keywords aren't visible during macro expansion.

1 Like