Merge pull request #656 from scratchcpp/refactor_numeric_string_constant_optimization

adazem009 · web-flow · commit 2094a7abacd9 · 2025-08-30T19:29:40.000+02:00
Refactor numeric string constant optimization
diff --git a/src/engine/internal/llvm/llvmbuildutils.cpp b/src/engine/internal/llvm/llvmbuildutils.cpp
@@ -5,6 +5,8 @@
 #include <scratchcpp/list.h>
 #include <scratchcpp/blockprototype.h>
 #include <scratchcpp/compiler.h>
+#include <scratchcpp/iengine.h>
+#include <scratchcpp/costume.h>
 
 #include "llvmbuildutils.h"
 #include "llvmfunctions.h"
@@ -40,6 +42,35 @@ LLVMBuildUtils::LLVMBuildUtils(LLVMCompilerContext *ctx, llvm::IRBuilder<> &buil
     initTypes();
     createVariableMap();
     createListMap();
+
+    // Find unsafe numeric string constants in costume and sound names
+    if (m_target) {
+        IEngine *engine = m_target->engine();
+
+        if (engine) {
+            auto checkConstant = [this](const std::string &str) {
+                Value stringValue(str);
+                Value numberValue(stringValue.toDouble());
+
+                if (stringValue.isValidNumber() && str == numberValue.toString())
+                    m_unsafeConstants.insert(str);
+            };
+
+            const auto &targets = engine->targets();
+            bool found = false;
+
+            for (const auto &target : targets) {
+                const auto &costumes = target->costumes();
+                const auto &sounds = target->sounds();
+
+                for (const auto &costume : costumes)
+                    checkConstant(costume->name());
+
+                for (const auto &sound : sounds)
+                    checkConstant(sound->name());
+            }
+        }
+    }
 }
 
 void LLVMBuildUtils::init(llvm::Function *function, BlockPrototype *procedurePrototype, bool warp, const std::vector<std::shared_ptr<LLVMRegister>> &regs)
@@ -423,13 +454,20 @@ std::vector<LLVMLoop> &LLVMBuildUtils::loops()
     return m_loops;
 }
 
-Compiler::StaticType LLVMBuildUtils::optimizeRegisterType(const LLVMRegister *reg)
+Compiler::StaticType LLVMBuildUtils::optimizeRegisterType(const LLVMRegister *reg) const
 {
     Compiler::StaticType ret = reg->type();
 
     // Optimize string constants that represent numbers
-    if (reg->isConst() && reg->type() == Compiler::StaticType::String && reg->constValue().isValidNumber())
-        ret = Compiler::StaticType::Number;
+    if (reg->isConst() && reg->type() == Compiler::StaticType::String) {
+        const Value &value = reg->constValue();
+        Value numberValue(value.toDouble());
+        std::string str = value.toString();
+
+        // Apply this optimization only if the number matches the string and the constant is safe
+        if (value.isValidNumber() && numberValue.toString() == str && m_unsafeConstants.find(str) == m_unsafeConstants.cend())
+            ret = Compiler::StaticType::Number;
+    }
 
     return ret;
 }
diff --git a/src/engine/internal/llvm/llvmbuildutils.h b/src/engine/internal/llvm/llvmbuildutils.h
@@ -82,7 +82,8 @@ class LLVMBuildUtils
         std::vector<LLVMIfStatement> &ifStatements();
         std::vector<LLVMLoop> &loops();
 
-        static Compiler::StaticType optimizeRegisterType(const LLVMRegister *reg);
+        Compiler::StaticType optimizeRegisterType(const LLVMRegister *reg) const;
+
         static Compiler::StaticType mapType(ValueType type);
         static ValueType mapType(Compiler::StaticType type);
         static bool isSingleType(Compiler::StaticType type);
@@ -173,6 +174,8 @@ class LLVMBuildUtils
         bool m_warp = false;
         Compiler::CodeType m_codeType = Compiler::CodeType::Script;
 
+        std::unordered_set<std::string> m_unsafeConstants;
+
         llvm::Value *m_executionContextPtr = nullptr;
         llvm::Value *m_targetPtr = nullptr;
         llvm::Value *m_targetVariables = nullptr;
