Skip to content

Use the SwiftIfConfig library for evaluating #if conditions #75688

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
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions include/swift/AST/ParseRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ class IDEInspectionSecondPassRequest
readDependencySource(const evaluator::DependencyRecorder &) const;
};

class EvaluateIfConditionRequest
: public SimpleRequest<EvaluateIfConditionRequest,
std::pair<bool, bool>(SourceFile *, SourceRange conditionRange,
bool shouldEvaluate),
RequestFlags::Uncached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

// Evaluation.
std::pair<bool, bool> evaluate(
Evaluator &evaluator, SourceFile *SF, SourceRange conditionRange,
bool shouldEvaluate) const;
};

/// The zone number for the parser.
#define SWIFT_TYPEID_ZONE Parse
#define SWIFT_TYPEID_HEADER "swift/AST/ParseTypeIDZone.def"
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/ParseTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ SWIFT_REQUEST(Parse, ParseTopLevelDeclsRequest,
SWIFT_REQUEST(Parse, ExportedSourceFileRequest,
void *(const SourceFile *), Cached,
NoLocationInfo)
SWIFT_REQUEST(Parse, EvaluateIfConditionRequest,
(std::pair<bool, bool>)(SourceFile *, SourceRange, bool), Uncached,
NoLocationInfo)
9 changes: 9 additions & 0 deletions include/swift/Basic/SourceLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,15 @@ class SourceRange {
}

SWIFT_DEBUG_DUMPER(dump(const SourceManager &SM));

friend size_t hash_value(SourceRange range) {
return llvm::hash_combine(range.Start, range.End);
}

friend void simple_display(raw_ostream &OS, const SourceRange &loc) {
// Nothing meaningful to print.
}

};

/// A half-open character-based source range.
Expand Down
71 changes: 70 additions & 1 deletion lib/ASTGen/Sources/ASTGen/CompilerBuildConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import ASTBridging
import SwiftDiagnostics
import SwiftIfConfig
@_spi(Compiler) import SwiftIfConfig
import SwiftParser
import SwiftSyntax

