Skip to content

Work around SwiftSyntax-related warning in CI. #183

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 3 commits into from
Jan 17, 2024

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Jan 16, 2024

We are getting a new warning in CI:

.../swift-testing/Sources/TestingMacros/ConditionMacro.swift:46:13: warning: let '_sourceLocationLabel' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor; this is an error in Swift 6
private let _sourceLocationLabel = TokenSyntax.identifier("sourceLocation")
            ^

Work around it by using private var ... {} instead of private let. I'm assuming that creating a TokenSyntax is not prohibitively expensive, so the change should be safe.

@ahoppen confirms that this operation is reasonably cheap and that syntax nodes are not safely sendable in the 5.9 tag of swift-syntax, so the warning is technically valid. Although our code does not access these constants across concurrency domains, the compiler is (in theory) free to invoke our macro expansion code on multiple tasks concurrently.

Concurrency safety in swift-syntax is tracked with swiftlang/swift-syntax#2425.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@grynspan
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@grynspan
Copy link
Contributor Author

@swift-ci please test

@grynspan grynspan self-assigned this Jan 16, 2024
@grynspan grynspan added darwin 🍎 macOS, iOS, watchOS, tvOS, and visionOS support workaround Workaround for an issue in another component (may need to revert later) labels Jan 16, 2024
We are getting a new warning in CI:

```
.../swift-testing/Sources/TestingMacros/ConditionMacro.swift:46:13: warning: let '_sourceLocationLabel' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor; this is an error in Swift 6
private let _sourceLocationLabel = TokenSyntax.identifier("sourceLocation")
            ^
```

Work around it by using `private var ... {}` instead of `private let`. I'm assuming that creating a `TokenSyntax` is not prohibitively expensive, so the change should be safe.

We're also getting new errors on macOS:

```
While building module 'std' imported from .../swift-testing/Sources/TestingInternals/WillThrow.cpp:13:
In file included from <module-includes>:34:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/complex.h:27:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/ccomplex:21:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/complex:243:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/sstream:191:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/istream:165:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/ostream:168:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__memory/shared_ptr.h:31:
In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__memory/unique_ptr.h:17:
/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__functional/hash.h:154:41: error: reference to unresolved using declaration
  154 |                            (static_cast<uint32_t>(__b) << 8);
      |                                         ^
/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/cstdint:169:1: note: using declaration annotated with 'using_if_exists' here
  169 | using ::uint32_t _LIBCPP_USING_IF_EXISTS;
      | ^
```

(And related.) This looks like a toolchain regression; we should be able to work around it by explicitly including C's stdint.h before the C++ headers that use the C types.
@grynspan grynspan force-pushed the jgrynspan/workaround-CI-failures branch from e7f050d to 3f9bbff Compare January 17, 2024 16:59
@grynspan grynspan changed the title Work around failures in CI. Work around SwiftSyntax-related warning in CI. Jan 17, 2024
@grynspan grynspan merged commit 33622d7 into main Jan 17, 2024
@grynspan grynspan deleted the jgrynspan/workaround-CI-failures branch January 17, 2024 17:32
@grynspan grynspan added concurrency 🔀 Swift concurrency/sendability issues and removed darwin 🍎 macOS, iOS, watchOS, tvOS, and visionOS support labels Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency 🔀 Swift concurrency/sendability issues workaround Workaround for an issue in another component (may need to revert later)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants