Skip to content

Commit f1e2b7a

Browse files
j-huij-hui
authored
Merge pull request #85723 from an0/my-branch
[ClangImporter] Fix NS_CLOSED_ENUM to validate raw values
2 parents 47eda91 + 7f7e41b commit f1e2b7a

File tree

3 files changed

+254
-0
lines changed

3 files changed

+254
-0
lines changed

‎lib/ClangImporter/SwiftDeclSynthesizer.cpp‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,49 @@ SwiftDeclSynthesizer::makeIndirectFieldAccessors(
12951295

12961296
// MARK: Enum RawValue initializers
12971297

1298+
/// Clone a literal expression for use in pattern matching.
1299+
/// Based on cloneRawLiteralExpr from DerivedConformanceRawRepresentable.cpp
1300+
static LiteralExpr *cloneRawLiteralExpr(ASTContext &C, LiteralExpr *expr) {
1301+
LiteralExpr *clone;
1302+
if (auto intLit = dyn_cast<IntegerLiteralExpr>(expr)) {
1303+
clone = new (C) IntegerLiteralExpr(intLit->getDigitsText(), expr->getLoc(),
1304+
/*implicit*/ true);
1305+
if (intLit->isNegative())
1306+
cast<IntegerLiteralExpr>(clone)->setNegative(expr->getLoc());
1307+
} else if (isa<NilLiteralExpr>(expr)) {
1308+
clone = new (C) NilLiteralExpr(expr->getLoc());
1309+
} else if (auto stringLit = dyn_cast<StringLiteralExpr>(expr)) {
1310+
clone = new (C) StringLiteralExpr(stringLit->getValue(), expr->getLoc());
1311+
} else if (auto floatLit = dyn_cast<FloatLiteralExpr>(expr)) {
1312+
clone = new (C) FloatLiteralExpr(floatLit->getDigitsText(), expr->getLoc(),
1313+
/*implicit*/ true);
1314+
if (floatLit->isNegative())
1315+
cast<FloatLiteralExpr>(clone)->setNegative(expr->getLoc());
1316+
} else {
1317+
llvm_unreachable("invalid raw literal expr");
1318+
}
1319+
clone->setImplicit();
1320+
return clone;
1321+
}
1322+
12981323
/// Synthesize the body of \c init?(rawValue:RawType) for an imported enum.
1324+
///
1325+
/// For non-frozen (open) enums, this generates:
1326+
/// init?(rawValue: RawType) {
1327+
/// self = Builtin.reinterpretCast(rawValue)
1328+
/// }
1329+
/// This allows arbitrary raw values for C compatibility.
1330+
///
1331+
/// For frozen (closed) enums, this generates:
1332+
/// init?(rawValue: RawType) {
1333+
/// switch rawValue {
1334+
/// case <value1>, <value2>, ...:
1335+
/// self = Builtin.reinterpretCast(rawValue)
1336+
/// default:
1337+
/// return nil
1338+
/// }
1339+
/// }
1340+
/// This ensures that only declared raw values are accepted.
12991341
static std::pair<BraceStmt *, bool>
13001342
synthesizeEnumRawValueConstructorBody(AbstractFunctionDecl *afd,
13011343
void *context) {
@@ -1338,10 +1380,70 @@ synthesizeEnumRawValueConstructorBody(AbstractFunctionDecl *afd,
13381380
/*implicit*/ true);
13391381
assign->setType(TupleType::getEmpty(ctx));
13401382

1383+
// Check if the enum is frozen (closed). If so, we need to validate
1384+
// that the raw value is one of the declared cases.
1385+
bool isFrozen = enumDecl->getAttrs().hasAttribute<FrozenAttr>();
1386+
1387+
if (isFrozen) {
1388+
// For frozen enums, generate a switch statement to validate the raw value.
1389+
1390+
// Collect all case labels for valid enum values
1391+
SmallVector<CaseLabelItem, 8> validCaseLabels;
1392+
for (auto *elt : enumDecl->getAllElements()) {
1393+
// Get the raw value literal for this element
1394+
auto rawValueExpr = elt->getStructuralRawValueExpr();
1395+
if (!rawValueExpr)
1396+
continue;
1397+
1398+
// Clone the raw value expression for pattern matching
1399+
auto *litExpr = cloneRawLiteralExpr(ctx, cast<LiteralExpr>(rawValueExpr));
1400+
auto *litPat = ExprPattern::createImplicit(ctx, litExpr, ctorDecl);
1401+
1402+
// Add to the list of valid case labels
1403+
validCaseLabels.push_back(CaseLabelItem(litPat));
1404+
}
1405+
1406+
// Create a single case statement with all valid raw values
1407+
// All valid values perform the same action: reinterpret cast
1408+
auto *caseBody = BraceStmt::create(ctx, SourceLoc(), {assign},
1409+
SourceLoc(), /*implicit*/ true);
1410+
auto *validCase = CaseStmt::createImplicit(ctx, CaseParentKind::Switch,
1411+
validCaseLabels, caseBody);
1412+
1413+
// Create default case that returns nil
1414+
auto *defaultPattern = AnyPattern::createImplicit(ctx);
1415+
auto defaultLabelItem = CaseLabelItem(defaultPattern);
1416+
1417+
auto *failStmt = new (ctx) FailStmt(SourceLoc(), SourceLoc(), /*implicit*/ true);
1418+
auto *defaultBody = BraceStmt::create(ctx, SourceLoc(), {failStmt},
1419+
SourceLoc(), /*implicit*/ true);
1420+
1421+
auto *defaultCase = CaseStmt::createImplicit(ctx, CaseParentKind::Switch,
1422+
{defaultLabelItem}, defaultBody);
1423+
1424+
// Create the switch statement
1425+
auto *switchParamRef = new (ctx) DeclRefExpr(param, DeclNameLoc(),
1426+
/*implicit*/ true);
1427+
switchParamRef->setType(param->getTypeInContext());
1428+
1429+
auto *switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(),
1430+
switchParamRef, SourceLoc(),
1431+
{validCase, defaultCase}, SourceLoc(), SourceLoc(), ctx);
1432+
1433+
auto body = BraceStmt::create(ctx, SourceLoc(), {switchStmt}, SourceLoc(),
1434+
/*implicit*/ true);
1435+
// Return isTypeChecked=false because the switch statement contains patterns
1436+
// and expressions that need type inference and semantic analysis.
1437+
return {body, /*isTypeChecked=*/false};
1438+
}
1439+
1440+
// For non-frozen enums, use the simple reinterpret cast approach
13411441
auto *ret = ReturnStmt::createImplicit(ctx, /*expr*/ nullptr);
13421442

13431443
auto body = BraceStmt::create(ctx, SourceLoc(), {assign, ret}, SourceLoc(),
13441444
/*implicit*/ true);
1445+
// Return isTypeChecked=true because all types are explicitly set
1446+
// and no type inference is needed.
13451447
return {body, /*isTypeChecked=*/true};
13461448
}
13471449