Expand Down Expand Up @@ -109,6 +109,13 @@ struct CompilerBuildConfiguration: BuildConfiguration {

func isActiveTargetRuntime(name: String) throws -> Bool {
var name = name

// Complain if the provided runtime isn't one of the known values.
switch name {
case "_Native", "_ObjC", "_multithreaded": break
default: throw IfConfigError.unexpectedRuntimeCondition
}

return name.withBridgedString { nameRef in
ctx.langOptsIsActiveTargetRuntime(nameRef)
}
Expand Down Expand Up @@ -161,6 +168,18 @@ struct CompilerBuildConfiguration: BuildConfiguration {
}
}

enum IfConfigError: Error, CustomStringConvertible {
case unexpectedRuntimeCondition

var description: String {
switch self {
case .unexpectedRuntimeCondition:
return "unexpected argument for the '_runtime' condition; expected '_Native' or '_ObjC'"
Copy link
Member

Choose a reason for hiding this comment

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

Isn’t _multithreaded also allowed, based on the check above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Heh, yes. I was originally following the actually-incorrect diagnostic text in the compiler itself to try to minimize the diffs in this PR. We can fix this now.

}
}
}


/// Extract the #if clause range information for the given source file.
@_cdecl("swift_ASTGen_configuredRegions")
public func configuredRegions(
Expand Down Expand Up @@ -301,3 +320,53 @@ public func freeConfiguredRegions(
) {
UnsafeMutableBufferPointer(start: regions, count: numRegions).deallocate()
}

/// Evaluate the #if condition at ifClauseLocationPtr.
@_cdecl("swift_ASTGen_evaluatePoundIfCondition")
public func evaluatePoundIfCondition(
astContext: BridgedASTContext,
diagEnginePtr: UnsafeMutableRawPointer,
sourceFileBuffer: BridgedStringRef,
ifConditionText: BridgedStringRef,
shouldEvaluate: Bool
) -> Int {
// Retrieve the #if condition that we're evaluating here.
// FIXME: Use 'ExportedSourceFile' when C++ parser is replaced.
let textBuffer = UnsafeBufferPointer<UInt8>(start: ifConditionText.data, count: ifConditionText.count)
Copy link
Member

Choose a reason for hiding this comment

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

Don’t we have an ExportedSourceFile already (the one that we also use for macro evaluation)? Or is this happening earlier where we haven’t created the ExportedSourceFile yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm following the path @rintaro used for checking the definitions of macros, which avoids running the swift-syntax parser on the whole file (for performance reasons). This also avoids the somewhat brittle "go find this exact swift-syntax node at this source location" lookup we need to do with that approach, which can fail when the C++ parser and SwiftParser don't recover in exactly the same way.

Copy link
Member

Choose a reason for hiding this comment

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

I see. I was still thinking we had the syntax tree for the entire file anyway and forgot that we no longer parse it completely.

var parser = Parser(textBuffer)
let conditionExpr = ExprSyntax.parse(from: &parser)

let isActive: Bool
let syntaxErrorsAllowed: Bool
let diagnostics: [Diagnostic]
if shouldEvaluate {
// Evaluate the condition against the compiler's build configuration.
let configuration = CompilerBuildConfiguration(
ctx: astContext,
sourceBuffer: textBuffer
)

let state: IfConfigRegionState
(state, syntaxErrorsAllowed, diagnostics) = IfConfigRegionState.evaluating(conditionExpr, in: configuration)
isActive = (state == .active)
} else {
// Don't evaluate the condition, because we know it's inactive. Determine
// whether syntax errors are permitted within this region according to the
// condition.
isActive = false
(syntaxErrorsAllowed, diagnostics) = IfConfigClauseSyntax.syntaxErrorsAllowed(conditionExpr)
}

// Render the diagnostics.
for diagnostic in diagnostics {
emitDiagnostic(
diagnosticEngine: BridgedDiagnosticEngine(raw: diagEnginePtr),
sourceFileBuffer: UnsafeBufferPointer(start: sourceFileBuffer.data, count: sourceFileBuffer.count),
sourceFileBufferOffset: ifConditionText.data! - sourceFileBuffer.data!,
diagnostic: diagnostic,
diagnosticSeverity: diagnostic.diagMessage.severity
)
}

return (isActive ? 0x1 : 0) | (syntaxErrorsAllowed ? 0x2 : 0)
}
31 changes: 30 additions & 1 deletion lib/Parse/ParseIfConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "swift/Parse/Parser.h"

#include "swift/AST/ASTBridging.h"
#include "swift/AST/ASTVisitor.h"
#include "swift/AST/DiagnosticSuppression.h"
#include "swift/Basic/Assertions.h"
Expand Down Expand Up @@ -766,6 +767,26 @@ static Expr *findAnyLikelySimulatorEnvironmentTest(Expr *Condition) {

} // end anonymous namespace

/// Call into the Swift implementation of #if condition evaluation.
///
/// \returns std::nullopt if the Swift implementation is not available, or
/// a pair (isActive, allowSyntaxErrors) describing whether the evaluated
/// condition indicates that the region is active and whether, if inactive,
/// the code in that region is allowed to have syntax errors.
static std::optional<std::pair<bool, bool>> evaluateWithSwiftIfConfig(
Parser &parser,
SourceRange conditionRange,
bool shouldEvaluate
) {
#if SWIFT_BUILD_SWIFT_SYNTAX
return evaluateOrDefault(
parser.Context.evaluator,
EvaluateIfConditionRequest{&parser.SF, conditionRange, shouldEvaluate},
std::pair(false, false));
#else
return std::nullopt;
#endif
}

/// Parse and populate a #if ... #endif directive.
/// Delegate callback function to parse elements in the blocks.
Expand Down Expand Up @@ -842,7 +863,15 @@ Result Parser::parseIfConfigRaw(
if (result.isNull())
return makeParserError();
Condition = result.get();
if (validateIfConfigCondition(Condition, Context, Diags)) {
if (std::optional<std::pair<bool, bool>> evalResult =
evaluateWithSwiftIfConfig(*this,
Condition->getSourceRange(),
shouldEvaluate)) {
if (!foundActive) {
isActive = evalResult->first;
isVersionCondition = evalResult->second;
}
} else if (validateIfConfigCondition(Condition, Context, Diags)) {
// Error in the condition;
isActive = false;
isVersionCondition = false;
Expand Down
29 changes: 29 additions & 0 deletions lib/Sema/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "TypeCheckType.h"
#include "CodeSynthesis.h"
#include "MiscDiagnostics.h"
#include "swift/AST/ASTBridging.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/ASTVisitor.h"
#include "swift/AST/Attr.h"
Expand Down Expand Up @@ -765,3 +766,31 @@ bool TypeChecker::diagnoseInvalidFunctionType(

return hadAnyError;
}

extern "C" intptr_t swift_ASTGen_evaluatePoundIfCondition(
BridgedASTContext astContext,
void *_Nonnull diagEngine,
BridgedStringRef sourceFileBuffer,
BridgedStringRef conditionText,
bool);

std::pair<bool, bool> EvaluateIfConditionRequest::evaluate(
Evaluator &evaluator, SourceFile *sourceFile, SourceRange conditionRange,
bool shouldEvaluate
) const {
// FIXME: When we migrate to SwiftParser, use the parsed syntax tree.
ASTContext &ctx = sourceFile->getASTContext();
auto &sourceMgr = ctx.SourceMgr;
StringRef sourceFileText =
sourceMgr.getEntireTextForBuffer(*sourceFile->getBufferID());
StringRef conditionText =
sourceMgr.extractText(Lexer::getCharSourceRangeFromSourceRange(
sourceMgr, conditionRange));
intptr_t evalResult = swift_ASTGen_evaluatePoundIfCondition(
ctx, &ctx.Diags, sourceFileText, conditionText, shouldEvaluate
);

bool isActive = (evalResult & 0x01) != 0;
bool allowSyntaxErrors = (evalResult & 0x02) != 0;
return std::pair(isActive, allowSyntaxErrors);
}
55 changes: 17 additions & 38 deletions test/Parse/ConditionalCompilation/basicParseErrors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var x = 0
var y = 0
#endif

#if foo(BAR) // expected-error {{unexpected platform condition (expected 'os', 'arch', or 'swift')}}
#if foo(BAR) // expected-error {{invalid conditional compilation expression}}
var z = 0
#endif

Expand All @@ -27,7 +27,7 @@ func h() {}
#else /* aaa */
#endif /* bbb */

#if foo.bar() // expected-error {{unexpected platform condition (expected 'os', 'arch', or 'swift')}}
#if foo.bar() // expected-error {{invalid conditional compilation expression}}
.baz()

#endif
Expand Down Expand Up @@ -67,30 +67,23 @@ struct S {
#else
#endif

#if os(ios) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note{{did you mean 'iOS'?}} {{8-11=iOS}}
#if os(ios)
#endif

#if os(uOS) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note{{did you mean 'iOS'?}} {{8-11=iOS}}
#if os(uOS)
#endif

#if os(xxxxxxd) // expected-warning {{unknown operating system for build configuration 'os'}}
// expected-note@-1{{did you mean 'Linux'?}} {{8-15=Linux}}
// expected-note@-2{{did you mean 'FreeBSD'?}} {{8-15=FreeBSD}}
// expected-note@-3{{did you mean 'Android'?}} {{8-15=Android}}
// expected-note@-4{{did you mean 'OSX'?}} {{8-15=OSX}}
// expected-note@-5{{did you mean 'OpenBSD'?}} {{8-15=OpenBSD}}
// expected-note@-6{{did you mean 'xrOS'?}} {{8-15=xrOS}}
#if os(xxxxxxd)
#endif

#if os(bisionos) // expected-warning {{unknown operating system for build configuration 'os'}}
// expected-note@-1{{did you mean 'visionOS'?}} {{8-16=visionOS}}
#if os(bisionos)

#endif

#if arch(arn) // expected-warning {{unknown architecture for build configuration 'arch'}} expected-note{{did you mean 'arm'?}} {{10-13=arm}}
#if arch(arn)
#endif

#if _endian(mid) // expected-warning {{unknown endianness for build configuration '_endian'}} expected-note{{did you mean 'big'?}} {{13-16=big}}
#if _endian(mid) // expected-warning {{unknown endianness for build configuration '_endian'}}
#endif

LABEL: #if true // expected-error {{expected statement}}
Expand All @@ -104,7 +97,7 @@ func fn_j() {}
#endif
fn_j() // OK

#if foo || bar || nonExistent() // expected-error {{expected argument to platform condition}}
#if foo || bar || nonExistent() // expected-error {{invalid conditional compilation expression}}
#endif

#if FOO = false
Expand All @@ -123,47 +116,33 @@ undefinedFunc() // expected-error {{cannot find 'undefinedFunc' in scope}}
#endif

/// Invalid platform condition arguments don't invalidate the whole condition.
// expected-warning@+1{{unknown endianness}}
#if !arch(arn) && !os(ystem) && !_endian(ness)
// expected-warning@-1 {{unknown architecture for build configuration 'arch'}}
// expected-note@-2 {{did you mean 'arm'?}} {{11-14=arm}}
// expected-warning@-3 {{unknown operating system for build configuration 'os'}}
// expected-note@-4 {{did you mean 'OSX'?}} {{23-28=OSX}}
// expected-note@-5 {{did you mean 'PS4'?}} {{23-28=PS4}}
// expected-note@-6 {{did you mean 'none'?}} {{23-28=none}}
// expected-warning@-7 {{unknown endianness for build configuration '_endian'}}
// expected-note@-8 {{did you mean 'big'?}} {{42-46=big}}
func fn_k() {}
#endif
fn_k()

#if os(cillator) || arch(i3rm)
// expected-warning@-1 {{unknown operating system for build configuration 'os'}}
// expected-note@-2 {{did you mean 'macOS'?}} {{8-16=macOS}}
// expected-note@-3 {{did you mean 'iOS'?}} {{8-16=iOS}}
// expected-warning@-4 {{unknown architecture for build configuration 'arch'}}
// expected-note@-5 {{did you mean 'arm'?}} {{26-30=arm}}
// expected-note@-6 {{did you mean 'i386'?}} {{26-30=i386}}
// expected-note@-7 {{did you mean 'visionOS'?}} {{8-16=visionOS}}
func undefinedFunc() // ignored.
#endif
undefinedFunc() // expected-error {{cannot find 'undefinedFunc' in scope}}

#if os(simulator) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note {{did you mean 'targetEnvironment'}} {{5-7=targetEnvironment}}
#if os(simulator)
#endif

#if arch(iOS) // expected-warning {{unknown architecture for build configuration 'arch'}} expected-note {{did you mean 'os'}} {{5-9=os}}
#if arch(iOS)
#endif

#if _endian(arm64) // expected-warning {{unknown endianness for build configuration '_endian'}} expected-note {{did you mean 'arch'}} {{5-12=arch}}
#if _endian(arm64) // expected-warning {{unknown endianness for build configuration '_endian'}}
#endif

#if targetEnvironment(_ObjC) // expected-warning {{unknown target environment for build configuration 'targetEnvironment'}} expected-note {{did you mean 'macabi'}} {{23-28=macabi}}
#if targetEnvironment(_ObjC)
#endif

#if os(iOS) || os(simulator) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note {{did you mean 'targetEnvironment'}} {{16-18=targetEnvironment}}
#if os(iOS) || os(simulator)
#endif

#if arch(ios) // expected-warning {{unknown architecture for build configuration 'arch'}} expected-note {{did you mean 'os'}} {{5-9=os}} expected-note {{did you mean 'iOS'}} {{10-13=iOS}}
#if arch(ios)
#endif

#if FOO
Expand All @@ -179,5 +158,5 @@ if true {}

// rdar://83017601 Make sure we don't crash
#if canImport()
// expected-error@-1 {{expected argument to platform condition}}
// expected-error@-1 {{'canImport' requires a module name}}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#endif

#if canImport(SomeModule)
// CHECK: :[[@LINE-1]]:15: error: could not find module 'SomeModule' for target '{{.*}}'; found: i386
// CHECK: :[[@LINE-1]]:{{.*}}: error: could not find module 'SomeModule' for target '{{.*}}'; found: i386
#endif

import SomeModule
10 changes: 5 additions & 5 deletions test/Parse/ConditionalCompilation/can_import_submodule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ import A.B.C
#endif


#if canImport(A(_:).B) // expected-error@:15 {{unexpected platform condition argument: expected module name}}
#if canImport(A(_:).B) // expected-error@:15 {{expected module name}}
#endif

#if canImport(A.B(c: "arg")) // expected-error@:15 {{unexpected platform condition argument: expected module name}}
#if canImport(A.B(c: "arg")) // expected-error@:15 {{expected module name}}
#endif

#if canImport(A(b: 1, c: 2).B.C) // expected-error@:15 {{unexpected platform condition argument: expected module name}}
#if canImport(A(b: 1, c: 2).B.C) // expected-error@:15 {{expected module name}}
#endif

#if canImport(A.B("arg")(3).C) // expected-error@:15 {{unexpected platform condition argument: expected module name}}
#if canImport(A.B("arg")(3).C) // expected-error@:15 {{expected module name}}
#endif

#if canImport(A.B.C()) // expected-error@:15 {{unexpected platform condition argument: expected module name}}
#if canImport(A.B.C()) // expected-error@:15 {{expected module name}}
#endif
Loading