Bug 18541 – comparison `==` of two typeid() should always be rewritten as a "is"

I cant see why anyone would want a full bit per bit comparison of the result of two `typeid()` since `typeid()` returns static instances, shared for each stuff of a given type. The idea is to have an AST rewrite of `typeid(T1) == typeid(T2)` as `typeid(T1) is typeid(T2)` which is compiled as faster code.

I thought in the cases of DLLs, the typeids from different object files could be identical, but different instances.

(In reply to Steven Schveighoffer from comment #1) > I thought in the cases of DLLs, the typeids from different object files > could be identical, but different instances. Nobody has said you're wrong. It's even a case where the lowering is invalid.

With this dll stuff it's hardly applicable...

All I'm saying is that I don't think we need to enforce this. There are reasons to use ==.

Note that == will shortcut and return true if the two instances are the same object.

(In reply to Steven Schveighoffer from comment #5) > Note that == will shortcut and return true if the two instances are the same > object. With someone yesterday we have verified that it's really not the case. In a specific hotspot, using "==" instead of "is" made something running in 7 minutes instead of 4 ! This is exactly why i opened this issue but unfortunately i understand now that it's not applicable, we just take care to use "is".

(In reply to Basile B. from comment #6) > (In reply to Steven Schveighoffer from comment #5) > > Note that == will shortcut and return true if the two instances are the same > > object. > > With someone yesterday we have verified that it's really not the case. In a > specific hotspot, using "==" instead of "is" made something running in 7 > minutes instead of 4 ! > > This is exactly why i opened this issue but unfortunately i understand now > that it's not applicable, we just take care to use "is". The very first thing in the implementation of the free function opEquals is if (lhs is rhs) return true; So, it most definitely shortcuts. Now, that doesn't mean that it's as efficient, since unless it's inlined, you're calling a function that isn't called if you use is directly, but the shortcut definitely happens.

Yeah, there is no inlining of it. There is a bit of magic in the compiler for Object == Object, and I think that magic is stopping the inlining. Of course, the compiler magic could be made to simply do the `is` comparison first, and then call the object function (we can take `lhs is rhs` out of the function itself). I think this would be a worthy enhancement request, especially considering the performance benefits. I'm not sure how keen someone is to delve into updating that code, it's kind of a mess.

compiler transforms `==` for objects to virtual call, and dmd cannot devirtualize calls (yet? ;-). so yeah, no inlining. it is quite possible to rewrite the call into `e1 is e1 || .object.opEquals(e1, e2)`, tho. --- a/src/ddmd/opover.d +++ b/src/ddmd/opover.d @@ -30,6 +30,7 @@ import ddmd.globals; import ddmd.id; import ddmd.identifier; import ddmd.mtype; +import ddmd.sideeffect; import ddmd.statement; import ddmd.tokens; import ddmd.visitor; @@ -1162,7 +1163,7 @@ extern (C++) Expression op_overload(Expression e, Scope* sc) if (!(cd1.cpp || cd2.cpp)) { /* Rewrite as: - * .object.opEquals(e1, e2) + * e1 is e2 || .object.opEquals(e1, e2) */ Expression e1x = e.e1; Expression e2x = e.e2; @@ -1176,12 +1177,38 @@ extern (C++) Expression op_overload(Expression e, Scope* sc) if (cd2.isInterfaceDeclaration()) e2x = new CastExp(e.loc, e.e2, t2.isMutable() ? to : to.constOf()); + /* create temporaries, to avoid invalid rewriting of this: + * if (arr[i++] is obj || .object.opEquals(arr[i++], obj) + * rewrite to: + * tmp1 = e1x, tmp2 = e2x, then use temps + */ + // load e1x to temporary + auto tmp1 = copyToTemp(STCnodtor, "__opEqualsTmp1", e1x); + auto e1xtmp = new CommaExp(e.loc, new DeclarationExp(e.loc, tmp1), new VarExp(e.loc, tmp1)); + + // load e2x to temporary + auto tmp2 = copyToTemp(STCnodtor, "__opEqualsTmp2", e2x); + auto e2xtmp = new CommaExp(e.loc, new DeclarationExp(e.loc, tmp2), new VarExp(e.loc, tmp2)); + + // e1 is e2 + auto expis = new IdentityExp(TOKidentity, e.loc, e1xtmp, e2xtmp); + + // and use fresh varexps with temps (previous ones can be changed by semanticing) + e1x = new VarExp(e.loc, tmp1); + e2x = new VarExp(e.loc, tmp2); + + // .object.opEquals(e1, e2) result = new IdentifierExp(e.loc, Id.empty); result = new DotIdExp(e.loc, result, Id.object); result = new DotIdExp(e.loc, result, Id.eq); result = new CallExp(e.loc, result, e1x, e2x); + + // expis || result + result = new LogicalExp(e.loc, TOKoror, expis, result); + if (e.op == TOKnotequal) result = new NotExp(e.loc, result); + result = result.semantic(sc); return; }