Warn on float-to-double coercion for arguments. (#2802)

* Warn on float-to-double coercion for arguments.

* Fix test.

* Rename.

* Fixup.

---------

Co-authored-by: Yong He <yhe@nvidia.com>

12 files changed, 83 insertions, 18 deletions

diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 9b2932446..3f1e99880 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -405,10 +405,15 @@ ${{{{
         IROp intrinsicOpCode = getBaseTypeConversionOp(
             kBaseTypes[tt],
             kBaseTypes[ss]);
+
+        BuiltinConversionKind builtinConversionKind = kBuiltinConversion_Unknown;
+        if (kBaseTypes[tt].tag == BaseType::Double &&
+            kBaseTypes[ss].tag == BaseType::Float)
+            builtinConversionKind = kBuiltinConversion_FloatToDouble;
 }}}}
 
     __intrinsic_op($(intrinsicOpCode))
-    __implicit_conversion($(conversionCost))
+    __implicit_conversion($(conversionCost), $(builtinConversionKind))
     __init($(kBaseTypes[ss].name) value);
 
 ${{{{
diff --git a/source/slang/slang-ast-modifier.h b/source/slang/slang-ast-modifier.h
index 00a6570ef..0b077477d 100644
--- a/source/slang/slang-ast-modifier.h
+++ b/source/slang/slang-ast-modifier.h
@@ -956,6 +956,9 @@ class ImplicitConversionModifier : public Modifier
  
     // The conversion cost, used to rank conversions
     ConversionCost cost;
+
+    // A builtin identifier for identifying conversions that need special treatment.
+    BuiltinConversionKind builtinConversionKind;
 };
 
 class FormatAttribute : public Attribute 
diff --git a/source/slang/slang-ast-support-types.h b/source/slang/slang-ast-support-types.h
index 78b0ccfcc..443b6e7bd 100644
--- a/source/slang/slang-ast-support-types.h
+++ b/source/slang/slang-ast-support-types.h
@@ -128,6 +128,13 @@ namespace Slang
         kConversionCost_Impossible = 0xFFFFFFFF,
     };
 
+    typedef unsigned int BuiltinConversionKind;
+    enum : BuiltinConversionKind
+    {
+        kBuiltinConversion_Unknown = 0,
+        kBuiltinConversion_FloatToDouble = 1,
+    };
+
     enum class ImageFormat
     {
 #define FORMAT(NAME, OTHER) NAME,
diff --git a/source/slang/slang-check-conversion.cpp b/source/slang/slang-check-conversion.cpp
index abfe7afa0..7c4560b5b 100644
--- a/source/slang/slang-check-conversion.cpp
+++ b/source/slang/slang-check-conversion.cpp
@@ -23,6 +23,17 @@ namespace Slang
         return kConversionCost_Explicit;
     }
 
+    BuiltinConversionKind SemanticsVisitor::getImplicitConversionBuiltinKind(
+        Decl* decl)
+    {
+        if (auto modifier = decl->findModifier<ImplicitConversionModifier>())
+        {
+            return modifier->builtinConversionKind;
+        }
+
+        return kBuiltinConversion_Unknown;
+    }
+
     bool SemanticsVisitor::isEffectivelyScalarForInitializerLists(
         Type*    type)
     {
@@ -121,6 +132,7 @@ namespace Slang
         {
             ioInitArgIndex++;
             return _coerce(
+                CoercionSite::Initializer,
                 toType,
                 outToExpr,
                 firstInitExpr->type,
@@ -211,6 +223,7 @@ namespace Slang
             {
                 auto arg = fromInitializerListExpr->args[ioArgIndex++];
                 return _coerce(
+                    CoercionSite::Initializer,
                     toType,
                     outToExpr,
                     arg->type,
@@ -616,6 +629,7 @@ namespace Slang
     }
 
     bool SemanticsVisitor::_coerce(
+        CoercionSite site,
         Type*    toType,
         Expr**   outToExpr,
         Type*    fromType,
@@ -852,6 +866,7 @@ namespace Slang
             }
 
             if(!_coerce(
+                site,
                 toType,
                 outToExpr,
                 fromElementType,
@@ -908,6 +923,7 @@ namespace Slang
             }
 
             if (!_coerce(
+                site,
                 toType,
                 outToExpr,
                 fromValueType,
@@ -1012,7 +1028,7 @@ namespace Slang
             // cost associated with the initializer we are invoking.
             //
             ConversionCost cost = getImplicitConversionCost(
-                    overloadContext.bestCandidate->item.declRef.getDecl());;
+                    overloadContext.bestCandidate->item.declRef.getDecl());
 
             // If the cost is too high to be usable as an
             // implicit conversion, then we will report the
@@ -1053,6 +1069,17 @@ namespace Slang
                         getSink()->diagnose(fromExpr, Diagnostics::unrecommendedImplicitConversion, fromType, toType);
                     }
                 }
+                
+                if (site == CoercionSite::Argument)
+                {
+                    auto builtinConversionKind = getImplicitConversionBuiltinKind(
+                        overloadContext.bestCandidate->item.declRef.getDecl());
+                    if (builtinConversionKind == kBuiltinConversion_FloatToDouble)
+                    {
+                        if (!as<FloatingPointLiteralExpr>(fromExpr))
+                            getSink()->diagnose(fromExpr, Diagnostics::implicitConversionToDouble);
+                    }
+                }
             }
             if(outCost)
                 *outCost = cost;
@@ -1146,6 +1173,7 @@ namespace Slang
         // during the coercion process.
         //
         bool rs = _coerce(
+            CoercionSite::General,
             toType,
             nullptr,
             fromType,
@@ -1215,11 +1243,13 @@ namespace Slang
 
 
     Expr* SemanticsVisitor::coerce(
+        CoercionSite site,
         Type*    toType,
         Expr*    fromExpr)
     {
         Expr* expr = nullptr;
         if (!_coerce(
+            site,
             toType,
             &expr,
             fromExpr->type,
diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index 63c7d9741..9b94ba492 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -1191,7 +1191,7 @@ namespace Slang
                 if (auto initExpr = varDecl->initExpr)
                 {
                     initExpr = CheckTerm(initExpr);
-                    initExpr = coerce(varDecl->type.Ptr(), initExpr);
+                    initExpr = coerce(CoercionSite::Initializer, varDecl->type.Ptr(), initExpr);
                     varDecl->initExpr = initExpr;
 
                     maybeInferArraySizeForVariable(varDecl);
@@ -1355,7 +1355,7 @@ namespace Slang
             // it to the type of the variable.
             //
             initExpr = CheckTerm(initExpr);
-            initExpr = coerce(varDecl->type.Ptr(), initExpr);
+            initExpr = coerce(CoercionSite::Initializer, varDecl->type.Ptr(), initExpr);
             varDecl->initExpr = initExpr;
 
             // We need to ensure that any variable doesn't introduce
@@ -2860,7 +2860,7 @@ namespace Slang
         // so we also need to coerce the result of the call to
         // the expected type.
         //
-        auto coercedCall = subVisitor.coerce(resultType, checkedCall);
+        auto coercedCall = subVisitor.coerce(CoercionSite::Return, resultType, checkedCall);
 
         // If our overload resolution or type coercion failed,
         // then we have not been able to synthesize a witness
@@ -3182,7 +3182,7 @@ namespace Slang
                 // which involves coercing the member access `this.name` to
                 // the expected type of the property.
                 //
-                auto coercedMemberRef = subVisitor.coerce(propertyType, synMemberRef);
+                auto coercedMemberRef = subVisitor.coerce(CoercionSite::Return, propertyType, synMemberRef);
                 auto synReturn = m_astBuilder->create<ReturnStmt>();
                 synReturn->expression = coercedMemberRef;
 
@@ -4684,7 +4684,7 @@ namespace Slang
         if(auto initExpr = decl->tagExpr)
         {
             initExpr = CheckTerm(initExpr);
-            initExpr = coerce(tagType, initExpr);
+            initExpr = coerce(CoercionSite::General, tagType, initExpr);
 
             // We want to enforce that this is an integer constant
             // expression, but we don't actually care to retain
@@ -5551,7 +5551,7 @@ namespace Slang
             // actual type of the parameter.
             //
             initExpr = CheckTerm(initExpr);
-            initExpr = coerce(typeExpr.type, initExpr);
+            initExpr = coerce(CoercionSite::Initializer, typeExpr.type, initExpr);
             paramDecl->initExpr = initExpr;
 
             // TODO: a default argument expression needs to
diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp
index 0f744997d..507b8c30f 100644
--- a/source/slang/slang-check-expr.cpp
+++ b/source/slang/slang-check-expr.cpp
@@ -1618,13 +1618,13 @@ namespace Slang
         switch (coercionType)
         {
         case IntegerConstantExpressionCoercionType::SpecificType:
-            expr = coerce(expectedType, inExpr);
+            expr = coerce(CoercionSite::General, expectedType, inExpr);
             break;
         case IntegerConstantExpressionCoercionType::AnyInteger:
             if (isScalarIntegerType(inExpr->type))
                 expr = inExpr;
             else
-                expr = coerce(m_astBuilder->getIntType(), inExpr);
+                expr = coerce(CoercionSite::General, m_astBuilder->getIntType(), inExpr);
             break;
         default:
             break;
@@ -1862,7 +1862,7 @@ namespace Slang
         expr->left = maybeOpenRef(expr->left);
         auto type = expr->left->type;
         auto right = maybeOpenRef(expr->right);
-        expr->right = coerce(type, right);
+        expr->right = coerce(CoercionSite::Assignment, type, right);
 
         if (!type.isLeftValue)
         {
@@ -2650,7 +2650,7 @@ namespace Slang
                             initListExpr->loc = expr->loc;
                             auto checkedInitListExpr = visitInitializerListExpr(initListExpr);
 
-                            return coerce(typeExp.type, checkedInitListExpr);
+                            return coerce(CoercionSite::General, typeExp.type, checkedInitListExpr);
                         }
                     }
                 }
diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h
index 50992b00b..18a60b8b3 100644
--- a/source/slang/slang-check-impl.h
+++ b/source/slang/slang-check-impl.h
@@ -219,6 +219,15 @@ namespace Slang
         Dictionary<BasicTypeKeyPair, ConversionCost> conversionCostCache;
     };
 
+    enum class CoercionSite
+    {
+        General,
+        Assignment,
+        Argument,
+        Return,
+        Initializer,
+    };
+
         /// Shared state for a semantics-checking session.
     struct SharedSemanticsContext
     {
@@ -773,6 +782,9 @@ namespace Slang
         ConversionCost getImplicitConversionCost(
             Decl* decl);
 
+        BuiltinConversionKind getImplicitConversionBuiltinKind(
+            Decl* decl);
+
         bool isEffectivelyScalarForInitializerLists(
             Type*    type);
 
@@ -886,6 +898,7 @@ namespace Slang
             /// should be emitted on failure.
             ///
         bool _coerce(
+            CoercionSite site,
             Type*    toType,
             Expr**   outToExpr,
             Type*    fromType,
@@ -924,6 +937,7 @@ namespace Slang
 
             /// Implicitly coerce `fromExpr` to `toType` and diagnose errors if it isn't possible
         Expr* coerce(
+            CoercionSite site,
             Type*    toType,
             Expr*    fromExpr);
 
diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp
index a068f19d6..d96d9e39e 100644
--- a/source/slang/slang-check-modifier.cpp
+++ b/source/slang/slang-check-modifier.cpp
@@ -559,7 +559,7 @@ namespace Slang
                     if (!typeChecked)
                     {
                         arg = CheckTerm(arg);
-                        arg = coerce(paramDecl->getType(), arg);
+                        arg = coerce(CoercionSite::Argument, paramDecl->getType(), arg);
                     }
                 }
                 paramIndex++;
diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp
index f786089f8..d544ce5a1 100644
--- a/source/slang/slang-check-overload.cpp
+++ b/source/slang/slang-check-overload.cpp
@@ -320,7 +320,7 @@ namespace Slang
                 }
                 else
                 {
-                    arg = coerce(getType(m_astBuilder, valParamRef), arg);
+                    arg = coerce(CoercionSite::Argument, getType(m_astBuilder, valParamRef), arg);
                 }
 
                 // If we have an argument to work with, then we will
@@ -427,7 +427,7 @@ namespace Slang
             }
             else
             {
-                arg = coerce(paramType, arg);
+                arg = coerce(CoercionSite::Argument, paramType, arg);
             }
         }
         return true;
diff --git a/source/slang/slang-check-stmt.cpp b/source/slang/slang-check-stmt.cpp
index 6efc3e52e..b2830febe 100644
--- a/source/slang/slang-check-stmt.cpp
+++ b/source/slang/slang-check-stmt.cpp
@@ -157,7 +157,7 @@ namespace Slang
         }
         Expr* e = expr;
         e = CheckTerm(e);
-        e = coerce(m_astBuilder->getBoolType(), e);
+        e = coerce(CoercionSite::General, m_astBuilder->getBoolType(), e);
         return e;
     }
 
@@ -313,7 +313,7 @@ namespace Slang
             {
                 if (function)
                 {
-                    stmt->expression = coerce(function->returnType.Ptr(), stmt->expression);
+                    stmt->expression = coerce(CoercionSite::Return, function->returnType.Ptr(), stmt->expression);
                 }
                 else
                 {
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 128142d84..dcd376474 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -302,6 +302,7 @@ DIAGNOSTIC(33071, Error, expectedAStringLiteral, "expected a string literal")
 DIAGNOSTIC(   -1, Note, noteExplicitConversionPossible, "explicit conversion from '$0' to '$1' is possible")
 DIAGNOSTIC(30080, Error, ambiguousConversion, "more than one implicit conversion exists from '$0' to '$1'")
 DIAGNOSTIC(30081, Warning, unrecommendedImplicitConversion, "implicit conversion from '$0' to '$1' is not recommended")
+DIAGNOSTIC(30082, Warning, implicitConversionToDouble, " implicit float-to-double conversion may cause unexpected performance issues, use explicit cast if intended.")
 DIAGNOSTIC(30090, Error, tryClauseMustApplyToInvokeExpr, "expression in a 'try' clause must be a call to a function or operator overload.")
 DIAGNOSTIC(30091, Error, tryInvokeCalleeShouldThrow, "'$0' called from a 'try' clause does not throw an error, make sure the callee is marked as 'throws'")
 DIAGNOSTIC(30092, Error, calleeOfTryCallMustBeFunc, "callee in a 'try' clause must be a function")
diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp
index 45f4be477..555c79445 100644
--- a/source/slang/slang-parser.cpp
+++ b/source/slang/slang-parser.cpp
@@ -6560,14 +6560,19 @@ namespace Slang
     static NodeBase* parseImplicitConversionModifier(Parser* parser, void* /*userData*/)
     {
         ImplicitConversionModifier* modifier = parser->astBuilder->create<ImplicitConversionModifier>();
-
+        BuiltinConversionKind builtinKind = kBuiltinConversion_Unknown;
         ConversionCost cost = kConversionCost_Default;
         if( AdvanceIf(parser, TokenType::LParent) )
         {
             cost = ConversionCost(StringToInt(parser->ReadToken(TokenType::IntegerLiteral).getContent()));
+            if (AdvanceIf(parser, TokenType::Comma))
+            {
+                builtinKind = BuiltinConversionKind(StringToInt(parser->ReadToken(TokenType::IntegerLiteral).getContent()));
+            }
             parser->ReadToken(TokenType::RParent);
         }
         modifier->cost = cost;
+        modifier->builtinConversionKind = builtinKind;
         return modifier;
     }