Skip to content
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.

Sign up for GitHub

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

Build without swift host tools #77815

Closed
wants to merge 11 commits into from
Closed

Build without swift host tools #77815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
307c62c
Only add SwiftMacros dependencies when we are building with swift-syn…
DougGregor Nov 22, 2024
524a446
Make sure EvaluateIfConditionRequest isn't using Swift code in C++-on…
DougGregor Nov 22, 2024
0367e8c
Parse `#isolation` as CurrentContextIsolationExpr when we don't have …
DougGregor Nov 22, 2024
a4b2ca8
Block the use of @DebugDescription behind hasFeature(Macros)
DougGregor Nov 22, 2024
0b36e13
[build-script] Add buildbot_linux,without_host_swift preset
DougGregor Nov 22, 2024
2d931cc
Disable bootstrapping when we aren't building SwiftSyntax
DougGregor Nov 23, 2024
aa8fdb6
Disable @_noLocks annotations when we don't have a host Swift compiler
DougGregor Nov 23, 2024
d332b27
[C++-only build] Disable fast paths that require SIL optimizer pieces…
DougGregor Nov 23, 2024
1afce80
Skip testing when building without host Swift
DougGregor Dec 1, 2024
70b1c3b
Disable testing with no_test
DougGregor Dec 1, 2024
04ce3fe
Fix up compiler and standard library build when there's no host Swift
DougGregor Feb 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,13 @@ if (NOT BOOTSTRAPPING_MODE)
message(FATAL_ERROR "turning off bootstrapping is not supported anymore")
endif()

Copy link
Contributor

@eeckstein eeckstein Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against removing this check.

We should only allow turning off BOOTSTRAPPING in the very specific case of cross compilation.

IMO, the risk is too high that someone accidentally turns it off and it gets unnoticed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving it as a default to hosttools is fine. The compiler itself still needs to produce valid output with swift sources disabled, even if it is less optimized. Given that the off-mode also disables macro support, it seems likely that it will be noticed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the off-mode also disables macro support

Does it? The BOOTSTRAPPING Cmake flag only impacts SwiftCompilerSources, AFAIK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about letting the compiler print a warning if SwiftCompilerSources are disabled?

Copy link
Contributor

@eeckstein eeckstein Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler itself still needs to produce valid output with swift sources disabled

Sure, but there shouldn't be another motivation to disable SwiftCompilerSources than for cross-compiling where no host-tools are available. And for that case we already do this in the cmake file:

set(SWIFT_EXEC_FOR_SWIFT_MODULES "${CMAKE_Swift_COMPILER}")
  if(NOT SWIFT_EXEC_FOR_SWIFT_MODULES)
    message(WARNING "BOOTSTRAPPING set to OFF because no Swift compiler is defined")
    set(BOOTSTRAPPING_MODE "OFF")
  endif()

Therefore I don't understand why we have to remove this check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, there are different flags controlling disabling swift-syntax in the compiler (which drops macros support, among other things) vs. disabling SwiftCompilerSources in the compiler (which drops embedded support and breaks no-locks/no-allocation checking, among other optimizer effects).

I'm a bit dubious on the notion that we have to work hard to prevent people from using bootstrapping=off and being surprised. If it really is that important to prevent misconfigurations, we could create a new CMake + build-script option for this "minimal C++-only build" that disables both of those at once. Checking SWIFT_EXEC_FOR_SWIFT_MODULES isn't that great, because it doesn't let you build this configuration if there's a Swift in your path.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would be good to have a single flag which turns off everything (macros, swiftcompilersource, etc.). This would make accidental misconfiguration very unlikely. Plus a compiler warning as a bonus.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've added build-script --minimal-cxx-bootstrap to do this, which maps down to the CMake option SWIFT_MINIMAL_CXX_BOOTSTRAP and centralizes most of the logic for turning off stuff in the build.

endif()

# Disable bootstrapping when we aren't building SwiftSyntax
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not very helpful - it just describes what one can read from the next two source lines, anyway.

Can you add more context here? I assume that SWIFT_BUILD_SWIFT_SYNTAX is the main "switch" which tells if host tools are available.
If so, why not call it SWIFT_HOST_TOOLS_AVAILABLE or something like that. It feels like SWIFT_BUILD_SWIFT_SYNTAX is abused for that purpose.

if(NOT SWIFT_BUILD_SWIFT_SYNTAX)
Copy link
Contributor

