From 2423b9379841cda4d1f6f5146a05dc5820225a7b Mon Sep 17 00:00:00 2001 From: Teodor Dutu Date: Mon, 19 Feb 2024 19:45:18 +0200 Subject: [PATCH] Fix Bugzilla Issue 24371 - String array concatenation does not respect operator precedence Rethink lowering logic to account for parentheses and operator associativity. Signed-off-by: Teodor Dutu --- compiler/src/dmd/expressionsem.d | 45 ++++++++++++++++-------------- compiler/test/runnable/test24371.d | 15 ++++++++++ 2 files changed, 39 insertions(+), 21 deletions(-) create mode 100644 compiler/test/runnable/test24371.d diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index 996213c65e..6cd282defa 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -12196,30 +12196,33 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return result; } - void handleCatArgument(Expressions *arguments, Expression e, Type catType) + void handleCatArgument(Expressions *arguments, Expression e, Type catType, bool isRightArg) { auto tb = e.type.toBasetype(); - if (!e.parens || (catType.equals(tb) && (tb.ty == Tarray || tb.ty == Tsarray))) - if (auto ce = e.isCatExp()) + if ((isRightArg && e.parens) || (!isRightArg && !tb.equals(catType))) + { + arguments.push(e); + return; + } + + if (auto ce = e.isCatExp()) + { + Expression lowering = ce.lowering; + + /* Skip `file`, `line`, and `funcname` if the hook of the parent + * `CatExp` is `_d_arraycatnTXTrace`. + */ + if (auto callExp = isRuntimeHook(lowering, hook)) { - Expression lowering = ce.lowering; - - /* Skip `file`, `line`, and `funcname` if the hook of the parent - * `CatExp` is `_d_arraycatnTXTrace`. - */ - if (auto callExp = isRuntimeHook(lowering, hook)) - { - if (hook == Id._d_arraycatnTX) - arguments.pushSlice((*callExp.arguments)[]); - else - arguments.pushSlice((*callExp.arguments)[3 .. $]); - } - - return; + if (hook == Id._d_arraycatnTX) + arguments.pushSlice((*callExp.arguments)[]); + else + arguments.pushSlice((*callExp.arguments)[3 .. $]); } - - arguments.push(e); + } + else + arguments.push(e); } auto arguments = new Expressions(); @@ -12232,8 +12235,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor arguments.push(new StringExp(exp.loc, funcname.toDString())); } - handleCatArgument(arguments, exp.e1, exp.type.toBasetype()); - handleCatArgument(arguments, exp.e2, exp.type.toBasetype()); + handleCatArgument(arguments, exp.e1, exp.type.toBasetype(), false); + handleCatArgument(arguments, exp.e2, exp.type.toBasetype(), true); Expression id = new IdentifierExp(exp.loc, Id.empty); id = new DotIdExp(exp.loc, id, Id.object); diff --git a/compiler/test/runnable/test24371.d b/compiler/test/runnable/test24371.d new file mode 100644 index 0000000000..885f9b8648 --- /dev/null +++ b/compiler/test/runnable/test24371.d @@ -0,0 +1,15 @@ +// https://issues.dlang.org/show_bug.cgi?id=24371 + +void main() +{ + assert("b" ~ "c" == "bc"); + assert(["a"] ~ "b" == ["a", "b"]); + assert(["a"] ~ ("b" ~ "c") == ["a", "bc"]); + + auto strArr = ["a"]; + assert(strArr ~ ("b" ~ "c") == ["a", "bc"]); + auto str = "c"; + assert(["a"] ~ ("b" ~ str) == ["a", "bc"]); + + assert(strArr ~ ("b" ~ str) == ["a", "bc"]); +}