Skip to content

Loading…

Isue 4424 & 6216 Relax opAssign signature #166

Merged
merged 5 commits into from

7 participants

@9rnsr
D Programming Language member
This change is based on #94 (already merged).

Issue 4424 - Copy constructor and templated opAssign cannot coexist
Issue 6174 - Initialize const fixed-size array in constructor
Issue 6216 - Built-in opAssign implicitly defined should call field's opAssign
Issue 6336 - Can't return ref T where T has const/immutable members

Following is removed from this pull request:
Issue 4596 - [tdpl] Rebinding this in class method compiles

@cristicbz

I would be really interested in this request going through as Issue 4424 seriously affects the interface of my GSoC project.

@9rnsr
D Programming Language member

I would be really interested in this request going through as Issue 4424 seriously affects the interface of my GSoC project.

A workaround exists against Issue 4424 like follows:

struct X {
    private mixin template _workaround4424()
    {
        @disable void opAssign(typeof(this) );
    }
    mixin _workaround4424;

    void opAssign(R)(R rhs) { /*...*/ }
}

It is already used std.typecons.Tuple.

@9rnsr 9rnsr closed this
@9rnsr 9rnsr reopened this
@9rnsr
D Programming Language member

Update patch.
This additionaly fix issue 6174.

@9rnsr
D Programming Language member

Updated to fix 4596.

@repeatedly
D Programming Language member

I hit this issue in msgpack-d today.
I removed 'immutable' keyword from member variables for avoiding compilation error.

This patch is needed for correct implementation.

@9rnsr
D Programming Language member

This pull also contains a fix for bug 6336.
Sorry, I had completely forgotten to post/find an issue about that problem in bugzilla.

@yebblies
D Programming Language member

Would it be possible to break this up a bit to make it easier to review and merge?

@klickverbot
D Programming Language member

@yebblies: As far as I can see, the pull request is already broken up into atomic commits?!

@nazriel

After excluding unittests this pull isn't THAT big, but still I think @yebblies is right here.
1 bug - 1 pull request would be easier to review for folks even if Pull request is divided into atomic commits.

@yebblies
D Programming Language member

@klickverbot It's easier if I can review once bugfix at a time. Is each commit completely independent of the others? If so, it will be fairly easy to split them into multiple pull requests. If not I'll need to work out how they are related before I can be confident enough to pull them.

9rnsr added some commits
@9rnsr 9rnsr Relax opAssign signature.
Class type that has identity opAssign is disallowed both it is templated or not.
9ee798f
@9rnsr 9rnsr fix Issue 6216 - Built-in opAssign implicitly defined should call fie…
…ld's opAssign

Separate 'top assignable' (see opAssign first) from 'blit assignable' (memberwise), and now they are not related to 'modifiable' directly.
b9896d1
@9rnsr 9rnsr fix issue 4424 - Copy constructor and templated opAssign cannot coexist 959e2f0
@9rnsr 9rnsr fix Issue 6336 - Can't return ref T where T has const/immutable members
Remove fail7695.d, because it was incorrectly failed to compile by bug 6336.
b10781e
@9rnsr 9rnsr fix Issue 6174 - Initialize const fixed-size array in constructor
Improve checking of whether an expression is modifiable.
If an expression is *initializing*, that is part of construction, then it bypass type check.
9553d0c
@9rnsr
D Programming Language member

Later commits depends on early ones, and all of them are related to the assignable language semantics.
OK. The commits for 6174 and 4596 are additional changes, so we don't need merge them at once. I'll remove the additionals from this pull request.

@9rnsr
D Programming Language member

Updated.

@yebblies
D Programming Language member

This seems to be causing some failures in std.container

@9rnsr
D Programming Language member

The error in std.container is fixed by the change for 6174...
Unfortunately, I can not break up the changes, except for 4596.

@yebblies
D Programming Language member

Fair enough.

@yebblies yebblies merged commit 5b42e51 into D-Programming-Language:master

1 check was pending

Details default Pass: 8, In Progress: 1
@9rnsr
D Programming Language member

I opened #1169 for issue 4596.

@MartinNowak MartinNowak commented on the diff
src/class.c
((17 lines not shown))
+ if (TemplateDeclaration *td = assign->isTemplateDeclaration())
+ { fd = td->deduceFunctionTemplate(sc, loc, NULL, e, arguments, 1+2);
+ if (fd && !(fd->storage_class & STCdisable))
+ goto Lassignerr;
+ }
+
+Lassignerr:
+ if (fd && !(fd->storage_class & STCdisable))
+ error("identity assignment operator overload is illegal");
@MartinNowak D Programming Language member

Why would you allow to explicitly @disable the operator if it is illegal anyhow?

@9rnsr D Programming Language member
9rnsr added a note

Because @disable opAssign can disallow class reference rebinding. If we allow normal opAssign for class types, it will cause null dereference if assigned class reference is null. On the other hand, disabled opAssign is always checked in compile time and no runtime error happens. I thought it would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 6, 2012
  1. @9rnsr

    Relax opAssign signature.

    9rnsr committed
    Class type that has identity opAssign is disallowed both it is templated or not.
  2. @9rnsr

    fix Issue 6216 - Built-in opAssign implicitly defined should call fie…

    9rnsr committed
    …ld's opAssign
    
    Separate 'top assignable' (see opAssign first) from 'blit assignable' (memberwise), and now they are not related to 'modifiable' directly.
  3. @9rnsr
  4. @9rnsr

    fix Issue 6336 - Can't return ref T where T has const/immutable members

    9rnsr committed
    Remove fail7695.d, because it was incorrectly failed to compile by bug 6336.
  5. @9rnsr

    fix Issue 6174 - Initialize const fixed-size array in constructor

    9rnsr committed
    Improve checking of whether an expression is modifiable.
    If an expression is *initializing*, that is part of construction, then it bypass type check.
Something went wrong with that request. Please try again.