Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Control Flow type Refinements for "break" and "continue" statements #1004

Merged
merged 7 commits into from
Sep 21, 2023
Merged

Support Control Flow type Refinements for "break" and "continue" statements #1004

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1364b16
Add feature flag and test cases for loop CFA
AmberGraceSoftware Aug 6, 2023
a309ebb
Accurately enumerate all control flow states
AmberGraceSoftware Aug 6, 2023
4f80629
Enable both control flow flags on new tests
AmberGraceSoftware Aug 6, 2023
ccef633
Test CFA for mixed loop+function exit edge cases
AmberGraceSoftware Aug 6, 2023
8c486b8
Formatting.
alexmccord Sep 20, 2023
94dc9d4
Merge branch 'master' of https://github.com/Roblox/luau into pr1004
alexmccord Sep 20, 2023
5b953e0
Fold in my review.
alexmccord Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions Analysis/include/Luau/ControlFlow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,24 @@ enum class ControlFlow
None = 0b00001,
Returns = 0b00010,
Throws = 0b00100,
Break = 0b01000, // Currently unused.
Continue = 0b10000, // Currently unused.
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,
alexmccord marked this conversation as resolved.
Show resolved Hide resolved
};
// 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)
{
Expand Down
38 changes: 28 additions & 10 deletions Analysis/src/ConstraintGraphBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ LUAU_FASTINT(LuauCheckRecursionLimit);
LUAU_FASTFLAG(DebugLuauLogSolverToJson);
LUAU_FASTFLAG(DebugLuauMagicTypes);
LUAU_FASTFLAG(LuauParseDeclareClassIndexer);
LUAU_FASTFLAG(LuauLoopControlFlowAnalysis);
LUAU_FASTFLAG(LuauFloorDivision);

namespace Luau
Expand Down Expand Up @@ -537,11 +538,10 @@ ControlFlow ConstraintGraphBuilder::visit(const ScopePtr& scope, AstStat* stat)
return visit(scope, s);
else if (auto s = stat->as<AstStatRepeat>())
return visit(scope, s);
else if (stat->is<AstStatBreak>() || stat->is<AstStatContinue>())
{
// Nothing
return ControlFlow::None; // TODO: ControlFlow::Break/Continue
}
else if (stat->is<AstStatBreak>())
return FFlag::LuauLoopControlFlowAnalysis ? ControlFlow::Breaks : ControlFlow::None;
else if (stat->is<AstStatContinue>())
return FFlag::LuauLoopControlFlowAnalysis ? ControlFlow::Continues : ControlFlow::None;
else if (auto r = stat->as<AstStatReturn>())
return visit(scope, r);
else if (auto e = stat->as<AstStatExpr>())
Expand Down Expand Up @@ -1067,20 +1067,38 @@ 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;

alexmccord marked this conversation as resolved.
Show resolved Hide resolved
ControlFlow thencf = visit(thenScope, ifStatement->thenbody);
ControlFlow elsecf = ControlFlow::None;
if (ifStatement->elsebody)
elsecf = visit(elseScope, ifStatement->elsebody);

if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && elsecf == ControlFlow::None)
if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None)
alexmccord marked this conversation as resolved.
Show resolved Hide resolved
scope->inheritRefinements(elseScope);
else if (thencf == ControlFlow::None && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows))
alexmccord marked this conversation as resolved.
Show resolved Hide resolved
scope->inheritRefinements(thenScope);

if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return ControlFlow::Returns;
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;
}
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)
Expand Down
38 changes: 28 additions & 10 deletions Analysis/src/TypeInfer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ LUAU_FASTFLAG(LuauInstantiateInSubtyping)
LUAU_FASTFLAGVARIABLE(LuauAllowIndexClassParameters, false)
LUAU_FASTFLAG(LuauOccursIsntAlwaysFailure)
LUAU_FASTFLAGVARIABLE(LuauTinyControlFlowAnalysis, false)
LUAU_FASTFLAGVARIABLE(LuauLoopControlFlowAnalysis, false)
LUAU_FASTFLAGVARIABLE(LuauVariadicOverloadFix, false)
LUAU_FASTFLAGVARIABLE(LuauAlwaysCommitInferencesOfFunctionCalls, false)
LUAU_FASTFLAG(LuauParseDeclareClassIndexer)
Expand Down Expand Up @@ -350,11 +351,10 @@ ControlFlow TypeChecker::check(const ScopePtr& scope, const AstStat& program)
return check(scope, *while_);
else if (auto repeat = program.as<AstStatRepeat>())
return check(scope, *repeat);
else if (program.is<AstStatBreak>() || program.is<AstStatContinue>())
{
// Nothing to do
return ControlFlow::None;
}
else if (program.is<AstStatBreak>())
return FFlag::LuauLoopControlFlowAnalysis ? ControlFlow::Breaks : ControlFlow::None;
else if (program.is<AstStatContinue>())
return FFlag::LuauLoopControlFlowAnalysis ? ControlFlow::Continues : ControlFlow::None;
else if (auto return_ = program.as<AstStatReturn>())
return check(scope, *return_);
else if (auto expr = program.as<AstStatExpr>())
Expand Down Expand Up @@ -747,20 +747,38 @@ 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, ControlFlow::Returns | ControlFlow::Throws) && elsecf == ControlFlow::None)
if (matches(thencf, guardClauseFlows) && elsecf == ControlFlow::None)
scope->inheritRefinements(elseScope);
else if (thencf == ControlFlow::None && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
else if (thencf == ControlFlow::None && matches(elsecf, guardClauseFlows))
scope->inheritRefinements(thenScope);

if (matches(thencf, ControlFlow::Returns | ControlFlow::Throws) && matches(elsecf, ControlFlow::Returns | ControlFlow::Throws))
return ControlFlow::Returns;
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;
}
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
{
Expand Down
Loading
Loading