diff --git a/explorer/ast/pattern.h b/explorer/ast/pattern.h index 4d7ff7284aea..ee39270c067a 100644 --- a/explorer/ast/pattern.h +++ b/explorer/ast/pattern.h @@ -106,8 +106,8 @@ class Pattern : public AstNode { }; // Call the given `visitor` on all patterns nested within the given pattern, -// including `pattern` itself. Aborts and returns `false` if `visitor` returns -// `false`, otherwise returns `true`. +// including `pattern` itself, in a preorder traversal. Aborts and returns +// `false` if `visitor` returns `false`, otherwise returns `true`. auto VisitNestedPatterns(const Pattern& pattern, llvm::function_ref visitor) -> bool; diff --git a/explorer/ast/value.cpp b/explorer/ast/value.cpp index f4bf04a1aae9..599884af5d58 100644 --- a/explorer/ast/value.cpp +++ b/explorer/ast/value.cpp @@ -13,6 +13,7 @@ #include "explorer/ast/declaration.h" #include "explorer/ast/element.h" #include "explorer/ast/element_path.h" +#include "explorer/ast/value_transform.h" #include "explorer/common/arena.h" #include "explorer/common/error_builders.h" #include "llvm/ADT/STLExtras.h" @@ -27,6 +28,86 @@ using llvm::dyn_cast; using llvm::dyn_cast_or_null; using llvm::isa; +namespace { +// A visitor that walks the Value*s nested within a value. +struct NestedValueVisitor { + template + auto VisitParts(const T& decomposable) -> bool { + return decomposable.Decompose( + [&](const auto&... parts) { return (Visit(parts) && ...); }); + } + + auto Visit(Nonnull value) -> bool { + if (!callback(value)) { + return false; + } + + return value->Visit( + [&](const auto* derived_value) { return VisitParts(*derived_value); }); + } + + auto Visit(Nonnull bindings) -> bool { + for (auto [binding, value] : bindings->args()) { + if (!Visit(value)) { + return false; + } + } + for (auto [binding, value] : bindings->witnesses()) { + if (!Visit(value)) { + return false; + } + } + return true; + } + + template + auto Visit(const std::vector& vec) -> bool { + for (auto& v : vec) { + if (!Visit(v)) { + return false; + } + } + return true; + } + + template + auto Visit(const std::optional& opt) -> bool { + return !opt || Visit(*opt); + } + + template >> + auto Visit(Nonnull value) -> bool { + return VisitParts(*value); + } + template >> + auto Visit(const T& value) -> bool { + return VisitParts(value); + } + + // Other value components can't refer to a value. + auto Visit(Nonnull) -> bool { return true; } + auto Visit(ValueNodeView) -> bool { return true; } + auto Visit(int) -> bool { return true; } + auto Visit(Address) -> bool { return true; } + auto Visit(const std::string&) -> bool { return true; } + auto Visit(Nonnull) -> bool { + // This is the pointer to the most-derived value within a class value, + // which is not "within" this value, so we shouldn't visit it. + return true; + } + auto Visit(const VTable&) -> bool { return true; } + + llvm::function_ref callback; +}; +} // namespace + +auto VisitNestedValues(Nonnull value, + llvm::function_ref visitor) -> bool { + return NestedValueVisitor{.callback = visitor}.Visit(value); +} + auto StructValue::FindField(std::string_view name) const -> std::optional> { for (const NamedValue& element : elements_) { diff --git a/explorer/ast/value.h b/explorer/ast/value.h index e8798b43ed7e..49c8c91d7224 100644 --- a/explorer/ast/value.h +++ b/explorer/ast/value.h @@ -122,6 +122,12 @@ auto ValueEqual(Nonnull v1, Nonnull v2, std::optional> equality_ctx) -> bool; +// Call the given `visitor` on all values nested within the given value, +// including `value` itself, in a preorder traversal. Aborts and returns +// `false` if `visitor` returns `false`, otherwise returns `true`. +auto VisitNestedValues(Nonnull value, + llvm::function_ref visitor) -> bool; + // An integer value. class IntValue : public Value { public: diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index 951938f9851c..85a82707ccc2 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -454,27 +454,17 @@ static auto ExpectResolvedBindingType(const BindingPattern& binding, } // Returns whether the given value is template-dependent, that is, if it -// depends on any template paramaeter. -static auto IsTemplateDependent(Nonnull value) -> bool { - // A VariableType is template dependent if it names a template binding. - if (const auto* var_type = dyn_cast(value)) { - return var_type->binding().binding_kind() == - GenericBinding::BindingKind::Template; - } - - static constexpr auto is_dependent_value = [](auto&& x) -> bool { - if constexpr (std::is_convertible_v) { - return IsTemplateDependent(x); - } - return false; - }; - - // Any other value is template dependent if any part of it is. - return value->Visit([](auto* derived_value) { - return derived_value->Decompose([](auto&&... parts) { - return (is_dependent_value(decltype(parts)(parts)) || ...); - }); - }); +// depends on any template parameter. +static auto DependsOnTemplateParameter(Nonnull value) -> bool { + bool mentions_no_template_parameters = + VisitNestedValues(value, [](Nonnull nested) -> bool { + if (const auto* var_type = dyn_cast(nested)) { + return var_type->binding().binding_kind() != + GenericBinding::BindingKind::Template; + } + return true; + }); + return !mentions_no_template_parameters; } // Returns whether all template parameters in `bindings` are saturated: that @@ -483,7 +473,7 @@ static auto IsTemplateDependent(Nonnull value) -> bool { static auto IsTemplateSaturated(const Bindings& bindings) -> bool { for (auto [binding, value] : bindings.args()) { if (binding->binding_kind() == GenericBinding::BindingKind::Template && - IsTemplateDependent(value)) { + DependsOnTemplateParameter(value)) { return false; } } @@ -6407,6 +6397,12 @@ auto TypeChecker::InstantiateImplDeclaration( } } + // TODO: Make this method non-const and remove the const-cast here. The + // requirement to perform template instantiation unfortunately means that a + // lot of type-checking stops being free of side-effects, so this means + // removing `const` throughout most of the type-checker. + auto* type_checker = const_cast(this); + // TODO: It's probably not correct to use the top-level impl scope here. It's // not obvious what we should use, though -- which impls are in scope in // template instantiation? @@ -6414,11 +6410,34 @@ auto TypeChecker::InstantiateImplDeclaration( << "can't perform template instantiation with no top-level scope"; ImplScope scope(*top_level_impl_scope_); - // TODO: Remove the const-cast here. The requirement to perform template - // instantiation unfortunately means that a lot of type-checking stops being - // free of side-effects, so this means removing `const` throughout most of - // the type-checker. - auto* type_checker = const_cast(this); + // Bring all impls from any checked generic bindings in the template + // arguments into scope. + // + // TODO: There shouldn't be any checked generic bindings in the template + // arguments by the time we come to perform an instantiation, but in order + // for that to work, we need to defer instantiating templates until we know + // the values of checked generic parameters, such as by performing + // monomorphization for checked generics (see #2153 for details). However, + // explorer doesn't yet support that. + // + // As a workaround for the lack of support for #2153, we can instantiate + // templates with the argument equal to a generic parameter. When we do so, + // the constraints on that generic parameter need to be in scope in the + // instantiation. This is imperfect: it misses constraints on the binding + // that come from anywhere other than its type. + for (auto [param, value] : bindings->args()) { + if (param->binding_kind() != GenericBinding::BindingKind::Template) { + continue; + } + VisitNestedValues(value, [&](Nonnull nested) -> bool { + if (auto* var_type = dyn_cast(nested)) { + if (auto impl_binding = var_type->binding().impl_binding()) { + type_checker->BringImplBindingIntoScope(*impl_binding, scope); + } + } + return true; + }); + } // Type-check the new impl. // diff --git a/explorer/testdata/template/instantiate_from_generic.carbon b/explorer/testdata/template/instantiate_from_generic.carbon new file mode 100644 index 000000000000..785d67199950 --- /dev/null +++ b/explorer/testdata/template/instantiate_from_generic.carbon @@ -0,0 +1,34 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// CHECK:STDOUT: 1 +// CHECK:STDOUT: result: 0 + +package ExplorerTest api; + +interface I { fn F[self: Self](); } + +fn CheckTimplsI[T:! I](x: T) { x.F(); } + +interface J { fn F[self: Self](); } + +impl forall [template T:! I] T as J { + fn F[self: Self]() { CheckTimplsI(self); } +} + +// Ensure that the instantiated `impl T as J` is type-checked in an impl scope +// where `T impls I` is available, so that its call to `CheckTimplsI` is valid. +fn UseTemplatedImplFromGeneric[T:! I](x: T) { x.(J.F)(); } + +impl i32 as I { + fn F[self: i32]() { + Print("{0}", self); + } +} + +fn Main() -> i32 { + UseTemplatedImplFromGeneric(1); + return 0; +}