Fix conversions between function pointers #96

Merged
merged 1 commit into from Sep 2, 2011

Conversation

Projects
None yet
4 participants
@yebblies
Member

yebblies commented Jun 8, 2011

This is a second attempt at pull 88. It fixes bugs 3797 and 3268 and implements part of 3180 for function pointers.

When combined with https://github.com/klickverbot/druntime/tree/fp-types it passes the dmd, druntime and phobos tests.

While disallowing incorrect implicit conversions, this patch allows a bunch of sensible conversions for return types and attributes.

Thanks to David Nadlinger for the base conversion implementation and fixing the implicit conversion tests.

Fixes:
http://d.puremagic.com/issues/show_bug.cgi?id=3797
http://d.puremagic.com/issues/show_bug.cgi?id=3268
Partially fixes:
http://d.puremagic.com/issues/show_bug.cgi?id=3180
Requires:
dlang/druntime#26 (has been merged already)

@klickverbot

View changes

src/parse.c
@@ -2624,6 +2624,18 @@ Type *Parser::parseDeclarator(Type *t, Identifier **pident, TemplateParameters *
tf = tf->makeWild();
}
+ //printf("-\nts is %s and t is %s\n", ts->toChars(), t->toChars());
+#if 1

This comment has been minimized.

@klickverbot

klickverbot Jun 8, 2011

Member

Is there a special reason for which you want to keep the old code around? If so, a comment would be nice, otherwise I think leaving commented-out code around in VCS-managed files is just confusing.

@klickverbot

klickverbot Jun 8, 2011

Member

Is there a special reason for which you want to keep the old code around? If so, a comment would be nice, otherwise I think leaving commented-out code around in VCS-managed files is just confusing.

@klickverbot

View changes

src/cast.c
+ TypeFunction *tf2 = (TypeFunction *)t2->nextOf();
+ TypeFunction *d = (TypeFunction*)tf1->syntaxCopy();
+
+ d->purity = (tf1->purity == tf2->purity ? tf1->purity : PUREimpure );

This comment has been minimized.

@klickverbot

klickverbot Jun 8, 2011

Member

I didn't have time for a closer look at the code yet, hence just a quick idea: Could this cause a strongly pure and a weakly pure function to be converted to a non-pure one?

@klickverbot

klickverbot Jun 8, 2011

Member

I didn't have time for a closer look at the code yet, hence just a quick idea: Could this cause a strongly pure and a weakly pure function to be converted to a non-pure one?

This comment has been minimized.

@yebblies

yebblies Jun 8, 2011

Member

Yes I think so. This was based off my older 3797 patch which did not include return type covariance. I'll see if I can find a reasonable fix for this.

@yebblies

yebblies Jun 8, 2011

Member

Yes I think so. This was based off my older 3797 patch which did not include return type covariance. I'll see if I can find a reasonable fix for this.

@klickverbot

This comment has been minimized.

Show comment
Hide comment
@klickverbot

klickverbot Jun 8, 2011

Member

I submitted the fix you mentioned above as druntime pull request 26. Also, everybody else, please consider my own first attempt on fixing this (pull request 91) obsolete.

Member

klickverbot commented Jun 8, 2011

I submitted the fix you mentioned above as druntime pull request 26. Also, everybody else, please consider my own first attempt on fixing this (pull request 91) obsolete.

@9rnsr

This comment has been minimized.

Show comment
Hide comment
@9rnsr

9rnsr Jun 9, 2011

Member

I think introduce special type class like TypeFunctionPtr is not good code design.
It spread special case code many place.
It seems to me that you should merge codes relating function pointer type conversion into TypePointer.
(Design improvement example: TypeAmbiguous in #70 and isAmbiguous into TypeFunction for transfer unresolved overloaded function type.)

Member

9rnsr commented Jun 9, 2011

I think introduce special type class like TypeFunctionPtr is not good code design.
It spread special case code many place.
It seems to me that you should merge codes relating function pointer type conversion into TypePointer.
(Design improvement example: TypeAmbiguous in #70 and isAmbiguous into TypeFunction for transfer unresolved overloaded function type.)

@yebblies

This comment has been minimized.

Show comment
Hide comment
@yebblies

yebblies Jun 9, 2011

Member

That might be true. Having TypeFunctionPtr has made the code more complicated in some parts, but simpler in others. I do not like the fact that it requires altering the parser and/or pointerTo in order to work.
It might be worth merging the types, but the main idea was to separate the conv functions as they have completely different behavior for function pointers and normal pointers. Most of the special cases would exist either way.
One other advantage of it is symmetry with TypeDelegate.
I might have a go refactoring it that way and see if it actually is any cleaner.

Member

yebblies commented Jun 9, 2011

That might be true. Having TypeFunctionPtr has made the code more complicated in some parts, but simpler in others. I do not like the fact that it requires altering the parser and/or pointerTo in order to work.
It might be worth merging the types, but the main idea was to separate the conv functions as they have completely different behavior for function pointers and normal pointers. Most of the special cases would exist either way.
One other advantage of it is symmetry with TypeDelegate.
I might have a go refactoring it that way and see if it actually is any cleaner.

@yebblies

This comment has been minimized.

Show comment
Hide comment
@yebblies

yebblies Jul 2, 2011

Member

Refactored and rebased.

Member

yebblies commented Jul 2, 2011

Refactored and rebased.

Issue 3797 - Regression(2.038): Implicit conversion between incompati…
…ble function pointers

This fixes behaviour that seems to have been broken since const and immutable were introduced to D.
Conversions from function pointer a to function pointer b are only allowed when function b is covariant with function a.
Changing the constness of the function pointer does not change the constness of the function's this reference.
This also allows finding the common type of two function pointers.
@yebblies

This comment has been minimized.

Show comment
Hide comment
@yebblies

yebblies Aug 29, 2011

Member

@WalterBright Is there anything I can do to help this get merged in? Are there issues with this patch or has it not just been evaluated yet? Please let me know if more documentation, more unit tests, breaking the patch into smaller patches etc would be helpful. The core issue (3797) is one of the oldest regressions and has been reported 9 times. 3180 has become a lot more important recently with template and lambda purity, nothrow and safety inference.

Member

yebblies commented Aug 29, 2011

@WalterBright Is there anything I can do to help this get merged in? Are there issues with this patch or has it not just been evaluated yet? Please let me know if more documentation, more unit tests, breaking the patch into smaller patches etc would be helpful. The core issue (3797) is one of the oldest regressions and has been reported 9 times. 3180 has become a lot more important recently with template and lambda purity, nothrow and safety inference.

WalterBright added a commit that referenced this pull request Sep 2, 2011

Merge pull request #96 from yebblies/functionpointers
Fix conversions between function pointers

@WalterBright WalterBright merged commit 306df8e into dlang:master Sep 2, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment