Skip to content

Commit a27a827

Browse files
authored
Merge pull request #388 from PhysShell/fix/protocol-attribute-recognition
fix(roslyn): protocol attribute names reserved only in the profile's shapes; a token callback outside a region entry is refused
2 parents a08b733 + e0ec98b commit a27a827

12 files changed

Lines changed: 407 additions & 11 deletions

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

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
// [ProtocolToken] a ref struct: every instance METHOD is a transition that consumes the
1414
// token (and borrows the entity for the call); a PROPERTY is a read.
1515
//
16+
// RESERVED NAMES. Matching by name makes the two names a convention the profile owns, but only
17+
// in those shapes: a method marked [ProtocolRegion] that takes a delegate, a ref struct marked
18+
// [ProtocolToken]. An attribute of the same name on anything else (a class, a plain struct, a
19+
// method with no callback) belongs to somebody else's code and is ignored — a scan of a
20+
// codebase that declares no state protocol is never refused over a name.
21+
//
1622
// This is a LOWERING, not an analysis. It reads the syntax of one method at a time and looks
1723
// symbols up; it never tracks state across statements, never follows aliases, and never
1824
// decides whether a program is right. Everything it emits is checked by the core:
@@ -93,6 +99,9 @@ private sealed class Ctx
9399
public Dictionary<ISymbol, (string Name, Region Region)> Tokens { get; } =
94100
new(SymbolEqualityComparer.Default);
95101
public HashSet<SyntaxNode> LoweredEntries { get; } = new();
102+
/// The lambdas that were lowered as region bodies: the only callables in consumer
103+
/// code that may take a token.
104+
public HashSet<SyntaxNode> LoweredBodies { get; } = new();
96105
public int Counter;
97106
/// How many times the scan has lowered a mention of a borrowed entity. A construct
98107
/// that runs unknown code is refused UNLESS it moved this counter: then the core
@@ -118,6 +127,8 @@ public static Result Lower(Compilation compilation,
118127
var model = compilation.GetSemanticModel(tree);
119128
var root = tree.GetRoot();
120129
var loweredEntries = new HashSet<SyntaxNode>();
130+
var loweredBodies = new HashSet<SyntaxNode>();
131+
var refusedMethods = new List<SyntaxNode>();
121132

122133
foreach (var method in root.DescendantNodes().OfType<BaseMethodDeclarationSyntax>())
123134
{
@@ -189,6 +200,7 @@ public static Result Lower(Compilation compilation,
189200
throw new Refused(At(file, v,
190201
$"protocol token '{l.Name}' is declared outside a protocol region"));
191202
loweredEntries.UnionWith(ctx.LoweredEntries);
203+
loweredBodies.UnionWith(ctx.LoweredBodies);
192204

193205
if (ops.Count > 0)
194206
functions.Add(new Dictionary<string, object?>
@@ -199,9 +211,33 @@ public static Result Lower(Compilation compilation,
199211
catch (Refused r)
200212
{
201213
refusals.Add(r.Message);
214+
refusedMethods.Add(method);
202215
}
203216
}
204217

218+
// A token exists only inside a region. A method cannot take one (refused above),
219+
// and a lambda or local function may take one only as the in-place body of a
220+
// region entry that was lowered. Any other callable with a token parameter is
221+
// consumer code that receives a token some other way — from a method of the
222+
// protocol's own type that is not marked as a region entry, typically — and that
223+
// nothing reads: without this it is simply absent from the facts, and a token
224+
// spent twice in it is clean.
225+
foreach (var callable in root.DescendantNodes().Where(n =>
226+
n is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax))
227+
{
228+
if (loweredBodies.Contains(callable)
229+
|| refusedMethods.Any(m => m.Span.Contains(callable.Span)))
230+
continue; // a lowered region body, or inside a method already refused
231+
var symbol = callable is LocalFunctionStatementSyntax
232+
? model.GetDeclaredSymbol(callable) as IMethodSymbol
233+
: model.GetSymbolInfo(callable).Symbol as IMethodSymbol;
234+
var taken = symbol?.Parameters.FirstOrDefault(p => IsToken(p.Type));
235+
if (taken is null || Enclosing(model, callable).Any(IsApiType))
236+
continue; // no token; or the protocol's own types, which hand tokens out
237+
refusals.Add(At(file, callable,
238+
$"a callback that takes protocol token '{taken.Type.Name}' is not the in-place body of a region entry: a token exists only inside a region, and code that receives one any other way is not analysed"));
239+
}
240+
205241
// A region entry outside any method body (an accessor, a field initializer, a
206242
// top-level statement) is not modelled either.
207243
foreach (var inv in root.DescendantNodes().OfType<InvocationExpressionSyntax>())
@@ -481,7 +517,7 @@ bool Outside(INamedTypeSymbol? owner)
481517
? info.ConvertedType : info.Type ?? info.ConvertedType)?.OriginalDefinition;
482518
if (IsToken(made) && !Within(api))
483519
refusals.Add(At(file, node,
484-
$"a protocol token '{made!.Name}' is created outside the protocol's own types: a token comes only from a region entry or a transition"));
520+
$"a protocol token '{made!.Name}' is created outside the protocol's own types: a token comes only from a region entry or a transition" + ReservedNames));
485521
else if (node is BaseObjectCreationExpressionSyntax
486522
&& Bound(model.GetSymbolInfo(node)) is IMethodSymbol
487523
{ DeclaredAccessibility: not Accessibility.Public } ctor
@@ -583,11 +619,37 @@ private static bool HasAttribute(ISymbol? symbol, string name) =>
583619
symbol is not null && symbol.GetAttributes().Any(a =>
584620
a.AttributeClass?.Name == name || a.AttributeClass?.Name == name + "Attribute");
585621

586-
private static bool IsToken(ITypeSymbol? type) => HasAttribute(type, "ProtocolToken");
622+
// The two attributes are matched by NAME, so the names are reserved — but only in the
623+
// SHAPE the profile gives them. A codebase that never declared a state protocol can own an
624+
// attribute called ProtocolToken (a wire-protocol token class, say), and a name alone must
625+
// not turn its scan into a refusal. So:
626+
//
627+
// a state token is a REF STRUCT marked [ProtocolToken];
628+
// a region entry is a method marked [ProtocolRegion] that TAKES A DELEGATE.
629+
//
630+
// Anything else carrying one of the names is not part of a protocol and is left alone.
631+
// The shapes are deliberately the widest ones that still catch a protocol declared wrongly:
632+
// a marked region entry whose callback does not take a token, or takes a marked type that
633+
// is not a ref struct, is still a region entry and is refused where it is used.
634+
635+
/// Marked [ProtocolToken], whatever it is. Only for saying WHY a marked type is not a token.
636+
private static bool HasTokenName(ITypeSymbol? type) => HasAttribute(type, "ProtocolToken");
637+
638+
private static bool IsToken(ITypeSymbol? type) =>
639+
type is { IsRefLikeType: true } && HasTokenName(type);
640+
641+
private static bool IsRegionEntry(IMethodSymbol? method)
642+
{
643+
if (method is null)
644+
return false;
645+
var declared = (method.ReducedFrom ?? method).OriginalDefinition;
646+
// an UNRESOLVED parameter type may be the callback of a degraded scan: keep it in
647+
return HasAttribute(declared, "ProtocolRegion")
648+
&& declared.Parameters.Any(p => p.Type.TypeKind is TypeKind.Delegate or TypeKind.Error);
649+
}
587650

588-
private static bool IsRegionEntry(IMethodSymbol? method) =>
589-
method is not null && HasAttribute((method.ReducedFrom ?? method).OriginalDefinition,
590-
"ProtocolRegion");
651+
private const string ReservedNames =
652+
" (the attribute names ProtocolRegion and ProtocolToken are reserved in these shapes: a method marked [ProtocolRegion] that takes a delegate is a region entry, a ref struct marked [ProtocolToken] is a state token)";
591653

592654
private static bool IsApiType(INamedTypeSymbol? type) =>
593655
type is not null && (IsToken(type)
@@ -759,7 +821,7 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
759821
{
760822
var args = entry.ArgumentList.Arguments;
761823
if (args.Count != 2)
762-
throw Refuse(ctx, entry, "a protocol region entry takes (entity, callback)");
824+
throw Refuse(ctx, entry, "a protocol region entry takes (entity, callback)" + ReservedNames);
763825
var entityExpr = Unparen(args[0].Expression);
764826
var entity = ctx.Model.GetSymbolInfo(entityExpr).Symbol;
765827
if (entityExpr is not IdentifierNameSyntax || entity is not (ILocalSymbol or IParameterSymbol))
@@ -779,9 +841,9 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
779841
};
780842
if (parameter is null
781843
|| ctx.Model.GetDeclaredSymbol(parameter) is not IParameterSymbol token
782-
|| !IsToken(token.Type))
844+
|| !HasTokenName(token.Type))
783845
throw Refuse(ctx, entry,
784-
"the callback of a protocol region takes exactly one protocol token");
846+
"the callback of a protocol region takes exactly one protocol token" + ReservedNames);
785847
// A token that is not a ref struct can be captured by a nested lambda, stored in a
786848
// field, or carried across an await — every one of which lets it reach ANOTHER
787849
// region, the shape the core cannot check. The language forbids all three for a
@@ -831,6 +893,7 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
831893
if (existing is null)
832894
ops.Add(Release(owner, line));
833895
ctx.LoweredEntries.Add(entry);
896+
ctx.LoweredBodies.Add(lambda);
834897
}
835898

