Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

fix Issue 14439 - aa's keys, values, byKey, byValue not usable in @sa… #1644

Closed
WalterBright wants to merge 2 commits into from
Closed

fix Issue 14439 - aa's keys, values, byKey, byValue not usable in @sa… #1644

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db4d68e
fix Issue 14439 - aa's keys, values, byKey, byValue not usable in @sa…
WalterBright Sep 4, 2016
6bc34e6
Walter's version
WalterBright Oct 6, 2016
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
89 changes: 71 additions & 18 deletions src/object.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1923,12 +1923,13 @@ class __cpp_type_info_ptr
extern (C)
{
// from druntime/src/rt/aaA.d
private:

// size_t _aaLen(in void* p) pure nothrow @nogc;
private void* _aaGetY(void** paa, const TypeInfo_AssociativeArray ti, in size_t valuesize, in void* pkey) pure nothrow;
void* _aaGetY(void** paa, const TypeInfo_AssociativeArray ti, in size_t valuesize, in void* pkey) pure nothrow;
// inout(void)* _aaGetRvalueX(inout void* p, in TypeInfo keyti, in size_t valuesize, in void* pkey);
inout(void)[] _aaValues(inout void* p, in size_t keysize, in size_t valuesize, const TypeInfo tiValArray) pure nothrow;
inout(void)[] _aaKeys(inout void* p, in size_t keysize, const TypeInfo tiKeyArray) pure nothrow;
inout(void)[] _aaValues(inout void* p, in size_t keysize, in size_t valuesize, const TypeInfo tiValArray) pure nothrow @trusted;
inout(void)[] _aaKeys(inout void* p, in size_t keysize, const TypeInfo tiKeyArray) pure nothrow @trusted;
void* _aaRehash(void** pp, in TypeInfo keyti) pure nothrow;
void _aaClear(void* p) pure nothrow;

Expand All @@ -1938,7 +1939,13 @@ extern (C)
// alias _dg2_t = extern(D) int delegate(void*, void*);
// int _aaApply2(void* aa, size_t keysize, _dg2_t dg);

private struct AARange { void* impl; size_t idx; }
struct AARange
{
private:
this(void *impl) { this.impl = impl; }
void* impl;
size_t idx;
}
AARange _aaRange(void* aa) pure nothrow @nogc;
bool _aaRangeEmpty(AARange r) pure nothrow @nogc;
void* _aaRangeFrontKey(AARange r) pure nothrow @nogc;
Expand Down Expand Up @@ -2039,21 +2046,22 @@ V[K] dup(T : V[K], K, V)(T* aa)
return (*aa).dup;
}

auto byKey(T : V[K], K, V)(T aa) pure nothrow @nogc
auto byKey(T : V[K], K, V)(T aa) pure nothrow @nogc @trusted
{
import core.internal.traits : substInout;

static struct Result
{
AARange r;

pure nothrow @nogc:
pure nothrow @nogc @trusted:
Copy link

@schuetzm schuetzm Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing @trusted followed by : makes me nervous... IMO it's better to add it to each function signature individually.

Copy link
Contributor

@JackStouffer JackStouffer Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; besides the safety issue, I have run into over applied attribute issues in other PRs.

Copy link
Member Author

@WalterBright WalterBright Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the functions it applies to are all right there and one liners, this isn't an issue, and implementing it would add clutter.

Copy link
Contributor

@aG0aep6G aG0aep6G Sep 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, this is also applies to std.range.front for normal arrays when compiling in release mode without bounds checking, and that's marked as safe.

But that's -boundscheck=off's fault. It flat-out breaks @safe. An @safe/@trusted function may be unsafe with the switch, but it must be safe without it.

@property bool empty() { return _aaRangeEmpty(r); }
@property ref front() { return *cast(substInout!K*)_aaRangeFrontKey(r); }
void popFront() { _aaRangePopFront(r); }
@property ref front() { assert(!empty); return *cast(substInout!K*)_aaRangeFrontKey(r); }
void popFront() { assert(!empty); _aaRangePopFront(r); }
@property Result save() { return this; }
}

static assert(!__traits(compiles, __traits(getAliasThis, T)), "cannot have alias this of associative array");
return Result(_aaRange(cast(void*)aa));
}

Expand All @@ -2062,21 +2070,22 @@ auto byKey(T : V[K], K, V)(T* aa) pure nothrow @nogc
return (*aa).byKey();
}

