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.

1 files changed, 36 insertions, 2 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);
                                     }