@eeckstein eeckstein Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do that in a safer way? I'm afraid that this can lead to accidentally disabling SwiftCompilerSources in a shipped release-compiler (e.g. by someone removing swift-syntax from a build-preset). This could get mostly unnoticed because it only affects optimizations, i.e. the qualify of generated code.

E.g. by excluding platforms which we expect to have hosttools (macos, linux, windows).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd expect it to go in the other direction. Fully bring back the BOOTSTRAPPING_MODE "OFF" as an option, then if bootstrapping=off we also disable the other parts that depend on Swift. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A released compiler without swift-syntax wouldn't support macros, so I don't think it can happen accidentally. That said, my if(NOT SWIFT_BUILD_SWIFT_SYNTAX) hack is awful. I'm all for bringing back BOOTSTRAPPING_MODE "OFF" to make this explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! It turns out that we don't specifically need either my hack or BOOTSTRAPPING_MODE "OFF", since we still have the code that disables SwiftCompilerSources when there is no Swift toolchain, here: https://github.com/swiftlang/swift/blob/main/CMakeLists.txt#L966

I'm going to remove this commit entirely for now, and we can (independently) decide whether to bring back an easier way to turn off bootstrapping and ignore any host Swift toolchain. For now, we'll test this in an environment where there is no swift toolchain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I'm going to refute my own comment 2 hours later. I went ahead and reverted the change that dropped bootstrapping=off support so we can more easily turn off bootstrapping locally for testing. I'm still trying to bring this up in a container with no host Swift just to be sure (and that's probably how we would want to configure CI).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd expect it to go in the other direction. Fully bring back the BOOTSTRAPPING_MODE "OFF" as an option

No. I intentionally made it to not be turned off because people disabled it in their local development and wondered why some things are not working in the compiler.
SwiftCompilerSources are mandatory, except for the purpose of bootstrapping the compiler on new platforms.

set(BOOTSTRAPPING_MODE "OFF")
endif()

set(SWIFT_RUNTIME_OUTPUT_INTDIR "${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin")
set(SWIFT_LIBRARY_OUTPUT_INTDIR "${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib")
if("${SWIFT_NATIVE_SWIFT_TOOLS_PATH}" STREQUAL "")
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/ASTScope.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ void ASTScope::unqualifiedLookup(
if (auto *s = SF->getASTContext().Stats)
++s->getFrontendCounters().NumASTScopeLookups;

#if SWIFT_BUILD_SWIFT_SYNTAX
// Perform validation of SwiftLexicalLookup if option
// Feature::UnqualifiedLookupValidation is enabled and lookup was not
// performed in a macro.
Expand All @@ -171,6 +172,9 @@ void ASTScope::unqualifiedLookup(
} else {
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
}
#else
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
#endif
}

llvm::SmallVector<LabeledStmt *, 4> ASTScope::lookupLabeledStmts(
Expand Down
14 changes: 14 additions & 0 deletions lib/Parse/ParseExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3471,6 +3471,20 @@ ParserResult<Expr> Parser::parseExprMacroExpansion(bool isExprBasic) {
if (!macroNameRef)
return status;

#if !SWIFT_BUILD_SWIFT_SYNTAX
// If we don't have swift-syntax and therefore have no support for macros,
// recognize #isolation as special and route it through
// CurrentContextIsolationExpr.
if (macroNameRef.getBaseName().userFacingName() == "isolation" &&
genericArgs.empty() &&
(!argList || argList->empty())) {
return makeParserResult(
status,
new (Context) CurrentContextIsolationExpr(
macroNameLoc.getStartLoc(), Type()));
}
#endif

return makeParserResult(
status,
MacroExpansionExpr::create(
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,7 @@ std::pair<bool, bool> EvaluateIfConditionRequest::evaluate(
Evaluator &evaluator, SourceFile *sourceFile, SourceRange conditionRange,
bool shouldEvaluate
) const {
#if SWIFT_BUILD_SWIFT_SYNTAX
// FIXME: When we migrate to SwiftParser, use the parsed syntax tree.
ASTContext &ctx = sourceFile->getASTContext();
auto &sourceMgr = ctx.SourceMgr;
Expand All @@ -775,4 +776,7 @@ std::pair<bool, bool> EvaluateIfConditionRequest::evaluate(
bool isActive = (evalResult & 0x01) != 0;
bool allowSyntaxErrors = (evalResult & 0x02) != 0;
return std::pair(isActive, allowSyntaxErrors);
#else
llvm_unreachable("Must not be used in C++-only build");
#endif
}
7 changes: 5 additions & 2 deletions stdlib/cmake/modules/SwiftSource.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,8 +886,11 @@ function(_compile_swift_files
# cross-compiling the compiler.
list(APPEND swift_compiler_tool_dep "swift-frontend${target_suffix}")

# If we aren't cross compiling, also depend on SwiftMacros.
list(APPEND swift_compiler_tool_dep SwiftMacros)
# If we aren't cross compiling and have swift-syntax, also depend on
# SwiftMacros.
if(SWIFT_BUILD_SWIFT_SYNTAX)
list(APPEND swift_compiler_tool_dep SwiftMacros)
endif()
endif()

# If there are more than one output files, we assume that they are specified
Expand Down
8 changes: 8 additions & 0 deletions stdlib/public/core/Availability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ public func _stdlib_isOSVersionAtLeast(
@_semantics("availability.osversion")
@_effects(readnone)
@_unavailableInEmbedded
#if hasFeature(Macros)
@_noLocks
#endif
public func _stdlib_isOSVersionAtLeast(
_ major: Builtin.Word,
_ minor: Builtin.Word,
Expand All @@ -65,7 +67,9 @@ public func _stdlib_isOSVersionAtLeast(
@_semantics("availability.osversion")
@_effects(readnone)
@_alwaysEmitIntoClient
#if hasFeature(Macros)
@_noLocks
#endif
public func _stdlib_isOSVersionAtLeast_AEIC(
_ major: Builtin.Word,
_ minor: Builtin.Word,
Expand Down Expand Up @@ -110,7 +114,9 @@ public func _stdlib_isOSVersionAtLeast_AEIC(
@_semantics("availability.osversion")
@_effects(readnone)
@available(macOS 10.15, iOS 13.0, *)
#if hasFeature(Macros)
@_noLocks
#endif
public func _stdlib_isVariantOSVersionAtLeast(
_ major: Builtin.Word,
_ minor: Builtin.Word,
Expand Down Expand Up @@ -153,7 +159,9 @@ public func _stdlib_isVariantOSVersionAtLeast(
@_semantics("availability.osversion")
@_effects(readnone)
@_unavailableInEmbedded
#if hasFeature(Macros)
@_noLocks
#endif
public func _stdlib_isOSVersionAtLeastOrVariantVersionAtLeast(
_ major: Builtin.Word,
_ minor: Builtin.Word,
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/ObjectIdentifier+DebugDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//

#if !$Embedded
#if !$Embedded && hasFeature(Macros)
@DebugDescription
extension ObjectIdentifier {
var lldbDescription: String {
Expand Down
2 changes: 2 additions & 0 deletions stdlib/public/core/StringBridge.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,7 @@ extension StringProtocol {

public // SPI(Foundation)
func _toUTF16Indices(_ range: Range<Int>) -> Range<Index> {
#if hasFeature(Macros)
if Self.self == String.self {
let s = unsafeBitCast(self, to: String.self)
return s.utf16._indexRange(for: range, from: s.startIndex)
Expand All @@ -759,6 +760,7 @@ extension StringProtocol {
let s = unsafeBitCast(self, to: Substring.self)
return s._slice._base.utf16._indexRange(for: range, from: s.startIndex)
}
#endif
let lowerbound = _toUTF16Index(range.lowerBound)
let upperbound = _toUTF16Index(range.upperBound)
return Range(uncheckedBounds: (lower: lowerbound, upper: upperbound))
Expand Down
2 changes: 2 additions & 0 deletions stdlib/public/core/SwiftifyImport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public enum _SwiftifyInfo {
///
/// Parameter paramInfo: information about how the function uses the pointer passed to it. The
/// safety of the generated wrapper function depends on this info being extensive and accurate.
#if hasFeature(Macros)
@attached(peer, names: overloaded)
public macro _SwiftifyImport(_ paramInfo: _SwiftifyInfo..., typeMappings: [String: String] = [:]) =
#externalMacro(module: "SwiftMacros", type: "SwiftifyImportMacro")
#endif
7 changes: 7 additions & 0 deletions utils/build-presets.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,13 @@ mixin-preset=

skip-test-swiftdocc

[preset: buildbot_linux,without_host_swift]
mixin-preset=
buildbot_linux,no_test

skip-early-swiftsyntax
skip-early-swift-driver

[preset: buildbot_linux_crosscompile_wasm]
mixin-preset=buildbot_linux

Expand Down