Fold in my review.

This commit is contained in:
Alexander McCord 2023-09-21 14:47:20 -07:00
parent 94dc9d40c1
commit 5b953e0681
4 changed files with 20 additions and 67 deletions

View file

@ -16,22 +16,7 @@ enum class ControlFlow
Throws = 0b00100, Throws = 0b00100,
Breaks = 0b01000, Breaks = 0b01000,
Continues = 0b10000, 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) inline ControlFlow operator&(ControlFlow a, ControlFlow b)
{ {

View file

@ -1067,38 +1067,22 @@ ControlFlow ConstraintGraphBuilder::visit(const ScopePtr& scope, AstStatIf* ifSt
ScopePtr elseScope = childScope(ifStatement->elsebody ? ifStatement->elsebody : ifStatement, scope); ScopePtr elseScope = childScope(ifStatement->elsebody ? ifStatement->elsebody : ifStatement, scope);
applyRefinements(elseScope, ifStatement->elseLocation.value_or(ifStatement->condition->location), refinementArena.negation(refinement)); 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 thencf = visit(thenScope, ifStatement->thenbody);
ControlFlow elsecf = ControlFlow::None; ControlFlow elsecf = ControlFlow::None;
if (ifStatement->elsebody) if (ifStatement->elsebody)
elsecf = visit(elseScope, ifStatement->elsebody); elsecf = visit(elseScope, ifStatement->elsebody);
if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None) if (thencf != ControlFlow::None && elsecf == ControlFlow::None)
scope->inheritRefinements(elseScope); scope->inheritRefinements(elseScope);
else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows)) else if (thencf == ControlFlow::None && elsecf != ControlFlow::None)
scope->inheritRefinements(thenScope); scope->inheritRefinements(thenScope);
if (FFlag::LuauLoopControlFlowAnalysis) if (FFlag::LuauLoopControlFlowAnalysis && thencf == elsecf)
{ return thencf;
if (thencf == elsecf) else if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return thencf; return ControlFlow::Returns;
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;
}
else else
{ return ControlFlow::None;
if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return ControlFlow::Returns;
else
return ControlFlow::None;
}
} }
static bool occursCheck(TypeId needle, TypeId haystack) static bool occursCheck(TypeId needle, TypeId haystack)

View file

@ -747,38 +747,22 @@ ControlFlow TypeChecker::check(const ScopePtr& scope, const AstStatIf& statement
ScopePtr elseScope = childScope(scope, statement.elsebody ? statement.elsebody->location : statement.location); ScopePtr elseScope = childScope(scope, statement.elsebody ? statement.elsebody->location : statement.location);
resolve(result.predicates, elseScope, false); resolve(result.predicates, elseScope, false);
const ControlFlow guardClauseFlows = FFlag::LuauLoopControlFlowAnalysis ? ExitingControlFlows : ControlFlow::Returns | ControlFlow::Throws;
ControlFlow thencf = check(thenScope, *statement.thenbody); ControlFlow thencf = check(thenScope, *statement.thenbody);
ControlFlow elsecf = ControlFlow::None; ControlFlow elsecf = ControlFlow::None;
if (statement.elsebody) if (statement.elsebody)
elsecf = check(elseScope, *statement.elsebody); elsecf = check(elseScope, *statement.elsebody);
if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None) if (thencf != ControlFlow::None && elsecf == ControlFlow::None)
scope->inheritRefinements(elseScope); scope->inheritRefinements(elseScope);
else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows)) else if (thencf == ControlFlow::None && elsecf != ControlFlow::None)
scope->inheritRefinements(thenScope); scope->inheritRefinements(thenScope);
if (FFlag::LuauLoopControlFlowAnalysis) if (FFlag::LuauLoopControlFlowAnalysis && thencf == elsecf)
{ return thencf;
if (thencf == elsecf) else if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return thencf; return ControlFlow::Returns;
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;
}
else else
{ return ControlFlow::None;
if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return ControlFlow::Returns;
else
return ControlFlow::None;
}
} }
else else
{ {

View file

@ -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 elseif math.random() > 0.5 then
break break
elseif not recordY.value then elseif not recordY.value then
end end
local foo = recordX.value 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 elseif math.random() > 0.5 then
continue continue
elseif not recordY.value then elseif not recordY.value then
end end
local foo = recordX.value 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 if not recordX.value then
break break
elseif not recordY.value then elseif not recordY.value then
elseif not recordZ.value then elseif not recordZ.value then
break break
end end
@ -448,7 +448,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_continue_elif_not_y_fallthrough_eli
if not recordX.value then if not recordX.value then
continue continue
elseif not recordY.value then elseif not recordY.value then
elseif not recordZ.value then elseif not recordZ.value then
continue continue
end end
@ -483,7 +483,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "if_not_x_continue_elif_not_y_throw_elif_not_
elseif not recordY.value then elseif not recordY.value then
error("Y value not defined") error("Y value not defined")
elseif not recordZ.value then elseif not recordZ.value then
end end
local foo = recordX.value 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 if not recordX.value then
return return
elseif not recordY.value then elseif not recordY.value then
elseif not recordZ.value then elseif not recordZ.value then
break break
end end