Skip to content

Make dumping work with the irM.instN tagged InstId format #6168

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Make dumping work with the irM.instN tagged InstId format #6168

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
567d7a9
debug-irs
danakj Oct 6, 2025
5b5b7b6
review
danakj Oct 7, 2025
d3d29f9
wording
danakj Oct 7, 2025
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
41 changes: 25 additions & 16 deletions scripts/lldbinit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ def print_usage() -> None:
Dumps the value of an associated ID, using the C++ Dump() functions.

Usage:
dump <CONTEXT> [<EXPR>|-- <EXPR>|<TYPE><ID>|<TYPE> <ID>]
dump <CONTEXT> [<EXPR>|-- <EXPR>|<TYPE><ID>|<IR>.<TYPE><ID>]

Args:
CONTEXT is the dump context, such a SemIR::Context reference, a SemIR::File,
a Parse::Context, or a Lex::TokenizeBuffer.
EXPR is a C++ expression such as a variable name. Use `--` to prevent it from
being treated as a TYPE and ID.
IR is the CheckIRId(N) in the form `irN`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this mention that it only supports inst? That seems new, and doesn't appear to be otherwise documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will become wrong in the future, so I don't feel that we should. Maybe I can say something about what the dumped ids look like instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps still document that it's a subset, and where to look for the supported list? Or change the approach, and just drop the IR on the floor when it's not used?

Right now reading TYPE documentation says everything is allowed, so as long as it's a subset I think it's worth clarifying that the disagreement between TYPE documentation and behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I have added some clarification and mentioned InstId without saying it's the only one that will be like this.

TYPE can be `inst`, `constant`, `generic`, `impl`, `entity_name`, etc. See
the `Label` string in `IdBase` classes to find possible TYPE names,
though only Id types that have a matching `Make...Id()` function are
Expand All @@ -74,8 +75,13 @@ def print_usage() -> None:

context = args[0]