‎test/ClangImporter/Inputs/enum-closed-raw-value.h‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#ifndef ENUM_CLOSED_RAWVALUE_H
2+
#define ENUM_CLOSED_RAWVALUE_H
3+
4+
// Test NS_CLOSED_ENUM with explicit enum_extensibility(closed) attribute
5+
typedef enum __attribute__((enum_extensibility(closed))) IntEnum : long {
6+
IntEnumZero = 0,
7+
IntEnumOne = 1
8+
} IntEnum;
9+
10+
// Test NS_CLOSED_ENUM with non-contiguous values
11+
typedef enum __attribute__((enum_extensibility(closed))) NonContiguousEnum : long {
12+
NonContiguousEnumFirst = 10,
13+
NonContiguousEnumSecond = 20,
14+
NonContiguousEnumThird = 30
15+
} NonContiguousEnum;
16+
17+
// Test NS_CLOSED_ENUM with negative values
18+
typedef enum __attribute__((enum_extensibility(closed))) NegativeEnum : long {
19+
NegativeEnumNegative = -1,
20+
NegativeEnumZero = 0,
21+
NegativeEnumPositive = 1
22+
} NegativeEnum;
23+
24+
// Test regular NS_ENUM (non-frozen) for comparison - should accept any value
25+
typedef enum __attribute__((enum_extensibility(open))) OpenEnum : long {
26+
OpenEnumZero = 0,
27+
OpenEnumOne = 1
28+
} OpenEnum;
29+
30+
// Test NS_CLOSED_ENUM with single case
31+
typedef enum __attribute__((enum_extensibility(closed))) SingleCaseEnum : long {
32+
SingleCaseEnumOnly = 42
33+
} SingleCaseEnum;
34+
35+
#endif // ENUM_CLOSED_RAWVALUE_H

