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

Basic name poisoning support #4654

Merged
merged 49 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
24c0d19
Basic name poisoning support
bricknerb Dec 9, 2024
8d13dcb
Address some review comments:
bricknerb Dec 10, 2024
5654c06
Change import poison test to be successful as expected
bricknerb Dec 10, 2024
81bbb1c
Ignore poisoned entries from imported from API
bricknerb Dec 10, 2024
4bcd3c5
Add missing library declaration
bricknerb Dec 10, 2024
c5bb4bc
Add a test for using a poisoned name without trying to define it
bricknerb Dec 10, 2024
30126a4
Rephrase poisoned name declaration error
bricknerb Dec 10, 2024
1f30769
Avoid trying to poison the same name twice
bricknerb Dec 10, 2024
494ae99
Make sure LookupOrAdd() is not called with a poisoned InstId
bricknerb Dec 10, 2024
fd700dc
Add a TODO to improve the poisoned name diagnostic
bricknerb Dec 10, 2024
2b442ca
Fix DiagnosePoisonedName() param name to match definition
bricknerb Dec 10, 2024
74d8c3f
Add support for poisoning in interface declarations
bricknerb Dec 10, 2024
3ccfede
Add support for poisoning in namespaces declarations
bricknerb Dec 10, 2024
beef0f4
Test declaraing data member using a poisoned name
bricknerb Dec 10, 2024
6c95298
Support name poisoning in function declaration
bricknerb Dec 10, 2024
850c91c
Add missing library in test
bricknerb Dec 10, 2024
5362b03
Remove extra SemIR::
bricknerb Dec 10, 2024
5018b08
Rephrase comments
bricknerb Dec 10, 2024
e8e59c3
Clarify TODO for improving name poisoning diagnostic
bricknerb Dec 10, 2024
e4e95a5
Change name poisoning error to error without a location for now and n…
bricknerb Dec 12, 2024
6d62e04
Disable modernize-use-trailing-return-type on MATCHER_P (#4656)
danakj Dec 9, 2024
69eec75
Improve diagnostics for missing qualified names. (#4638)
zygoloid Dec 9, 2024
eef1ffb
Track complete types required by a generic. (#4652)
zygoloid Dec 10, 2024
a32804d
Rename mutable accessor in `InstBlock` store. (#4659)
zygoloid Dec 10, 2024
c4a8f4b
Mark some //common, //toolchain/driver, //‎toolchain/install tests as…
bricknerb Dec 10, 2024
c91f51a
Rename various `TryToCompleteType` functions to better describe what …
zygoloid Dec 10, 2024
e372be3
Refactor single-unit checking out of check.cpp (#4649)
jonmeow Dec 10, 2024
a383f3d
Update the vscode extension for publishing. (#4660)
jonmeow Dec 10, 2024
6890290
Switch from recommending a local workspace extension to recommending …
zygoloid Dec 10, 2024
93b4924
Proposal: Variadics (#2240)
geoffromer Dec 11, 2024
a24e4b7
Include a fully-qualified name when stringifying types. (#4657)
zygoloid Dec 11, 2024
bc6e444
Make more use of llvm STLExtras (#4668)
jonmeow Dec 11, 2024
2cbe3a7
Pacify CHECK failure on invalid code. (#4665)
zygoloid Dec 11, 2024
233ae2f
Produce a note indicating where the specific was used from if monomor…
zygoloid Dec 11, 2024
369dded
Provide a location for monomorphization failures resulting from `TryT…
zygoloid Dec 12, 2024
26bbbfb
Add `Dump` functions to Check, Parse, and Lex (#4669)
jonmeow Dec 12, 2024
99b5733
Allow defining .h files in tests without trying to compile them as Ca…
bricknerb Dec 12, 2024
9b2c5cc
Lower global variables as global definitions, not global declarations…
zygoloid Dec 12, 2024
266798d
Import support for array types. (#4675)
zygoloid Dec 12, 2024
7f96e88
Support stringifying tuple values. (#4664)
zygoloid Dec 12, 2024
28be4bf
Use a filename without a line number as a cue for autoupdate. (#4677)
jonmeow Dec 13, 2024
f6feb1e
Clean up missing library in test (#4678)
jonmeow Dec 13, 2024
d121b91
Avoid printing enums as characters (#4676)
danakj Dec 13, 2024
a4f8811
Add a --remote switch to new_proposal.py (#4681)
danakj Dec 13, 2024
7f3357e
Fix mem usage tracking of semir (#4684)
jonmeow Dec 14, 2024
ddc1f3b
Fix lowering of array indexing with an int literal. (#4686)
zygoloid Dec 14, 2024
4b63218
Update name poisoning test following merge with trunk
bricknerb Dec 16, 2024
340dbb1
Merge branch 'trunk' into poison4
bricknerb Dec 16, 2024
b92ee03
Merge trunk
jonmeow Dec 17, 2024
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
30 changes: 16 additions & 14 deletions toolchain/check/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,13 @@ auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
.Emit();
}

auto Context::DiagnosePoisonedName(SemIRLoc dup_def) -> void {
auto Context::DiagnosePoisonedName(SemIRLoc loc) -> void {
// TODO: Improve the diagnostic to point to where the name was poisoned.
CARBON_DIAGNOSTIC(
NameDeclPoisoned, Error,
"cannot declare this name in this scope since is was already used "
"without qualification in the context of this scope");
emitter_->Build(dup_def, NameDeclPoisoned).Emit();
NameDeclUsedUnqualifiedBefore, Error,
"name previously used by unqualified name lookup and not found; "
"cannot later be declared");
emitter_->Build(loc, NameDeclUsedUnqualifiedBefore).Emit();
}

auto Context::DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id)
Expand Down Expand Up @@ -335,25 +336,26 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id,

// Walk the non-lexical scopes and perform lookups into each of them.
// Collect scopes to poison this name when it's found.
std::vector<LookupScope> scopes_to_poison;
llvm::SmallVector<LookupScope> scopes_to_poison;
for (auto [index, lookup_scope_id, specific_id] :
llvm::reverse(non_lexical_scopes)) {
if (auto non_lexical_result =
LookupQualifiedName(node_id, name_id,
LookupScope{.name_scope_id = lookup_scope_id,
.specific_id = specific_id},
/*required=*/false);
non_lexical_result.inst_id.is_valid() &&
!non_lexical_result.inst_id.is_poisoned()) {
// Poison the scopes for this name.
for (const auto [scope_id, specific_id] : scopes_to_poison) {
name_scopes().Get(scope_id).AddPoison(name_id);
}
if (non_lexical_result.inst_id.is_valid()) {
// Poison the scopes for this name.
for (const auto [scope_id, specific_id] : scopes_to_poison) {
name_scopes().Get(scope_id).AddPoison(name_id);
}

return non_lexical_result;
return non_lexical_result;
}
scopes_to_poison.push_back(
{.name_scope_id = lookup_scope_id, .specific_id = specific_id});
}
scopes_to_poison.push_back(
{.name_scope_id = lookup_scope_id, .specific_id = specific_id});
}

if (lexical_result.is_valid()) {
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ class Context {
auto DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def) -> void;

// Prints a diagnostic for a poisoned name.
auto DiagnosePoisonedName(SemIRLoc dup_def) -> void;
auto DiagnosePoisonedName(SemIRLoc loc) -> void;

// Prints a diagnostic for a missing name.
auto DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id) -> void;
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/decl_name_stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ static auto CheckQualifierIsResolved(
}
return true;

case DeclNameStack::NameContext::State::Unresolved:
case DeclNameStack::NameContext::State::Poisoned:
case DeclNameStack::NameContext::State::Unresolved:
// Because more qualifiers were found, we diagnose that the earlier
// qualifier failed to resolve.
context.DiagnoseNameNotFound(name_context.loc_id,
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/handle_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static auto MergeOrAddName(Context& context, Parse::AnyClassDeclId node_id,
}

if (prev_id.is_poisoned()) {
// This is declaration of a poisoned name.
// This is a declaration of a poisoned name.
context.DiagnosePoisonedName(class_decl_id);
return;
}
Expand Down
5 changes: 5 additions & 0 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
return;
}

if (prev_id.is_poisoned()) {
context.DiagnosePoisonedName(function_info.latest_decl_id());
return;
}

auto prev_function_id = SemIR::FunctionId::Invalid;
auto prev_import_ir_id = SemIR::ImportIRId::Invalid;
CARBON_KIND_SWITCH(context.insts().Get(prev_id)) {
Expand Down
9 changes: 7 additions & 2 deletions toolchain/check/handle_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,13 @@ static auto BuildInterfaceDecl(Context& context,
auto existing_id = context.decl_name_stack().LookupOrAddName(
name_context, interface_decl_id, introducer.modifier_set.GetAccessKind());
if (existing_id.is_valid()) {
if (auto existing_interface_decl =
context.insts().Get(existing_id).TryAs<SemIR::InterfaceDecl>()) {
if (existing_id.is_poisoned()) {
// This is a declaration of a poisoned name.
context.DiagnosePoisonedName(interface_decl_id);
} else if (auto existing_interface_decl =
context.insts()
.Get(existing_id)
.TryAs<SemIR::InterfaceDecl>()) {
auto existing_interface =
context.interfaces().Get(existing_interface_decl->interface_id);
if (CheckRedeclParamsMatch(
Expand Down
11 changes: 7 additions & 4 deletions toolchain/check/handle_namespace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool {
auto existing_inst_id = context.decl_name_stack().LookupOrAddName(
name_context, namespace_id, SemIR::AccessKind::Public);
if (existing_inst_id.is_valid()) {
// If there's a name conflict with a namespace, "merge" by using the
// previous declaration. Otherwise, diagnose the issue.
if (auto existing =
context.insts().TryGetAs<SemIR::Namespace>(existing_inst_id)) {
if (existing_inst_id.is_poisoned()) {
context.DiagnosePoisonedName(namespace_id);
} else if (auto existing = context.insts().TryGetAs<SemIR::Namespace>(
existing_inst_id)) {
// If there's a name conflict with a namespace, "merge" by using the
// previous declaration. Otherwise, diagnose the issue.

// Point at the other namespace.
namespace_inst.name_scope_id = existing->name_scope_id;

Expand Down
3 changes: 3 additions & 0 deletions toolchain/check/import.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ static auto ImportScopeFromApiFile(Context& context,
auto& impl_scope = context.name_scopes().Get(impl_scope_id);

for (const auto& api_entry : api_scope.entries()) {
if (api_entry.inst_id.is_poisoned()) {
continue;
}
auto impl_name_id =
CopyNameFromImportIR(context, api_sem_ir, api_entry.name_id);
if (auto ns =
Expand Down
Loading