Issue 11094 - Disuniform error messages with overloaded + and ^ operators
Summary: Disuniform error messages with overloaded + and ^ operators
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: x86 Windows
: P3 minor
Assignee: No Owner
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2013-09-22 04:41 UTC by bearophile_hugs
Modified: 2022-12-17 10:45 UTC (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description bearophile_hugs 2013-09-22 04:41:14 UTC 
With DMD 2.064alpha this code gives no errors, it seems correct:


struct Foo {
    Foo opBinary(string op)(in Foo r) inout if (op == "+") {
        return Foo();
    }
    Foo opBinary(string op)(in Foo r) inout if (op == "^") {
        return Foo();
    }
}
void main() {
    const x = Foo();
    auto r1 = x + Foo();
    auto r2 = x ^ Foo();
}


But if I forget the inouts:

struct Foo {
    Foo opBinary(string op)(in Foo r) if (op == "+") {
        return Foo();
    }
    Foo opBinary(string op)(in Foo r) if (op == "^") {
        return Foo();
    }
}
void main() {
    const x = Foo();
    auto r1 = x + Foo();
    auto r2 = x ^ Foo();
}


DMD gives:

test.d(11): Error: incompatible types for ((x) + (Foo())): 'const(Foo)' and 'Foo'
test.d(12): Error: 'x' is not of integral type, it is a const(Foo)
test.d(12): Error: 'Foo()' is not of integral type, it is a Foo

I suggest:
- To give only one error message for the "^" case;
- To give the same error messages for both "+" and "^";
- Perhaps the error message could explain better the problem, or/and perhaps it could suggest the use of 'inout'.
Comment 1 Andrej Mitrovic 2014-04-27 19:43:07 UTC 
Note that it is unrelated to the overloads, you could have a single opBinary to reproduce:

-----
struct Foo
{
    Foo opBinary(string op)(in Foo r) if (op == "+" || op == "^")
    {
        return Foo();
    }
}

void main()
{
    const x = Foo();
    auto r1 = x + Foo();
    auto r2 = x ^ Foo();
}
-----
Comment 2 Ben 2022-05-26 20:31:49 UTC 
I took a look at this, and the difference seems to be related to the extra check(s) added at the end of the expressionsem visitors for AddExp vs XorExp:

//in xor only
if (exp.checkIntegralBin() || exp.checkSharedAccessBin(sc))
    return setError();


A couple of questions.  This happens earlier in the visitor:

Expression e = exp.op_overload(sc);
if (e)
{
    result = e;
    return;
}


I assume e is null if the op overload doesn't apply (in this case there are constness issues), but putting an error in opover.visitBin seems like maybe the wrong fix?

Those extra checks make sense for xor, is it worth trying to add a supplemental error or something that sees if they tried to do op overloading?

There's a TON of duplication between the addexp and xorexp visitors in expressionsem, and I assume probably with most binop visitors.  Is it worth trying to refactor to reduce the duplication?