summaryrefslogtreecommitdiff
path: root/source/slang/slang-check-overload.cpp
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-12-06 15:50:32 -0800
committerGitHub <noreply@github.com>2019-12-06 15:50:32 -0800
commit2e52217cb870b4101c1639fed78224f89bf119b3 (patch)
tree3290c233716b809d7e453364c20fc448b2c821a5 /source/slang/slang-check-overload.cpp
parent895fcff7df1a71af59fad6bb31939ff370920eb4 (diff)
Support conversion from int/uint to enum types (#1147)
* Support conversion from int/uint to enum types The basic feature here is tiny, and is summarized in the code added to the stdlib: ``` extension __EnumType { __init(int val); __init(uint val); } ``` The front-end already makes all `enum` types implicitly conform to `__EnumType` behind the scenes, and this `extension` makes it so that all such types inherit some initializers (`__init` declarations, aka. "constructors") that take `int` and `uint`. (Note: right now all `__init` declarations in Slang are assumed to be implemented as intrinsics using `kIROp_Construct`. This obviously needs to change some day, especially so that we can support user-defined initializers.) Actually making this *work* required a bit of fleshing out pieces of the compiler that had previously been a bit ad hoc to be a bit more "correct." Most of the rest of this description is focused on those details, since the main feature is not itself very exciting. When overload resolution sees an attempt to "call" a type (e.g., `MyType(3.0)`) it needs to add appropriate overload candidates for the initializers in that type, which may take different numbers and types of parameters. The existing code for handling this case was using an ad hoc approach to try to enumerate the initializer declarations to consider, which might be found via inheritance, `extension` declarations, etc. In practice, the ad hoc logic for looking up initializers was just doing a subset of the work that already goes into doing member lookup. Changing the code so that it effectively does lookup for `MyType.__init` allows us to look up initializers in a way that is consistent with any other case of member lookup. Generalizing this lookup step brings us one step closer to being able to go from an `enum` type `E` to an initializer defined on an `extension` of an `interface` that `E` conforms to. One casualty of using the ordinary lookup logic for initializers is that we used to pass the type being constructed down into the logic that enumerated the initializers, which made it easier to short-circuit the part of overload resolution that usually asks "what type does this candidate return." It might seem "obvious" that an initializer/constructor on type `Foo` should return a value of type `Foo`, but that isn't necessarily true. Consider the `__BuiltinFloatingPointType` interface, which requires all the built-in floating-point types (`float`, `double`, `half`) to have an initializer that can take a `float`. If we call that interface in a generic context for `T : __BuiltinFloatingPointType`, then we want to treat that initializer as returning `T` and not `__BuiltinFloatingPointType`. Without the ad hoc logic in initializer overload resolution, this is the exact problem that surfaced for the stdlib definition of `clamp`. The solution to the "what type does an initializer return" problem was to introduce a notion of a `ThisType`, which refers to the type of `this` in the body of an interface. More generally, we will eventually want to have the keyword `This` be the type-level equivalent of `this`, and be usable inside any type. The `calcThisType` function introduced here computes a reasonable `Type` to represent the value of `This` within a given declaration. Inside of concrete type it refers to the type itself, while in an `interface` it will always be a `ThisType`. The existing `ThisTypeSubstitution`s, previously only applied to associated types, now apply to `ThisType`s as well, in the same situations. The next roadblock for making the simple declarations for `__EnumType` work was that the lookup logic was only doing lookup through inheritance relationships when the type being looked up in was an `interface`. The logic in play was reasonable: if you are doing lookup in a type `T` that inherits from `IFoo`, then why bother looking for `IFoo::bar` when there must be a `T::bar` if `T` actually implements the interface? The catch in this case is that `IFoo::bar` might not be a requirement of `IFoo`, but rather a concrete method added via an `extension`, in which case `T` need not have its own concrete `bar`. The simple/obvious fix here was to make the lookup logic always include inherited members, even when looking up through a concrete type. Of course, if we allow lookup to see `IFoo::bar` when looking up on `T`, then we have the problem that both `T::bar` and `IFoo::bar` show up in the lookup results, and potentially lead to an "ambiguous overload" error. This problem arises for any interface rquirement (so both methods and associated types right now). In order to get around it, I added a somewhat grungy check for comparing overload candidates (during overload resolution) or `LookupResultItem`s (during resolution of simple overloaded identifiers) that considers a member of a concrete type as automatically "better" than a member of an interface. The Right Way to solve this problem in the long run requires some more subtlety, but for now this check should Just Work. One final wrinkle is that due to our IR lowering pass being a bit overzealous, we currently end up trying to emit IR for those new `__init` declarations, which ends up causing us to try and emit IR for a `ThisType`. That is a case that will require some subtlty to handle correctly down the line, for for now we do the expedient thing and emit the `ThisType` for `IFoo` as `IFoo` itself, which is not especially correct, but doesn't matter since the concrete initializer won't ever be called. * testing: add more debug output to Unix process launch function * testing: increase timeout when running command-line tests
Diffstat (limited to 'source/slang/slang-check-overload.cpp')
-rw-r--r--source/slang/slang-check-overload.cpp248
1 files changed, 114 insertions, 134 deletions
diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp
index a7ae187a7..dc013b781 100644
--- a/source/slang/slang-check-overload.cpp
+++ b/source/slang/slang-check-overload.cpp
@@ -1,6 +1,8 @@
// slang-check-overload.cpp
#include "slang-check-impl.h"
+#include "slang-lookup.h"
+
// This file implements semantic checking logic related
// to resolving overloading call operations, by checking
// the applicability and relative priority of various candidates.
@@ -482,6 +484,59 @@ namespace Slang
}
}
+ /// Does the given `declRef` represent an interface requirement?
+ bool isInterfaceRequirement(DeclRef<Decl> const& declRef)
+ {
+ if(!declRef)
+ return false;
+
+ auto parent = declRef.GetParent();
+ if(parent.as<GenericDecl>())
+ parent = parent.GetParent();
+
+ if(parent.as<InterfaceDecl>())
+ return true;
+
+ return false;
+ }
+
+ int SemanticsVisitor::CompareLookupResultItems(
+ LookupResultItem const& left,
+ LookupResultItem const& right)
+ {
+ // It is possible for lookup to return both an interface requirement
+ // and the concrete function that satisfies that requirement.
+ // We always want to favor a concrete method over an interface
+ // requirement it might override.
+ //
+ // TODO: This should turn into a more detailed check such that
+ // a candidate for declaration A is always better than a candidate
+ // for declaration B if A is an override of B. We can't
+ // easily make that check right now because we aren't tracking
+ // this kind of "is an override of ..." information on declarations
+ // directly (it is only visible through the requirement witness
+ // information for inheritance declarations).
+ //
+ bool leftIsInterfaceRequirement = isInterfaceRequirement(left.declRef);
+ bool rightIsInterfaceRequirement = isInterfaceRequirement(right.declRef);
+ if(leftIsInterfaceRequirement != rightIsInterfaceRequirement)
+ return int(leftIsInterfaceRequirement) - int(rightIsInterfaceRequirement);
+
+ // TODO: We should always have rules such that in a tie a declaration
+ // A::m is better than B::m when all other factors are equal and
+ // A inherits from B.
+
+ // TODO: There are other cases like this we need to add in terms
+ // of ranking/prioritizing overloads, around things like
+ // "transparent" members, or when lookup proceeds from an "inner"
+ // to an "outer" scope. In many cases the right way to proceed
+ // could involve attaching a distance/cost/rank to things directly
+ // as part of lookup, and in other cases it might be best handled
+ // as a semantic check based on the actual declarations found.
+
+ return 0;
+ }
+
int SemanticsVisitor::CompareOverloadCandidates(
OverloadCandidate* left,
OverloadCandidate* right)
@@ -496,6 +551,15 @@ namespace Slang
{
if (left->conversionCostSum != right->conversionCostSum)
return left->conversionCostSum - right->conversionCostSum;
+
+ // If all conversion costs match, then we should consider
+ // whether one of the two items/declarations should be
+ // preferred based on grounds that have nothing to do
+ // with applicability or conversion costs.
+ //
+ auto itemDiff = CompareLookupResultItems(left->item, right->item);
+ if(itemDiff)
+ return itemDiff;
}
return 0;
@@ -758,135 +822,39 @@ namespace Slang
return DeclRef<Decl>(innerDecl, constraintSubst);
}
- void SemanticsVisitor::AddAggTypeOverloadCandidates(
- LookupResultItem typeItem,
+ void SemanticsVisitor::AddTypeOverloadCandidates(
RefPtr<Type> type,
- DeclRef<AggTypeDecl> aggTypeDeclRef,
- OverloadResolveContext& context,
- RefPtr<Type> resultType)
+ OverloadResolveContext& context)
{
- for (auto ctorDeclRef : getMembersOfType<ConstructorDecl>(aggTypeDeclRef))
- {
- // now work through this candidate...
- AddCtorOverloadCandidate(typeItem, type, ctorDeclRef, context, resultType);
- }
-
- // Also check for generic constructors.
+ // The code being checked is trying to apply `type` like a function.
+ // Semantically, the operations `T(args...)` is equivalent to
+ // `T.__init(args...)` if we had a surface syntax that supported
+ // looking up `__init` declarations by that name.
//
- // TODO: There is way too much duplication between this case and the extension
- // handling below, and all of this is *also* duplicative with the ordinary
- // overload resolution logic for function.
+ // Internally, all `__init` declarations are stored with the name
+ // `$init`, to avoid potential conflicts if a user decided to name
+ // a field/method `__init`.
//
- // The right solution is to handle a "constructor" call expression by
- // first doing member lookup in the type (for initializer members, which
- // should all share a common name), and then to do overload resolution using
- // the (possibly overloaded) result of that lookup.
+ // We will look up all the initializers on `type` by looking up
+ // its members named `$init`, and then proceed to perform overload
+ // resolution with what we find.
//
- for (auto genericDeclRef : getMembersOfType<GenericDecl>(aggTypeDeclRef))
- {
- if (auto ctorDecl = as<ConstructorDecl>(genericDeclRef.getDecl()->inner))
- {
- DeclRef<Decl> innerRef = SpecializeGenericForOverload(genericDeclRef, context);
- if (!innerRef)
- continue;
-
- DeclRef<ConstructorDecl> innerCtorRef = innerRef.as<ConstructorDecl>();
- AddCtorOverloadCandidate(typeItem, type, innerCtorRef, context, resultType);
- }
- }
-
- // Now walk through any extensions we can find for this types
- for (auto ext = GetCandidateExtensions(aggTypeDeclRef); ext; ext = ext->nextCandidateExtension)
- {
- auto extDeclRef = ApplyExtensionToType(ext, type);
- if (!extDeclRef)
- continue;
-
- for (auto ctorDeclRef : getMembersOfType<ConstructorDecl>(extDeclRef))
- {
- // TODO(tfoley): `typeItem` here should really reference the extension...
-
- // now work through this candidate...
- AddCtorOverloadCandidate(typeItem, type, ctorDeclRef, context, resultType);
- }
-
- // Also check for generic constructors
- for (auto genericDeclRef : getMembersOfType<GenericDecl>(extDeclRef))
- {
- if (auto ctorDecl = genericDeclRef.getDecl()->inner.as<ConstructorDecl>())
- {
- DeclRef<Decl> innerRef = SpecializeGenericForOverload(genericDeclRef, context);
- if (!innerRef)
- continue;
-
- DeclRef<ConstructorDecl> innerCtorRef = innerRef.as<ConstructorDecl>();
-
- AddCtorOverloadCandidate(typeItem, type, innerCtorRef, context, resultType);
-
- // TODO(tfoley): need a way to do the solving step for the constraint system
- }
- }
- }
- }
-
- void SemanticsVisitor::addGenericTypeParamOverloadCandidates(
- DeclRef<GenericTypeParamDecl> typeDeclRef,
- OverloadResolveContext& context,
- RefPtr<Type> resultType)
- {
- // We need to look for any constraints placed on the generic
- // type parameter, since they will give us information on
- // interfaces that the type must conform to.
-
- // We expect the parent of the generic type parameter to be a generic...
- auto genericDeclRef = typeDeclRef.GetParent().as<GenericDecl>();
- SLANG_ASSERT(genericDeclRef);
-
- for(auto constraintDeclRef : getMembersOfType<GenericTypeConstraintDecl>(genericDeclRef))
- {
- // Does this constraint pertain to the type we are working on?
- //
- // We want constraints of the form `T : Foo` where `T` is the
- // generic parameter in question, and `Foo` is whatever we are
- // constraining it to.
- auto subType = GetSub(constraintDeclRef);
- auto subDeclRefType = as<DeclRefType>(subType);
- if(!subDeclRefType)
- continue;
- if(!subDeclRefType->declRef.Equals(typeDeclRef))
- continue;
-
- // The super-type in the constraint (e.g., `Foo` in `T : Foo`)
- // will tell us a type we should use for lookup.
- auto bound = GetSup(constraintDeclRef);
-
- // Go ahead and use the target type:
- //
- // TODO: Need to consider case where this might recurse infinitely.
- AddTypeOverloadCandidates(bound, context, resultType);
- }
- }
-
- void SemanticsVisitor::AddTypeOverloadCandidates(
- RefPtr<Type> type,
- OverloadResolveContext& context,
- RefPtr<Type> resultType)
- {
- if (auto declRefType = as<DeclRefType>(type))
- {
- auto declRef = declRefType->declRef;
- if (auto aggTypeDeclRef = declRef.as<AggTypeDecl>())
- {
- AddAggTypeOverloadCandidates(LookupResultItem(aggTypeDeclRef), type, aggTypeDeclRef, context, resultType);
- }
- else if(auto genericTypeParamDeclRef = declRef.as<GenericTypeParamDecl>())
- {
- addGenericTypeParamOverloadCandidates(
- genericTypeParamDeclRef,
- context,
- resultType);
- }
- }
+ // TODO: One wrinkle here is single-argument constructor syntax.
+ // An operation like `(T) oneArg` or `T(oneArg)` is currently
+ // treated as a call expression, but we might want such cases
+ // to go through the type coercion logic first/instead, because
+ // by doing so we could weed out cases where a type is "constructed"
+ // from a value of the same type. There is no need in Slang for
+ // "copy constructors" but the stdlib currently has to define
+ // some just to make code that does, e.g., `float(1.0f)` work.
+
+ LookupResult initializers = lookUpMember(
+ getSession(),
+ this,
+ getName("$init"),
+ type);
+
+ AddOverloadCandidates(initializers, context);
}
void SemanticsVisitor::AddDeclRefOverloadCandidates(
@@ -904,7 +872,7 @@ namespace Slang
auto type = DeclRefType::Create(
getSession(),
aggTypeDeclRef);
- AddAggTypeOverloadCandidates(item, type, aggTypeDeclRef, context, type);
+ AddTypeOverloadCandidates(type, context);
}
else if (auto genericDeclRef = item.declRef.as<GenericDecl>())
{
@@ -938,14 +906,14 @@ namespace Slang
else if( auto typeDefDeclRef = item.declRef.as<TypeDefDecl>() )
{
auto type = getNamedType(getSession(), typeDefDeclRef);
- AddTypeOverloadCandidates(GetType(typeDefDeclRef), context, type);
+ AddTypeOverloadCandidates(type, context);
}
else if( auto genericTypeParamDeclRef = item.declRef.as<GenericTypeParamDecl>() )
{
auto type = DeclRefType::Create(
getSession(),
genericTypeParamDeclRef);
- addGenericTypeParamOverloadCandidates(genericTypeParamDeclRef, context, type);
+ AddTypeOverloadCandidates(type, context);
}
else
{
@@ -954,6 +922,23 @@ namespace Slang
}
void SemanticsVisitor::AddOverloadCandidates(
+ LookupResult const& result,
+ OverloadResolveContext& context)
+ {
+ if(result.isOverloaded())
+ {
+ for(auto item : result.items)
+ {
+ AddDeclRefOverloadCandidates(item, context);
+ }
+ }
+ else
+ {
+ AddDeclRefOverloadCandidates(result.item, context);
+ }
+ }
+
+ void SemanticsVisitor::AddOverloadCandidates(
RefPtr<Expr> funcExpr,
OverloadResolveContext& context)
{
@@ -973,12 +958,7 @@ namespace Slang
}
else if (auto overloadedExpr = as<OverloadedExpr>(funcExpr))
{
- auto lookupResult = overloadedExpr->lookupResult2;
- SLANG_RELEASE_ASSERT(lookupResult.isOverloaded());
- for(auto item : lookupResult.items)
- {
- AddDeclRefOverloadCandidates(item, context);
- }
+ AddOverloadCandidates(overloadedExpr->lookupResult2, context);
}
else if (auto overloadedExpr2 = as<OverloadedExpr2>(funcExpr))
{
@@ -996,7 +976,7 @@ namespace Slang
// TODO(tfoley): are there any meaningful types left
// that aren't declaration references?
auto type = typeType->type;
- AddTypeOverloadCandidates(type, context, type);
+ AddTypeOverloadCandidates(type, context);
return;
}
}