Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibly wrong interaction of Variant and const arrays #9931

Open
dlangBugzillaToGithub opened this issue Jul 31, 2012 · 8 comments
Open

Possibly wrong interaction of Variant and const arrays #9931

dlangBugzillaToGithub opened this issue Jul 31, 2012 · 8 comments

Comments

@dlangBugzillaToGithub
Copy link

cybevnm reported this on 2012-07-31T12:36:58Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=8486

CC List

Description

During initializing Variant, D discards top level const of array, which 
leads to little unintuitive behaviour. Consider code:

import std.stdio;
import std.variant;
void main()
{
  const int[] arr;
  Variant v = Variant( arr );
  writeln( v.peek!( typeof( arr ) )() );
  writeln( v.peek!( const(int)[] )() );
  writeln( v.type() );
}

...and output:
%dmd main.d && ./main.d
null
7FFF358AE298
const(int)[]

As you can see peek works successfully not for original array type, but 
for type without top level const. Is Variant supposed to work in that way ?
@dlangBugzillaToGithub
Copy link
Author

code (@MartinNowak) commented on 2012-07-31T13:07:32Z

Variant should generally ignore top-level const for »value types« (i.e. array slices, pointers, integral types, …).

@dlangBugzillaToGithub
Copy link
Author

code (@MartinNowak) commented on 2012-07-31T13:09:26Z

… the point being that this is, if anything, analogous to the constness of the Variant instance itself, not the contained type.

@dlangBugzillaToGithub
Copy link
Author

cybevnm commented on 2012-08-01T12:58:14Z

(In reply to comment #2)
> … the point being that this is, if anything, analogous to the constness of the
> Variant instance itself, not the contained type.

Unfortunately, Variant's handling of arrays is different to handling of class references. So we have next situtation:

import std.variant;
class C { }
void main()
{
  {
    const C c;
    Variant v = Variant( c );
    assert( v.peek!( typeof(c) )() != null );
  }
  {
    const C[] cs;
    Variant v = Variant( cs );
    assert( v.peek!( typeof(cs) )() == null );
 }
}
But array top-level const cutting is understandable, it's the way it works in all other places...

@dlangBugzillaToGithub
Copy link
Author

code (@MartinNowak) commented on 2012-08-01T13:08:10Z

I meant that Variant should regard e.g. const(T[]) and const(T)[] or const(T*) and const(T)* equivalent, i.e. allow implicit conversion between them.

The fact that const is automatically stripped from arrays is not Variant-specific, template functions generally work like that in D.

@dlangBugzillaToGithub
Copy link
Author

cybevnm commented on 2012-08-01T13:43:19Z

(In reply to comment #4)
> I meant that Variant should regard e.g. const(T[]) and const(T)[] or const(T*)
> and const(T)* equivalent, i.e. allow implicit conversion between them.

Agree, it will resolve the problem.

@dlangBugzillaToGithub
Copy link
Author

bugzilla (@WalterBright) commented on 2019-10-18T12:12:52Z

This seems not to be a Phobos bug:

import std.stdio;

void main()
{
    const int[] arr;
    writeln(typeof(arr).stringof);
    Test(arr);
}

struct Test
{
    this(T)(T value)
    {
        writeln(T.stringof);
    }
}

writes:

const(int[])
const(int)[]

For me the question remains: Is this intentional (for reasons I don't understand yet) or a language bug?

@dlangBugzillaToGithub
Copy link
Author

code (@MartinNowak) commented on 2019-10-18T12:17:48Z

(In reply to berni44 from comment #6)
> For me the question remains: Is this intentional (for reasons I don't
> understand yet) or a language bug?

Removal of "head const", i.e. the outer layer, is intentional. This avoid duplicate template instantiations for code that has identical semantics anyway (like for a const int parameter, where it is copied anyway even if non-const, so from the caller side, there isn't a difference), and allows range functions to work in a natural way with constness (where, again, the slice is copied anyway, so const or not isn't visible to the caller).

@dlangBugzillaToGithub
Copy link
Author

bugzilla (@WalterBright) commented on 2019-10-18T19:51:14Z

(In reply to David Nadlinger from comment #7)
> Removal of "head const", i.e. the outer layer, is intentional.
OK. In that case this is a Phobos bug. The solution would be to make peek remove "head const" on the given type und check if that's the type of the Variant. I've got too few experience on const, so I cannot fix this. I tried to add a small template which returns what it get's (just out of curiosity, if that could work). With this, peek could compare the type of the returned value to the value of the Variant. But this did not work out well. Partly, because the template needs to be given a value and not a type, partly because of inout gets in the way.

I don't know if it's of much use, because this can be easily derived from the post above, but I created this unittest which currently does not pass but a corrected version of peek should pass:

unittest
{
    const int[] arr;
    Variant a = Variant(arr);
    assert(a.peek!(typeof(arr)) !is null);
}

@LightBender LightBender removed the P3 label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants