From 58c3cc6b2c273101201854336a30d21b920c7405 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Mon, 31 Jan 2022 16:23:57 +0100 Subject: [PATCH 1/4] Fix Issue 22717 - TypeInfo_Struct.equals swaps lhs and rhs parameters Leading to a swapped `rhs.opEquals(lhs)` check *if* the opEquals method took its parameter by ref. --- VERSION | 2 +- src/dmd/clone.d | 9 +++++---- src/dmd/dtemplate.d | 9 ++++++++- test/runnable/test22717.d | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 test/runnable/test22717.d diff --git a/VERSION b/VERSION index 0aa03f4b61ab..ceb50d8ce346 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v2.098.1 +v2.099.0-beta.0 diff --git a/src/dmd/clone.d b/src/dmd/clone.d index 73f7e02b37b2..b46741612e2a 100644 --- a/src/dmd/clone.d +++ b/src/dmd/clone.d @@ -521,12 +521,12 @@ FuncDeclaration buildOpEquals(StructDeclaration sd, Scope* sc) /****************************************** * Build __xopEquals for TypeInfo_Struct - * static bool __xopEquals(ref const S p, ref const S q) + * static bool __xopEquals(ref const S q, ref const S p) * { * return p == q; * } * - * This is called by TypeInfo.equals(p1, p2). If the struct does not support + * This is called by TypeInfo.equals(p, q). If the struct does not support * const objects comparison, it will throw "not implemented" Error in runtime. */ FuncDeclaration buildXopEquals(StructDeclaration sd, Scope* sc) @@ -570,8 +570,9 @@ FuncDeclaration buildXopEquals(StructDeclaration sd, Scope* sc) Loc declLoc; // loc is unnecessary so __xopEquals is never called directly Loc loc; // loc is unnecessary so errors are gagged auto parameters = new Parameters(); - parameters.push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.p, null, null)) - .push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.q, null, null)); + // TODO: get rid of parameter reversal by making __xopEquals a method + parameters.push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.q, null, null)) + .push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.p, null, null)); auto tf = new TypeFunction(ParameterList(parameters), Type.tbool, LINK.d); Identifier id = Id.xopEquals; auto fop = new FuncDeclaration(declLoc, Loc.initial, id, STC.static_, tf); diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d index fb36c5e199e5..61653a1573eb 100644 --- a/src/dmd/dtemplate.d +++ b/src/dmd/dtemplate.d @@ -7799,15 +7799,22 @@ struct TemplateInstanceBox { bool res = void; if (ti.inst && s.ti.inst) + { /* This clause is only used when an instance with errors * is replaced with a correct instance. */ res = ti is s.ti; + } else + { /* Used when a proposed instance is used to see if there's * an existing instance. */ - res = (cast()s.ti).equalsx(cast()ti); + static if (__VERSION__ >= 2099) + res = (cast()ti).equalsx(cast()s.ti); + else // https://issues.dlang.org/show_bug.cgi?id=22717 + res = (cast()s.ti).equalsx(cast()ti); + } debug (FindExistingInstance) ++(res ? nHits : nCollisions); return res; diff --git a/test/runnable/test22717.d b/test/runnable/test22717.d new file mode 100644 index 000000000000..5d2cc43c6199 --- /dev/null +++ b/test/runnable/test22717.d @@ -0,0 +1,33 @@ +// PERMUTE_ARGS: -version=XopEquals + +static assert(__VERSION__ >= 2099); + +void main() +{ + static struct S + { + int value; + + version (XopEquals) + { + bool opEquals(const S rhs) const + { + assert(this.value == 42); + return true; + } + } + else + { + bool opEquals(const ref S rhs) const + { + assert(this.value == 42); + return true; + } + } + } + + auto a = S(42); + auto b = S(24); + auto ti = typeid(S); + assert(ti.equals(&a, &b)); +} From 09c43ae59ec3bc5f21e7a9969f9fdc1097786bda Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Mon, 31 Jan 2022 20:09:12 +0100 Subject: [PATCH 2/4] CircleCI: Try to use same-named druntime/Phobos branches for PRs from official repo --- .circleci/run.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/.circleci/run.sh b/.circleci/run.sh index 9da64a805b86..7594b1e0b827 100755 --- a/.circleci/run.sh +++ b/.circleci/run.sh @@ -73,14 +73,19 @@ install_deps() { } setup_repos() { - # set a default in case we run into rate limit restrictions local base_branch="" - if [ -n "${CIRCLE_PR_NUMBER:-}" ]; then - base_branch=$((curl -fsSL https://api.github.com/repos/dlang/$CIRCLE_PROJECT_REPONAME/pulls/$CIRCLE_PR_NUMBER || echo) | jq -r '.base.ref') - else + if [ -z "${CIRCLE_PR_NUMBER:-}" ]; then + # no PR base_branch=$CIRCLE_BRANCH + elif [ -z "${CIRCLE_PR_REPONAME:-}" ]; then + # PR originating from the official dlang repo + base_branch=$CIRCLE_BRANCH + else + # PR from a fork + base_branch=$( (curl -fsSL https://api.github.com/repos/dlang/$CIRCLE_PROJECT_REPONAME/pulls/$CIRCLE_PR_NUMBER || echo) | jq -r '.base.ref') + # set a default in case we run into rate limit restrictions + base_branch=${base_branch:-master} fi - base_branch=${base_branch:-"master"} # merge testee PR with base branch (master) before testing if [ -n "${CIRCLE_PR_NUMBER:-}" ]; then From bd4bae1311f8807a01d066c0d5651e7c06dd569b Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Mon, 31 Jan 2022 20:42:42 +0100 Subject: [PATCH 3/4] [TEMP] Enforce VERSION file --- src/build.d | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/build.d b/src/build.d index 9fa152d8beb0..d69818564708 100755 --- a/src/build.d +++ b/src/build.d @@ -401,12 +401,14 @@ alias backend = makeRuleWithArgs!((MethodInitializer!BuildRule builder, BuildRul /// Returns: the rules that generate required string files: VERSION and SYSCONFDIR.imp alias versionFile = makeRule!((builder, rule) { alias contents = memoize!(() { + /* if (dmdRepo.buildPath(".git").exists) { auto gitResult = tryRun([env["GIT"], "describe", "--dirty"]); if (gitResult.status == 0) return gitResult.output.strip; } + */ // version fallback return dmdRepo.buildPath("VERSION").readText; }); From 92823bd118f20bf2036faccd0f02863026d49816 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 1 Feb 2022 00:19:37 +0100 Subject: [PATCH 4/4] Only change xopEquals behavior if `__VERSION__` is going to be >= 2099 druntime has been adapted accordingly too. --- src/build.d | 2 -- src/dmd/clone.d | 14 ++++++++------ test/runnable/test22717.d | 6 ++++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/build.d b/src/build.d index d69818564708..9fa152d8beb0 100755 --- a/src/build.d +++ b/src/build.d @@ -401,14 +401,12 @@ alias backend = makeRuleWithArgs!((MethodInitializer!BuildRule builder, BuildRul /// Returns: the rules that generate required string files: VERSION and SYSCONFDIR.imp alias versionFile = makeRule!((builder, rule) { alias contents = memoize!(() { - /* if (dmdRepo.buildPath(".git").exists) { auto gitResult = tryRun([env["GIT"], "describe", "--dirty"]); if (gitResult.status == 0) return gitResult.output.strip; } - */ // version fallback return dmdRepo.buildPath("VERSION").readText; }); diff --git a/src/dmd/clone.d b/src/dmd/clone.d index b46741612e2a..613ff842a492 100644 --- a/src/dmd/clone.d +++ b/src/dmd/clone.d @@ -521,12 +521,12 @@ FuncDeclaration buildOpEquals(StructDeclaration sd, Scope* sc) /****************************************** * Build __xopEquals for TypeInfo_Struct - * static bool __xopEquals(ref const S q, ref const S p) + * static bool __xopEquals(ref const S p, ref const S q) * { * return p == q; * } * - * This is called by TypeInfo.equals(p, q). If the struct does not support + * This is called by TypeInfo.equals(p1, p2). If the struct does not support * const objects comparison, it will throw "not implemented" Error in runtime. */ FuncDeclaration buildXopEquals(StructDeclaration sd, Scope* sc) @@ -570,16 +570,18 @@ FuncDeclaration buildXopEquals(StructDeclaration sd, Scope* sc) Loc declLoc; // loc is unnecessary so __xopEquals is never called directly Loc loc; // loc is unnecessary so errors are gagged auto parameters = new Parameters(); - // TODO: get rid of parameter reversal by making __xopEquals a method - parameters.push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.q, null, null)) - .push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.p, null, null)); + parameters.push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.p, null, null)) + .push(new Parameter(STC.ref_ | STC.const_, sd.type, Id.q, null, null)); auto tf = new TypeFunction(ParameterList(parameters), Type.tbool, LINK.d); Identifier id = Id.xopEquals; auto fop = new FuncDeclaration(declLoc, Loc.initial, id, STC.static_, tf); fop.generated = true; Expression e1 = new IdentifierExp(loc, Id.p); Expression e2 = new IdentifierExp(loc, Id.q); - Expression e = new EqualExp(EXP.equal, loc, e1, e2); + // TODO: simplify as soon as `git describe` for DMD master yields v2.099+ + Expression e = global.versionNumber() >= 2099 + ? new EqualExp(EXP.equal, loc, e2, e1) + : new EqualExp(EXP.equal, loc, e1, e2); fop.fbody = new ReturnStatement(loc, e); uint errors = global.startGagging(); // Do not report errors Scope* sc2 = sc.push(); diff --git a/test/runnable/test22717.d b/test/runnable/test22717.d index 5d2cc43c6199..3fd284f6db65 100644 --- a/test/runnable/test22717.d +++ b/test/runnable/test22717.d @@ -1,9 +1,10 @@ // PERMUTE_ARGS: -version=XopEquals -static assert(__VERSION__ >= 2099); - void main() { + // TODO: remove as soon as `git describe` for DMD master yields v2.099+ + static if (__VERSION__ >= 2099) + { static struct S { int value; @@ -30,4 +31,5 @@ void main() auto b = S(24); auto ti = typeid(S); assert(ti.equals(&a, &b)); + } }