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

Issue 9383 - Wrong context for contracts if closure [dis]appears in override function #4788

Merged
merged 1 commit into from Jul 6, 2015

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jun 30, 2015

/* __ensure is always called directly,
* so it never becomes closure.
/* __require and __ensure will always get called directly,
* so they never make outer funcitons closure.
Copy link
Member

Choose a reason for hiding this comment

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

s/funcitons/functions/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed.

WalterBright added a commit that referenced this pull request Jul 6, 2015
Issue 9383 - Wrong context for contracts if closure [dis]appears in override function
@WalterBright WalterBright merged commit 8e87814 into dlang:master Jul 6, 2015
@9rnsr 9rnsr deleted the fix9383 branch July 6, 2015 23:03
@9rnsr
Copy link
Contributor Author

9rnsr commented Jul 6, 2015

Thanks!

@schuetzm
Copy link
Contributor

schuetzm commented Jul 7, 2015

I think this broke something in my program. With this commit, I'm getting segfaults. Unfortunately this is a larger project, it will take some time to reduce, I guess.

Anyway, here is part of the backtrace:

#0  0x00007fffffffd800 in ?? ()
#1  0x0000000000aef840 in invariant._d_invariant(Object) ()
#2  0x0000000000997d67 in _long_mangled_name (this=0x7fffffffd630, __HID556=0x7fffffffd588) at orm/persistence.d:46
#3  0x0000000000990111 in _another_long_name (this=0x7ffff7ebff00) at orm/relations.d:22

The code around these lines is:

    // relations.d:
    @property accessor_(Class value)
    in {
        import vibe.data.bson : BsonObjectID;
        assert(value is null || value.id != BsonObjectID.init);
    } body {
        cache = value;
        if(value is null)
            mixin(key ~ " = BsonObjectID.init;");
        else
            mixin(key ~ " = value.id;");    // line 22
    }

    // persistence.d:
    static if(!hasField!(typeof(this), "_id")) {
        @Field
        BsonObjectID _id;
        @property id() const {    // line 46
            return _id;
        }
    }

@9rnsr Do you see anything suspicious? I can't be sure that I haven't done anything wrong, but I specifically checked (using writeln) that value is not null, so this can't be the reason.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jul 7, 2015

@schuetzm This change actually introduced a git-head only regression 14779, and it's in progress to fix that in #4803.

@schuetzm
Copy link
Contributor

schuetzm commented Jul 7, 2015

Thank you! I can confirm that the SEGV disappears with your fix :-)

9rnsr pushed a commit to 9rnsr/dmd that referenced this pull request Aug 7, 2015
Issue 9383 - Wrong context for contracts if closure [dis]appears in override function
9rnsr pushed a commit to 9rnsr/dmd that referenced this pull request Aug 10, 2015
Issue 9383 - Wrong context for contracts if closure [dis]appears in override function
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 7, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788). In Win64, some
function parameters are passed by reggisters (`SCshadowreg`). When the
parameters are placed in closure context, their access was not well handled in dlang#4788.
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 7, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788). In Win64, some
function parameters are passed by registers (`SCshadowreg`). When the
parameters are placed in closure context, their access was not well handled in dlang#4788.
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 13, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788). In Win64, some
function parameters are passed by registers (`SCshadowreg`). When the
parameters are placed in closure context, their access was not well handled in dlang#4788.
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 13, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788). In Win64, some
function parameters are passed by registers (`SCshadowreg`). When the
parameters are placed in closure context, their access was not well handled in dlang#4788.
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 13, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788).
In Win64, some function parameters are passed by registers (`SCshadowreg`). When the parameters are placed in closure context, their access was not well handled in dlang#4788.
Also in Posix 64bit platforms, parameter variables have SCauto. So there was same issue.
9rnsr added a commit to 9rnsr/dmd that referenced this pull request Feb 16, 2016
The regression has been introduce by issue 9383 fix (PR dlang#4788).
In Win64, some function parameters are passed by registers (`SCshadowreg`). When the parameters are placed in closure context, their access was not well handled in dlang#4788.
Also in Posix 64bit platforms, parameter variables have SCauto. So there was same issue.
@WalterBright
Copy link
Member

Introduced regression https://issues.dlang.org/show_bug.cgi?id=15984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants