Skip to content

Get rid of obsolete __ArrayEq lowering & revise core.internal.array.equality #3456

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions dmd/dcast.d
Original file line number Diff line number Diff line change
Expand Up @@ -3523,29 +3523,6 @@ void fix16997(Scope* sc, UnaExp ue)
}
}

/***********************************
* See if both types are arrays that can be compared
* for equality. Return true if so.
* If they are arrays, but incompatible, issue error.
* This is to enable comparing things like an immutable
* array with a mutable one.
*/
extern (C++) bool arrayTypeCompatible(Loc loc, Type t1, Type t2)
{
t1 = t1.toBasetype().merge2();
t2 = t2.toBasetype().merge2();

if ((t1.ty == Tarray || t1.ty == Tsarray || t1.ty == Tpointer) && (t2.ty == Tarray || t2.ty == Tsarray || t2.ty == Tpointer))
{
if (t1.nextOf().implicitConvTo(t2.nextOf()) < MATCH.constant && t2.nextOf().implicitConvTo(t1.nextOf()) < MATCH.constant && (t1.nextOf().ty != Tvoid && t2.nextOf().ty != Tvoid))
{
error(loc, "array equality comparison type mismatch, `%s` vs `%s`", t1.toChars(), t2.toChars());
}
return true;
}
return false;
}

/***********************************
* See if both types are arrays that can be compared
* for equality without any casting. Return true if so.
Expand Down
52 changes: 30 additions & 22 deletions dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -10973,7 +10973,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
Type t1 = exp.e1.type.toBasetype();
Type t2 = exp.e2.type.toBasetype();

bool needsDirectEq(Type t1, Type t2)
// Indicates whether the comparison of the 2 specified array types
// requires an object.__equals() lowering.
static bool needsDirectEq(Type t1, Type t2, Scope* sc)
{
Type t1n = t1.nextOf().toBasetype();
Type t2n = t2.nextOf().toBasetype();
Expand All @@ -10988,13 +10990,16 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
Type t = t1n;
while (t.toBasetype().nextOf())
t = t.nextOf().toBasetype();
if (t.ty != Tstruct)
return false;
if (auto ts = t.isTypeStruct())
{
// semanticTypeInfo() makes sure hasIdentityEquals has been computed
if (global.params.useTypeInfo && Type.dtypeinfo)
semanticTypeInfo(sc, ts);

if (global.params.useTypeInfo && Type.dtypeinfo)
semanticTypeInfo(sc, t);
return ts.sym.hasIdentityEquals;
}

return (cast(TypeStruct)t).sym.hasIdentityEquals;
return false;
}

if (auto e = exp.op_overload(sc))
Expand All @@ -11003,8 +11008,11 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return;
}

const isArrayComparison = (t1.ty == Tarray || t1.ty == Tsarray) &&
(t2.ty == Tarray || t2.ty == Tsarray);
const needsArrayLowering = isArrayComparison && needsDirectEq(t1, t2, sc);

if (!(t1.ty == Tarray && t2.ty == Tarray && needsDirectEq(t1, t2)))
if (!needsArrayLowering)
{
if (auto e = typeCombine(exp, sc))
{
Expand All @@ -11020,26 +11028,20 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor

exp.type = Type.tbool;

// Special handling for array comparisons
if (!(t1.ty == Tarray && t2.ty == Tarray && needsDirectEq(t1, t2)))
if (!isArrayComparison)
{
if (!arrayTypeCompatible(exp.loc, exp.e1.type, exp.e2.type))
if (exp.e1.type != exp.e2.type && exp.e1.type.isfloating() && exp.e2.type.isfloating())
{
if (exp.e1.type != exp.e2.type && exp.e1.type.isfloating() && exp.e2.type.isfloating())
{
// Cast both to complex
exp.e1 = exp.e1.castTo(sc, Type.tcomplex80);
exp.e2 = exp.e2.castTo(sc, Type.tcomplex80);
}
// Cast both to complex
exp.e1 = exp.e1.castTo(sc, Type.tcomplex80);
exp.e2 = exp.e2.castTo(sc, Type.tcomplex80);
}
}

if (t1.ty == Tarray && t2.ty == Tarray)
// lower some array comparisons to object.__equals(e1, e2)
if (needsArrayLowering || (t1.ty == Tarray && t2.ty == Tarray))
{
//printf("Lowering to __equals %s %s\n", e1.toChars(), e2.toChars());

// For e1 and e2 of struct type, lowers e1 == e2 to object.__equals(e1, e2)
// and e1 != e2 to !(object.__equals(e1, e2)).
//printf("Lowering to __equals %s %s\n", exp.e1.toChars(), exp.e2.toChars());

if (!verifyHookExist(exp.loc, *sc, Id.__equals, "equal checks on arrays"))
return setError();
Expand All @@ -11058,7 +11060,13 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
{
__equals = new NotExp(exp.loc, __equals);
}
__equals = __equals.expressionSemantic(sc);
__equals = __equals.trySemantic(sc); // for better error message
if (!__equals)
{
exp.error("incompatible types for array comparison: `%s` and `%s`",
exp.e1.type.toChars(), exp.e2.type.toChars());
__equals = new ErrorExp();
}

result = __equals;
return;
Expand Down
1 change: 0 additions & 1 deletion dmd/id.d
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ immutable Msgtable[] msgtable =
{ "monitorexit", "_d_monitorexit" },
{ "criticalenter", "_d_criticalenter" },
{ "criticalexit", "_d_criticalexit" },
{ "__ArrayEq" },
{ "__ArrayPostblit" },
{ "__ArrayDtor" },
{ "_d_delThrowable" },
Expand Down
1 change: 0 additions & 1 deletion dmd/mtype.h
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,6 @@ class TypeNull : public Type

/**************************************************************/

