Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow compile time bindings where they aren't clearly supported. #4338

Merged
merged 5 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions toolchain/check/handle_binding_pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
inst_id.is_valid() && context.insts().Is<SemIR::InterfaceDecl>(inst_id);
}

bool needs_compile_time_binding = is_generic && !is_associated_constant;

// Create the appropriate kind of binding for this pattern.
auto make_bind_name = [&](SemIR::TypeId type_id,
SemIR::InstId value_id) -> SemIR::LocIdAndInst {
Expand All @@ -42,7 +44,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
.parent_scope_id = context.scope_stack().PeekNameScopeId(),
// TODO: Don't allocate a compile-time binding index for an associated
// constant declaration.
.bind_index = is_generic && !is_associated_constant
.bind_index = needs_compile_time_binding
? context.scope_stack().AddCompileTimeBinding()
: SemIR::CompileTimeBindIndex::Invalid});
if (is_generic) {
Expand All @@ -63,7 +65,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
// stack.
auto push_bind_name = [&](SemIR::InstId bind_id) {
context.node_stack().Push(node_id, bind_id);
if (is_generic && !is_associated_constant) {
if (needs_compile_time_binding) {
context.scope_stack().PushCompileTimeBinding(bind_id);
}
};
Expand Down Expand Up @@ -200,7 +202,21 @@ auto HandleParseNode(Context& context, Parse::BindingPatternId node_id)

auto HandleParseNode(Context& context,
Parse::CompileTimeBindingPatternId node_id) -> bool {
return HandleAnyBindingPattern(context, node_id, /*is_generic=*/true);
bool is_generic = true;
if (context.decl_introducer_state_stack().innermost().kind ==
Lex::TokenKind::Let) {
auto scope_inst = context.insts().Get(context.scope_stack().PeekInstId());
if (!scope_inst.Is<SemIR::InterfaceDecl>() &&
!scope_inst.Is<SemIR::FunctionDecl>()) {
CARBON_DIAGNOSTIC(LetCompileTimeBindingUnsupported, Error,
"`let` compile time bindings require function or "
"interface (may be supported later)");
context.emitter().Emit(node_id, LetCompileTimeBindingUnsupported);
is_generic = false;
}
}

return HandleAnyBindingPattern(context, node_id, is_generic);
}

auto HandleParseNode(Context& context, Parse::AddrId node_id) -> bool {
Expand Down
31 changes: 17 additions & 14 deletions toolchain/check/scope_stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ auto ScopeStack::VerifyOnFinish() -> void {
CARBON_CHECK(scope_stack_.empty(), "{0}", scope_stack_.size());
}

auto ScopeStack::VerifyNextCompileTimeBindIndex(llvm::StringLiteral label,
const ScopeStackEntry& scope)
-> void {
CARBON_CHECK(
static_cast<int32_t>(compile_time_binding_stack_.all_values_size()) ==
scope.next_compile_time_bind_index.index,
"Wrong number of entries in compile-time binding stack after {0}: have "
"{1}, expected {2}",
label, compile_time_binding_stack_.all_values_size(),
scope.next_compile_time_bind_index.index);
}

auto ScopeStack::Push(SemIR::InstId scope_inst_id, SemIR::NameScopeId scope_id,
SemIR::SpecificId specific_id,
bool lexical_lookup_has_load_error) -> void {
Expand Down Expand Up @@ -49,6 +61,8 @@ auto ScopeStack::Push(SemIR::InstId scope_inst_id, SemIR::NameScopeId scope_id,
CARBON_CHECK(next_scope_index_.index != std::numeric_limits<int32_t>::max(),
"Ran out of scopes");
++next_scope_index_.index;

VerifyNextCompileTimeBindIndex("Push", scope_stack_.back());
}

auto ScopeStack::Pop() -> void {
Expand All @@ -72,13 +86,7 @@ auto ScopeStack::Pop() -> void {
return_scope_stack_.back().returned_var = SemIR::InstId::Invalid;
}

CARBON_CHECK(
scope.next_compile_time_bind_index.index ==
static_cast<int32_t>(compile_time_binding_stack_.all_values_size()),
"Wrong number of entries in compile-time binding stack, have {0}, "
"expected {1}",
compile_time_binding_stack_.all_values_size(),
scope.next_compile_time_bind_index.index);
VerifyNextCompileTimeBindIndex("Pop", scope);
compile_time_binding_stack_.PopArray();
}

Expand Down Expand Up @@ -216,13 +224,8 @@ auto ScopeStack::Restore(SuspendedScope scope) -> void {
}
}

CARBON_CHECK(
scope.entry.next_compile_time_bind_index.index ==
static_cast<int32_t>(compile_time_binding_stack_.all_values_size()),
"Wrong number of entries in compile-time binding stack when restoring, "
"have {0}, expected {1}",
compile_time_binding_stack_.all_values_size(),
scope.entry.next_compile_time_bind_index.index);
// Still verify the index isn't too high.
VerifyNextCompileTimeBindIndex("Restore", scope.entry);

if (scope.entry.scope_id.is_valid()) {
non_lexical_scope_stack_.push_back(
Expand Down
7 changes: 7 additions & 0 deletions toolchain/check/scope_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ class ScopeStack {
scope_stack_.back().lexical_lookup_has_load_error;
}

// Checks that the provided scope's `next_compile_time_bind_index` matches the
// full size of the current `compile_time_binding_stack_`. The values should
// always match, and this is used to validate the correspondence during
// significant changes.
auto VerifyNextCompileTimeBindIndex(llvm::StringLiteral label,
const ScopeStackEntry& scope) -> void;

// A stack of scopes from which we can `return`.
llvm::SmallVector<ReturnScope> return_scope_stack_;

Expand Down
Loading
Loading