From c5b0708ead5de2d90ef14f20b5b8e3ed4f576373 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Fri, 30 Jun 2023 15:25:59 -0400 Subject: Fix for operator assignment issue (#2951) * WIP handling LValue coercion via LValueImplicitCast * Need to have the ptr type for the cast. * Casting conversion working on C++. * Make the LValue casts record if in or in/out as we can produce better code if we know the difference. * WIP LValueCast pass * Fix tests so we don't fail because downstream compilers detect use of uninitialized variable. * Do conversions through through tmp for l-value scenarios that can't work other ways. * Fix a typo. * Change diagnostic implicit-cast-lvalue for a type that still exhibits the issue. * Add matrix test. * Added a bit more clarity around LValue casting choices. * Small comment improvements. Improvements based on comments on PR. * Use findOuterGeneric. --- source/slang/slang-check-expr.cpp | 107 ++++++++++++++++++++++++++++++++++---- 1 file changed, 96 insertions(+), 11 deletions(-) (limited to 'source/slang/slang-check-expr.cpp') diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index e0084e08e..00ece3628 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -1952,6 +1952,51 @@ namespace Slang return checkedExpr; } + static bool _canLValueCoerceScalarType(Type* a, Type* b) + { + auto basicTypeA = as(a); + auto basicTypeB = as(b); + + if (basicTypeA && basicTypeB) + { + const auto& infoA = BaseTypeInfo::getInfo(basicTypeA->baseType); + const auto& infoB = BaseTypeInfo::getInfo(basicTypeB->baseType); + + // TODO(JS): Initially this tries to limit where LValueImplict casts happen. + // We could in principal allow different sizes, as long as we converted to a temprorary + // and back again. + // + // For now we just stick with the simple case. + // // We only allow on integer types for now. In effect just allowing any size uint/int conversions + if (infoA.sizeInBytes == infoB.sizeInBytes && + (infoA.flags & infoB.flags & BaseTypeInfo::Flag::Integer)) + { + return true; + } + + } + return false; + } + + static bool _canLValueCoerce(Type* a, Type* b) + { + // We can *assume* here that if they are coercable, that dimensions of vectors + // and matrices match. We might want to assert to be sure... + SLANG_ASSERT(a != b); + if (a->astNodeType == b->astNodeType) + { + if (auto matA = as(a)) + { + return _canLValueCoerceScalarType(matA->getElementType(), static_cast(b)->getElementType()); + } + else if (auto vecA = as(a)) + { + return _canLValueCoerceScalarType(vecA->getScalarType(), static_cast(b)->getScalarType()); + } + } + return _canLValueCoerceScalarType(a, b); + } + Expr* SemanticsVisitor::CheckInvokeExprWithCheckedOperands(InvokeExpr *expr) { auto rs = ResolveInvoke(expr); @@ -1985,23 +2030,63 @@ namespace Slang if( pp < expr->arguments.getCount() ) { auto argExpr = expr->arguments[pp]; - if( !argExpr->type.isLeftValue ) + if( !argExpr->type.isLeftValue) { - getSink()->diagnose( - argExpr, - Diagnostics::argumentExpectedLValue, - pp); + auto implicitCastExpr = as(argExpr); - if( auto implicitCastExpr = as(argExpr) ) + if (implicitCastExpr && _canLValueCoerce(implicitCastExpr->arguments[0]->type, implicitCastExpr->type)) + { + // This is to work around issues like + // + // ``` + // int a = 0; + // uint b = 1; + // a += b; + // ``` + // That strictly speaking it's not allowed, but we are going to allow it for now + // for situations were the types are uint/int and vector/matrix varieties of those types + // + // Then in lowering we are going to insert code to do something like + // ``` + // var OutType: tmp = arg; + // f(... tmp); + // arg = tmp; + // ``` + + TypeCastExpr* lValueImplicitCast; + + // We want to record if the cast is being used for `out` or `inout`/`ref` as + // if it's just `out` we won't need to convert before passing in. + if (as(paramType)) + { + lValueImplicitCast = getASTBuilder()->create(*implicitCastExpr); + } + else + { + lValueImplicitCast = getASTBuilder()->create(*implicitCastExpr); + } + + // Replace the expression. This should make this situation easier to detect. + expr->arguments[pp] = lValueImplicitCast; + } + else { getSink()->diagnose( argExpr, - Diagnostics::implicitCastUsedAsLValue, - implicitCastExpr->arguments[0]->type, - implicitCastExpr->type); + Diagnostics::argumentExpectedLValue, + pp); + + if(implicitCastExpr) + { + getSink()->diagnose( + argExpr, + Diagnostics::implicitCastUsedAsLValue, + implicitCastExpr->arguments[pp]->type, + implicitCastExpr->type); + } + + maybeDiagnoseThisNotLValue(argExpr); } - - maybeDiagnoseThisNotLValue(argExpr); } } else -- cgit v1.2.3