Skip to content

Commit 10df475

Browse files
authored
Merge pull request #409 from PhysShell/fix/408-sibling-locals
fix(roslyn): same-named locals in sibling scopes are no longer conflated (#408)
2 parents 6b6a0ab + f88fe63 commit 10df475

3 files changed

Lines changed: 337 additions & 7 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,13 @@ jobs:
818818
env:
819819
OWN_TIERB_REQUIRED: "1"
820820
run: python tests/test_extractor_deep_flow.py
821+
- name: Same-named locals in sibling scopes are not conflated (Own.NET#408)
822+
# The flow pass keys locals by name: an untracked local sharing a tracked one's name in a
823+
# sibling branch aborted the whole document in the core (internal OWN030), and in a later
824+
# block read as a false OWN002. REQUIRED here; skips cleanly in the offline Tier-A job.
825+
env:
826+
OWN_TIERB_REQUIRED: "1"
827+
run: python tests/test_extractor_sibling_locals.py
821828
- name: Guarded release is not a call-site handoff (INF-S2, #380)
822829
# A callee that releases its parameter only under a guard is a PARTIAL release, so
823830
# its call must not be lowered as the caller's `release` (the fabricated `must`),

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3597,6 +3597,63 @@ static int FlowNestingDepth(IReadOnlyList<object> nodes, int level = 0)
35973597
return deepest;
35983598
}
35993599

3600+
// #408: C# lets two DIFFERENT locals share a name when their scopes are disjoint (two branches, two
3601+
// blocks, a branch and a loop). The flow pass keys locals by NAME, so every reference to an untracked
3602+
// local named like a tracked one became an op on the tracked local: a `use` with no `acquire` on that
3603+
// path aborted the whole document in the core (internal OWN030 "undefined name", MessagePack-CSharp),
3604+
// and a `use` after the tracked local's release was a false OWN002. Map each scope that declares such
3605+
// an untracked same-named local (its block, or the foreach / for / using / switch / fixed statement it
3606+
// belongs to) to the names it hides; the lowering drops those names from `tracked` inside it. Scopes of
3607+
// two same-named locals never overlap (CS0128 / CS0136), so inside that scope the name can only mean
3608+
// the untracked local. Lambda and local-function parameters are not mapped.
3609+
static Dictionary<SyntaxNode, HashSet<string>>? ShadowingScopes(BlockSyntax body, HashSet<string> tracked,
3610+
HashSet<SyntaxNode> candidateDecls)
3611+
{
3612+
Dictionary<SyntaxNode, HashSet<string>>? map = null;
3613+
void Hide(SyntaxNode? scope, string name)
3614+
{
3615+
if (scope is null || !tracked.Contains(name)) return;
3616+
map ??= new Dictionary<SyntaxNode, HashSet<string>>();
3617+
if (!map.TryGetValue(scope, out var names))
3618+
map[scope] = names = new HashSet<string>(StringComparer.Ordinal);
3619+
names.Add(name);
3620+
}
3621+
static SyntaxNode? StatementScope(SyntaxNode? owner) => owner switch
3622+
{
3623+
SwitchSectionSyntax sec => sec.Parent, // a section's locals belong to the whole switch block
3624+
_ => owner,
3625+
};
3626+
foreach (var node in body.DescendantNodes())
3627+
switch (node)
3628+
{
3629+
case VariableDeclaratorSyntax d when !candidateDecls.Contains(d):
3630+
Hide(d.Parent?.Parent switch
3631+
{
3632+
LocalDeclarationStatementSyntax ld => StatementScope(ld.Parent),
3633+
{ } owner => owner, // for / using / fixed statement
3634+
null => null,
3635+
}, d.Identifier.Text);
3636+
break;
3637+
case ForEachStatementSyntax fe:
3638+
Hide(fe, fe.Identifier.Text);
3639+
break;
3640+
case SingleVariableDesignationSyntax sv: // `out var x`, `is T x`, deconstruction
3641+
Hide(StatementScope(sv.Ancestors().FirstOrDefault(a => a is BlockSyntax or SwitchSectionSyntax)),
3642+
sv.Identifier.Text);
3643+
break;
3644+
}
3645+
return map;
3646+
}
3647+
3648+
static HashSet<string> WithoutShadowed(SyntaxNode scope, HashSet<string> tracked)
3649+
{
3650+
if (FlowShadows.Hidden is not { } map || !map.TryGetValue(scope, out var hidden) || !tracked.Overlaps(hidden))
3651+
return tracked;
3652+
var narrowed = new HashSet<string>(tracked, StringComparer.Ordinal);
3653+
narrowed.ExceptWith(hidden);
3654+
return narrowed;
3655+
}
3656+
36003657
static List<object>? LowerFlowBody(BlockSyntax block, HashSet<string> tracked, SemanticModel model)
36013658
{
36023659
var nodes = new List<object>();
@@ -3617,6 +3674,8 @@ static bool LowerFlowStatements(IReadOnlyList<StatementSyntax> stmts, int start,
36173674
bool canEscape, List<object>? onThrow, List<object>? onReturn,
36183675
bool onThrowDefinite = false)
36193676
{
3677+
if (start == 0 && stmts.Count > 0 && stmts[0].Parent is { } scope)
3678+
tracked = WithoutShadowed(scope, tracked);
36203679
for (var i = start; i < stmts.Count; i++)
36213680
{
36223681
var st = stmts[i];
@@ -3685,6 +3744,7 @@ static bool LowerFlowStmt(StatementSyntax st, HashSet<string> tracked, SemanticM
36853744
// skipped anyway: bail the method as unmodelled instead.
36863745
if (!System.Runtime.CompilerServices.RuntimeHelpers.TryEnsureSufficientExecutionStack())
36873746
return false;
3747+
tracked = WithoutShadowed(st, tracked);
36883748
switch (st)
36893749
{
36903750
case BlockSyntax b:
@@ -7032,6 +7092,14 @@ when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol
70327092
}
70337093
}
70347094
var candidates = new HashSet<string>();
7095+
// #408: the DECLARATORS behind `candidates` — the flow pass keys locals by name, so a
7096+
// different local that merely shares a tracked name must be told apart by its declarator.
7097+
var candidateDecls = new HashSet<SyntaxNode>();
7098+
void AddCandidate(VariableDeclaratorSyntax d)
7099+
{
7100+
candidates.Add(d.Identifier.Text);
7101+
candidateDecls.Add(d);
7102+
}
70357103
var poolBuffers = new HashSet<string>(); // candidates that are ArrayPool<T> buffers
70367104
var usingMemoryOwners = new HashSet<string>(); // `using`-declared MemoryPool owners
70377105
var newedDisposables = new HashSet<string>(); // candidates created via `new` (NOT a pool rental / factory)
@@ -7045,7 +7113,7 @@ when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol
70457113
foreach (var v in ld.Declaration.Variables)
70467114
if (IsMemoryPoolRent(v.Initializer?.Value, model))
70477115
{
7048-
candidates.Add(v.Identifier.Text);
7116+
AddCandidate(v);
70497117
usingMemoryOwners.Add(v.Identifier.Text);
70507118
}
70517119
continue;
@@ -7059,25 +7127,25 @@ or ImplicitObjectCreationExpressionSyntax } init
70597127
// resource -> never disposing a local of it cannot leak.
70607128
&& !HasEmptyDisposeBody(dt))
70617129
{
7062-
candidates.Add(v.Identifier.Text);
7130+
AddCandidate(v);
70637131
newedDisposables.Add(v.Identifier.Text);
70647132
}
70657133
else if (IsPoolRent(v.Initializer?.Value, model)) // an ArrayPool<T> buffer
70667134
{
7067-
candidates.Add(v.Identifier.Text);
7135+
AddCandidate(v);
70687136
poolBuffers.Add(v.Identifier.Text);
70697137
}
70707138
else if (IsOwningFactory(v.Initializer?.Value, model)) // File / crypto Create* factory
7071-
candidates.Add(v.Identifier.Text);
7139+
AddCandidate(v);
70727140
else if (IsMemoryPoolRent(v.Initializer?.Value, model)) // MemoryPool<T> IMemoryOwner (Dispose-released, NOT a poolBuffer)
7073-
candidates.Add(v.Identifier.Text);
7141+
AddCandidate(v);
70747142
else if (IsFirstPartyDisposableFactory(v.Initializer?.Value, model,
70757143
out _, out _))
70767144
// P-005 D5.2: `var r = FirstPartyFactory()` — a candidate acquire
70777145
// IFF the core proves the callee returns `fresh` (it emits a `call`
70787146
// op, not an `acquire`; the core decides). Checked last so `new` /
70797147
// pool / BCL-factory initializers keep their existing classification.
7080-
candidates.Add(v.Identifier.Text);
7148+
AddCandidate(v);
70817149
}
70827150
// OWN053 (promoted from ownership-semantics-lab H-29): the orphaned-awaitable sites of this body,
70837151
// always on under --flow-locals (the own-check default); the frozen scope is the detector's own comment.
@@ -7090,7 +7158,7 @@ or ImplicitObjectCreationExpressionSyntax } init
70907158
foreach (var v in usd.Variables)
70917159
if (IsMemoryPoolRent(v.Initializer?.Value, model))
70927160
{
7093-
candidates.Add(v.Identifier.Text);
7161+
AddCandidate(v);
70947162
usingMemoryOwners.Add(v.Identifier.Text);
70957163
}
70967164
// Eligibility, NOT a deleted guard. This is the gate that drops a method
@@ -7226,7 +7294,9 @@ or ImplicitObjectCreationExpressionSyntax } init
72267294
// shape `params[]` is paired with.
72277295
var flowNames = new HashSet<string>(tracked, StringComparer.Ordinal);
72287296
flowNames.UnionWith(ownedParamNames);
7297+
FlowShadows.Hidden = ShadowingScopes(mbody, flowNames, candidateDecls);
72297298
var fbody = LowerFlowBody(mbody, flowNames, model);
7299+
FlowShadows.Hidden = null;
72307300
if (fbody is null || fbody.Count == 0)
72317301
{
72327302
statMethodsSkipped++; // unmodelled construct -> honestly skipped
@@ -7447,3 +7517,10 @@ static class OrphanedAwaitables
74477517

74487518
internal readonly record struct SourceRange(SourcePos Start, SourcePos End);
74497519
}
7520+
7521+
// #408: the shadowing scopes of the method being lowered (see ShadowingScopes), ambient for the
7522+
// recursive lowering so its many signatures stay unchanged; null outside LowerFlowBody.
7523+
static class FlowShadows
7524+
{
7525+
[ThreadStatic] internal static Dictionary<SyntaxNode, HashSet<string>>? Hidden;
7526+
}

0 commit comments

Comments
 (0)