Disable l-value coercion for ref types (#2960)

* Make lvalue coercion not work for ref, to stop problem with atomics (for GLSL output).

* Improve some comments.

6 files changed, 115 insertions, 47 deletions

diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp
index 9dc359a5e..587dcc2b5 100644
--- a/source/slang/slang-check-expr.cpp
+++ b/source/slang/slang-check-expr.cpp
@@ -2051,7 +2051,20 @@ namespace Slang
                             {
                                 auto implicitCastExpr = as<ImplicitCastExpr>(argExpr);
 
-                                if (implicitCastExpr && _canLValueCoerce(implicitCastExpr->arguments[0]->type, implicitCastExpr->type))
+                                // NOTE: 
+                                // This is currently only enabled for in/inout based scenarios. Ie NOT ref.
+                                // 
+                                // Depending on the target there can be an issue around atomics.
+                                // The fall back transformation with InOut/OutImplicitCast is to introduce 
+                                // a temporary, and do the work on that and copy back.
+                                // 
+                                // This doesn't work with an atomic. So the work around is to not enable
+                                // the transformation with ref types, which atomics are defined on.
+                                // 
+                                // An argument can be made that transformation shouldn't apply to the ref scenario in general.
+                                if (implicitCastExpr && 
+                                    as<OutTypeBase>(paramType) && 
+                                    _canLValueCoerce(implicitCastExpr->arguments[0]->type, implicitCastExpr->type))
                                 {
                                     // This is to work around issues like
                                     //
@@ -2093,11 +2106,32 @@ namespace Slang
                                         Diagnostics::argumentExpectedLValue,
                                         pp);
 
+                                    
                                     if(implicitCastExpr)
                                     {
+                                        const DiagnosticInfo* diagnostic = nullptr;
+
+                                        // Try and determine reason for failure
+                                        if (as<RefType>(paramType))
+                                        {
+                                            // Ref types are not allowed to use this mechanism because it breaks atomics 
+                                            diagnostic = &Diagnostics::implicitCastUsedAsLValueRef;
+                                        }
+                                        else if (!_canLValueCoerce(implicitCastExpr->arguments[0]->type, implicitCastExpr->type))
+                                        {
+                                            // We restict what types can use this mechanism - currently int/uint and same sized matrix/vectors
+                                            // of those types.
+                                            diagnostic = &Diagnostics::implicitCastUsedAsLValueType;
+                                        }
+                                        else
+                                        {
+                                            // Fall back, in case there are other reasons...
+                                            diagnostic = &Diagnostics::implicitCastUsedAsLValue;
+                                        }
+
                                         getSink()->diagnose(
                                             argExpr,
-                                            Diagnostics::implicitCastUsedAsLValue,
+                                            *diagnostic,
                                             implicitCastExpr->arguments[pp]->type,
                                             implicitCastExpr->type);
                                     }
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 0fd962614..922348d61 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -270,8 +270,9 @@ DIAGNOSTIC(30031, Error, projectTypeMismatch, "'project': expression must evalua
 DIAGNOSTIC(30033, Error, invalidTypeForLocalVariable, "cannot declare a local variable of this type.")
 DIAGNOSTIC(30035, Error, componentOverloadTypeMismatch, "'$0': type of overloaded component mismatches previous definition.")
 DIAGNOSTIC(30041, Error, bitOperationNonIntegral, "bit operation: operand must be integral type.")
+DIAGNOSTIC(30043, Error, getStringHashRequiresStringLiteral, "getStringHash parameter can only accept a string literal")
 DIAGNOSTIC(30047, Error, argumentExpectedLValue, "argument passed to parameter '$0' must be l-value.")
-DIAGNOSTIC(30048, Note,  implicitCastUsedAsLValue, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value")
+
 DIAGNOSTIC(30049, Note,  thisIsImmutableByDefault, "a 'this' parameter is an immutable parameter by default in Slang; apply the `[mutating]` attribute to the function declaration to opt in to a mutable `this`")
 DIAGNOSTIC(30050, Error,  mutatingMethodOnImmutableValue, "mutating method '$0' cannot be called on an immutable value")
 
@@ -284,11 +285,13 @@ DIAGNOSTIC(30056, Warning, useOfNonShortCircuitingOperator, "non-short-circuitin
 DIAGNOSTIC(30057, Error, assignmentInPredicateExpr, "use an assignment operation as predicate expression is not allowed, wrap the assignment with '()' to clarify the intent.")
 DIAGNOSTIC(30058, Warning, danglingEqualityExpr, "result of '==' not used, did you intend '='?")
 
-DIAGNOSTIC(30043, Error, getStringHashRequiresStringLiteral, "getStringHash parameter can only accept a string literal")
-
 DIAGNOSTIC(30060, Error, expectedAType, "expected a type, got a '$0'")
 DIAGNOSTIC(30061, Error, expectedANamespace, "expected a namespace, got a '$0'")
 
+DIAGNOSTIC(30062, Note,  implicitCastUsedAsLValueRef, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value with a reference")
+DIAGNOSTIC(30063, Note,  implicitCastUsedAsLValueType, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value with this type")
+DIAGNOSTIC(30064, Note,  implicitCastUsedAsLValue, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value for this usage")
+
 DIAGNOSTIC(30065, Error, newCanOnlyBeUsedToInitializeAClass, "`new` can only be used to initialize a class")
 DIAGNOSTIC(30066, Error, classCanOnlyBeInitializedWithNew, "a class can only be initialized by a `new` clause")
 
diff --git a/tests/bugs/atomic-coerce.slang b/tests/bugs/atomic-coerce.slang
new file mode 100644
index 000000000..1e56a8d25
--- /dev/null
+++ b/tests/bugs/atomic-coerce.slang
@@ -0,0 +1,27 @@
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -shaderobj 
+//TEST(compute,vulkan):COMPARE_COMPUTE_EX:-vk -slang -compute -shaderobj
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0 ], stride=4):out,name outputBuffer
+RWStructuredBuffer<int> outputBuffer;
+
+groupshared uint gs_values[2];
+
+[shader("compute")]
+[numthreads(4, 1, 1)]
+void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+    int index = (int)dispatchThreadID.x;
+    
+    // Initialize first
+    if (index < 2)
+    {
+        gs_values[index] = 2;
+    }
+
+    GroupMemoryBarrierWithGroupSync();
+ 
+    // NOTE! We have to cast to uint, to make atomic work
+    InterlockedAdd(gs_values[index & 1], uint(index * index));
+           
+    outputBuffer[index] = gs_values[index & 1];
+}
diff --git a/tests/bugs/atomic-coerce.slang.expected.txt b/tests/bugs/atomic-coerce.slang.expected.txt
new file mode 100644
index 000000000..9f774a3a1
--- /dev/null
+++ b/tests/bugs/atomic-coerce.slang.expected.txt
@@ -0,0 +1,4 @@
+6
+C
+6
+C
diff --git a/tests/diagnostics/bad-operator-call.slang.expected b/tests/diagnostics/bad-operator-call.slang.expected
index 21cb6bd41..82eaef848 100644
--- a/tests/diagnostics/bad-operator-call.slang.expected
+++ b/tests/diagnostics/bad-operator-call.slang.expected
@@ -3,63 +3,63 @@ standard error = {
 tests/diagnostics/bad-operator-call.slang(18): error 39999: no overload for '+=' applicable to arguments of type (int, S)
     a += b;
       ^~
-core.meta.slang(2430): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, T) -> matrix<T,R,C>
-core.meta.slang(2422): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, matrix<T,R,C>) -> matrix<T,R,C>
-core.meta.slang(2414): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, T) -> vector<T,N>
-core.meta.slang(2406): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, vector<T,N>) -> vector<T,N>
-core.meta.slang(2398): note 39999: candidate: func +=<T>(out T, T) -> T
+core.meta.slang(2655): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, T) -> matrix<T,R,C>
+core.meta.slang(2647): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, matrix<T,R,C>) -> matrix<T,R,C>
+core.meta.slang(2639): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, T) -> vector<T,N>
+core.meta.slang(2631): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, vector<T,N>) -> vector<T,N>
+core.meta.slang(2623): note 39999: candidate: func +=<T>(out T, T) -> T
 tests/diagnostics/bad-operator-call.slang(20): error 39999: no overload for '+' applicable to arguments of type (int, S)
     a = a + b;
           ^
