From 52a45890b5ab71d7dbfdd01955afce129728d67e Mon Sep 17 00:00:00 2001 From: Yong He Date: Tue, 22 Jul 2025 08:47:50 -0700 Subject: Fix crash when private ctor is used for coercion. (#7858) * Fix crash when private ctor is used for coercion. * Fix tests. * Fix. * Fix test error. --- source/slang/slang-check-conversion.cpp | 31 ++++++++++++++++++++---- source/slang/slang-check-expr.cpp | 3 +++ source/slang/slang-check-overload.cpp | 43 +++++++++++++++++++++++++++------ 3 files changed, 64 insertions(+), 13 deletions(-) (limited to 'source') diff --git a/source/slang/slang-check-conversion.cpp b/source/slang/slang-check-conversion.cpp index 3ace9f999..61d132627 100644 --- a/source/slang/slang-check-conversion.cpp +++ b/source/slang/slang-check-conversion.cpp @@ -346,6 +346,8 @@ Expr* SemanticsVisitor::_createCtorInvokeExpr( auto* varExpr = getASTBuilder()->create(); varExpr->type = (QualType)getASTBuilder()->getTypeType(toType); varExpr->declRef = isDeclRefTypeOf(toType); + varExpr->loc = loc; + varExpr->checked = true; auto* constructorExpr = getASTBuilder()->create(); constructorExpr->functionExpr = varExpr; @@ -382,6 +384,8 @@ bool SemanticsVisitor::createInvokeExprForExplicitCtor( fromInitializerListExpr->args); DiagnosticSink tempSink(getSourceManager(), nullptr); + tempSink.setFlags(getSink()->getFlags()); + SemanticsVisitor subVisitor(withSink(&tempSink)); ctorInvokeExpr = subVisitor.CheckTerm(ctorInvokeExpr); @@ -395,6 +399,12 @@ bool SemanticsVisitor::createInvokeExprForExplicitCtor( getSink()->diagnoseRaw( Severity::Error, static_cast(blob->getBufferPointer())); + // For non-c-style types, we will always return true when there + // is a ctor, so that we do not fallback to legacy initializer list logic + // in `_coerceInitializerList()` and produce unrelated errors. + if (outExpr) + *outExpr = CreateErrorExpr(ctorInvokeExpr); + return true; } return false; } @@ -956,13 +966,13 @@ bool SemanticsVisitor::_coerceInitializerList( } // We will fall back to the legacy logic of initialize list. + Expr* outInitListExpr = nullptr; if (!_readAggregateValueFromInitializerList( toType, - outToExpr, + &outInitListExpr, fromInitializerListExpr, argIndex)) return false; - if (argIndex != argCount) { if (outToExpr) @@ -974,6 +984,8 @@ bool SemanticsVisitor::_coerceInitializerList( argCount); } } + if (outToExpr) + *outToExpr = outInitListExpr; return true; } @@ -1868,9 +1880,18 @@ bool SemanticsVisitor::_coerce( castExpr->arguments.add(args[0]); } if (!cachedMethod) - getShared()->cacheImplicitCastMethod( - implicitCastKey, - ImplicitCastMethod{*overloadContext.bestCandidate, cost}); + { + // We can only cache the method if it is a public, otherwise we may not be able to + // use this method depending on where we are performing the coercion. + if (overloadContext.bestCandidate->item.declRef && + getDeclVisibility(overloadContext.bestCandidate->item.declRef.getDecl()) == + DeclVisibility::Public) + { + getShared()->cacheImplicitCastMethod( + implicitCastKey, + ImplicitCastMethod{*overloadContext.bestCandidate, cost}); + } + } return true; } if (!cachedMethod) diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index 2891c316f..6fed59621 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -2845,6 +2845,9 @@ Expr* SemanticsVisitor::CheckInvokeExprWithCheckedOperands(InvokeExpr* expr) auto rs = ResolveInvoke(expr); if (auto invoke = as(rs)) { + if (!invoke->functionExpr) + return rs; + // if this is still an invoke expression, test arguments passed to inout/out parameter are // LValues if (auto funcType = as(invoke->functionExpr->type)) diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp index 18fb9798b..8a8da3eb2 100644 --- a/source/slang/slang-check-overload.cpp +++ b/source/slang/slang-check-overload.cpp @@ -245,13 +245,6 @@ bool SemanticsVisitor::TryCheckOverloadCandidateVisibility( OverloadResolveContext& context, OverloadCandidate const& candidate) { - // Always succeeds when we are trying out constructors. - if (context.mode == OverloadResolveContext::Mode::JustTrying) - { - if (as(candidate.item.declRef)) - return true; - } - if (!context.sourceScope) return true; @@ -1280,6 +1273,21 @@ error: if (context.originalExpr) { + // Even when there is an error, we still want to update + // the expr we return to refer to the candidate we found so far + // so language server can still provide info on the potential callee. + if (candidate.flavor == OverloadCandidate::Flavor::Func) + { + if (auto invokeExpr = as(context.originalExpr)) + { + invokeExpr->functionExpr = ConstructLookupResultExpr( + candidate.item, + context.baseExpr, + candidate.item.declRef.getName(), + context.funcLoc, + context.originalExpr); + } + } return CreateErrorExpr(context.originalExpr); } else @@ -2711,9 +2719,15 @@ Expr* SemanticsVisitor::ResolveInvoke(InvokeExpr* expr) // equivalent in (almost) all cases. // If callee is a type, and we are calling with one argument, then treat it as a // type coercion. + // + // Exception: if the argument is an initializer list, such as + // Foo({1,2,3}), we should not coerce {1,2,3} to Foo, but rather + // treat it as a ctor call with {1,2,3} as the first argument. + // bool typeOverloadChecked = false; - if (expr->arguments.getCount() == 1 && !as(expr)) + if (expr->arguments.getCount() == 1 && !as(expr) && + !as(expr->arguments[0])) { if (const auto typeType = as(funcExpr->type)) { @@ -2943,7 +2957,20 @@ Expr* SemanticsVisitor::ResolveInvoke(InvokeExpr* expr) initListExpr->type = m_astBuilder->getInitializerListType(); Expr* outExpr = nullptr; if (_coerceInitializerList(typetype->getType(), &outExpr, initListExpr)) + { + // If there is a coercion error, make sure we return a valid original expr + // for language server to use. + if (IsErrorExpr(outExpr)) + { + if (auto invokeExpr = as(outExpr)) + { + invokeExpr->originalFunctionExpr = typeExpr; + return CreateErrorExpr(invokeExpr); + } + return CreateErrorExpr(typeExpr); + } return outExpr; + } } // Nothing at all was found that we could even consider invoking. -- cgit v1.2.3