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

[REG 2.077] Type merging immutable -> mutable broken in AST #19449

Open
dlangBugzillaToGithub opened this issue Jun 24, 2018 · 3 comments
Open
Labels
P1 Severity:Regression PRs that fix regressions

Comments

@dlangBugzillaToGithub
Copy link

Iain Buclaw (@ibuclaw) reported this on 2018-06-24T14:19:16Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=19021

CC List

Description

Commit that caused regression.

https://github.com/dlang/dmd/pull/6998/commits/46b0f6b9a2a089ed6dcd106ca03fc7b2e040385d#diff-33cd340a268c63317687a0a372cd3d94L403


It happen that there are two mutable types T that have different deco, but point to the same immutable!T because of failure to recognize that T and T are the same type.

---
struct MultiwayMerge()
{ 
    bool compFront()
    {
        return true;
    }

    struct BinaryHeap(alias less)
    {
        struct Impl
        {
            int _payload;
        }

        void initialize()
        {
            immutable Impl init;
            checkTypes(init);
        }

        void checkTypes(T)(immutable T init)
        {
            alias IT = typeof(init);    // immutable(Impl)
            alias MT = T;               // Impl
            static assert(MT.mangleof == "S6setops__T13MultiwayMergeZQq__T10BinaryHeapS_DQBu__TQBqZQBu9compFrontMFNaNbNiNfZbZQBz4Impl");
            static assert(IT.mangleof == "yS6setops__T13MultiwayMergeZQq__T10BinaryHeapS_DQBu__TQBqZQBu9compFrontMFNaNbNiNfZbZQBz4Impl");
        }

        // Should be same as MT
        static assert(Impl.mangleof == "S6setops__T13MultiwayMergeZQq__T10BinaryHeapS_DQBu__TQBqZQBu9compFrontMFNaNbNiNfZbZQBz4Impl");
    }

    BinaryHeap!(compFront) _heap;
}

MultiwayMerge!() multiwayMerge;

---


What looks to be the case is that mangleToBuffer is called before compFront has finished its semantic, so the first mutable T does not have pure nothrow @nogc encoded into its symbol.
@dlangBugzillaToGithub
Copy link
Author

ibuclaw (@ibuclaw) commented on 2018-06-24T14:21:25Z

Commit looks related https://github.com/dlang/dmd/commit/2c0ead859f208d80190d4e634b5a0737d4b64645

Introduced adding a link between `shared(T)`, `inout(T)` and `const(T)` with `T`.  But left out `immutable(T)` despite this comment here that seems to suggest that there should be one.

https://github.com/dlang/dmd/commit/2c0ead859f208d80190d4e634b5a0737d4b64645#diff-ffafa03255a57832dd09031af6cb915dR470

@dlangBugzillaToGithub
Copy link
Author

ibuclaw (@ibuclaw) commented on 2018-06-24T14:24:48Z

Adding in the reference fixes immediate bug.

https://github.com/dlang/dmd/pull/8398


But now `pure nothrow @nogc` is no longer part of the symbol mangle.

@dlangBugzillaToGithub
Copy link
Author

ibuclaw (@ibuclaw) commented on 2018-06-24T16:12:27Z

Blocking GDC, as there's an ICE in the glue/back-end.

Said ICE is valid, the problem is in the front-end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Severity:Regression PRs that fix regressions
Projects
None yet
Development

No branches or pull requests

1 participant