Adopt SwiftIfConfig library to help with #if clause handling
#75014
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DougGregor
requested review from
ktoso,
ahoppen,
bnbarham,
CodaFi,
hamishknight,
rintaro,
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci please smoke test |
DougGregor
force-pushed
the
swiftifconfig-build-configuration
branch
from
7813360 to
444a9d6
Compare
|
I forgot a |
|
@swift-ci please smoke test |
DougGregor
force-pushed
the
swiftifconfig-build-configuration
branch
from
444a9d6 to
37ebac2
Compare
|
Turns out that we had to model where the |
|
@swift-ci please smoke test |
hamishknight
approved these changes
Jul 8, 2024
Comment on lines
+22
to
+24
| var name = name | ||
| return name.withBridgedString { nameRef in |
There was a problem hiding this comment.
Could take this opportunity to sink the mutating-ness into withBridgedString, it doesn't seem like any clients are really benefitting from it
There was a problem hiding this comment.
I thought it was intentional (because String's withUTF8 is mutating), but if we don't care... yeah, we could do that.
|
@swift-ci please smoke test |
DougGregor
force-pushed
the
swiftifconfig-build-configuration
branch
from
5a3ff4a to
a48871d
Compare
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
This concrete implementation of the BuildConfiguration allows the use of the SwiftIfConfig library's APIs where the build configuration comes from the compiler itself.
The SwiftIfConfig library provides APIs for evaluating and extracting the active #if regions in source code. Use its "configured regions" API along with the ASTContext-backed build configuration to reimplement the extraction of active/inactive regions from the source. This approach has the benefit of being effectively stateless: where the existing solution relies on the C++ parser recording all of the `#if` clauses it sees as it is parsing (and then might have to sort them later), this version does a scan of source to collect the list without requiring any other state. The newer implementation is also conceptually cleaner, and can be shared with other clients that have their own take on the build configuration. The primary client of this information is the SourceKit request that identifies "inactive" regions within the source file, which IDEs can use to grey out inactive code within the current build configuration. There is also some profiling information that uses it. Those clients should be unaffected by this under-the-hood change. For the moment, I'm leaving the old code path in place for compiler builds that don't have swift-syntax. This should be considered temporary, and that code should be removed in favor of request'ifying this function and removing the incrementally-built state entirely.
…SwiftIfConfig The SIL coverage map generation depends on the locations of the `#endif` directives, but the mapping from SwiftIfConfig's configured regions wasn't producing them. The information is implicitly available in the SwiftIfConfig configured regions, so reconstitute it as we translate regions.
This implementation depends on ASTGen, which isn't linked as part of the AST library.
DougGregor
force-pushed
the
swiftifconfig-build-configuration
branch
from
7d52780 to
a844f4f
Compare
|
@swift-ci please smoke test |
…dConfiguration Unlike all of the other build configuration checks, `canImport` has side effects including the emission of various diagnostics. Thread a source location through here so the diagnostics end up on the right line.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
The failures from that debug build are also failing on |
hyp
added a commit
to swiftlang/swift-installer-scripts
that referenced
this pull request
Aug 6, 2024
…nstaller step swiftlang/swift#75014 added the new dependency, that the swift compiler depends on, and thus we need to make sure that this DLL is installed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Start adopting the new
SwiftIfConfiglibrary to reason about#ifclauses in a few places in the compiler. There are two aspects to this:CompilerBuildConfiguration, a wrapper around anASTContextthat conforms to the newBuildConfigurationprotocol from theSwiftIfConfiglibrary. This allows us to use the APIs inSwiftIfConfigbacked by the logic for compiler conditionals (hasFeature,os(XYZ),canImport, etc.) within the compiler itself.SourceFile::getIfConfigClauseRanges()with one based onSwiftIfConfig's enumeration of configured regions. This has the advantage of being less "stateful", because it doesn't need to rely on the C++ parser populating an array as it parses.