Skip to content

Conversation

yebblies
Copy link
Contributor

@yebblies yebblies commented Feb 2, 2012

arrayExpressionsToCommonType incorrectly uses condexp.e2->type rather than condexp.type to determine the common type of the array.

http://d.puremagic.com/issues/show_bug.cgi?id=5498

@Aatch
Copy link

Aatch commented May 4, 2012

Any reason this hasn't gone through?

@yebblies
Copy link
Contributor Author

yebblies commented May 4, 2012

Well, it fails in the autotester. My guess is that something special needs to be done for template lambdas.

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
Minor: Fix a link to TickDuration in std.datetime docs
@yebblies
Copy link
Contributor Author

With the following code:
pragma(msg, typeof(true ? (x => x) : ((int x) => x * 2)));

The issue is that in CondExp::semantic e1 has type void and it is special cased to combine void+anything to void.

If this is avoided by excluding TOKfunction expressions, typeCombine/typeMerge fails.

@9rnsr Are there existing functions that could be used to do the type merging for function literals, or does it need to be done from scratch?

@9rnsr
Copy link
Contributor

9rnsr commented Jan 17, 2013

In basic, if a type t is implicitly required for a expression e, it should be set by calling e->inferType(t) before the semantic of e.
But, typeMerge should look at the both side of operands at the same time. It is yet not implemented enough. IMHO we need to add a case for FuncExp vs FuncExp in typeMerge

@yebblies
Copy link
Contributor Author

yebblies commented May 7, 2013

It seems this change is correct, so I'll leave this pull open pending typeMerge improvements.

@andralex
Copy link
Member

@yebblies time to revive this?

@yebblies
Copy link
Contributor Author

This pull is correct, but it will have to wait until typeMerge is updated to merge function literals.

@andralex
Copy link
Member

@yebblies ping?

@mrkline
Copy link

mrkline commented Mar 4, 2014

@yebblies What's the current status of this? As you saw, this came up again in https://d.puremagic.com/issues/show_bug.cgi?id=12283

arrayExpressionsToCommonType incorrectly uses condexp.e2->type rather than condexp.type to determine the common type of the array.
@yebblies
Copy link
Contributor Author

yebblies commented Mar 5, 2014

Here's a hack that keeps the old behavior when merging two FuncExps.

@yebblies
Copy link
Contributor Author

yebblies commented Mar 5, 2014

Hmm, actually seems to work.

@andralex
Copy link
Member

no guts, no glory

andralex added a commit that referenced this pull request Mar 10, 2014
Issue 5498 - array of elements of subtypes of a common supertype
@andralex andralex merged commit 1ede8e2 into dlang:master Mar 10, 2014
@yebblies yebblies deleted the issue5498 branch March 10, 2014 05:21
@yebblies
Copy link
Contributor Author

@andralex Did you merge this based on trust or are you actually confident this is the correct way to solve the problem?

Either way, thanks!

@andralex
Copy link
Member

Some of both...

@9rnsr
Copy link
Contributor

9rnsr commented Apr 21, 2014

With the following code:
pragma(msg, typeof(true ? (x => x) : ((int x) => x * 2)));

For the case, I opened an enhancement issue 12605, and as its first step I opened a refactoring PR #3473.

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

Successfully merging this pull request may close these issues.

5 participants