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

fix Issue 19136 - is expressions don't work as documented #13387

Closed
wants to merge 1 commit into from

Conversation

sorin-gabriel
Copy link
Contributor

Before this patch, is ( Type Identifier : TypeSpecialization ) expressions would evaluate to true with basic types, like this:

static assert(!is(int T : long));
static assert(!is(int T : float));
static assert(!is(int T : double));
static assert(!is(int T : dchar));
..
static assert(!is(bool T : byte));
static assert(!is(byte T : bool));
..

..and many more - you can replace the two types inside the isExpression however you like; the only cases where this would correctly evaluate to false seem to be when one of the types is void or Type has a size strictly bigger than TypeSpecialization.

This fix makes it so that, as per spec, if both Type and TypeSpecialization are basic types, the isExpressions defined like above will evaluate to false if the types are not identical.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @sorin-gabriel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
19136 major is expressions don't work as documented

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#13387"

@adamdruppe
Copy link
Contributor

This is a spec bug, not a compiler bug!

@adamdruppe
Copy link
Contributor

adamdruppe commented Dec 5, 2021

The general pattern of is(A B op C) is that you get the same logic as is(A op C), except you also declare the alias B based on some matching interaction.

If is(A op C) returns true - which it unambiguously would for is(int : long) because "The condition is satisfied if Type is semantically correct and it is the same as or can be implicitly converted to TypeSpecialization." - then is(A B op C) ought to also return true because all you're doing is adding the alias.

This has been the implementation for as long as I can remember, this is the documented behavior in the printed books (see Alexandrescu's "The D Programming Language", published 2010, page 48-49 and my own D Cookbook, published 2014, page 212-214), and it is just consistent with everything else. Moreover, why have an operator == and operator : if they do the same thing in these cases anyway?

The spec simply didn't write it out as it should have.

@RazvanN7
Copy link
Contributor

RazvanN7 commented Dec 6, 2021

Yep, the spec should be changed to match the behavior.

@RazvanN7 RazvanN7 closed this Dec 6, 2021
@radcapricorn
Copy link
Contributor

Yep, the spec should be changed to match the behavior.

Close the issue or update it so it reflects that error is in the spec? Will it pick up a new pull with the same issue ID?

@sorin-gabriel
Copy link
Contributor Author

Thanks for the clarifications, I have submitted a PR for the spec. You can view it here.

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