Skip to content

Commit adaea0e

Browse files
csyonghecsyonghe
authored
Warning on lossy implicit casts. (shader-slang#2367)
* Warning on bool to float conversion. * Fix test cases. * Improve. * LanguageServer: don't show constant value for non constant variables. * Fix tests. * Fix warnings in tests. Co-authored-by: Yong He <yhe@nvidia.com>
1 parent d65c618 commit adaea0e

File tree

163 files changed

+546
-329
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

163 files changed

+546
-329
lines changed

‎source/slang/slang-check-conversion.cpp

+28-4
Original file line numberDiff line numberDiff line change
@@ -955,15 +955,39 @@ namespace Slang
955955
// but then emit a diagnostic when actually reifying
956956
// the result expression.
957957
//
958-
if( cost >= kConversionCost_Explicit )
958+
if (outToExpr)
959959
{
960-
if( outToExpr )
960+
if (cost >= kConversionCost_Explicit)
961961
{
962962
getSink()->diagnose(fromExpr, Diagnostics::typeMismatch, toType, fromType);
963-
getSink()->diagnose(fromExpr, Diagnostics::noteExplicitConversionPossible, fromType, toType);
963+
getSink()->diagnose(
964+
fromExpr, Diagnostics::noteExplicitConversionPossible, fromType, toType);
965+
}
966+
else if (cost >= kConversionCost_Default)
967+
{
968+
// For general types of implicit conversions, we issue a warning, unless `fromExpr` is a known constant
969+
// and we know it won't cause a problem.
970+
bool shouldEmitGeneralWarning = true;
971+
if (isScalarIntegerType(toType))
972+
{
973+
if (auto intVal = tryFoldIntegerConstantExpression(fromExpr, nullptr))
974+
{
975+
if (auto val = as<ConstantIntVal>(intVal))
976+
{
977+
if (isIntValueInRangeOfType(val->value, toType))
978+
{
979+
// OK.
980+
shouldEmitGeneralWarning = false;
981+
}
982+
}
983+
}
984+
}
985+
if (shouldEmitGeneralWarning)
986+
{
987+
getSink()->diagnose(fromExpr, Diagnostics::unrecommendedImplicitConversion, fromType, toType);
988+
}
964989
}
965990
}
966-
967991
if(outCost)
968992
*outCost = cost;
969993

‎source/slang/slang-check-decl.cpp

+33-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#include "slang-syntax.h"
1616

17+
#include <limits>
18+
1719
namespace Slang
1820
{
1921
/// Visitor to transition declarations to `DeclCheckState::CheckedModifiers`
@@ -3487,7 +3489,36 @@ namespace Slang
34873489
if(!basicType)
34883490
return false;
34893491

3490-
return isIntegerBaseType(basicType->baseType);
3492+
return isIntegerBaseType(basicType->baseType) || basicType->baseType == BaseType::Bool;
3493+
}
3494+
3495+
bool SemanticsVisitor::isIntValueInRangeOfType(IntegerLiteralValue value, Type* type)
3496+
{
3497+
auto basicType = as<BasicExpressionType>(type);
3498+
if (!basicType)
3499+
return false;
3500+
3501+
switch (basicType->baseType)
3502+
{
3503+
case BaseType::UInt8:
3504+
return (value >= 0 && value <= std::numeric_limits<uint8_t>::max()) || (value == -1);
3505+
case BaseType::UInt16:
3506+
return (value >= 0 && value <= std::numeric_limits<uint16_t>::max()) || (value == -1);
3507+
case BaseType::UInt:
3508+
return (value >= 0 && value <= std::numeric_limits<uint32_t>::max()) || (value == -1);
3509+
case BaseType::UInt64:
3510+
return true;
3511+
case BaseType::Int8:
3512+
return value >= std::numeric_limits<int8_t>::min() && value <= std::numeric_limits<int8_t>::max();
3513+
case BaseType::Int16:
3514+
return value >= std::numeric_limits<int16_t>::min() && value <= std::numeric_limits<int16_t>::max();
3515+
case BaseType::Int:
3516+
return value >= std::numeric_limits<int32_t>::min() && value <= std::numeric_limits<int32_t>::max();
3517+
case BaseType::Int64:
3518+
return value >= std::numeric_limits<int64_t>::min() && value <= std::numeric_limits<int64_t>::max();
3519+
default:
3520+
return false;
3521+
}
34913522
}
34923523

34933524
void SemanticsVisitor::validateEnumTagType(Type* type, SourceLoc const& loc)
@@ -3772,7 +3803,7 @@ namespace Slang
37723803
// We want to enforce that this is an integer constant
37733804
// expression, but we don't actually care to retain
37743805
// the value.
3775-
CheckIntegerConstantExpression(initExpr);
3806+
CheckIntegerConstantExpression(initExpr, IntegerConstantExpressionCoercionType::AnyInteger, nullptr);
37763807

37773808
decl->tagExpr = initExpr;
37783809
}