auto byValue(T : V[K], K, V)(T aa) pure nothrow @nogc
auto byValue(T : V[K], K, V)(T aa) pure nothrow @nogc @trusted
{
import core.internal.traits : substInout;

static struct Result
{
AARange r;

pure nothrow @nogc:
pure nothrow @nogc @trusted:
Copy link
Contributor

@dnadlinger dnadlinger Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

int main() @safe {
  auto a = ["foo": 1];
  auto r = a.byValue;
  r.popFront;
  return r.front; // oops
}

Copy link
Contributor

@JackStouffer JackStouffer Sep 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, this is also applies to std.range.front for normal arrays when compiling in release mode without bounds checking, and that's marked as safe.

Copy link
Member Author

@WalterBright WalterBright Sep 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@property bool empty() { return _aaRangeEmpty(r); }
@property ref front() { return *cast(substInout!V*)_aaRangeFrontValue(r); }
void popFront() { _aaRangePopFront(r); }
@property ref front() { assert(!empty); return *cast(substInout!V*)_aaRangeFrontValue(r); }
void popFront() { assert(!empty); _aaRangePopFront(r); }
@property Result save() { return this; }
}

static assert(!__traits(compiles, __traits(getAliasThis, T)), "cannot have alias this of associative array");
return Result(_aaRange(cast(void*)aa));
}

Expand All @@ -2085,15 +2094,15 @@ auto byValue(T : V[K], K, V)(T* aa) pure nothrow @nogc
return (*aa).byValue();
}

auto byKeyValue(T : V[K], K, V)(T aa) pure nothrow @nogc
auto byKeyValue(T : V[K], K, V)(T aa) pure nothrow @nogc @trusted
{
import core.internal.traits : substInout;

static struct Result
{
AARange r;

pure nothrow @nogc:
pure nothrow @nogc @trusted:
@property bool empty() { return _aaRangeEmpty(r); }
@property auto front() @trusted
{
Expand All @@ -2114,6 +2123,7 @@ auto byKeyValue(T : V[K], K, V)(T aa) pure nothrow @nogc
@property Result save() { return this; }
}

static assert(!__traits(compiles, __traits(getAliasThis, T)), "cannot have alias this of associative array");
return Result(_aaRange(cast(void*)aa));
}

Expand All @@ -2124,8 +2134,15 @@ auto byKeyValue(T : V[K], K, V)(T* aa) pure nothrow @nogc

Key[] keys(T : Value[Key], Value, Key)(T aa) @property
{
auto a = cast(void[])_aaKeys(cast(inout(void)*)aa, Key.sizeof, typeid(Key[]));
auto res = *cast(Key[]*)&a;
static assert(!__traits(compiles, __traits(getAliasThis, T)), "cannot have alias this of associative array");
void* trustedCastVoid(T aa) @trusted { return cast(void*)aa; }
void* p = trustedCastVoid(aa);

void[] a = _aaKeys(p, Key.sizeof, typeid(Key[]));

Key[] trustedCastKeyArray(ref void[] a) @trusted { return *cast(Key[]*)&a; }
auto res = trustedCastKeyArray(a);

_doPostblit(res);
return res;
}
Expand All @@ -2137,8 +2154,15 @@ Key[] keys(T : Value[Key], Value, Key)(T *aa) @property

Value[] values(T : Value[Key], Value, Key)(T aa) @property
{
auto a = cast(void[])_aaValues(cast(inout(void)*)aa, Key.sizeof, Value.sizeof, typeid(Value[]));
auto res = *cast(Value[]*)&a;
static assert(!__traits(compiles, __traits(getAliasThis, T)), "cannot have alias this of associative array");
void* trustedCastVoid(T aa) @trusted { return cast(void*)aa; }
void* p = trustedCastVoid(aa);

auto a = _aaValues(p, Key.sizeof, Value.sizeof, typeid(Value[]));

Value[] trustedCastValueArray(ref void[] a) @trusted { return *cast(Value[]*)&a; }
auto res = trustedCastValueArray(a);

_doPostblit(res);
return res;
}
Expand Down Expand Up @@ -2175,6 +2199,20 @@ unittest
assert(T.count == 2);
}

@safe unittest
{
string[string] saa = ["a" : "1", "b" : "2"];
string s = saa["a"];
saa["c"] = "3";
if ("c" in saa) {}
size_t l = saa.length;
foreach(k; saa.keys) {}
foreach(k; saa.byKey) {}
foreach(v; saa.values) {}
foreach(v; saa.byValue) {}
foreach(k, v; saa) {}
}

inout(V) get(K, V)(inout(V[K]) aa, K key, lazy inout(V) defaultValue)
{
auto p = key in aa;
Expand Down Expand Up @@ -2480,6 +2518,21 @@ unittest
static assert(is(typeof(caa.byValue().front) == const int));
}

unittest
{
static struct Foo {
int[int] aa;
auto opCast() pure nothrow @nogc { *cast(uint*)0xdeadbeef = 0xcafebabe; return null; }
alias aa this;
}

static void test() @safe {
Foo f;
static assert(!__traits(compiles, { int i = !f.byKey.empty; }));
}
}


private void _destructRecurse(S)(ref S s)
if (is(S == struct))
{
Expand Down