Skip to content

Commit

Permalink
Improve support for static arrays and non-CTFEable opEquals
Browse files Browse the repository at this point in the history
The original intent of this commit was to avoid calling `opEquals`
on types that we were evaluating for optionality, as a simple `is`
comparison is enough. However, `is` was not used previously because
it triggers a deprecation in D: Doing `a is b` when `a` and `b`
are static array would compare their `.ptr`.

Re-writing `isOptional` and adding test yield some issues with static
array support, including compilation error. This is now working and
the library will throw a proper error if the length do not match.
  • Loading branch information
Geod24 committed Feb 4, 2024
1 parent f80478a commit 2e9eae6
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 7 deletions.
35 changes: 35 additions & 0 deletions source/configy/Exceptions.d
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,38 @@ public class ConstructionException : ConfigException
sink(this.next.message);
}
}

/// Thrown when an array read from config does not match a static array size
public class ArrayLengthException : ConfigException
{
private size_t actual;
private size_t expected;

/// Constructor
public this (size_t actual, size_t expected,
string path, string key, Mark position,
string file = __FILE__, size_t line = __LINE__)
@safe pure nothrow @nogc
{
assert(actual != expected);
this.actual = actual;
this.expected = expected;
super(path, key, position, file, line);
}

/// Format the message with or without colors
protected override void formatMessage (
scope SinkType sink, in FormatSpec!char spec)
const scope @trusted
{
import core.internal.string : unsignedToTempString;

char[20] buffer = void;
sink("Too ");
sink((this.actual > this.expected) ? "many" : "few");
sink(" entries for sequence: Expected ");
sink(unsignedToTempString(this.expected, buffer));
sink(", got ");
sink(unsignedToTempString(this.actual, buffer));
}
}
19 changes: 14 additions & 5 deletions source/configy/FieldRef.d
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,20 @@ package template FieldRef (alias T, string name, bool forceOptional = false)

/// Evaluates to `true` if this field is to be considered optional
/// (does not need to be present in the YAML document)
public enum Optional = forceOptional ||
hasUDA!(Ref, CAOptional) ||
is(immutable(Type) == immutable(bool)) ||
is(Type : SetInfo!FT, FT) ||
(Default != Type.init);
static if (forceOptional || hasUDA!(Ref, CAOptional))
public enum Optional = true;
// Booleans are always optional
else static if (is(immutable(Type) == immutable(bool)))
public enum Optional = true;
// A mandatory SetInfo would not make sense
else static if (is(Type : SetInfo!FT, FT))
public enum Optional = true;
// Use `is` to avoid calling `opEquals` which might not be CTFEable,
// except for static arrays as that triggers a deprecation warning.
else static if (is(Type : E[k], E, size_t k))
public enum Optional = (Default[] !is Type.init[]);
else
public enum Optional = (Default !is Type.init);
}

unittest
Expand Down
19 changes: 17 additions & 2 deletions source/configy/Read.d
Original file line number Diff line number Diff line change
Expand Up @@ -766,13 +766,28 @@ package FR.Type parseField (alias FR)
if (node.nodeID != NodeID.sequence)
throw new TypeConfigException(node, "sequence (array)", path);

typeof(return) validateLength (E[] res)
{
static if (is(FR.Type : E_[k], E_, size_t k))
{
if (res.length != k)
throw new ArrayLengthException(
res.length, k, path, null, node.startMark());
return res[0 .. k];
}
else
return res;
}

// We pass `E.init` as default value as it is not going to be used:
// Either there is something in the YAML document, and that will be
// converted, or `sequence` will not iterate.
return node.sequence.enumerate.map!(
return validateLength(
node.sequence.enumerate.map!(
kv => kv.value.parseField!(NestedFieldRef!(E, FR))(
format("%s[%s]", path, kv.index), E.init, ctx))
.array();
.array()
);
}
}
else
Expand Down
111 changes: 111 additions & 0 deletions source/configy/Test.d
Original file line number Diff line number Diff line change
Expand Up @@ -816,3 +816,114 @@ unittest
assert(v2.v2.fileVersion == 2);
assert(v2.v2.str == "hello world");
}

/// Don't call `opCmp` / `opEquals` as they might not be CTFEable
/// Also various tests around static arrays
unittest
{
static struct NonCTFEAble
{
int value;

public bool opEquals (const NonCTFEAble other) const scope
{
assert(0);
}

public bool opEquals (const ref NonCTFEAble other) const scope
{
assert(0);
}

public int opCmp (const NonCTFEAble other) const scope
{
assert(0);
}

public int opCmp (const ref NonCTFEAble other) const scope
{
assert(0);
}
}

static struct Config
{
NonCTFEAble fixed;
@Name("static") NonCTFEAble[3] static_;
NonCTFEAble[] dynamic;
}

auto c = parseConfigString!Config(`fixed:
value: 42
static:
- value: 84
- value: 126
- value: 168
dynamic:
- value: 420
- value: 840
`, "/dev/null");

assert(c.fixed.value == 42);
assert(c.static_[0].value == 84);
assert(c.static_[1].value == 126);
assert(c.static_[2].value == 168);
assert(c.dynamic.length == 2);
assert(c.dynamic[0].value == 420);
assert(c.dynamic[1].value == 840);

try parseConfigString!Config(`fixed:
value: 42
dynamic:
- value: 420
- value: 840
`, "/dev/null");
catch (ConfigException e)
assert(e.toString() == "/dev/null(0:0): static: Required key was not found in configuration or command line arguments");

try parseConfigString!Config(`fixed:
value: 42
static:
- value: 1
- value: 2
dynamic:
- value: 420
- value: 840
`, "/dev/null");
catch (ConfigException e)
assert(e.toString() == "/dev/null(3:2): static: Too few entries for sequence: Expected 3, got 2");

try parseConfigString!Config(`fixed:
value: 42
static:
- value: 1
- value: 2
- value: 3
- value: 4
dynamic:
- value: 420
- value: 840
`, "/dev/null");
catch (ConfigException e)
assert(e.toString() == "/dev/null(3:2): static: Too many entries for sequence: Expected 3, got 4");

// Check that optional static array work
static struct ConfigOpt
{
NonCTFEAble fixed;
@Name("static") NonCTFEAble[3] static_ = [
NonCTFEAble(69),
NonCTFEAble(70),
NonCTFEAble(71),
];
}

auto c1 = parseConfigString!ConfigOpt(`fixed:
value: 1100
`, "/dev/null");

assert(c1.fixed.value == 1100);
assert(c1.static_[0].value == 69);
assert(c1.static_[1].value == 70);
assert(c1.static_[2].value == 71);
}

0 comments on commit 2e9eae6

Please sign in to comment.