-core.meta.slang(2256): note 39999: candidate: func +(uintptr_t, uintptr_t) -> uintptr_t
-core.meta.slang(2248): note 39999: candidate: func +(uint64_t, uint64_t) -> uint64_t
-core.meta.slang(2240): note 39999: candidate: func +(uint, uint) -> uint
-core.meta.slang(2232): note 39999: candidate: func +(uint16_t, uint16_t) -> uint16_t
-core.meta.slang(2224): note 39999: candidate: func +(uint8_t, uint8_t) -> uint8_t
-core.meta.slang(2216): note 39999: candidate: func +(double, double) -> double
-core.meta.slang(2208): note 39999: candidate: func +(float, float) -> float
-core.meta.slang(2200): note 39999: candidate: func +(half, half) -> half
-core.meta.slang(2192): note 39999: candidate: func +(intptr_t, intptr_t) -> intptr_t
-core.meta.slang(2184): note 39999: candidate: func +(int64_t, int64_t) -> int64_t
+core.meta.slang(2481): note 39999: candidate: func +(uintptr_t, uintptr_t) -> uintptr_t
+core.meta.slang(2473): note 39999: candidate: func +(uint64_t, uint64_t) -> uint64_t
+core.meta.slang(2465): note 39999: candidate: func +(uint, uint) -> uint
+core.meta.slang(2457): note 39999: candidate: func +(uint16_t, uint16_t) -> uint16_t
+core.meta.slang(2449): note 39999: candidate: func +(uint8_t, uint8_t) -> uint8_t
+core.meta.slang(2441): note 39999: candidate: func +(double, double) -> double
+core.meta.slang(2433): note 39999: candidate: func +(float, float) -> float
+core.meta.slang(2425): note 39999: candidate: func +(half, half) -> half
+core.meta.slang(2417): note 39999: candidate: func +(intptr_t, intptr_t) -> intptr_t
+core.meta.slang(2409): note 39999: candidate: func +(int64_t, int64_t) -> int64_t
 tests/diagnostics/bad-operator-call.slang(20): note 39999: 3 more overload candidates
 tests/diagnostics/bad-operator-call.slang(22): error 39999: no overload for '~' applicable to arguments of type (S)
     a = ~b;
         ^
