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.

8 files changed, 133 insertions, 19 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.
diff --git a/tests/autodiff/diff-assoctype-generic-interface.slang b/tests/autodiff/diff-assoctype-generic-interface.slang
index 79e0eff08..6640e1d94 100644
--- a/tests/autodiff/diff-assoctype-generic-interface.slang
+++ b/tests/autodiff/diff-assoctype-generic-interface.slang
@@ -15,6 +15,7 @@ struct GetterImpl : IGetter
 {
     float[8] data;
 
+    [Differentiable]
     __init(float[8] data)
     { this.data = data; }
 
diff --git a/tests/diagnostics/private-visibility.slang b/tests/diagnostics/private-visibility.slang
index 7c0bad970..b7cb628f4 100644
--- a/tests/diagnostics/private-visibility.slang
+++ b/tests/diagnostics/private-visibility.slang
@@ -24,21 +24,20 @@ struct MyType
         get { return member; }
         set { member = newValue; }
     }
-    // CHECK:{{.*}}(28): error 30601:
+    // CHECK:{{.*}}([[# @LINE+1]]): error 30601:
     public void publicMethod() {} // ERROR.
 }
 
 void test()
 {
-    // CHECK:{{.*}}(34): error 30600:
-    MyType t; // ERROR.
+    MyType t; // should leave uninitialized.
     // CHECK-NOT:{{.*}}error
     t.func1(); // OK.
-    // CHECK:{{.*}}(38): error 30600:
+    // CHECK:{{.*}}([[# @LINE+1]]): error 30600:
     t.func(); // ERROR.
-    // CHECK:{{.*}}(40): error 30600:
+    // CHECK:{{.*}}([[# @LINE+1]]): error 30600:
     t[0] = 1; // ERROR.
 
-    // CHECK:{{.*}}(43): error 30600:
+    // CHECK:{{.*}}([[# @LINE+1]]): error 30600:
     t.member = 2;
 }
diff --git a/tests/language-server/private-ctor-call/bsdfs.slang b/tests/language-server/private-ctor-call/bsdfs.slang
new file mode 100644
index 000000000..ff50ab352
--- /dev/null
+++ b/tests/language-server/private-ctor-call/bsdfs.slang
@@ -0,0 +1,28 @@
+module bsdfs;
+__include lambert_diffuse_btdf;
+
+public struct BSDFContext
+{
+    float iorI;  ///< IOR from incidence medium
+    float iorT;  ///< IOR trom transmission medium
+    bool inited; ///< Flag to indicate if the struct was initialized
+
+    __init(float iorI_, float iorT_)
+    {
+        iorI = iorI_;
+        iorT = iorT_;
+        inited = true;
+    }
+
+    __init()
+    {
+        iorI = 1.f;
+        iorT = 1.f;
+        inited = false;
+    }
+}
+
+public interface IBSDF
+{
+    public float3 eval(const float3 wi, const float3 wo, BSDFContext bc);
+}
diff --git a/tests/language-server/private-ctor-call/lambert_diffuse_btdf.slang b/tests/language-server/private-ctor-call/lambert_diffuse_btdf.slang
new file mode 100644
index 000000000..82f766da4
--- /dev/null
+++ b/tests/language-server/private-ctor-call/lambert_diffuse_btdf.slang
@@ -0,0 +1,22 @@
+implementing bsdfs;
+public struct LambertDiffuseBTDF : IBSDF, IDifferentiable
+{
+    float3 albedo = {}; ///< Diffuse albedo.
+#if 0
+    public __init(float3 albedo_)
+#else
+    __init(float3 albedo_)
+#endif
+    {
+        this.albedo = albedo_;
+    }
+
+    [Differentiable]
+    public float3 eval(const float3 wi, const float3 wo, BSDFContext bc)
+    {
+        if (min(wi.z, -wo.z) < 1e-6f)
+            return float3(0.f);
+
+        return (1.0 / 3.1415) * albedo * -wo.z;
+    }
+}
diff --git a/tests/language-server/private-ctor-call/test-main.slang b/tests/language-server/private-ctor-call/test-main.slang
new file mode 100644
index 000000000..2074c3b8d
--- /dev/null
+++ b/tests/language-server/private-ctor-call/test-main.slang
@@ -0,0 +1,13 @@
+//TEST:LANG_SERVER(filecheck=CHECK):
+
+module "test-main";
+
+import bsdfs;
+
+LambertDiffuseBTDF createTranslucent(float3 color, float3 normal)
+{
+//HOVER:10,21
+    return LambertDiffuseBTDF( color );
+}
+
+//CHECK: LambertDiffuseBTDF.init