D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 5080 - breaking const-correctness with class/interface
Summary: breaking const-correctness with class/interface
Status: RESOLVED DUPLICATE of issue 3731
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: x86 Windows
: P2 normal
Assignee: No Owner
URL:
Keywords: accepts-invalid, patch
Depends on:
Blocks: 2573
  Show dependency treegraph
 
Reported: 2010-10-19 04:28 UTC by Kenji Hara
Modified: 2012-01-30 18:13 UTC (History)
5 users (show)

See Also:


Attachments
test code (4.00 KB, application/octet-stream)
2010-10-19 04:30 UTC, Kenji Hara
Details
test code(fixed) (4.13 KB, text/d-source)
2010-10-19 10:23 UTC, Kenji Hara
Details
Patch for DMD svn r716 (513 bytes, patch)
2010-10-19 10:26 UTC, Kenji Hara
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Kenji Hara 2010-10-19 04:28:46 UTC
Result of attached test code is follows:
----
part1 ----
true,   false,  false,  false,  false
true,   true,   false,  false,  true
false,  false,  true,   true,   false
false,  false,  true,   true,   false
false,  false,  false,  false,  true
part2 ----
true,   false,  false,  false,  false
true,   true,   false,  false,  true
false,  false,  true,   true,   false
false,  false,  true,   true,   false
false,  false,  false,  false,  true
part3 ----
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
----

I discovered two issues.

Part 1 & 2:
  Shared const should not be implicitly convertible to shared, is it?
Part 3:
  All of them are completely broken.
Comment 1 Kenji Hara 2010-10-19 04:30:48 UTC
Created attachment 786 [details]
test code
Comment 2 bearophile_hugs 2010-10-19 04:58:23 UTC
By the way, you have just shown the right way to design (or test) a language: you need to test all the items in the matrix/tensor of all possible combinations. One of the the inventor of Java did the same.
Comment 3 Kenji Hara 2010-10-19 08:08:39 UTC
This code should not work.
----
interface I
{
    void set(int v);
}
class A : I
{
    this(int v){val = v;}
    int val = 10;
    override void set(int v){val = v;}
}
void change(I i)
{
    i.set(100);
}
void main()
{
    auto a = new immutable(A)(10);
    assert(a.val == 10);
    change(a);              // immutable(A) is converted to (mutable) I.
    assert(a.val == 100);   // breaking const-correctness!
}
----
Comment 4 Kenji Hara 2010-10-19 10:23:14 UTC
Created attachment 787 [details]
test code(fixed)

test code fixed.
I withdraw part 1 and 2.

test code result is follows:
----
part1 ----
true,   false,  false,  false,  false
true,   true,   false,  false,  true
false,  false,  true,   false,  false
false,  false,  true,   true,   true
false,  false,  false,  false,  true
part2 ----
true,   false,  false,  false,  false
true,   true,   false,  false,  true
false,  false,  true,   false,  false
false,  false,  true,   true,   true
false,  false,  false,  false,  true
part3 ----
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
true,   true,   true,   true,   true
----
Comment 5 Kenji Hara 2010-10-19 10:26:44 UTC
Created attachment 788 [details]
Patch for DMD svn r716
Comment 6 Tomasz Sowiński 2011-01-07 14:30:10 UTC
(In reply to comment #3)
> interface I
> {
>     void set(int v);
> }
> class A : I
> {
>     this(int v){val = v;}
>     int val = 10;
>     override void set(int v){val = v;}
> }
> void change(I i)
> {
>     i.set(100);
> }
> void main()
> {
>     auto a = new immutable(A)(10);
>     assert(a.val == 10);
>     change(a);              // immutable(A) is converted to (mutable) I.
>     assert(a.val == 100);   // breaking const-correctness!
> }

Looks like duplicate of bug 3731 (read Steven's comment).
Comment 7 Brad Roberts 2011-02-06 15:38:47 UTC
Mass migration of bugs marked as x86-64 to just x86.  The platform run on isn't what's relevant, it's if the app is a 32 or 64 bit app.
Comment 8 Don 2011-02-09 08:27:11 UTC
The patch causes this code to fail to compile:
----
class S { }

void main()
{
    S s1 = new S;
    const S s2 = new S;
    assert(s1!=s2);
}
---
Even so, I think the patch is probably correct -- it's a problem with opEquals.
But this means that more work is required before this patch could be applied.
Comment 9 Steven Schveighoffer 2011-02-09 10:18:44 UTC
(In reply to comment #8)
> The patch causes this code to fail to compile:
> ----
> class S { }
> 
> void main()
> {
>     S s1 = new S;
>     const S s2 = new S;
>     assert(s1!=s2);
> }
> ---
> Even so, I think the patch is probably correct -- it's a problem with opEquals.
> But this means that more work is required before this patch could be applied.

That code should fail to compile on the current compiler without patches, because the prototype for opEquals for objects is:

bool opEquals(Object lhs, Object rhs);

Clearly, this should not accept any const objects.  Because opEquals is special somehow, the compiler rams it through.

So yeah, that code is not indicative of the patch being bad.
Comment 10 yebblies 2012-01-30 18:13:15 UTC

*** This issue has been marked as a duplicate of issue 3731 ***