Jump to page: 1 2
Thread overview
why won't byPair work with a const AA?
Jul 30, 2017
Matthew Gamble
Jul 30, 2017
Ali Çehreli
Jul 30, 2017
Matthew Gamble
Jul 30, 2017
Ali Çehreli
Aug 01, 2017
H. S. Teoh
Aug 01, 2017
H. S. Teoh
Aug 01, 2017
H. S. Teoh
Aug 02, 2017
H. S. Teoh
Aug 02, 2017
H. S. Teoh
Aug 03, 2017
Olivier FAURE
Aug 02, 2017
H. S. Teoh
July 30, 2017
I have a class member function from which I'm trying to return a sorted array of key, value tuples stored in an associative array as a private member. The member function should be able to be marked const to prevent the AA from being modified. I have reduced the problem to the simple case below which won't compile with DMD v2.072.2.

import std.array;
import std.algorithm;

class A
{
	this() { aa = ["a":1, "b" : 2, "c" : 3]; }
	auto pairs() @property const { return aa.byPair.array.sort().release; }
private:
	int[string] aa;
}

If I remove const from the pairs function it compiles fine. I'm just not sure this is a behavior I want. Any help/recommendation would be appreciated.
July 29, 2017
On 07/29/2017 09:19 PM, Matthew Gamble wrote:
> I have a class member function from which I'm trying to return a sorted
> array of key, value tuples stored in an associative array as a private
> member. The member function should be able to be marked const to prevent
> the AA from being modified. I have reduced the problem to the simple
> case below which won't compile with DMD v2.072.2.
>
> import std.array;
> import std.algorithm;
>
> class A
> {
>     this() { aa = ["a":1, "b" : 2, "c" : 3]; }
>     auto pairs() @property const { return aa.byPair.array.sort().release; }
> private:
>     int[string] aa;
> }
>
> If I remove const from the pairs function it compiles fine. I'm just not
> sure this is a behavior I want. Any help/recommendation would be
> appreciated.

I think it should work. I think a cast to unqualified is a safe workaround in this case:

    auto pairs() @property const { return (cast(int[string])aa).byPair.array.sort().release; }

Ali

July 30, 2017
On Sunday, 30 July 2017 at 04:36:19 UTC, Ali Çehreli wrote:
> On 07/29/2017 09:19 PM, Matthew Gamble wrote:
>> I have a class member function from which I'm trying to return a sorted
>> array of key, value tuples stored in an associative array as a private
>> member. The member function should be able to be marked const to prevent
>> the AA from being modified. I have reduced the problem to the simple
>> case below which won't compile with DMD v2.072.2.
>>
>> import std.array;
>> import std.algorithm;
>>
>> class A
>> {
>>     this() { aa = ["a":1, "b" : 2, "c" : 3]; }
>>     auto pairs() @property const { return aa.byPair.array.sort().release; }
>> private:
>>     int[string] aa;
>> }
>>
>> If I remove const from the pairs function it compiles fine. I'm just not
>> sure this is a behavior I want. Any help/recommendation would be
>> appreciated.
>
> I think it should work. I think a cast to unqualified is a safe workaround in this case:
>
>     auto pairs() @property const { return (cast(int[string])aa).byPair.array.sort().release; }
>
> Ali

Thanks Ali,

That works to solve the compile problem. Seems the data in aa was safe from modification without the const or casting, even if the values are reference types (i.e. ClassB[string]). Is that expected?

Best,
Matt
July 29, 2017
On 07/29/2017 10:15 PM, Matthew Gamble wrote:

>> I think it should work. I think a cast to unqualified is a safe
>> workaround in this case:
>>
>>     auto pairs() @property const { return
>> (cast(int[string])aa).byPair.array.sort().release; }

As your question reveals, casting away const is not safe in general and not for this code. :-/

> That works to solve the compile problem. Seems the data in aa was safe
> from modification without the const or casting, even if the values are
> reference types (i.e. ClassB[string]). Is that expected?

No, they are not safe as you can mutate values in the AA:

import std.stdio;
import std.array;
import std.algorithm;

class B {
    int i;
    void mutate() {
        ++i;
    }
}

class A
{
    this() { aa = ["a" : new B(), "b" : new B(), "c" : new B()]; }
    auto pairs() @property const { return (cast(B[string])aa).byPair.array.sort().release; }
private:
    B[string] aa;
}

void main() {
    auto a = new A();
    auto p = a.pairs();
    p.front[1].mutate();
    assert(p.front[1].i == 1);    // <-- Mutated :(
}

Of course, the problem is the same without const and the cast.

Ali

August 01, 2017
On 7/30/17 12:19 AM, Matthew Gamble wrote:
> I have a class member function from which I'm trying to return a sorted array of key, value tuples stored in an associative array as a private member. The member function should be able to be marked const to prevent the AA from being modified. I have reduced the problem to the simple case below which won't compile with DMD v2.072.2.
> 
> import std.array;
> import std.algorithm;
> 
> class A
> {
>      this() { aa = ["a":1, "b" : 2, "c" : 3]; }
>      auto pairs() @property const { return aa.byPair.array.sort().release; }
> private:
>      int[string] aa;
> }
> 
> If I remove const from the pairs function it compiles fine. I'm just not sure this is a behavior I want. Any help/recommendation would be appreciated.

byPair must store a pointer to the data in the AA. If you mark the AA const, then it must store a const pointer to AA data.

However, because we don't have tail modifiers, you can't construct just one range that would make this work. We would need many different ranges, one for each flavor of mutability.

So you are stuck doing what Ali recommended -- cast away the const.

In this case, no harm comes if you don't cast back to const (since the value of the AA is `int`), but in general this solution isn't valid if the value type contains references.

-Steve
August 01, 2017
On Tue, Aug 01, 2017 at 10:04:18AM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
> On 7/30/17 12:19 AM, Matthew Gamble wrote:
[...]
> > import std.array;
> > import std.algorithm;
> > 
> > class A
> > {
> >      this() { aa = ["a":1, "b" : 2, "c" : 3]; }
> >      auto pairs() @property const { return
> > aa.byPair.array.sort().release; }
> > private:
> >      int[string] aa;
> > }
> > 
> > If I remove const from the pairs function it compiles fine. I'm just not sure this is a behavior I want. Any help/recommendation would be appreciated.
> 
> byPair must store a pointer to the data in the AA. If you mark the AA const, then it must store a const pointer to AA data.
[...]

Actually, there's nothing about the implementation of both byKeyValue (the underlying implementation in druntime) and byPair in std.array that would preclude them from being used with const AA's.  The only flaw is that the declaration of byPair doesn't match const AA's:

	https://issues.dlang.org/show_bug.cgi?id=17711

Here's the fix:

	https://github.com/dlang/phobos/pull/5668


T

-- 
Sometimes the best solution to morale problems is just to fire all of the unhappy people. -- despair.com
August 01, 2017
On 8/1/17 6:50 PM, H. S. Teoh via Digitalmars-d-learn wrote:
> On Tue, Aug 01, 2017 at 10:04:18AM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
>> On 7/30/17 12:19 AM, Matthew Gamble wrote:
> [...]
>>> import std.array;
>>> import std.algorithm;
>>>
>>> class A
>>> {
>>>       this() { aa = ["a":1, "b" : 2, "c" : 3]; }
>>>       auto pairs() @property const { return
>>> aa.byPair.array.sort().release; }
>>> private:
>>>       int[string] aa;
>>> }
>>>
>>> If I remove const from the pairs function it compiles fine. I'm just
>>> not sure this is a behavior I want. Any help/recommendation would be
>>> appreciated.
>>
>> byPair must store a pointer to the data in the AA. If you mark the AA
>> const, then it must store a const pointer to AA data.
> [...]
> 
> Actually, there's nothing about the implementation of both byKeyValue
> (the underlying implementation in druntime) and byPair in std.array that
> would preclude them from being used with const AA's.  The only flaw is
> that the declaration of byPair doesn't match const AA's:
> 
> 	https://issues.dlang.org/show_bug.cgi?id=17711
> 
> Here's the fix:
> 
> 	https://github.com/dlang/phobos/pull/5668

It works, because the byKeyValue implementation is so... ugly.

For instance this:

return Result(_aaRange(cast(void*)aa));

Just throws away all const/mutability. However, the Pair struct inside restores the correct modifiers. I hope...

If this were a true implementation without the opaqueness, it would not work properly.

-Steve
August 01, 2017
On Tue, Aug 01, 2017 at 07:09:45PM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
> On 8/1/17 6:50 PM, H. S. Teoh via Digitalmars-d-learn wrote:
[...]
> > Actually, there's nothing about the implementation of both byKeyValue (the underlying implementation in druntime) and byPair in std.array that would preclude them from being used with const AA's. The only flaw is that the declaration of byPair doesn't match const AA's:
> > 
> > 	https://issues.dlang.org/show_bug.cgi?id=17711
> > 
> > Here's the fix:
> > 
> > 	https://github.com/dlang/phobos/pull/5668
> 
> It works, because the byKeyValue implementation is so... ugly.
> 
> For instance this:
> 
> return Result(_aaRange(cast(void*)aa));
> 
> Just throws away all const/mutability. However, the Pair struct inside restores the correct modifiers. I hope...
> 
> If this were a true implementation without the opaqueness, it would not work properly.
[...]

