diff options
| -rw-r--r-- | source/slang/slang-check-overload.cpp | 151 | ||||
| -rw-r--r-- | tests/bugs/generic-type-arg-overloaded.slang | 33 | ||||
| -rw-r--r-- | tests/bugs/generic-type-arg-overloaded.slang.expected | 11 |
3 files changed, 168 insertions, 27 deletions
diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp index 8eba71767..e64fef4aa 100644 --- a/source/slang/slang-check-overload.cpp +++ b/source/slang/slang-check-overload.cpp @@ -148,55 +148,148 @@ namespace Slang { auto genericDeclRef = candidate.item.declRef.as<GenericDecl>(); - // We will go ahead and hang onto the arguments that we've - // already checked, since downstream validation might need - // them. + // The basic idea here is that we need to check that the + // arguments to a generic application (e.g., `F<A1, A2, ...>`) + // have the right "type," which in this context means + // checking that: + // + // * The argument for any generic type parameter is a (proper) type. + // + // * The argument for any generic value parameter is a + // specialization-time constant value of the appropriate type. + // + // Some additional checks are *not* handled at this point: + // + // * We don't check that a type argument actually conforms to + // the constraints on the parameter. + // + // Along the way we will build up a `GenericSubstitution` + // to represent the arguments that have been coerced to + // appropriateforms. + // auto genSubst = new GenericSubstitution(); candidate.subst = genSubst; auto& checkedArgs = genSubst->args; + // Rather than bail out as soon as we hit a problem, + // we are going to process *all* of the parameters of the + // generic and place suitable arguments into the `checkedArgs` + // array. This is important so that we don't cause crashes + // in cases where the arguments fail this step of checking, + // but we decide to proceed with subsequent steps (e.g., + // because the candidate we are trying here is the *only* + // candidate). + // + bool success = true; + Index aa = 0; for (auto memberRef : getMembers(genericDeclRef)) { if (auto typeParamRef = memberRef.as<GenericTypeParamDecl>()) { - if (aa >= context.argCount) + // We have a type parameter, and we expect to find + // a type argument. + // + TypeExp typeArg; + if( aa >= context.argCount ) { - return false; + // If we have run out of arguments, then we definitely + // fail checking (in principle this should have been + // checked already by an earlier step). + // + success = false; } - auto arg = context.getArg(aa++); - - TypeExp typeExp; - if (context.mode == OverloadResolveContext::Mode::JustTrying) + else { - typeExp = tryCoerceToProperType(TypeExp(arg)); - if(!typeExp.type) + // If we have at least one argument left, we grab + // it and try to coerce it to a proper type. The + // manner in which we handle the coercion depends + // on whether we are "just trying" the candidate + // (so a failure would rule out the candidate, but + // shouldn't be reported to the user), or are doing + // the checking "for real" in which case any errors + // we run into need to be reported. + // + auto arg = context.getArg(aa++); + if (context.mode == OverloadResolveContext::Mode::JustTrying) { - return false; + typeArg = tryCoerceToProperType(TypeExp(arg)); + } + else + { + typeArg = CoerceToProperType(TypeExp(arg)); } } - else + + // If we failed to get a valid type (either because + // there was no matching argument, or because the + // "just trying" coercion failed), then we create + // an error type to stand in for the argument + // + if( !typeArg.type ) { - typeExp = CoerceToProperType(TypeExp(arg)); + typeArg.type = getSession()->getErrorType(); + success = false; } - checkedArgs.add(typeExp.type); + + checkedArgs.add(typeArg.type); } else if (auto valParamRef = memberRef.as<GenericValueParamDecl>()) { - auto arg = context.getArg(aa++); - - if (context.mode == OverloadResolveContext::Mode::JustTrying) + // The case for a generic value parameter is similar to that + // for a generic type parameter. + // + RefPtr<Expr> arg; + if( aa >= context.argCount ) { - ConversionCost cost = kConversionCost_None; - if (!canCoerce(GetType(valParamRef), arg->type, &cost)) + // If there are no arguments left to consume, then + // we have a definite failure. + // + success = false; + } + else + { + // If we have an argument then we need to coerce it + // to the type of the parameter (and fail if the + // coercion is not possible) + // + arg = context.getArg(aa++); + if (context.mode == OverloadResolveContext::Mode::JustTrying) { - return false; + ConversionCost cost = kConversionCost_None; + if (!canCoerce(GetType(valParamRef), arg->type, &cost)) + { + success = false; + } + candidate.conversionCostSum += cost; + } + else + { + arg = coerce(GetType(valParamRef), arg); } - candidate.conversionCostSum += cost; } - arg = coerce(GetType(valParamRef), arg); - auto val = ExtractGenericArgInteger(arg, context.mode == OverloadResolveContext::Mode::JustTrying ? nullptr : getSink()); + // If we have an argument to work with, then we will + // try to extract its speicalization-time constant value. + // + // TODO: This is one of the places where we will need to + // generalize in order to support generic value parameters + // with types other than `int`. + // + RefPtr<Val> val; + if( arg ) + { + val = ExtractGenericArgInteger(arg, context.mode == OverloadResolveContext::Mode::JustTrying ? nullptr : getSink()); + } + + // If any of the above checking steps fail and we don't + // have a value to work with here, we will instead + // use an "error" value to stand in for the argument. + // + if( !val ) + { + val = new ErrorIntVal(); + } checkedArgs.add(val); } else @@ -205,8 +298,12 @@ namespace Slang } } - // Okay, we've made it! - return true; + // Once we are done processing the parameters of the generic, + // we will have build up a usable `checkedArgs` array and + // can return to the caller a report of whether we + // were successful or not. + // + return success; } bool SemanticsVisitor::TryCheckOverloadCandidateTypes( @@ -1672,7 +1769,7 @@ namespace Slang else { // Nothing at all was found that we could even consider invoking - getSink()->diagnose(genericAppExpr, Diagnostics::unimplemented, "expected a generic"); + getSink()->diagnose(genericAppExpr, Diagnostics::expectedAGeneric, baseExpr->type); return CreateErrorExpr(genericAppExpr); } } diff --git a/tests/bugs/generic-type-arg-overloaded.slang b/tests/bugs/generic-type-arg-overloaded.slang new file mode 100644 index 000000000..99150d2f0 --- /dev/null +++ b/tests/bugs/generic-type-arg-overloaded.slang @@ -0,0 +1,33 @@ +// generic-type-arg-overloaded.slang + +//DIAGNOSTIC_TEST:SIMPLE: + +// Regression test to confirm that type checker +// doesn't crash when an overloaded identifier +// is used as a generic type argument. + +interface IThing { int getVal(); } + +struct Stuff : IThing { int getVal() { return 1; } } + +// Conflicting declaration: +struct Stuff {} + +int util<T : IThing>() { return 1; } + +struct G {} +int nonGeneric() { return 2; } + +int test() +{ + // This call should note the ambiguity, + // rather than crash. + // + return util<Stuff>() + + // Adding an extra call to also test the + // case of trying to speicalize something + // like a generic when it isn't one. + // + + nonGeneric<G>(); +} diff --git a/tests/bugs/generic-type-arg-overloaded.slang.expected b/tests/bugs/generic-type-arg-overloaded.slang.expected new file mode 100644 index 000000000..518abc2b1 --- /dev/null +++ b/tests/bugs/generic-type-arg-overloaded.slang.expected @@ -0,0 +1,11 @@ +result code = -1 +standard error = { +tests/bugs/generic-type-arg-overloaded.slang(14): error 30200: declaration of 'Stuff' conflicts with existing declaration +tests/bugs/generic-type-arg-overloaded.slang(11): note: see previous declaration of 'Stuff' +tests/bugs/generic-type-arg-overloaded.slang(26): error 39999: ambiguous reference to 'Stuff' +tests/bugs/generic-type-arg-overloaded.slang(14): note 39999: candidate: Stuff +tests/bugs/generic-type-arg-overloaded.slang(11): note 39999: candidate: Stuff +tests/bugs/generic-type-arg-overloaded.slang(32): error 39999: expected a generic when using '<...>' (found: '() -> int') +} +standard output = { +} |