bool arrayTypeCompatible(Loc loc, Type *t1, Type *t2);
bool arrayTypeCompatibleWithoutCasting(Type *t1, Type *t2);

// If the type is a class or struct, returns the symbol for it, else null.
Expand Down
51 changes: 5 additions & 46 deletions dmd/opover.d
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ private Expression checkAliasThisForRhs(AggregateDeclaration ad, Scope* sc, BinE
* sc = context
* pop = if not null, is set to the operator that was actually overloaded,
* which may not be `e.op`. Happens when operands are reversed to
* to match an overload
* match an overload
* Returns:
* `null` if not an operator overload,
* otherwise the lowered expression
Expand Down Expand Up @@ -917,52 +917,14 @@ Expression op_overload(Expression e, Scope* sc, TOK* pop = null)
Type t1 = e.e1.type.toBasetype();
Type t2 = e.e2.type.toBasetype();

/* Check for array equality.
/* Array equality is handled by expressionSemantic() potentially
* lowering to object.__equals(), which takes care of overloaded
* operators for the element types.
*/
if ((t1.ty == Tarray || t1.ty == Tsarray) &&
(t2.ty == Tarray || t2.ty == Tsarray))
{
bool needsDirectEq()
{
Type t1n = t1.nextOf().toBasetype();
Type t2n = t2.nextOf().toBasetype();
if ((t1n.ty.isSomeChar && t2n.ty.isSomeChar) ||
(t1n.ty == Tvoid || t2n.ty == Tvoid))
{
return false;
}
if (t1n.constOf() != t2n.constOf())
return true;

Type t = t1n;
while (t.toBasetype().nextOf())
t = t.nextOf().toBasetype();
if (t.ty != Tstruct)
return false;

if (global.params.useTypeInfo && Type.dtypeinfo)
semanticTypeInfo(sc, t);

return (cast(TypeStruct)t).sym.hasIdentityEquals;
}

if (needsDirectEq() && !(t1.ty == Tarray && t2.ty == Tarray))
{
/* Rewrite as:
* __ArrayEq(e1, e2)
*/
Expression eeq = new IdentifierExp(e.loc, Id.__ArrayEq);
result = new CallExp(e.loc, eeq, e.e1, e.e2);
if (e.op == TOK.notEqual)
result = new NotExp(e.loc, result);
result = result.trySemantic(sc); // for better error message
if (!result)
{
e.error("cannot compare `%s` and `%s`", t1.toChars(), t2.toChars());
result = new ErrorExp();
}
return;
}
return;
}

/* Check for class equality with null literal or typeof(null).
Expand Down Expand Up @@ -1028,9 +990,6 @@ Expression op_overload(Expression e, Scope* sc, TOK* pop = null)
return;
}

if (t1.ty == Tarray && t2.ty == Tarray)
return;

/* Check for pointer equality.
*/
if (t1.ty == Tpointer || t2.ty == Tpointer)
Expand Down
33 changes: 12 additions & 21 deletions gen/declarations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,28 +371,19 @@ class CodegenVisitor : public Visitor {
return;
}

// Force codegen if this is a templated function with pragma(inline, true).
if ((decl->members->length == 1) &&
((*decl->members)[0]->isFuncDeclaration()) &&
((*decl->members)[0]->isFuncDeclaration()->inlining == PINLINEalways)) {
Logger::println("needsCodegen() == false, but function is marked with "
"pragma(inline, true), so it really does need "
"codegen.");
} else {
// FIXME: This is #673 all over again.
if (!decl->needsCodegen()) {
Logger::println("Does not need codegen, skipping.");
return;
}
// FIXME: This is #673 all over again.
if (!decl->needsCodegen()) {
Logger::println("Does not need codegen, skipping.");
return;
}

if (irs->dcomputetarget && (decl->tempdecl == Type::rtinfo ||
decl->tempdecl == Type::rtinfoImpl)) {
// Emitting object.RTInfo(Impl) template instantiations in dcompute
// modules would require dcompute support for global variables.
Logger::println("Skipping object.RTInfo(Impl) template instantiations "
"in dcompute modules.");
return;
}
if (irs->dcomputetarget && (decl->tempdecl == Type::rtinfo ||
decl->tempdecl == Type::rtinfoImpl)) {
// Emitting object.RTInfo(Impl) template instantiations in dcompute
// modules would require dcompute support for global variables.
Logger::println("Skipping object.RTInfo(Impl) template instantiations "
"in dcompute modules.");
return;
}

for (auto &m : *decl->members) {
Expand Down
3 changes: 3 additions & 0 deletions gen/function-inlining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ bool isInlineCandidate(FuncDeclaration &fdecl) {
} // end anonymous namespace

bool alreadyOrWillBeDefined(FuncDeclaration &fdecl) {
if (fdecl.isFuncLiteralDeclaration()) // emitted into each referencing CU
return true;

for (FuncDeclaration *f = &fdecl; f;) {
if (!f->isInstantiated() && f->inNonRoot()) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion runtime/druntime
2 changes: 2 additions & 0 deletions tests/baremetal/wasm2.d
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ void _start() {}

void __assert(const(char)* msg, const(char)* file, uint line) {}

int memcmp(const void* s1, const void* s2, size_t n) { assert(0); }

export int myExportedFoo()
{
import std.algorithm, std.range;
Expand Down
10 changes: 4 additions & 6 deletions tests/codegen/inlining_templates.d
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ void main()
{
}

// CHECK-NOT: declare{{.*}}_D6inputs10inlinables__T12template_fooTiZQrUNaNbNiNfiZi
// CHECK-NOT: declare{{.*}}_D3std9exception__T7enforce
// CHECK-NOT: declare {{.*}}template_foo

// CHECK-DAG: define{{.*}}_D6inputs10inlinables__T12template_fooTiZQrUNaNbNiNfiZi{{.*}}) #[[ATTR:[0-9]+]]
// CHECK-DAG: define{{.*}}_D3std9exception__T7enforce{{.*}}) #[[ATTR]]

// CHECK-DAG: attributes #[[ATTR]] ={{.*}} alwaysinline
// CHECK-NOT: declare {{.*}}call_enforce_with_default_template_params
// CHECK-NOT: declare {{.*}}__lambda
// CHECK-NOT: declare {{.*}}_D3std9exception__T7enforce