diff --git a/src/engine/internal/llvm/llvmcodeanalyzer.cpp b/src/engine/internal/llvm/llvmcodeanalyzer.cpp
@@ -12,6 +12,11 @@ static const std::unordered_set<LLVMInstruction::Type> BEGIN_LOOP_INSTRUCTIONS =
 
 static const std::unordered_set<LLVMInstruction::Type> LIST_WRITE_INSTRUCTIONS = { LLVMInstruction::Type::AppendToList, LLVMInstruction::Type::InsertToList, LLVMInstruction::Type::ListReplace };
 
+LLVMCodeAnalyzer::LLVMCodeAnalyzer(const LLVMBuildUtils &utils) :
+    m_utils(utils)
+{
+}
+
 void LLVMCodeAnalyzer::analyzeScript(const LLVMInstructionList &script) const
 {
     std::unordered_set<LLVMInstruction *> typeAssignedInstructions;
@@ -289,5 +294,5 @@ Compiler::StaticType LLVMCodeAnalyzer::writeType(LLVMInstruction *ins) const
         }
     }
 
-    return LLVMBuildUtils::optimizeRegisterType(argReg);
+    return m_utils.optimizeRegisterType(argReg);
 }
diff --git a/src/engine/internal/llvm/llvmcodeanalyzer.h b/src/engine/internal/llvm/llvmcodeanalyzer.h
@@ -8,12 +8,16 @@
 namespace libscratchcpp
 {
 
+class LLVMBuildUtils;
 class LLVMInstructionList;
 class LLVMInstruction;
 
 class LLVMCodeAnalyzer
 {
     public:
+        LLVMCodeAnalyzer(const LLVMBuildUtils &utils);
+        LLVMCodeAnalyzer(const LLVMCodeAnalyzer &) = delete;
+
         void analyzeScript(const LLVMInstructionList &script) const;
 
     private:
@@ -50,6 +54,8 @@ class LLVMCodeAnalyzer
         bool isProcedureCall(const LLVMInstruction *ins) const;
 
         Compiler::StaticType writeType(LLVMInstruction *ins) const;
+
+        const LLVMBuildUtils &m_utils;
 };
 
 } // namespace libscratchcpp
diff --git a/src/engine/internal/llvm/llvmcodebuilder.cpp b/src/engine/internal/llvm/llvmcodebuilder.cpp
@@ -28,6 +28,7 @@ LLVMCodeBuilder::LLVMCodeBuilder(LLVMCompilerContext *ctx, BlockPrototype *proce
     m_module(ctx->module()),
     m_builder(m_llvmCtx),
     m_utils(ctx, m_builder, codeType),
+    m_codeAnalyzer(m_utils),
     m_procedurePrototype(procedurePrototype),
     m_defaultWarp(procedurePrototype ? procedurePrototype->warp() : false),
     m_warp(m_defaultWarp),
diff --git a/test/blocks/control_blocks_test.cpp b/test/blocks/control_blocks_test.cpp
@@ -23,6 +23,7 @@ using namespace libscratchcpp;
 using namespace libscratchcpp::test;
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::SaveArg;
 using ::testing::_;
 
@@ -35,6 +36,8 @@ class ControlBlocksTest : public testing::Test
             m_engine = m_project.engine().get();
             m_extension->registerBlocks(m_engine);
             registerBlocks(m_engine, m_extension.get());
+
+            EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));
         }
 
         std::unique_ptr<IExtension> m_extension;
diff --git a/test/blocks/event_blocks_test.cpp b/test/blocks/event_blocks_test.cpp
@@ -20,6 +20,7 @@ using namespace libscratchcpp;
 using namespace libscratchcpp::test;
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 
 class EventBlocksTest : public testing::Test
 {
@@ -29,6 +30,8 @@ class EventBlocksTest : public testing::Test
             m_extension = std::make_unique<EventBlocks>();
             m_engine = m_project.engine().get();
             m_extension->registerBlocks(m_engine);
+
+            EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));
         }
 
         std::unique_ptr<IExtension> m_extension;
diff --git a/test/blocks/looks_blocks_test.cpp b/test/blocks/looks_blocks_test.cpp
@@ -23,6 +23,7 @@ using namespace libscratchcpp;
 using namespace libscratchcpp::test;
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::ReturnArg;
 using ::testing::_;
 
@@ -36,6 +37,8 @@ class LooksBlocksTest : public testing::Test
             m_extension->registerBlocks(m_engine);
             m_extension->onInit(m_engine);
 
+            EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));
+
             // Create and register fake graphic effects
             auto colorEffect = std::make_shared<GraphicsEffectMock>();
             auto fisheyeEffect = std::make_shared<GraphicsEffectMock>();
diff --git a/test/blocks/motion_blocks_test.cpp b/test/blocks/motion_blocks_test.cpp
@@ -20,6 +20,7 @@ using namespace libscratchcpp;
 using namespace libscratchcpp::test;
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 
 class MotionBlocksTest : public testing::Test
 {
@@ -29,6 +30,8 @@ class MotionBlocksTest : public testing::Test
             m_extension = std::make_unique<MotionBlocks>();
             m_engine = m_project.engine().get();
             m_extension->registerBlocks(m_engine);
+
+            EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));
         }
 
         std::unique_ptr<IExtension> m_extension;
diff --git a/test/blocks/sensing_blocks_test.cpp b/test/blocks/sensing_blocks_test.cpp
@@ -47,6 +47,8 @@ class SensingBlocksTest : public testing::Test
             EXPECT_CALL(m_audioInput, audioLoudness()).WillRepeatedly(Return(m_audioLoudness));
 
             SensingBlocks::clock = &m_clock;
+
+            EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));
         }
 
         void TearDown() override
diff --git a/test/llvm/code_analyzer/list_type_analysis.cpp b/test/llvm/code_analyzer/list_type_analysis.cpp
diff --git a/test/llvm/code_analyzer/mixed_type_analysis.cpp b/test/llvm/code_analyzer/mixed_type_analysis.cpp
diff --git a/test/llvm/code_analyzer/variable_type_analysis.cpp b/test/llvm/code_analyzer/variable_type_analysis.cpp
diff --git a/test/llvm/llvmtestutils.cpp b/test/llvm/llvmtestutils.cpp
diff --git a/test/llvm/llvmtestutils.h b/test/llvm/llvmtestutils.h

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,11 @@ static const std::unordered_set<LLVMInstruction::Type> BEGIN_LOOP_INSTRUCTIONS =`
`12`	`12`
`13`	`13`	`static const std::unordered_set<LLVMInstruction::Type> LIST_WRITE_INSTRUCTIONS = { LLVMInstruction::Type::AppendToList, LLVMInstruction::Type::InsertToList, LLVMInstruction::Type::ListReplace };`
`14`	`14`
	`15`	`+LLVMCodeAnalyzer::LLVMCodeAnalyzer(const LLVMBuildUtils &utils) :`
	`16`	`+ m_utils(utils)`
	`17`	`+{`
	`18`	`+}`
	`19`	`+`
`15`	`20`	`void LLVMCodeAnalyzer::analyzeScript(const LLVMInstructionList &script) const`
`16`	`21`	`{`
`17`	`22`	`std::unordered_set<LLVMInstruction *> typeAssignedInstructions;`
`@@ -289,5 +294,5 @@ Compiler::StaticType LLVMCodeAnalyzer::writeType(LLVMInstruction *ins) const`
`289`	`294`	`}`
`290`	`295`	`}`
`291`	`296`
`292`		`- return LLVMBuildUtils::optimizeRegisterType(argReg);`
	`297`	`+ return m_utils.optimizeRegisterType(argReg);`
`293`	`298`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ using namespace libscratchcpp;`
`20`	`20`	`using namespace libscratchcpp::test;`
`21`	`21`
`22`	`22`	`using ::testing::Return;`
	`23`	`+using ::testing::ReturnRef;`
`23`	`24`
`24`	`25`	`class EventBlocksTest : public testing::Test`
`25`	`26`	`{`
`@@ -29,6 +30,8 @@ class EventBlocksTest : public testing::Test`
`29`	`30`	`m_extension = std::make_unique<EventBlocks>();`
`30`	`31`	`m_engine = m_project.engine().get();`
`31`	`32`	`m_extension->registerBlocks(m_engine);`
	`33`	`+`
	`34`	`+ EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));`
`32`	`35`	`}`
`33`	`36`
`34`	`37`	`std::unique_ptr<IExtension> m_extension;`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,8 @@ class SensingBlocksTest : public testing::Test`
`47`	`47`	`EXPECT_CALL(m_audioInput, audioLoudness()).WillRepeatedly(Return(m_audioLoudness));`
`48`	`48`
`49`	`49`	`SensingBlocks::clock = &m_clock;`
	`50`	`+`
	`51`	`+ EXPECT_CALL(m_engineMock, targets()).WillRepeatedly(ReturnRef(m_engine->targets()));`
`50`	`52`	`}`
`51`	`53`
`52`	`54`	`void TearDown() override`