mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-10-06 09:34:42 +01:00
Properly diagnose ambiguous .Self in T impls X where... (#7132)
A `where` expression nested inside a `T impls X` constraint makes `.Self` ambiguous on the right-hand side of the `where` if `T` is anything other than `.Self`. After the `where`, the value of a `.Self` could be `T` or could be the value of `.Self` before the `impls` constraint: the so-called top-level value of `.Self`. Implicit use of `.Self` in designators is always allowed, and they are bound (and replaced by a reference) to the inner-most possible value of `.Self`. On the right-hand side of the nested `where` above, they have the value `T as X`. `.Self impls ...` is also always allowed, since it acts more as a keyword here, and it always refers to the inner-most possible value of `.Self`. Any other explicit use of `.Self` is diagnosed when ambiguous, in any kind of constraint. This is done in the handling of `WhereExpr` since it has enough context to allow `.Self impls` (which is an explicit use) while disallowing other explicit uses. And because it has non-canonical instructions to work with, so it is able to diagnose errors with precise locations. Since `.Self` is no longer going to be marked with depth modifiers, the eval of `WhereExpr` does not need an input facet value instruction representing `.Self` to compare with, as they are now going to all be equivalent. So revert it back to just looking for the `PeriodSelf` name id, through a shared helper being introduced as `IsPeriodSelf`. And drop the period self InstId from the `WhereExpr` instruction. This causes most of the formatted SemIR changes. Move helpers for working with and replacing `.Self` to their own file, out of the `facet_type.h` header/cpp files. These are working with `.Self` facet values more than facet types, though `.Self` is a name that only exists inside the scope of a facet type.
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
#include "toolchain/check/generic.h"
|
||||
#include "toolchain/check/handle.h"
|
||||
#include "toolchain/check/inst.h"
|
||||
#include "toolchain/check/period_self.h"
|
||||
#include "toolchain/check/subst.h"
|
||||
#include "toolchain/check/type.h"
|
||||
#include "toolchain/check/unused.h"
|
||||
@@ -33,6 +34,12 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId node_id) -> bool {
|
||||
context.emitter().Emit(self_node, WhereOnNonFacetType);
|
||||
self_with_constraints_type_id = SemIR::ErrorInst::TypeId;
|
||||
}
|
||||
if (self_with_constraints_type_id == SemIR::ErrorInst::TypeId) {
|
||||
// Keep `self_id` in sync with `self_with_constraints_type_id`, if one is an
|
||||
// error they both are. Note that ExprAsType may have returned ErrorInst,
|
||||
// or we may have set it to ErrorInst in this function.
|
||||
self_id = SemIR::ErrorInst::InstId;
|
||||
}
|
||||
|
||||
// Strip off any constraints provided by a `WhereExpr` from the `Self` facet
|
||||
// type. For a facet type like `I & J where .X = .Y`, this will reduce it down
|
||||
@@ -62,12 +69,7 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId node_id) -> bool {
|
||||
context.scope_stack().PushForSameRegion();
|
||||
// Introduce `.Self` as a symbolic binding. Its type is the value of the
|
||||
// expression to the left of `where`, so `MyInterface` in the example above.
|
||||
auto period_self_inst_id =
|
||||
MakePeriodSelfFacetValue(context, period_self_type_id);
|
||||
|
||||
// Save the `.Self` symbolic binding on the node stack. It will become the
|
||||
// first argument to the `WhereExpr` instruction.
|
||||
context.node_stack().Push(node_id, period_self_inst_id);
|
||||
MakePeriodSelfFacetValue(context, period_self_type_id);
|
||||
|
||||
// Going to put each requirement on `args_type_info_stack`, so we can have an
|
||||
// inst block with the varying number of requirements but keeping other
|
||||
@@ -79,8 +81,7 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId node_id) -> bool {
|
||||
context.args_type_info_stack().AddInstId(
|
||||
AddInstInNoBlock<SemIR::RequirementBaseFacetType>(
|
||||
context, SemIR::LocId(node_id),
|
||||
{.base_type_inst_id =
|
||||
context.types().GetTypeInstId(self_with_constraints_type_id)}));
|
||||
{.base_type_inst_id = context.types().GetAsTypeInstId(self_id)}));
|
||||
|
||||
// Add a context stack for tracking rewrite constraints, that will be used to
|
||||
// allow later constraints to read from them eagerly.
|
||||
@@ -179,6 +180,11 @@ auto HandleParseNode(Context& context, Parse::RequirementImplsId node_id)
|
||||
// TODO: For things like `HashSet(.T) as type`, add an implied constraint
|
||||
// that `.T impls Hash`.
|
||||
|
||||
if (FindAndDiagnoseAmbiguousPeriodSelf(context, lhs_as_type.inst_id,
|
||||
rhs_id)) {
|
||||
rhs_as_type.inst_id = SemIR::ErrorInst::TypeInstId;
|
||||
}
|
||||
|
||||
// Build up the list of arguments for the `WhereExpr` inst.
|
||||
context.args_type_info_stack().AddInstId(
|
||||
AddInstInNoBlock<SemIR::RequirementImpls>(
|
||||
@@ -202,8 +208,6 @@ static auto FindDesignator(Context& context,
|
||||
llvm::SmallVector<SemIR::InstId> requirements;
|
||||
requirements.reserve(block.size() * 2);
|
||||
|
||||
// These requirement instructions don't have a constant value, but they
|
||||
// contain only canonical instructions.
|
||||
for (auto inst_id : block) {
|
||||
auto inst = context.insts().Get(inst_id);
|
||||
CARBON_KIND_SWITCH(inst) {
|
||||
@@ -291,22 +295,20 @@ auto HandleParseNode(Context& context, Parse::WhereExprId node_id) -> bool {
|
||||
// Remove `PeriodSelf` from name lookup, undoing the `Push` done for the
|
||||
// `WhereOperand`.
|
||||
context.scope_stack().Pop(/*check_unused=*/true);
|
||||
SemIR::InstId period_self_id =
|
||||
context.node_stack().Pop<Parse::NodeKind::WhereOperand>();
|
||||
SemIR::InstBlockId requirements_id = context.args_type_info_stack().Pop();
|
||||
|
||||
auto type_id = SemIR::TypeType::TypeId;
|
||||
if (!FindDesignator(context, requirements_id)) {
|
||||
CARBON_DIAGNOSTIC(WhereWithoutDesignator, Error,
|
||||
"`where` clause without a designator; expected `.Self` "
|
||||
"to appear in a requirement, or a member of `.Self`");
|
||||
context.emitter().Emit(node_id, WhereWithoutDesignator);
|
||||
period_self_id = SemIR::ErrorInst::InstId;
|
||||
type_id = SemIR::ErrorInst::TypeId;
|
||||
}
|
||||
|
||||
AddInstAndPush<SemIR::WhereExpr>(context, node_id,
|
||||
{.type_id = SemIR::TypeType::TypeId,
|
||||
.period_self_id = period_self_id,
|
||||
.requirements_id = requirements_id});
|
||||
AddInstAndPush<SemIR::WhereExpr>(
|
||||
context, node_id,
|
||||
{.type_id = type_id, .requirements_id = requirements_id});
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user