Skip to content

[REG2.061] Issue 13252 - ParameterDefaultValueTuple affects other instantiations#3846

Merged
yebblies merged 2 commits intodlang:masterfrom
9rnsr:fix13252
Aug 5, 2014
Merged

[REG2.061] Issue 13252 - ParameterDefaultValueTuple affects other instantiations#3846
yebblies merged 2 commits intodlang:masterfrom
9rnsr:fix13252

Conversation

@9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Aug 5, 2014

https://issues.dlang.org/show_bug.cgi?id=13252

Expression::equals should also compare operand types.

…ions

Expression::equals should also compare operand types.
@AndrewEdwards AndrewEdwards added this to the 2.066 milestone Aug 5, 2014
@yebblies
Copy link
Contributor

yebblies commented Aug 5, 2014

I don't like that you're changing the type before comparing. I know this is how equality works for template args, but this is supposed to be a general equals method. I assume the expressions will be cast to the correct type somewhere during template matching, so would it be possible to call equals sometime after that?

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 5, 2014

In general equals method, type should be compared. In floating point value comparison, different types mean different precisions. Please also see IntegerExp::equals, RealExp::equals, and CompexExp::equals.

And, if expressions are passed via alias or tuple parameters, there's no correct types and their types should be left as-is.

@yebblies
Copy link
Contributor

yebblies commented Aug 5, 2014

I agree that types should be compared, but not that toHeadMutable should be called on them first.

…quals

I'm not sure we should also remove toHeadMutable call from IntergerExp, RealExp, ComplexExp, and VarExp::equals. To avoid accidental existing code breaking, I'd leave them in 2.066 beta/rc phase.
@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 5, 2014

but not that toHeadMutable should be called on them first.

Ah, I see. OK, I fixed it.

But, I'm not sure we should also remove toHeadMutable call from IntergerExp, RealExp, ComplexExp, and VarExp::equals. To avoid accidental existing code breaking, I'd leave them in 2.066 beta/rc phase.

@yebblies
Copy link
Contributor

yebblies commented Aug 5, 2014

I think we should probably remove them, but not now is fine.

@yebblies
Copy link
Contributor

yebblies commented Aug 5, 2014

Auto-merge toggled on

yebblies added a commit that referenced this pull request Aug 5, 2014
[REG2.061] Issue 13252 - ParameterDefaultValueTuple affects other instantiations
@yebblies yebblies merged commit 895c56c into dlang:master Aug 5, 2014
@9rnsr 9rnsr deleted the fix13252 branch August 6, 2014 00:30
9rnsr pushed a commit to 9rnsr/dmd that referenced this pull request Aug 6, 2014
[REG2.061] Issue 13252 - ParameterDefaultValueTuple affects other instantiations
ibuclaw pushed a commit to ibuclaw/dmd that referenced this pull request Jul 10, 2022
RISCV: Add some definitions and vararg supports

Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Nicholas Wilson <thewilsonator@users.noreply.github.com>
Merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
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.

3 participants