From 5b953e06812f7f541eb97b7a5cbde4912a234046 Mon Sep 17 00:00:00 2001 From: Alexander McCord Date: Thu, 21 Sep 2023 14:47:20 -0700 Subject: [PATCH] Fold in my review. --- Analysis/include/Luau/ControlFlow.h | 15 ------------- Analysis/src/ConstraintGraphBuilder.cpp | 30 ++++++------------------- Analysis/src/TypeInfer.cpp | 30 ++++++------------------- tests/TypeInfer.cfa.test.cpp | 12 +++++----- 4 files changed, 20 insertions(+), 67 deletions(-) diff --git a/Analysis/include/Luau/ControlFlow.h b/Analysis/include/Luau/ControlFlow.h index c51da707..82c0403c 100644 --- a/Analysis/include/Luau/ControlFlow.h +++ b/Analysis/include/Luau/ControlFlow.h @@ -16,22 +16,7 @@ enum class ControlFlow Throws = 0b00100, Breaks = 0b01000, Continues = 0b10000, - - // Returns OR Throws (e.g. if predicate then return else error() end) - MixedFunctionExit = 0b100000, - - // Breaks OR Continues (e.g. if predicate then break else continue end) - MixedLoopExit = 0b1000000, - - // Exits a loop OR function (e.g. if prediate then continue else return end) - MixedExit = 0b10000000, }; -// Bitmask of all control flows which exit the nearest function scope -#define FunctionExitControlFlows ControlFlow::Returns | ControlFlow::Throws | ControlFlow::MixedFunctionExit -// Bitmask of all control flows which exit the nearest loop scope -#define LoopExitControlFlows ControlFlow::Breaks | ControlFlow::Continues | ControlFlow::MixedLoopExit -// Bitmask of all control flows which exit the nearest function or loop scopes -#define ExitingControlFlows ControlFlow::Returns | ControlFlow::Throws | ControlFlow::Breaks | ControlFlow::Continues | ControlFlow::MixedFunctionExit | ControlFlow::MixedLoopExit | ControlFlow::MixedExit inline ControlFlow operator&(ControlFlow a, ControlFlow b) { diff --git a/Analysis/src/ConstraintGraphBuilder.cpp b/Analysis/src/ConstraintGraphBuilder.cpp index 3b15ec62..f9b0dbf8 100644 --- a/Analysis/src/ConstraintGraphBuilder.cpp +++ b/Analysis/src/ConstraintGraphBuilder.cpp @@ -1067,38 +1067,22 @@ ControlFlow ConstraintGraphBuilder::visit(const ScopePtr& scope, AstStatIf* ifSt ScopePtr elseScope = childScope(ifStatement->elsebody ? ifStatement->elsebody : ifStatement, scope); applyRefinements(elseScope, ifStatement->elseLocation.value_or(ifStatement->condition->location), refinementArena.negation(refinement)); - const ControlFlow guardClauseFlows = FFlag::LuauLoopControlFlowAnalysis ? ExitingControlFlows : ControlFlow::Returns | ControlFlow::Throws; - ControlFlow thencf = visit(thenScope, ifStatement->thenbody); ControlFlow elsecf = ControlFlow::None; if (ifStatement->elsebody) elsecf = visit(elseScope, ifStatement->elsebody); - if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None) + if (thencf != ControlFlow::None && elsecf == ControlFlow::None) scope->inheritRefinements(elseScope); - else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows)) + else if (thencf == ControlFlow::None && elsecf != ControlFlow::None) scope->inheritRefinements(thenScope); - if (FFlag::LuauLoopControlFlowAnalysis) - { - if (thencf == elsecf) - return thencf; - else if (matches(thencf, FunctionExitControlFlows) && matches(elsecf, FunctionExitControlFlows)) - return ControlFlow::MixedFunctionExit; - else if (matches(thencf, LoopExitControlFlows) && matches(elsecf, LoopExitControlFlows)) - return ControlFlow::MixedLoopExit; - else if (matches(thencf, ExitingControlFlows) && matches(elsecf, ExitingControlFlows)) - return ControlFlow::MixedExit; - else - return ControlFlow::None; - } + if (FFlag::LuauLoopControlFlowAnalysis && thencf == elsecf) + return thencf; + else if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws)) + return ControlFlow::Returns; else - { - if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws)) - return ControlFlow::Returns; - else - return ControlFlow::None; - } + return ControlFlow::None; } static bool occursCheck(TypeId needle, TypeId haystack) diff --git a/Analysis/src/TypeInfer.cpp b/Analysis/src/TypeInfer.cpp index 48adc292..a29b1e06 100644 --- a/Analysis/src/TypeInfer.cpp +++ b/Analysis/src/TypeInfer.cpp @@ -747,38 +747,22 @@ ControlFlow TypeChecker::check(const ScopePtr& scope, const AstStatIf& statement ScopePtr elseScope = childScope(scope, statement.elsebody ? statement.elsebody->location : statement.location); resolve(result.predicates, elseScope, false); - const ControlFlow guardClauseFlows = FFlag::LuauLoopControlFlowAnalysis ? ExitingControlFlows : ControlFlow::Returns | ControlFlow::Throws; - ControlFlow thencf = check(thenScope, *statement.thenbody); ControlFlow elsecf = ControlFlow::None; if (statement.elsebody) elsecf = check(elseScope, *statement.elsebody); - if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None) + if (thencf != ControlFlow::None && elsecf == ControlFlow::None) scope->inheritRefinements(elseScope); - else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows)) + else if (thencf == ControlFlow::None && elsecf != ControlFlow::None) scope->inheritRefinements(thenScope); - if (FFlag::LuauLoopControlFlowAnalysis) - { - if (thencf == elsecf) - return thencf; - else if (matches(thencf, FunctionExitControlFlows) && matches(elsecf, FunctionExitControlFlows)) - return ControlFlow::MixedFunctionExit; - else if (matches(thencf, LoopExitControlFlows) && matches(elsecf, LoopExitControlFlows)) - return ControlFlow::MixedLoopExit; - else if (matches(thencf, ExitingControlFlows) && matches(elsecf, ExitingControlFlows)) - return ControlFlow::MixedExit; - else - return ControlFlow::None; - } + if (FFlag::LuauLoopControlFlowAnalysis && thencf == elsecf) + return thencf; + else if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws)) + return ControlFlow::Returns; else - { - if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws)) - return ControlFlow::Returns; - else - return ControlFlow::None; - } + return ControlFlow::None; } else { diff --git a/tests/TypeInfer.cfa.test.cpp b/tests/TypeInfer.cfa.test.cpp index 6198d2fd..19700d2c 100644 --- a/tests/TypeInfer.cfa.test.cpp +++ b/tests/TypeInfer.cfa.test.cpp @@ -330,7 +330,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_break_elif_rand_break_elif_not_y_fa elseif math.random() > 0.5 then break elseif not recordY.value then - + end local foo = recordX.value @@ -360,7 +360,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_continue_elif_rand_continue_elif_no elseif math.random() > 0.5 then continue elseif not recordY.value then - + end local foo = recordX.value @@ -415,7 +415,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_break_elif_not_y_fallthrough_elif_n if not recordX.value then break elseif not recordY.value then - + elseif not recordZ.value then break end @@ -448,7 +448,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_continue_elif_not_y_fallthrough_eli if not recordX.value then continue elseif not recordY.value then - + elseif not recordZ.value then continue end @@ -483,7 +483,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_continue_elif_not_y_throw_elif_not_ elseif not recordY.value then error("Y value not defined") elseif not recordZ.value then - + end local foo = recordX.value @@ -514,7 +514,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_return_elif_not_y_fallthrough_elif_ if not recordX.value then return elseif not recordY.value then - + elseif not recordZ.value then break end