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

Regression fix Issue 11591 - std.typecons.Tuple -s with classes fails at runtime as associative array keys #3054

Merged
merged 1 commit into from
Jan 2, 2014

Conversation

WalterBright
Copy link
Member

This pushes a runtime error to a compile time error. It therefore has the potential to break existing code, but I suspect any such code was already broken (like the test12.d test case, see below).

The new error message also helpfully gives the correct form for the required opCmp function.

https://d.puremagic.com/issues/show_bug.cgi?id=11591

@yebblies
Copy link
Member

yebblies commented Jan 1, 2014

Why do AAs need opCmp?

@WalterBright
Copy link
Member Author

For historical reasons they use opCmp instead of opEquals.

@yebblies
Copy link
Member

yebblies commented Jan 1, 2014

Why don't we fix that instead?

@WalterBright
Copy link
Member Author

Because 1) I don't want to break anything with this release and 2) that just pushes the problem to being an unsuitable opEquals()

@WalterBright
Copy link
Member Author

Needs this Phobos pull: dlang/phobos#1827

@yebblies
Copy link
Member

yebblies commented Jan 1, 2014

  1. according to phobos, this does break code, and 2) the compiler auto-generates a suitable opEquals, no?

@WalterBright
Copy link
Member Author

  1. it was a broken unittest
  2. not in all cases

@WalterBright
Copy link
Member Author

BTW, all the issues uncovered by this PR are variations on bug 11591 - the compiler accepting invalid code. The AA's would fail at runtime, but they didn't in these cases because either the AA was never actually used, or was used only in trivial cases.

I think it's a good sign this PR is uncovering latent bugs.

@yebblies
Copy link
Member

yebblies commented Jan 1, 2014

It's going to break code that works, because of a historical requirement of AAs that we don't need any more. Code I have uses AA literals to create AAs, and never modifies them, and I assume this will break it.

In what cases does the compiler not generate a suitable opEquals?

@WalterBright
Copy link
Member Author

The same cases for which a suitable opCmp is not generated - when an opEquals is supplied, but does not match the required type signature. Look through the source for xerreq.

It's going to break code that works,

The code does not work - code that used it was never run. It would have issued a runtime error if it had.

@denis-sh
Copy link
Contributor

denis-sh commented Jan 1, 2014

@denis-sh
Copy link
Contributor

denis-sh commented Jan 1, 2014

Also there are Issue 11588 and Issue 11589.

@WalterBright
Copy link
Member Author

It fixes 11591.

andralex added a commit that referenced this pull request Jan 2, 2014
Regression fix Issue 11591 - std.typecons.Tuple -s with classes fails at runtime as associative array keys
@andralex andralex merged commit 10de92b into dlang:master Jan 2, 2014
@WalterBright WalterBright deleted the fix11591 branch January 2, 2014 19:58
andralex added a commit that referenced this pull request Jan 7, 2014
Regression fix Issue 11591 - std.typecons.Tuple -s with classes fails at runtime as associative array keys
@WalterBright
Copy link
Member Author

@WalterBright
Copy link
Member Author

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.

4 participants