‎source/slang/slang-check-expr.cpp

+96-44
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,8 @@ namespace Slang
884884

885885
auto funcDeclRef = getDeclRef(m_astBuilder, funcDeclRefExpr);
886886
auto intrinsicMod = funcDeclRef.getDecl()->findModifier<IntrinsicOpModifier>();
887-
if (!intrinsicMod)
887+
auto implicitCast = funcDeclRef.getDecl()->findModifier<ImplicitConversionModifier>();
888+
if (!intrinsicMod && !implicitCast)
888889
{
889890
// We can't constant fold anything that doesn't map to a builtin
890891
// operation right now.
@@ -942,54 +943,91 @@ namespace Slang
942943

943944
// At this point, all the operands had simple integer values, so we are golden.
944945
IntegerLiteralValue resultValue = 0;
945-
auto opName = funcDeclRef.getName();
946-
947-
// handle binary operators
948-
if (opName == getName("-"))
946+
// If this is an implicit cast, we can try to fold.
947+
if (implicitCast)
949948
{
950-
if (argCount == 1)
949+
auto targetBasicType = as<BasicExpressionType>(invokeExpr.getExpr()->type.type);
950+
if (!targetBasicType)
951+
return nullptr;
952+
switch (targetBasicType->baseType)
951953
{
952-
resultValue = -constArgVals[0];
954+
case BaseType::Bool:
955+
resultValue = constArgVals[0] != 0;
956+
break;
957+
case BaseType::Int:
958+
case BaseType::UInt:
959+
case BaseType::UInt16:
960+
case BaseType::Int16:
961+
case BaseType::UInt8:
962+
case BaseType::Int8:
963+
resultValue = constArgVals[0];
964+
break;
965+
default:
966+
return nullptr;
953967
}
954-
else if (argCount == 2)
968+
}
969+
else
970+
{
971+
auto opName = funcDeclRef.getName();
972+
973+
// handle binary operators
974+
if (opName == getName("-"))
955975
{
956-
resultValue = constArgVals[0] - constArgVals[1];
976+
if (argCount == 1)
977+
{
978+
resultValue = -constArgVals[0];
979+
}
980+
else if (argCount == 2)
981+
{
982+
resultValue = constArgVals[0] - constArgVals[1];
983+
}
984+
}
985+
else if (opName == getName("!"))
986+
{
987+
resultValue = constArgVals[0] != 0;
988+
}
989+
else if (opName == getName("~"))
990+
{
991+
resultValue = ~constArgVals[0];
957992
}
958-
}
959993

960-
// simple binary operators
994+
// simple binary operators
961995
#define CASE(OP) \
962-
else if(opName == getName(#OP)) do { \
963-
if(argCount != 2) return nullptr; \
964-
resultValue = constArgVals[0] OP constArgVals[1]; \
965-
} while(0)
966-
967-
CASE(+); // TODO: this can also be unary...
968-
CASE(*);
969-
CASE(<<);
970-
CASE(>>);
971-
CASE(&);
972-
CASE(|);
973-
CASE(^);
996+
else if(opName == getName(#OP)) do { \
997+
if(argCount != 2) return nullptr; \
998+
resultValue = constArgVals[0] OP constArgVals[1]; \
999+
} while(0)
1000+
1001+
CASE(+); // TODO: this can also be unary...
1002+
CASE(*);
1003+
CASE(<<);
1004+
CASE(>>);
1005+
CASE(&);
1006+
CASE(|);
1007+
CASE(^);
1008+
CASE(!=);
1009+
CASE(==);
1010+
CASE(>=);
1011+
CASE(<=);
1012+
CASE(<);
1013+
CASE(>);
9741014
#undef CASE
975-
976-
// binary operators with chance of divide-by-zero
977-
// TODO: issue a suitable error in that case
1015+
// binary operators with chance of divide-by-zero
1016+
// TODO: issue a suitable error in that case
9781017
#define CASE(OP) \
979-
else if(opName == getName(#OP)) do { \
980-
if(argCount != 2) return nullptr; \
981-
if(!constArgVals[1]) return nullptr; \
982-
resultValue = constArgVals[0] OP constArgVals[1]; \
983-
} while(0)
984-
985-
CASE(/);
986-
CASE(%);
1018+
else if(opName == getName(#OP)) do { \
1019+
if(argCount != 2) return nullptr; \
1020+
if(!constArgVals[1]) return nullptr; \
1021+
resultValue = constArgVals[0] OP constArgVals[1]; \
1022+
} while(0)
1023+
CASE(/);
1024+
CASE(%);
9871025
#undef CASE
988-
989-
// TODO(tfoley): more cases
990-
else
991-
{
992-
return nullptr;
1026+
// TODO(tfoley): more cases
1027+
else
1028+
{
1029+
return nullptr;
1030+
}
9931031
}
9941032

9951033
IntVal* result = m_astBuilder->create<ConstantIntVal>(resultValue);
@@ -1126,13 +1164,27 @@ namespace Slang
11261164
return tryConstantFoldExpr(expr, circularityInfo);
11271165
}
11281166

1129-
IntVal* SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr, DiagnosticSink* sink)
1167+
IntVal* SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr, IntegerConstantExpressionCoercionType coercionType, Type* expectedType, DiagnosticSink* sink)
11301168
{
11311169
// No need to issue further errors if the expression didn't even type-check.
11321170
if(IsErrorExpr(inExpr)) return nullptr;
11331171

11341172
// First coerce the expression to the expected type
1135-
auto expr = coerce(m_astBuilder->getIntType(),inExpr);
1173+
Expr* expr = nullptr;
1174+
switch (coercionType)
1175+
{
1176+
case IntegerConstantExpressionCoercionType::SpecificType:
1177+
expr = coerce(expectedType, inExpr);
1178+
break;
1179+
case IntegerConstantExpressionCoercionType::AnyInteger:
1180+
if (isScalarIntegerType(inExpr->type))
1181+
expr = inExpr;
1182+
else
1183+
expr = coerce(m_astBuilder->getIntType(), inExpr);
1184+
break;
1185+
default:
1186+
break;
1187+
}
11361188

11371189
// No need to issue further errors if the type coercion failed.
11381190
if(IsErrorExpr(expr)) return nullptr;
@@ -1145,9 +1197,9 @@ namespace Slang
11451197
return result;
11461198
}
11471199

1148-
IntVal* SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr)
1200+
IntVal* SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr, IntegerConstantExpressionCoercionType coercionType, Type* expectedType)
11491201
{
1150-
return CheckIntegerConstantExpression(inExpr, getSink());
1202+
return CheckIntegerConstantExpression(inExpr, coercionType, expectedType, getSink());
11511203
}
11521204