836899
private static void LowerTokenLocal(Ctx ctx, ILocalSymbol local, ExpressionSyntax? init,

‎frontend/roslyn/README.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,18 @@ SQL, another process. Those belong to concurrency tokens, constraints and
118118
transactions (two of them are pinned as stated limits under
119119
`protocol-samples/efcore/known-gaps`).
120120

121+
**Reserved names.** Matching by name makes `ProtocolToken` and `ProtocolRegion`
122+
names the profile owns — but only in its own shapes: a **ref struct** marked
123+
`[ProtocolToken]` is a state token, a method marked `[ProtocolRegion]` **that
124+
takes a delegate** is a region entry. An attribute with one of these names on
125+
anything else (a class, a plain struct, a record, a method with no callback)
126+
belongs to somebody else's code and is ignored, so a codebase that declares no
127+
state protocol is not refused over a name. The two shapes themselves stay
128+
reserved: an unrelated attribute of the same name on a ref struct, or on a method
129+
that takes a delegate, is read as a protocol declared wrongly and refused — the
130+
profile cannot tell the two apart, and a mis-declared protocol must not go
131+
unanalysed.
132+
121133
**The trust boundary.** The types that DECLARE a protocol — the tokens, and the
122134
type holding a region entry — are its trusted definition surface: they construct
123135
tokens, enter regions and implement transitions, and their bodies are not
@@ -148,6 +160,10 @@ not skipped: the extractor exits `2` and writes no facts, for the whole scan.
148160
- *Trust boundary.* A region opened inside a type that declares the protocol
149161
(see above): write the code that uses a protocol outside the types that
150162
declare it.
163+
- *Tokens stay in regions.* A lambda or local function that takes a token must be
164+
the in-place body of a region entry. A token handed to a callback any other
165+
way — by a method of the protocol's own type that is not marked
166+
`[ProtocolRegion]`, for one — reaches consumer code nobody lowered.
151167
- *Binding.* The scan does not read `obj/`, so usings a project only gets
152168
**implicitly** are not there. A region whose entity does not bind is refused;
153169
write the usings out in the files that open regions (or qualify the names).
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
using System;
2+
using Own.Protocols.Sample;
3+
4+
namespace Own.Protocols.Cases;
5+
6+
// A token exists only inside a region. Here the protocol's own type hands one out through a
7+
// method that is NOT marked as a region entry, so the consumer's callback is not a region
8+
// body: nothing lowers it, and before this was refused it was simply absent from the facts —
9+
// a token spent twice, clean. The declaring type is trusted; the callback is consumer code,
10+
// and consumer code that receives a token is either a lowered region or a refusal.
11+
public sealed class Letter
12+
{
13+
public bool Sealed { get; private set; }
14+
15+
internal void MarkSealed() => Sealed = true;
16+
}
17+
18+
[ProtocolToken]
19+
public readonly ref struct OpenLetter
20+
{
21+
private readonly Letter _letter;
22+
internal OpenLetter(Letter letter) => _letter = letter;
23+
24+
public void Seal() => _letter.MarkSealed();
25+
}
26+
27+
public delegate void OpenLetterRegion(OpenLetter open);
28+
29+
public static class LetterProtocol
30+
{
31+
[ProtocolRegion]
32+
public static void WithOpen(Letter letter, OpenLetterRegion body) => body(new OpenLetter(letter));
33+
34+
// the same hand-out, without the mark
35+
public static void Peek(Letter letter, OpenLetterRegion body) => body(new OpenLetter(letter));
36+
}
37+
38+
public static class R16TokenHandedOutPastARegionEntry
39+
{
40+
public static void Run(Letter letter)
41+
{
42+
LetterProtocol.Peek(letter, open =>
43+
{
44+
open.Seal();
45+
open.Seal(); // a stale token: OWN002 inside a region
46+
});
47+
}
48+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
using System;
2+
using Own.Protocols.Sample;
3+
4+
namespace Own.Protocols.Cases;
5+
6+
// The same escape with no lambda at all: a LOCAL FUNCTION that takes the token, passed as a
7+
// method group to a hand-out that is not a region entry. A method may not take a token; a
8+
// local function is a method the consumer can write inside a handler.
9+
public sealed class Crate
10+
{
11+
public bool Shipped { get; private set; }
12+
13+
internal void MarkShipped() => Shipped = true;
14+
}
15+
16+
[ProtocolToken]
17+
public readonly ref struct PackedCrate
18+
{
19+
private readonly Crate _crate;
20+
internal PackedCrate(Crate crate) => _crate = crate;
21+
22+
public void Ship() => _crate.MarkShipped();
23+
}
24+
25+
public delegate void PackedCrateRegion(PackedCrate packed);
26+
27+
public static class CrateProtocol
28+
{
29+
[ProtocolRegion]
30+
public static void WithPacked(Crate crate, PackedCrateRegion body) => body(new PackedCrate(crate));
31+
32+
public static void Borrow(Crate crate, PackedCrateRegion body) => body(new PackedCrate(crate));
33+
}
34+
35+
public static class R17TokenTakenByALocalFunction
36+
{
37+
public static void Run(Crate crate)
38+
{
39+
void Twice(PackedCrate packed)
40+
{
41+
packed.Ship();
42+
packed.Ship();
43+
}
44+
45+
CrateProtocol.Borrow(crate, Twice);
46+
}
47+
}

‎frontend/roslyn/protocol-samples/refused/expected.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
"R13_public_mutator_reaches_protocol_state": "'Doc.Publish' is public and writes 'Published', state the protocol's transitions own",
66
"R14_public_setter_of_protocol_state": "'Ticket.State' is public and writes 'State', state the protocol's transitions own",
77
"R15_region_opened_inside_protocol_type": "a protocol region is opened inside 'ParcelHandlers', one of the protocol's own types",
8+
"R16_token_handed_out_past_a_region_entry": "a callback that takes protocol token 'OpenLetter' is not the in-place body of a region entry",
9+
"R17_token_taken_by_a_local_function": "a callback that takes protocol token 'PackedCrate' is not the in-place body of a region entry",
810
"R1_default_token": "has no lowerable origin",
911
"R2_token_outside_region": "is declared outside a protocol region",
1012
"R3_return_inside_region": "`return` inside a protocol region",
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
namespace Net.Wire;
2+
3+
// [ProtocolToken] on everything a state token is NOT: a class, a plain struct, a record, an
4+
// enum, an interface. They are created, copied and passed around like any other value. A
5+
// state token is a REF STRUCT, so none of these is one, and the scan has nothing to say.
6+
[ProtocolToken]
7+
public sealed class SessionToken
8+
{
9+
public string Value = "";
10+
}
11+
12+
[ProtocolToken]
13+
public struct Ticket
14+
{
15+
public int Id;
16+
}
17+
18+
[ProtocolToken]
19+
public sealed record Hello(int Version);
20+
21+
[ProtocolToken]
22+
public enum FrameKind { Data, Control }
23+
24+
[ProtocolToken]
25+
public interface IFrame
26+
{
27+
int Size { get; }
28+
}
29+
30+
public static class U1TokenNameOnOrdinaryTypes
31+
{
32+
public static int Read(Ticket ticket) => ticket.Id;
33+
34+
public static int Run(FrameKind kind)
35+
{
36+
var session = new SessionToken { Value = "x" };
37+
var hello = new Hello((int)kind);
38+
var copy = new Ticket { Id = hello.Version };
39+
var again = copy;
40+
return Read(again) + session.Value.Length;
41+
}
42+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
namespace Net.Wire;
2+
3+
// [ProtocolRegion] on methods that take no delegate: there is no callback, so there is no
4+
// region body and nothing a region entry could mean. Called from their own class and from
5+
// another one, as a statement and inside an expression.
6+
public static class Frames
7+
{
8+
[ProtocolRegion]
9+
public static int Parse(byte[] frame) => frame.Length;
10+
11+
[ProtocolRegion]
12+
public static void Reset(byte[] frame, int offset)
13+
{
14+
frame[offset] = 0;
15+
}
16+
17+
public static int Own() => Parse(new byte[4]);
18+
}
19+
20+
public static class U2RegionNameWithoutACallback
21+
{
22+
public static int Run()
23+
{
24+
var frame = new byte[8];
25+
Frames.Reset(frame, 0);
26+
return Frames.Parse(frame) + Frames.Own();
27+
}
28+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
using System;
2+
3+
namespace Net.Wire;
4+
5+
// THE RESIDUAL, pinned. [ProtocolRegion] on a method that DOES take a delegate has exactly the
6+
// shape of a region entry, and the profile cannot tell it from a protocol whose author forgot
7+
// to mark the token — which must stay a refusal, or a mis-declared protocol would go
8+
// unanalysed in silence. So this collision is still refused; the message says which names are
9+
// reserved and in which shapes.
10+
public static class Bytes
11+
{
12+
[ProtocolRegion]
13+
public static void Each(byte[] frame, Action<byte> body)
14+
{
15+
foreach (var b in frame)
16+
body(b);
17+
}
18+
}
19+
20+
public static class U3RegionNameWithACallback
21+
{
22+
public static int Run()
23+
{
24+
var frame = new byte[4];
25+
var sum = 0;
26+
Bytes.Each(frame, b => { sum += b; });
27+
return sum;
28+
}
29+
}

0 commit comments

Comments
 (0)