Skip to content

Commit 2dec76c

Browse files
l46kokcopybara-github
authored andcommitted
Fix type checking around comprehensions with empty iteration range
PiperOrigin-RevId: 979965644
1 parent 705fcdb commit 2dec76c

2 files changed

Lines changed: 79 additions & 1 deletion

File tree

checker/internal/type_checker_impl.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,8 @@ void ResolveVisitor::PostVisitComprehensionSubexpression(
838838
GetDeducedType(&comprehension.accu_init())));
839839
break;
840840
case ComprehensionArg::ITER_RANGE: {
841-
Type range_type = GetDeducedType(&comprehension.iter_range());
841+
Type range_type = inference_context_->FullySubstitute(
842+
GetDeducedType(&comprehension.iter_range()), /*free_to_dyn=*/false);
842843
Type iter_type = DynType(); // iter_var for non comprehensions v2.
843844
Type iter_type1 = DynType(); // iter_var for comprehensions v2.
844845
Type iter_type2 = DynType(); // iter_var2 for comprehensions v2.
@@ -851,7 +852,14 @@ void ResolveVisitor::PostVisitComprehensionSubexpression(
851852
iter_type = iter_type1 = range_type.GetMap().key();
852853
iter_type2 = range_type.GetMap().value();
853854
break;
855+
case TypeKind::kTypeParam:
856+
// Set the range type to DYN to prevent assignment to a potentially
857+
// incorrect type at a later point in type-checking. The IsAssignable
858+
// call will update the type substitutions for the type param.
859+
inference_context_->IsAssignable(DynType(), range_type);
860+
break;
854861
case TypeKind::kDyn:
862+
case TypeKind::kError:
855863
break;
856864
default:
857865
ReportIssue(TypeCheckIssue::CreateError(

checker/internal/type_checker_impl_test.cc

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,76 @@ TEST(TypeCheckerImplTest, ComprehensionDynRange) {
12871287
EXPECT_THAT(result.GetIssues(), IsEmpty());
12881288
}
12891289

1290+
TEST(TypeCheckerImplTest, EmptyListRangeNestedListComprehension) {
1291+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1292+
google::protobuf::Arena arena;
1293+
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
1294+
1295+
TypeCheckerImpl impl(std::move(env));
1296+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("[].map(x, x.map(y, y))"));
1297+
ASSERT_OK_AND_ASSIGN(ValidationResult result, impl.Check(std::move(ast)));
1298+
1299+
EXPECT_TRUE(result.IsValid()) << result.FormatError();
1300+
EXPECT_THAT(result.GetIssues(), IsEmpty());
1301+
1302+
ASSERT_OK_AND_ASSIGN(auto checked_ast, result.ReleaseAst());
1303+
EXPECT_EQ(checked_ast->GetReturnType(),
1304+
AstType(ListTypeSpec(std::make_unique<AstType>(
1305+
ListTypeSpec(std::make_unique<AstType>(DynTypeSpec()))))));
1306+
}
1307+
1308+
TEST(TypeCheckerImplTest, EmptyMapRangeNestedComprehension) {
1309+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1310+
google::protobuf::Arena arena;
1311+
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
1312+
1313+
TypeCheckerImpl impl(std::move(env));
1314+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("{}.map(k, k.map(y, y))"));
1315+
ASSERT_OK_AND_ASSIGN(ValidationResult result, impl.Check(std::move(ast)));
1316+
1317+
EXPECT_TRUE(result.IsValid()) << result.FormatError();
1318+
EXPECT_THAT(result.GetIssues(), IsEmpty());
1319+
1320+
ASSERT_OK_AND_ASSIGN(auto checked_ast, result.ReleaseAst());
1321+
EXPECT_EQ(checked_ast->GetReturnType(),
1322+
AstType(ListTypeSpec(std::make_unique<AstType>(
1323+
ListTypeSpec(std::make_unique<AstType>(DynTypeSpec()))))));
1324+
}
1325+
1326+
TEST(TypeCheckerImplTest, EmptyRangeNestedComprehensionPredicate) {
1327+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1328+
google::protobuf::Arena arena;
1329+
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
1330+
1331+
TypeCheckerImpl impl(std::move(env));
1332+
ASSERT_OK_AND_ASSIGN(auto ast,
1333+
MakeTestParsedAst("[].all(x, x.all(y, y == 1))"));
1334+
ASSERT_OK_AND_ASSIGN(ValidationResult result, impl.Check(std::move(ast)));
1335+
1336+
EXPECT_TRUE(result.IsValid()) << result.FormatError();
1337+
EXPECT_THAT(result.GetIssues(), IsEmpty());
1338+
1339+
ASSERT_OK_AND_ASSIGN(auto checked_ast, result.ReleaseAst());
1340+
EXPECT_EQ(checked_ast->GetReturnType(), AstType(PrimitiveType::kBool));
1341+
}
1342+
1343+
TEST(TypeCheckerImplTest, EmptyRangeNestedComprehensionBeforeConstraint) {
1344+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1345+
google::protobuf::Arena arena;
1346+
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
1347+
1348+
TypeCheckerImpl impl(std::move(env));
1349+
ASSERT_OK_AND_ASSIGN(
1350+
auto ast, MakeTestParsedAst("[].all(x, x.all(y, y == 1) && x == 1)"));
1351+
ASSERT_OK_AND_ASSIGN(ValidationResult result, impl.Check(std::move(ast)));
1352+
1353+
EXPECT_TRUE(result.IsValid()) << result.FormatError();
1354+
EXPECT_THAT(result.GetIssues(), IsEmpty());
1355+
1356+
ASSERT_OK_AND_ASSIGN(auto checked_ast, result.ReleaseAst());
1357+
EXPECT_EQ(checked_ast->GetReturnType(), AstType(PrimitiveType::kBool));
1358+
}
1359+
12901360
TEST(TypeCheckerImplTest, BasicOvlResolution) {
12911361
TypeCheckEnv env(GetSharedTestingDescriptorPool());
12921362
google::protobuf::Arena arena;

0 commit comments

Comments
 (0)