11531205
IntVal* SemanticsVisitor::CheckEnumConstantExpression(Expr* expr)
@@ -1218,7 +1270,7 @@ namespace Slang
12181270
IntVal* elementCount = nullptr;
12191271
if (indexExpr)
12201272
{
1221-
elementCount = CheckIntegerConstantExpression(indexExpr);
1273+
elementCount = CheckIntegerConstantExpression(indexExpr, IntegerConstantExpressionCoercionType::AnyInteger, nullptr);
12221274
}
12231275

12241276
auto elementType = CoerceToUsableType(TypeExp(baseExpr, baseTypeType->type));

‎source/slang/slang-check-impl.h

+11-4
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,8 @@ namespace Slang
562562

563563
Type* ExtractGenericArgType(Expr* exp);
564564

565-
IntVal* ExtractGenericArgInteger(Expr* exp, DiagnosticSink* sink);
566-
IntVal* ExtractGenericArgInteger(Expr* exp);
565+
IntVal* ExtractGenericArgInteger(Expr* exp, Type* genericParamType, DiagnosticSink* sink);
566+
IntVal* ExtractGenericArgInteger(Expr* exp, Type* genericParamType);
567567

568568
Val* ExtractGenericArgVal(Expr* exp);
569569

@@ -1035,6 +1035,8 @@ namespace Slang
10351035
/// Is `type` a scalar integer type.
10361036
bool isScalarIntegerType(Type* type);
10371037