‎test/ClangImporter/enum-closed-raw-value.swift‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// RUN: %target-run-simple-swift(-import-objc-header %S/Inputs/enum-closed-raw-value.h)
2+
// RUN: %target-run-simple-swift(-cxx-interoperability-mode=default -import-objc-header %S/Inputs/enum-closed-raw-value.h)
3+
4+
// REQUIRES: executable_test
5+
6+
import StdlibUnittest
7+
8+
var EnumClosedRawValueTestSuite = TestSuite("NS_CLOSED_ENUM raw value validation")
9+
10+
EnumClosedRawValueTestSuite.test("IntEnum: valid raw values") {
11+
let zero = IntEnum(rawValue: 0)
12+
expectNotNil(zero)
13+
expectEqual(zero!, .zero)
14+
15+
let one = IntEnum(rawValue: 1)
16+
expectNotNil(one)
17+
expectEqual(one!, .one)
18+
}
19+
20+
EnumClosedRawValueTestSuite.test("IntEnum: invalid raw values return nil") {
21+
// This should pass with the fix - invalid values return nil
22+
expectNil(IntEnum(rawValue: 100))
23+
expectNil(IntEnum(rawValue: -1))
24+
expectNil(IntEnum(rawValue: 2))
25+
}
26+
27+
EnumClosedRawValueTestSuite.test("NonContiguousEnum: valid raw values") {
28+
let first = NonContiguousEnum(rawValue: 10)
29+
expectNotNil(first)
30+
expectEqual(first!, .first)
31+
32+
let second = NonContiguousEnum(rawValue: 20)
33+
expectNotNil(second)
34+
expectEqual(second!, .second)
35+
36+
let third = NonContiguousEnum(rawValue: 30)
37+
expectNotNil(third)
38+
expectEqual(third!, .third)
39+
}
40+
41+
EnumClosedRawValueTestSuite.test("NonContiguousEnum: invalid raw values between valid ones") {
42+
expectNil(NonContiguousEnum(rawValue: 0))
43+
expectNil(NonContiguousEnum(rawValue: 15))
44+
expectNil(NonContiguousEnum(rawValue: 25))
45+
expectNil(NonContiguousEnum(rawValue: 100))
46+
}
47+
48+
EnumClosedRawValueTestSuite.test("NegativeEnum: valid raw values including negatives") {
49+
let negative = NegativeEnum(rawValue: -1)
50+
expectNotNil(negative)
51+
expectEqual(negative!, .negative)
52+
53+
let zero = NegativeEnum(rawValue: 0)
54+
expectNotNil(zero)
55+
expectEqual(zero!, .zero)
56+
57+
let positive = NegativeEnum(rawValue: 1)
58+
expectNotNil(positive)
59+
expectEqual(positive!, .positive)
60+
}
61+
62+
EnumClosedRawValueTestSuite.test("NegativeEnum: invalid raw values") {
63+
expectNil(NegativeEnum(rawValue: -2))
64+
expectNil(NegativeEnum(rawValue: 2))
65+
expectNil(NegativeEnum(rawValue: 100))
66+
}
67+
68+
EnumClosedRawValueTestSuite.test("OpenEnum: accepts ANY raw value for C compatibility") {
69+
// Open (non-frozen) enums should accept arbitrary values
70+
expectNotNil(OpenEnum(rawValue: 0))
71+
expectNotNil(OpenEnum(rawValue: 1))
72+
73+
// Invalid values should still succeed for open enums
74+
expectNotNil(OpenEnum(rawValue: 100))
75+
expectNotNil(OpenEnum(rawValue: -1))
76+
}
77+
78+
EnumClosedRawValueTestSuite.test("SingleCaseEnum: only valid value") {
79+
let only = SingleCaseEnum(rawValue: 42)
80+
expectNotNil(only)
81+
expectEqual(only!, .only)
82+
}
83+
84+
EnumClosedRawValueTestSuite.test("SingleCaseEnum: invalid raw values") {
85+
expectNil(SingleCaseEnum(rawValue: 0))
86+
expectNil(SingleCaseEnum(rawValue: 41))
87+
expectNil(SingleCaseEnum(rawValue: 43))
88+
}
89+
90+
EnumClosedRawValueTestSuite.test("Switch exhaustivity: frozen enums don't need default") {
91+
// Frozen enums should be exhaustive - no default needed
92+
let value = IntEnum.zero
93+
switch value {
94+
case .zero:
95+
expectTrue(true) // Expected path
96+
case .one:
97+
expectTrue(false, "Wrong case")
98+
}
99+
}
100+
101+
EnumClosedRawValueTestSuite.test("Switch with invalid raw value scenario from bug report") {
102+
// This was the crash scenario from issue #85701
103+
// With the fix, init should return nil, so this branch never executes
104+
if let ie = IntEnum(rawValue: 100) {
105+
switch ie {
106+
case .zero:
107+
expectTrue(false, "Should not reach here - rawValue 100 is invalid")
108+
case .one:
109+
expectTrue(false, "Should not reach here - rawValue 100 is invalid")
110+
}
111+
} else {
112+
// This is the expected path with the fix
113+
expectTrue(true, "Correctly returned nil for invalid raw value")
114+
}
115+
}
116+
117+
runAllTests()

0 commit comments

Comments
 (0)