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.

3 files changed, 64 insertions, 13 deletions

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>();
     varExpr->type = (QualType)getASTBuilder()->getTypeType(toType);
     varExpr->declRef = isDeclRefTypeOf<Decl>(toType);
+    varExpr->loc = loc;
+    varExpr->checked = true;
 
     auto* constructorExpr = getASTBuilder()->create<ExplicitCtorInvokeExpr>();
     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<char const*>(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<InvokeExpr>(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<FuncType>(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<ConstructorDecl>(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<InvokeExpr>(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<ExplicitCtorInvokeExpr>(expr))
+    if (expr->arguments.getCount() == 1 && !as<ExplicitCtorInvokeExpr>(expr) &&
+        !as<InitializerListExpr>(expr->arguments[0]))
     {
         if (const auto typeType = as<TypeType>(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<InvokeExpr>(outExpr))
+                {
+                    invokeExpr->originalFunctionExpr = typeExpr;
+                    return CreateErrorExpr(invokeExpr);
+                }
+                return CreateErrorExpr(typeExpr);
+            }
             return outExpr;
+        }
     }
 
     // Nothing at all was found that we could even consider invoking.