1038+
bool isIntValueInRangeOfType(IntegerLiteralValue value, Type* type);
1039+
10381040
// Validate that `type` is a suitable type to use
10391041
// as the tag type for an `enum`
10401042
void validateEnumTagType(Type* type, SourceLoc const& loc);
@@ -1135,8 +1137,13 @@ namespace Slang
11351137
ConstantFoldingCircularityInfo* circularityInfo);
11361138

11371139
// Enforce that an expression resolves to an integer constant, and get its value
1138-
IntVal* CheckIntegerConstantExpression(Expr* inExpr);
1139-
IntVal* CheckIntegerConstantExpression(Expr* inExpr, DiagnosticSink* sink);
1140+
enum class IntegerConstantExpressionCoercionType
1141+
{
1142+
SpecificType,
1143+
AnyInteger
1144+
};
1145+
IntVal* CheckIntegerConstantExpression(Expr* inExpr, IntegerConstantExpressionCoercionType coercionType, Type* expectedType);
1146+
IntVal* CheckIntegerConstantExpression(Expr* inExpr, IntegerConstantExpressionCoercionType coercionType, Type* expectedType, DiagnosticSink* sink);
11401147

11411148
IntVal* CheckEnumConstantExpression(Expr* expr);
11421149

‎source/slang/slang-check-modifier.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace Slang
1818
// First type-check the expression as normal
1919
expr = CheckExpr(expr);
2020

21-
auto intVal = CheckIntegerConstantExpression(expr);
21+
auto intVal = CheckIntegerConstantExpression(expr, IntegerConstantExpressionCoercionType::AnyInteger, nullptr);
2222
if(!intVal)
2323
return nullptr;
2424

‎source/slang/slang-check-overload.cpp

+1-5
Original file line numberDiff line numberDiff line change
@@ -314,14 +314,10 @@ namespace Slang
314314
// If we have an argument to work with, then we will
315315
// try to extract its speicalization-time constant value.
316316
//
317-
// TODO: This is one of the places where we will need to
318-
// generalize in order to support generic value parameters
319-
// with types other than `int`.
320-
//
321317
Val* val = nullptr;
322318
if( arg )
323319
{
324-
val = ExtractGenericArgInteger(arg, context.mode == OverloadResolveContext::Mode::JustTrying ? nullptr : getSink());
320+
val = ExtractGenericArgInteger(arg, getType(m_astBuilder, valParamRef), context.mode == OverloadResolveContext::Mode::JustTrying ? nullptr : getSink());
325321
}
326322

327323
// If any of the above checking steps fail and we don't

‎source/slang/slang-check-stmt.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ namespace Slang
140140
Expr* SemanticsVisitor::checkExpressionAndExpectIntegerConstant(Expr* expr, IntVal** outIntVal)
141141
{
142142
expr = CheckExpr(expr);
143-
auto intVal = CheckIntegerConstantExpression(expr);
143+
auto intVal = CheckIntegerConstantExpression(expr, IntegerConstantExpressionCoercionType::AnyInteger, nullptr);
144144
if (outIntVal)
145145
*outIntVal = intVal;
146146
return expr;

0 commit comments

Comments
 (0)