Skip to content

Commit

Permalink
Replace the "hidden" access operator with a public interface (#48)
Browse files Browse the repository at this point in the history
Expose the index of the currently stored value via the new `Index`
property and add `UnsafeGet()` methods that allow one to bypass the
index check and retrieve a value assuming it matches the stored value.

This direct access is now used in extension methods.

The previous approach would break if the access modifier on the
variant's interface was forced to be `internal` because conversion
operators must be `static` and `public`.

This change not only is robust to the access change but also allows
users low-level value access if needed.
  • Loading branch information
mknejp authored Nov 28, 2021
1 parent a8cbd97 commit 03889c6
Show file tree
Hide file tree
Showing 21 changed files with 2,090 additions and 2,004 deletions.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

205 changes: 113 additions & 92 deletions src/dotVariant.Generator.Test/samples/Variant-disposable.out.cs

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

128 changes: 70 additions & 58 deletions src/dotVariant.Generator.Test/samples/Variant-generic-class.out.cs

Large diffs are not rendered by default.

280 changes: 155 additions & 125 deletions src/dotVariant.Generator.Test/samples/Variant-generic-multiple.out.cs

Large diffs are not rendered by default.

128 changes: 70 additions & 58 deletions src/dotVariant.Generator.Test/samples/Variant-generic-notnull.out.cs

Large diffs are not rendered by default.

128 changes: 70 additions & 58 deletions src/dotVariant.Generator.Test/samples/Variant-generic-struct.out.cs

Large diffs are not rendered by default.

128 changes: 70 additions & 58 deletions src/dotVariant.Generator.Test/samples/Variant-generic-unbounded.out.cs

Large diffs are not rendered by default.

Large diffs are not rendered by default.

203 changes: 112 additions & 91 deletions src/dotVariant.Generator.Test/samples/Variant-public.out.cs

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

32 changes: 20 additions & 12 deletions src/dotVariant.Generator/templates/IObservable.scriban-cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
# Distributed under the Boost Software License, Version 1.0.
# (See accompanying file LICENSE.txt or copy at https://www.boost.org/LICENSE_1_0.txt)
~}}
{{~
func $get_n()
ret get_n "_variant"
end
func $get_value(param)
ret get_value param "_variant"
end
~}}
{{~ if Options.ExtensionClassNamespace ~}}
namespace {{ Options.ExtensionClassNamespace }}
{
Expand All @@ -28,8 +36,8 @@ namespace {{ Options.ExtensionClassNamespace }}
{{~ end ~}}
{
return global::System.Reactive.Linq.Observable.Select(
global::System.Reactive.Linq.Observable.Where(source, _variant => {{ get_n }} == {{ $p.Index }}),
_variant => {{ $p.Identifier }}({{ get_value $p }}));
global::System.Reactive.Linq.Observable.Where(source, _variant => {{ $get_n }} == {{ $p.Index }}),
_variant => {{ $p.Identifier }}({{ $get_value $p }}));
}
{{~ end ~}}

