Issue 18244: std.math generic functions need to have sig constraints #6040
Conversation
|
@quickfur do you plan on addressing all problems mentioned in https://issues.dlang.org/show_bug.cgi?id=18244, or just |
|
Haha, I didn't realize there was a whole list. I'll add them to this PR and update accordingly. |
quickfur
changed the title
|
Fixed everything except |
There was a problem hiding this comment.
If constraints need to be indented on the same level as the declaration: https://dlang.org/dstyle.html#phobos_declarations
otherwise LGTM 👍
std/math.d
Outdated
| @@ -5411,6 +5412,7 @@ if (isFloatingPoint!(X)) | |||
| * $(D true) if $(D_PARAM x) is finite. | |||
| */ | |||
| bool isFinite(X)(X x) @trusted pure nothrow @nogc | |||
| if (isFloatingPoint!X) | |||
There was a problem hiding this comment.
If constraints need to be indented on the same level as the declaration: https://dlang.org/dstyle.html#phobos_declarations
|
Fixed. |
…g constraints.
Not having any sig constraints causes needless conflicts with
user-defined numerical types, e.g.:
````
// myNumber.d
module myNumber;
struct MyNumber { ... }
int signbit(T : MyNumber)(T num) { ... }
// usercode.d
import myNumber;
import std.math;
MyNumber num;
auto x = signbit(num); // tries to call std.math.signbit, and fails
````
Add sig constraints to:
* signbit
* isFinite.
* isNormal.
* isSubnormal
* sgn.
* nextafter.
Should we use
|
|
(jenkins seems to be unable to find this PR) |
|
Something funny is going on, dlang bot didn't seem to have updated the bug even though it recognized the bug number in the commits. |
|
Yeah Martin set the GH integration active only to
In the mean time, we can do |
|
This broke all these functions for types with |
|
On Mon, Jan 22, 2018 at 01:26:55PM -0800, John Colvin wrote:
This broke all these functions for types with alias this to numeric
types, including ones used in dstats. Is there some policy that
phobos doesn't support these or was this an oversight?
[...]
This was an oversight on my part, I apologize. Please file a regression
so that we don't forget to fix this before the next release.
It's kinda annoying that the standard traits require exact type matches,
whereas what we need here are implicitly-converts-to constraints of the
`is(T : float)` kind.
…--T
|
|
Another regression: https://issues.dlang.org/show_bug.cgi?id=18473 |
|
Why did this even make it into a PATCH release ? |
|
Yeah, it should not have been merged into a patch release when there's a regression filed against it. |
|
It was indeed more of an enhancement than a bug fix and shouldn't have landed in stable. How should we proceed? For the time being I'd consider to just revert this. Any good idea how to test if sth. is convertible to a numeric type? This reminds me of the similar issue we had with constraints on strings that stopped working with |
|
It has already been reverted #6213. |
|
Similar to what we had to do for string constraints. |
|
Is there an easy way to check if something might be related to a regression? Because @John-Colvin had already filed one after this PR was merged, before it landed in stable. Anyway, about conversion, I'm thinking it's probably possible, but unlikely to be a simple one-liner. Probably have to factor out the test(s) into a helper template. |
Don't. It is always a mistake. Checking for "is it an integer or floating point type" makes no more sense than "is it a string or an integer". They look very similar to each other but actually have no non-trivial semantics in common. Even if you restrict yourself to floating point types, one immediate problem you face is, is it losslessly convertible to a numeric type? Lossy conversion is something you didn't have to worry about with strings. |
|
Supporting implicit conversions with templated functions is almost always a mistake. It generally causes subtle bugs, though which bugs it causes tends to depend on the types involved. Part of the problem we frequently have with that though is that a function that's not templated inherently supports various implicit conversions but in a way that the conversion is forced at the call site, and when we go and change the function so that it's templated, the semantics change even if it accepts exactly the same set of types. It doesn't look like that's the case with what happened here, but implicit conversions in generic code tend to be a nasty can of worms and are usually best avoided. |
Not having any sig constraints causes needless conflicts with user-defined numerical types, e.g.:
To anticipate the objection "user code should not use the name
signbit": sometimes, we want to deliberately reuse established names for certain operations, so that we can drop in a user-defined type as a replacement for built-in floats without having to modify a lot of user code. As long as it behaves similarly enough to a built-in float, it should be allowed with a minimum of roadblocks.