Actually, a proper implementation would still work, provided you declare your pointer types carefully.  Sketch of idea:

	auto byKeyValue(AA)(AA aa) {
		struct Result {
			const(Slot)* current; // N.B.: proper type
			bool empty() { ... }
			auto front() { return Pair(*current); }
			void popFront() {
				current = current.next;
				...
			}
		}
		return Result(aa);
	}

Basically, the type of `current` must be const(Slot)* rather than const(Slot*), which would be the default inferred type. But since it's legal to assign a const pointer to a pointer to const (you can't modify the original pointer, nor what it points to, but it's valid to copy the pointer to a mutable pointer variable, as long as what is pointed to is still const), this actually will work without breaking / bypassing the type system.


T

-- 
People tell me that I'm skeptical, but I don't believe them.
August 01, 2017
On 8/1/17 7:15 PM, H. S. Teoh via Digitalmars-d-learn wrote:
> On Tue, Aug 01, 2017 at 07:09:45PM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
>> If this were a true implementation without the opaqueness, it would
>> not work properly.
> [...]
> 
> Actually, a proper implementation would still work, provided you declare
> your pointer types carefully.  Sketch of idea:
> 
> 	auto byKeyValue(AA)(AA aa) {
> 		struct Result {
> 			const(Slot)* current; // N.B.: proper type
> 			bool empty() { ... }
> 			auto front() { return Pair(*current); }
> 			void popFront() {
> 				current = current.next;
> 				...
> 			}
> 		}
> 		return Result(aa);
> 	}
> 
> Basically, the type of `current` must be const(Slot)* rather than
> const(Slot*), which would be the default inferred type. But since it's
> legal to assign a const pointer to a pointer to const (you can't modify
> the original pointer, nor what it points to, but it's valid to copy the
> pointer to a mutable pointer variable, as long as what is pointed to is
> still const), this actually will work without breaking / bypassing the
> type system.

No, you can't const the Slot, because if the value type is a pointer, you can't then cast away the const-ness of it legally.

There are ways to get it right, it involves templating for mutability.

The current code is pretty horrific though, any way you slice it.

-Steve
August 01, 2017
On Tue, Aug 01, 2017 at 07:31:41PM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
> On 8/1/17 7:15 PM, H. S. Teoh via Digitalmars-d-learn wrote:
> > On Tue, Aug 01, 2017 at 07:09:45PM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote:
> > > If this were a true implementation without the opaqueness, it would not work properly.
> > [...]
> > 
> > Actually, a proper implementation would still work, provided you declare your pointer types carefully.  Sketch of idea:
> > 
> > 	auto byKeyValue(AA)(AA aa) {
> > 		struct Result {
> > 			const(Slot)* current; // N.B.: proper type
> > 			bool empty() { ... }
> > 			auto front() { return Pair(*current); }
> > 			void popFront() {
> > 				current = current.next;
> > 				...
> > 			}
> > 		}
> > 		return Result(aa);
> > 	}
> > 
> > Basically, the type of `current` must be const(Slot)* rather than const(Slot*), which would be the default inferred type. But since it's legal to assign a const pointer to a pointer to const (you can't modify the original pointer, nor what it points to, but it's valid to copy the pointer to a mutable pointer variable, as long as what is pointed to is still const), this actually will work without breaking / bypassing the type system.
> 
> No, you can't const the Slot, because if the value type is a pointer, you can't then cast away the const-ness of it legally.
> 
> There are ways to get it right, it involves templating for mutability.
[...]

Counter-proof:

	struct Slot {
		Slot* next;
		const(string) key;
		const(int) value;
	}
	struct AA {
		Slot*[] slots;
	}
	unittest {
		const(AA) aa;

		static assert(is(typeof(aa.slots[0]) == const(Slot*)));
		const(Slot)* p = aa.slots[0];	// N.B.: legal
		p = p.next;			// N.B.: legal
	}

Note especially the type checked for in the static assert: you cannot modify any of the Slot pointers in the AA, but you *can* assign them to mutable (but tail-const) pointers. Therefore you totally can iterate a const AA without any casts or any breaking of the type system. And there's no need to template for mutability either.


T

-- 
One Word to write them all, One Access to find them, One Excel to count them all, And thus to Windows bind them. -- Mike Champion
« First   ‹ Prev
1 2