-core.meta.slang(2125): note 39999: candidate: __prefix func ~(uintptr_t) -> uintptr_t
-core.meta.slang(2121): note 39999: candidate: __prefix func ~(uint64_t) -> uint64_t
-core.meta.slang(2117): note 39999: candidate: __prefix func ~(uint) -> uint
-core.meta.slang(2113): note 39999: candidate: __prefix func ~(uint16_t) -> uint16_t
-core.meta.slang(2109): note 39999: candidate: __prefix func ~(uint8_t) -> uint8_t
-core.meta.slang(2105): note 39999: candidate: __prefix func ~(intptr_t) -> intptr_t
-core.meta.slang(2101): note 39999: candidate: __prefix func ~(int64_t) -> int64_t
-core.meta.slang(2097): note 39999: candidate: __prefix func ~(int) -> int
-core.meta.slang(2093): note 39999: candidate: __prefix func ~(int16_t) -> int16_t
-core.meta.slang(2089): note 39999: candidate: __prefix func ~(int8_t) -> int8_t
+core.meta.slang(2350): note 39999: candidate: __prefix func ~(uintptr_t) -> uintptr_t
+core.meta.slang(2346): note 39999: candidate: __prefix func ~(uint64_t) -> uint64_t
+core.meta.slang(2342): note 39999: candidate: __prefix func ~(uint) -> uint
+core.meta.slang(2338): note 39999: candidate: __prefix func ~(uint16_t) -> uint16_t
+core.meta.slang(2334): note 39999: candidate: __prefix func ~(uint8_t) -> uint8_t
+core.meta.slang(2330): note 39999: candidate: __prefix func ~(intptr_t) -> intptr_t
+core.meta.slang(2326): note 39999: candidate: __prefix func ~(int64_t) -> int64_t
+core.meta.slang(2322): note 39999: candidate: __prefix func ~(int) -> int
+core.meta.slang(2318): note 39999: candidate: __prefix func ~(int16_t) -> int16_t
+core.meta.slang(2314): note 39999: candidate: __prefix func ~(int8_t) -> int8_t
 tests/diagnostics/bad-operator-call.slang(27): error 30047: argument passed to parameter '0' must be l-value.
     a += c;
     ^
-tests/diagnostics/bad-operator-call.slang(27): note 30048: argument was implicitly cast from 'int' to 'vector<int,4>', and Slang does not support using an implicit cast as an l-value
+tests/diagnostics/bad-operator-call.slang(27): note 30063: argument was implicitly cast from 'int' to 'vector<int,4>', and Slang does not support using an implicit cast as an l-value with this type
 tests/diagnostics/bad-operator-call.slang(31): error 39999: no overload for '+=' applicable to arguments of type (vector<float,3>, vector<int,4>)
     d += c;
       ^~