Expand All @@ -55,9 +63,9 @@ namespace {{ Options.ExtensionClassNamespace }}
{
return global::System.Reactive.Linq.Observable.Select(source, _variant =>
{
if ({{ get_n }} == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
return {{ $p.Identifier }}({{ get_value $p }});
return {{ $p.Identifier }}({{ $get_value $p }});
}
else
{
Expand Down Expand Up @@ -89,9 +97,9 @@ namespace {{ Options.ExtensionClassNamespace }}
{
return global::System.Reactive.Linq.Observable.Select(source, _variant =>
{
if ({{ get_n }} == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
return {{ $p.Identifier }}({{ get_value $p }});
return {{ $p.Identifier }}({{ $get_value $p }});
}
else
{
Expand Down Expand Up @@ -122,13 +130,13 @@ namespace {{ Options.ExtensionClassNamespace }}
{
return global::System.Reactive.Linq.Observable.Select(source, _variant =>
{
switch ({{ get_n }})
switch ({{ $get_n }})
{
case 0:
return {{ throw_empty_error "TResult" }};
{{~ for $p in Params ~}}
case {{ $p.Index }}:
return {{ $p.Identifier }}({{ get_value $p }});
return {{ $p.Identifier }}({{ $get_value $p }});
{{~ end ~}}
default:
return {{ throw_internal_error "TResult" }};
Expand Down Expand Up @@ -159,13 +167,13 @@ namespace {{ Options.ExtensionClassNamespace }}
{
return global::System.Reactive.Linq.Observable.Select(source, _variant =>
{
switch ({{ get_n }})
switch ({{ $get_n }})
{
case 0:
return _();
{{~ for $p in Params ~}}
case {{ $p.Index }}:
return {{ $p.Identifier }}({{ get_value $p }});
return {{ $p.Identifier }}({{ $get_value $p }});
{{~ end ~}}
default:
return {{ throw_internal_error "TResult" }};
Expand Down Expand Up @@ -345,7 +353,7 @@ namespace {{ Options.ExtensionClassNamespace }}

public void OnNext({{ Variant.QualifiedType }} _variant)
{
switch ({{ get_n }})
switch ({{ $get_n }})
{
case 0:
if (_accept0)
Expand All @@ -359,7 +367,7 @@ namespace {{ Options.ExtensionClassNamespace }}
break;
{{~ for $p in Params ~}}
case {{ $p.Index }}:
Subject{{ $p.Index }}.OnNext({{ get_value $p }});
Subject{{ $p.Index }}.OnNext({{ $get_value $p }});
break;
{{~ end ~}}
default:
Expand Down
73 changes: 42 additions & 31 deletions src/dotVariant.Generator/templates/Union.scriban-cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ for value types and still consumes *much* less memory than if
it had to store all values side-by-side.
## ~}}
{{~
func $get_n()
ret get_n "this"
end
func $storage(param)
#ret is_generic ? "_x._" + param.Index : "_x._" + param.Index + ".Value";
ret "_x._" + param.Index;
Expand Down Expand Up @@ -192,13 +195,19 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
{{~ end ~}}

private readonly Union _x;
private readonly byte _n;

/// <summary>
/// The 1-based index of the currently stored type,
/// counted left-ro-right from the <see cref="{{ cref Variant.QualifiedType }}.VariantOf()"/> parameter list.
/// <c>0</c> if the variant is empty.
/// </summary>
public readonly byte Index;

{{~ ## STORAGE CONSTRUCTORS ## ~}}
{{~ for $p in Params ~}}
public {{ Variant.Identifier }}({{ $p.Type }} {{ $p.Identifier }})
{
_n = {{ $p.Index }};
Index = {{ $p.Index }};
_x = new Union({{ $p.Identifier }});
}
{{~ end ~}}
Expand All @@ -210,7 +219,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// </summary>
public void Dispose()
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
break;
Expand All @@ -228,14 +237,6 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
}
{{~ end ~}}

{{~ ## INTERNAL ACCESS ## ~}}
public static explicit operator {{ discriminator }}(in {{ Variant.Type }} v)
=> ({{ discriminator }})v._n;
{{~ for $p in Params ~}}
public static explicit operator {{ accessor $p.Index $p.Type }}(in {{ Variant.Type }} v)
=> new {{ accessor $p.Index $p.Type }}(v.{{ $storage $p }});
{{~ end ~}}

/// <summary>
/// <see langword="true"/> if {{ Variant.Name }} was constructed without a value.
/// </summary>
Expand All @@ -246,7 +247,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// and never satisfy any matching attempts except for the wildcard <c>_</c> parameter.
/// </remarks>
{{~ end ~}}
public bool IsEmpty => _n == 0;
public bool IsEmpty => {{ $get_n }} == 0;

/// <summary>
/// The string representation of the stored value's type.
Expand All @@ -255,7 +256,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
{
get
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return "<empty>";
Expand All @@ -274,7 +275,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// </summary>
public override string ToString()
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return "";
Expand All @@ -294,7 +295,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
{
get
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return null;
Expand All @@ -310,11 +311,11 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}

public bool Equals(in {{ Variant.Type }} other)
{
if (_n != other._n)
if ({{ $get_n }} != {{ get_n "other" }})
{
return false;
}
switch (_n)
switch (Index)
{
case 0:
return true;
Expand All @@ -330,7 +331,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}

public override int GetHashCode()
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return 0;
Expand All @@ -349,6 +350,16 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
}

{{~ for $p in Params ~}}
/// <summary>
/// Retrieve the stored value assuming it is of type <see cref="{{ cref $p.Type }}"/>.
///
/// <b>Only call this if you have ensured that <c>Index == {{ $p.Index }}</c>,
/// otherwise the correctness of the returned value is not guaranteed,
/// nor that any value is returned at all.</b>
/// </summary>
public {{ $p.Type }} UnsafeGet({{ accessor $p }} _)
=> {{ $storage $p }};

{{~ ## UNION TryMatch ## ~}}
/// <summary>
/// Retrieve the value stored within {{ Variant.Name }} if it is of type <see cref="{{ cref $p.Type }}"/>.
Expand All @@ -357,8 +368,8 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <returns><see langword="true"/> if {{ Variant.Name }} contained a value of type <see cref="{{ cref $p.Type }}"/>.</returns>
public bool TryMatch({{ annotate_NotNullWhen $p }}out {{ $p.OutType }} {{ $p.Identifier }})
{
{{ $p.Identifier }} = _n == {{ $p.Index }} ? {{ $storage $p }} : default;
return _n == {{ $p.Index }};
{{ $p.Identifier }} = {{ $get_n }} == {{ $p.Index }} ? {{ $storage $p }} : default;
return {{ $get_n }} == {{ $p.Index }};
}

/// <summary>
Expand All @@ -369,7 +380,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"> is rethrown.</exception>
public bool TryMatch({{ action_type $p }} {{ $p.Identifier }})
{
if (_n == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
{{ $p.Identifier }}({{ $storage $p }});
return true;
Expand All @@ -386,7 +397,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.InvalidOperationException">{{ Variant.Name }} does not contain a value of type <see cref="{{ cref $p.Type }}"/>.</exception>
public void Match({{ annotate_NotNull $p }}out {{ $p.OutType }} {{ $p.Identifier }})
{
if (_n == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
{{ $p.Identifier }} = {{ $storage $p }};
return;
Expand All @@ -403,7 +414,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"/> is rethrown.</exception>
public void Match({{ action_type $p }} {{ $p.Identifier }})
{
if (_n == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
{{ $p.Identifier }}({{ $storage $p }});
return;
Expand All @@ -420,7 +431,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"/> or <paramref name="_"/> is rethrown.</exception>
public void Match({{ action_type $p }} {{ $p.Identifier }}, global::System.Action _)
{
if (_n == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
{{ $p.Identifier }}({{ $storage $p }});
}
Expand All @@ -440,7 +451,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"/> is rethrown.</exception>
public TResult Match<TResult>({{ func_type $p }} {{ $p.Identifier }})
{
if (_n == {{ $p.Index }})
if ({{ $get_n }} == {{ $p.Index }})
{
return {{ $p.Identifier }}({{ $storage $p }});
}
Expand All @@ -457,7 +468,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"/> or <paramref name="_"/> is rethrown.</exception>
public TResult Match<TResult>({{ func_type $p }} {{ $p.Identifier }}, TResult _)
{
return _n == {{ $p.Index }} ? {{ $p.Identifier }}({{ $storage $p }}) : _;
return {{ $get_n }} == {{ $p.Index }} ? {{ $p.Identifier }}({{ $storage $p }}) : _;
}

/// <summary>
Expand All @@ -469,7 +480,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from <paramref name="{{ $p.Name }}"/> or <paramref name="_"/> is rethrown.</exception>
public TResult Match<TResult>({{ func_type $p }} {{ $p.Identifier }}, global::System.Func<TResult> _)
{
return _n == {{ $p.Index }} ? {{ $p.Identifier }}({{ $storage $p }}) : _();
return {{ $get_n }} == {{ $p.Index }} ? {{ $p.Identifier }}({{ $storage $p }}) : _();
}
{{~ end ~}}

Expand All @@ -485,7 +496,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from a delegate is rethrown.</exception>
public void Visit({{ action_params }}, global::System.Action _)
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
_();
Expand Down Expand Up @@ -513,7 +524,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <exception cref="global::System.Exception">Any exception thrown from a delegate is rethrown.</exception>
public void Visit({{ action_params }})
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
{{ throw_empty_error }};
Expand Down Expand Up @@ -542,7 +553,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <typeparam name="TResult">The return type of all delegates, and by extension the return type of this function.</typeparam>
public TResult Visit<TResult>({{ func_params }}, global::System.Func<TResult> _)
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return _();
Expand All @@ -568,7 +579,7 @@ namespace dotVariant._G{{ if Variant.Namespace; "." + Variant.Namespace; end }}
/// <typeparam name="TResult">The return type of all delegates, and by extension the return type of this function.</typeparam>
public TResult Visit<TResult>({{ func_params }})
{
switch (_n)
switch ({{ $get_n }})
{
case 0:
return {{ throw_empty_error "TResult" }};
Expand Down
23 changes: 11 additions & 12 deletions src/dotVariant.Generator/templates/Variant.scriban-cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ namespace {{ Variant.Namespace }}
public {{ method_modifiers }}bool IsEmpty
=> _variant.IsEmpty;

/// <inheritdoc cref="{{ cref storage_type }}.Index"/>
[global::System.Diagnostics.DebuggerNonUserCode]
public {{ method_modifiers }}byte Index
=> _variant.Index;

{{~ ## VARIANT Equals ## ~}}
/// <inheritdoc/>
[global::System.Diagnostics.DebuggerNonUserCode]
Expand Down Expand Up @@ -111,6 +116,12 @@ namespace {{ Variant.Namespace }}
=> _variant.ToString();

{{~ for $p in Params ~}}
{{~ ## VARIANT UnsafeGet ## ~}}
/// <inheritdoc cref="{{ cref storage_type }}.UnsafeGet({{ cref (accessor $p) }})"/>
[global::System.Diagnostics.DebuggerNonUserCode]
public {{ method_modifiers }}{{ $p.Type }} UnsafeGet({{ accessor $p }} accessor)
=> _variant.UnsafeGet(accessor);

{{~ ## VARIANT TryMatch ## ~}}
/// <inheritdoc cref="{{ cref storage_type }}.TryMatch(out {{ cref $p.OutType }})"/>
[global::System.Diagnostics.DebuggerNonUserCode]
Expand Down Expand Up @@ -189,18 +200,6 @@ namespace {{ Variant.Namespace }}
#pragma warning restore 8604, 8625
}
}

{{~ ## INTERNAL ACCESS ## ~}}
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
[global::System.Diagnostics.DebuggerNonUserCode]
public static explicit operator {{ discriminator }}({{ Variant.Type }} v)
=> ({{ discriminator }})v._variant;
{{~ for $p in Params ~}}
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
[global::System.Diagnostics.DebuggerNonUserCode]
public static explicit operator {{ accessor $p.Index $p.Type }}({{ Variant.Type }} v)
=> ({{ accessor $p.Index $p.Type }})v._variant;
{{~ end ~}}
}
{{~ if Variant.Namespace ~}}
}
Expand Down
Loading

0 comments on commit 03889c6

Please sign in to comment.