Skip to content

Commit f2de9fb

Browse files
committed
Fix out of range return value of get list item instruction
1 parent 59bce80 commit f2de9fb

File tree

6 files changed

+34
-23
lines changed

6 files changed

+34
-23
lines changed

src/engine/internal/llvm/instructions/lists.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ LLVMInstruction *Lists::buildGetListItem(LLVMInstruction *ins)
246246
llvm::Value *index = m_utils.castValue(arg.second, arg.first);
247247
llvm::Value *inRange = m_builder.CreateAnd(m_builder.CreateFCmpOGE(index, min), m_builder.CreateFCmpOLT(index, size));
248248

249-
LLVMConstantRegister nullReg(listType, Value());
249+
LLVMConstantRegister nullReg(Compiler::StaticType::String, "");
250250
llvm::Value *null = m_utils.createValue(static_cast<LLVMRegister *>(&nullReg));
251251

252252
index = m_builder.CreateFPToUI(index, m_builder.getInt64Ty());

src/engine/internal/llvm/llvmcodeanalyzer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ void LLVMCodeAnalyzer::analyzeScript(const LLVMInstructionList &script) const
100100
updateListType(currentBranch, ins, typeAssignedInstructions, false);
101101

102102
// Store the type in the return register
103-
ins->functionReturnReg->setType(ins->targetType);
103+
// NOTE: Get list item returns empty string if index is out of range
104+
ins->functionReturnReg->setType(ins->targetType | Compiler::StaticType::String);
104105
}
105106

106107
ins = ins->next;

test/blocks/list_blocks_test.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -422,14 +422,14 @@ TEST_F(ListBlocksTest, ItemOfList)
422422
static const std::string expected =
423423
"dolor\n"
424424
"true\n"
425-
"0\n"
426-
"0\n"
425+
"\n"
426+
"\n"
427427
"true\n"
428428
"123\n"
429429
"Lorem\n"
430-
"0\n"
431-
"0\n"
432-
"0\n";
430+
"\n"
431+
"\n"
432+
"\n";
433433

434434
EXPECT_CALL(m_rng, randint(1, 5)).WillOnce(Return(4)).WillOnce(Return(1));
435435
testing::internal::CaptureStdout();

test/llvm/code_analyzer/list_type_analysis.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,8 +1002,8 @@ TEST(LLVMCodeAnalyzer_ListTypeAnalysis, CrossListDependency_SingleType)
10021002
// targetList first write has no previous type
10031003
ASSERT_EQ(appendList1_1->targetType, Compiler::StaticType::Void);
10041004

1005-
// targetList second write has Number type
1006-
ASSERT_EQ(appendList1_2->targetType, Compiler::StaticType::Number);
1005+
// targetList second write has Number | String type (get list item can always return string)
1006+
ASSERT_EQ(appendList1_2->targetType, Compiler::StaticType::Number | Compiler::StaticType::String);
10071007
}
10081008

10091009
TEST(LLVMCodeAnalyzer_ListTypeAnalysis, CrossListDependency_MultipleTypes)
@@ -1063,8 +1063,8 @@ TEST(LLVMCodeAnalyzer_ListTypeAnalysis, CrossListDependency_MultipleTypes)
10631063
// targetList first write has no previous type
10641064
ASSERT_EQ(appendList3->targetType, Compiler::StaticType::Void);
10651065

1066-
// targetList second write has Number or Bool type
1067-
ASSERT_EQ(appendList4->targetType, Compiler::StaticType::Number | Compiler::StaticType::Bool);
1066+
// targetList second write has Number or Bool type + String (get list item can always return string)
1067+
ASSERT_EQ(appendList4->targetType, Compiler::StaticType::Unknown);
10681068
}
10691069

10701070
TEST(LLVMCodeAnalyzer_ListTypeAnalysis, ChainedAssignmentsInLoop)
@@ -1243,8 +1243,8 @@ TEST(LLVMCodeAnalyzer_ListTypeAnalysis, ListReadReturnRegType)
12431243

12441244
analyzer.analyzeScript(list);
12451245

1246-
// sourceList read return register has Number type
1247-
ASSERT_EQ(sourceValue.type(), Compiler::StaticType::Number);
1246+
// sourceList read return register has Number | String type (get list item can always return string)
1247+
ASSERT_EQ(sourceValue.type(), Compiler::StaticType::Number | Compiler::StaticType::String);
12481248
}
12491249