-core.meta.slang(2430): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, T) -> matrix<T,R,C>
-core.meta.slang(2422): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, matrix<T,R,C>) -> matrix<T,R,C>
-core.meta.slang(2414): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, T) -> vector<T,N>
-core.meta.slang(2406): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, vector<T,N>) -> vector<T,N>
-core.meta.slang(2398): note 39999: candidate: func +=<T>(out T, T) -> T
+core.meta.slang(2655): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, T) -> matrix<T,R,C>
+core.meta.slang(2647): note 39999: candidate: func +=<T, R:int, C:int>(out matrix<T,R,C>, matrix<T,R,C>) -> matrix<T,R,C>
+core.meta.slang(2639): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, T) -> vector<T,N>
+core.meta.slang(2631): note 39999: candidate: func +=<T, N:int>(out vector<T,N>, vector<T,N>) -> vector<T,N>
+core.meta.slang(2623): note 39999: candidate: func +=<T>(out T, T) -> T
 tests/diagnostics/bad-operator-call.slang(33): error 39999: no overload for '+' applicable to arguments of type (vector<int,4>, vector<float,3>)
     d = c + d;
           ^
-core.meta.slang(2262): note 39999: candidate: func +<4>(uintptr_t4, uintptr_t) -> uintptr_t4
-core.meta.slang(2260): note 39999: candidate: func +<3>(uintptr_t, uintptr_t3) -> uintptr_t3
-core.meta.slang(2256): note 39999: candidate: func +(uintptr_t, uintptr_t) -> uintptr_t
-core.meta.slang(2254): note 39999: candidate: func +<4>(uint64_t4, uint64_t) -> uint64_t4
-core.meta.slang(2252): note 39999: candidate: func +<3>(uint64_t, uint64_t3) -> uint64_t3
-core.meta.slang(2248): note 39999: candidate: func +(uint64_t, uint64_t) -> uint64_t
-core.meta.slang(2246): note 39999: candidate: func +<4>(uint4, uint) -> uint4
-core.meta.slang(2244): note 39999: candidate: func +<3>(uint, uint3) -> uint3
-core.meta.slang(2240): note 39999: candidate: func +(uint, uint) -> uint
-core.meta.slang(2238): note 39999: candidate: func +<4>(uint16_t4, uint16_t) -> uint16_t4
+core.meta.slang(2487): note 39999: candidate: func +<4>(uintptr_t4, uintptr_t) -> uintptr_t4
+core.meta.slang(2485): note 39999: candidate: func +<3>(uintptr_t, uintptr_t3) -> uintptr_t3
+core.meta.slang(2481): note 39999: candidate: func +(uintptr_t, uintptr_t) -> uintptr_t
+core.meta.slang(2479): note 39999: candidate: func +<4>(uint64_t4, uint64_t) -> uint64_t4
+core.meta.slang(2477): note 39999: candidate: func +<3>(uint64_t, uint64_t3) -> uint64_t3
+core.meta.slang(2473): note 39999: candidate: func +(uint64_t, uint64_t) -> uint64_t
+core.meta.slang(2471): note 39999: candidate: func +<4>(uint4, uint) -> uint4
+core.meta.slang(2469): note 39999: candidate: func +<3>(uint, uint3) -> uint3
+core.meta.slang(2465): note 39999: candidate: func +(uint, uint) -> uint
+core.meta.slang(2463): note 39999: candidate: func +<4>(uint16_t4, uint16_t) -> uint16_t4
 tests/diagnostics/bad-operator-call.slang(33): note 39999: 29 more overload candidates
 }
 standard output = {
diff --git a/tests/diagnostics/implicit-cast-lvalue.slang.expected b/tests/diagnostics/implicit-cast-lvalue.slang.expected
index 920ad17c2..10ebfc656 100644
--- a/tests/diagnostics/implicit-cast-lvalue.slang.expected
+++ b/tests/diagnostics/implicit-cast-lvalue.slang.expected
@@ -6,7 +6,7 @@ tests/diagnostics/implicit-cast-lvalue.slang(19): warning 30081: implicit conver
 tests/diagnostics/implicit-cast-lvalue.slang(19): error 30047: argument passed to parameter '0' must be l-value.
     a(y);
       ^
-tests/diagnostics/implicit-cast-lvalue.slang(19): note 30048: argument was implicitly cast from 'double' to 'float', and Slang does not support using an implicit cast as an l-value
+tests/diagnostics/implicit-cast-lvalue.slang(19): note 30063: argument was implicitly cast from 'double' to 'float', and Slang does not support using an implicit cast as an l-value with this type
 }
 standard output = {
 }