# The set of "Make" functions in dump.cpp.
# The set of "Make" functions in dump.cpp. These use ADL to find the factory
# function in `context/` or in `sem_ir/`.
id_types = {
"inst": "MakeInstId",
}
# These id types don't have an IdTag embedded in them.
untagged_id_types = {
"class": "SemIR::MakeClassId",
"constant": "SemIR::MakeConstantId",
"symbolic_constant": "SemIR::MakeSymbolicConstantId",
Expand All @@ -85,7 +91,6 @@ def print_usage() -> None:
"generic": "SemIR::MakeGenericId",
"impl": "SemIR::MakeImplId",
"inst_block": "SemIR::MakeInstBlockId",
"inst": "SemIR::MakeInstId",
"interface": "SemIR::MakeInterfaceId",
"name": "SemIR::MakeNameId",
"name_scope": "SemIR::MakeNameScopeId",
Expand All @@ -110,27 +115,31 @@ def print_dump(context: str, expr: str) -> None:

# Try to find a type + id from the input args. If not, the id will be passed
# through directly to C++, as it can be a variable name.
id_type = None
found_id_type = False

# Look for <type><id> as a single argument.
if m := re.fullmatch("([a-z_]+)(\\d+)", args[1]):
if m[1] in id_types:
# Look for <irN>.<type><id> as a single argument.
if m := re.fullmatch("ir(\\d+)\\.([a-z_]+)(\\d+)", args[1]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had you considered making this optional (e.g.. (ir(\\d+)\\.)?) to make this a single regex?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arguments to the MakeInstId function are different depending on if there's an ir or not. I guess I could look for the group being present and branch, but this feels at least as straightforward to me.

if m[2] in id_types:
if len(args) != 2:
print_usage()
return
id_type = m[1]
print_dump(context, f"{id_types[id_type]}({m[2]})")
id_type = m[2]
print_dump(
context, f"{id_types[id_type]}({context}, {m[1]}, {m[3]})"
)
found_id_type = True

# Look for <type> <id> as two arguments.
if not id_type:
if args[1] in id_types:
if len(args) != 3:
# Look for <type><id> as a single argument.
if m := re.fullmatch("([a-z_]+)(\\d+)", args[1]):
if m[1] in untagged_id_types:
if len(args) != 2:
print_usage()
return
id_type = args[1]
print_dump(context, f"{id_types[id_type]}({args[2]})")
id_type = m[1]
print_dump(context, f"{untagged_id_types[id_type]}({m[2]})")
found_id_type = True

if not id_type:
if not found_id_type:
# Use `--` to escape a variable name like `inst22`.
if args[1] == "--":
expr = " ".join(args[2:])
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ cc_library(
"//toolchain/parse:tree",
"//toolchain/sem_ir:dump",
"//toolchain/sem_ir:file",
"//toolchain/sem_ir:typed_insts",
],
# Always link dump methods.
alwayslink = 1,
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ class Context {
// Directly expose SemIR::File data accessors for brevity in calls.
// --------------------------------------------------------------------------

auto check_ir_id() -> SemIR::CheckIRId { return sem_ir().check_ir_id(); }
auto identifiers() -> SharedValueStores::IdentifierStore& {
return sem_ir().identifiers();
}
Expand Down
15 changes: 13 additions & 2 deletions toolchain/check/dump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
// - lldb: `expr Dump(context, id)`
// - gdb: `call Dump(context, id)`

#include "toolchain/sem_ir/ids.h"
#ifndef NDEBUG

#include "toolchain/lex/dump.h"

#include <string>

#include "common/check.h"
#include "common/raw_string_ostream.h"
#include "toolchain/check/context.h"
#include "toolchain/lex/dump.h"
#include "toolchain/lex/tokenized_buffer.h"
#include "toolchain/parse/dump.h"
#include "toolchain/parse/tree.h"
Expand Down Expand Up @@ -138,6 +138,17 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::TypeId type_id)
return SemIR::Dump(context.sem_ir(), type_id);
}

LLVM_DUMP_METHOD static auto MakeInstId(Context& context, int check_ir_id,
int id) -> SemIR::InstId {
if (SemIR::CheckIRId(check_ir_id) == context.check_ir_id()) {
return SemIR::MakeInstId(context.sem_ir(), check_ir_id, id);
} else {
const auto& import_ir = context.import_irs().Get(
context.check_ir_map().Get(SemIR::CheckIRId(check_ir_id)));
return SemIR::MakeInstId(*import_ir.sem_ir, check_ir_id, id);
}
}

} // namespace Carbon::Check

#endif // NDEBUG
8 changes: 7 additions & 1 deletion toolchain/sem_ir/dump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,13 @@ LLVM_DUMP_METHOD static auto MakeImplId(int id) -> ImplId { return ImplId(id); }
LLVM_DUMP_METHOD static auto MakeInstBlockId(int id) -> InstBlockId {
return InstBlockId(id);
}
LLVM_DUMP_METHOD static auto MakeInstId(int id) -> InstId { return InstId(id); }
LLVM_DUMP_METHOD auto MakeInstId(const SemIR::File& file, int check_ir_id,
int id) -> InstId {
CARBON_CHECK(file.check_ir_id() == SemIR::CheckIRId(check_ir_id),
"SemIR::MakeInstId given a CheckIRId for the wrong SemIR::File. "
"Maybe you want to dump from a Check::Context.");
return InstId(file.insts().GetIdTag().Apply(id));
}
LLVM_DUMP_METHOD static auto MakeInterfaceId(int id) -> InterfaceId {
return InterfaceId(id);
}
Expand Down
2 changes: 2 additions & 0 deletions toolchain/sem_ir/dump.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ auto Dump(const File& file, StructTypeFieldsId struct_type_fields_id)
-> std::string;
auto Dump(const File& file, TypeId type_id) -> std::string;

auto MakeInstId(const SemIR::File& file, int check_ir_id, int id) -> InstId;

} // namespace Carbon::SemIR

#endif // NDEBUG
Expand Down
Loading