12501250
TEST(LLVMCodeAnalyzer_ListTypeAnalysis, CrossListWriteArgType)
@@ -1285,6 +1285,6 @@ TEST(LLVMCodeAnalyzer_ListTypeAnalysis, CrossListWriteArgType)
12851285

12861286
analyzer.analyzeScript(list);
12871287

1288-
// last write argument has Number type
1289-
ASSERT_EQ(appendList1_1->args.back().first, Compiler::StaticType::Number);
1288+
// last write argument has Number | String type (get list item can always return string)
1289+
ASSERT_EQ(appendList1_1->args.back().first, Compiler::StaticType::Number | Compiler::StaticType::String);
12901290
}

test/llvm/code_analyzer/mixed_type_analysis.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ TEST(LLVMCodeAnalyzer_MixedTypeAnalysis, ListToVariable_SimpleTransfer)
121121
// Variable write should see Unknown (first write)
122122
ASSERT_EQ(writeVar->targetType, Compiler::StaticType::Unknown);
123123

124-
// Return register should have Bool type
125-
ASSERT_EQ(readListReg.type(), Compiler::StaticType::Bool);
124+
// Return register should have Bool | String type (get list item can always return string)
125+
ASSERT_EQ(readListReg.type(), Compiler::StaticType::Bool | Compiler::StaticType::String);
126126
}
127127

128128
TEST(LLVMCodeAnalyzer_MixedTypeAnalysis, CircularVarListDependency)
@@ -502,5 +502,5 @@ TEST(LLVMCodeAnalyzer_MixedTypeAnalysis, ComplexChain_VarToListToVar_TypePropaga
502502
ASSERT_EQ(appendList->targetType, Compiler::StaticType::Void);
503503
ASSERT_EQ(readList->targetType, Compiler::StaticType::Number);
504504
ASSERT_EQ(writeTargetVar->targetType, Compiler::StaticType::Unknown);
505-
ASSERT_EQ(readListReg.type(), Compiler::StaticType::Number);
505+
ASSERT_EQ(readListReg.type(), Compiler::StaticType::Number | Compiler::StaticType::String); // get list item can always return string
506506
}

test/llvm/llvmcodebuilder_test.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,20 +1449,30 @@ TEST_F(LLVMCodeBuilderTest, GetListItem)
14491449
v = builder->addListItem(localList3.get(), v);
14501450
builder->addFunctionCall("test_print_number", Compiler::StaticType::Void, { Compiler::StaticType::Number }, { v });
14511451

1452+
// Establish number type
1453+
builder->createListClear(localList3.get());
1454+
builder->createListAppend(localList3.get(), builder->addConstValue(1.5));
1455+
1456+
// Out-of-range index should still return empty string
1457+
v = builder->addConstValue(2);
1458+
v = builder->addListItem(localList3.get(), v);
1459+
builder->addFunctionCall("test_print_string", Compiler::StaticType::Void, { Compiler::StaticType::String }, { v });
1460+
14521461
static const std::string expected =
14531462
"3\n"
14541463
"1\n"
1455-
"0\n"
1456-
"0\n"
1464+
"\n"
1465+
"\n"
14571466
"Lorem\n"
14581467
"dolor\n"
14591468
"sit\n"
1469+
"\n"
1470+
"\n"
14601471
"0\n"
14611472
"0\n"
14621473
"0\n"
14631474
"0\n"
1464-
"0\n"
1465-
"0\n";
1475+
"\n";
14661476

14671477
auto code = builder->build();
14681478
Script script(&sprite, nullptr, nullptr);
@@ -1477,7 +1487,7 @@ TEST_F(LLVMCodeBuilderTest, GetListItem)
14771487
ASSERT_EQ(globalList->toString(), "1 test 3");
14781488
ASSERT_EQ(localList1->toString(), "Lorem ipsum dolor sit");
14791489
ASSERT_EQ(localList2->toString(), "-564.121 4257.4");
1480-
ASSERT_EQ(localList3->toString(), "true false");
1490+
ASSERT_EQ(localList3->toString(), "1.5");
14811491
}
14821492

14831493
TEST_F(LLVMCodeBuilderTest, GetListSize)

0 commit comments

Comments
 (0)