Use python built in type annotations in LLDB visualizer scripts #134291

Walnut356 · 2024-12-14T02:49:17Z

Replaces type annotation comments with python's built-in type annotations.

Built-in type annotations were added in python 3.5. LLDB currently recommends (and as of LLVM 21, will enforce) a minimum python version of 3.8. Rust's test suite also requires python 3.10.

rustbot · 2024-12-14T02:49:25Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tgross35 · 2024-12-15T01:05:38Z

Do these check out with mypy where possible?

from typing import Dict, List, Optional work down to 3.5 to clarify some of the collection and optional types.

Walnut356 · 2024-12-15T02:41:40Z

The only thing that currently doesn't is EmptySyntheticProvider.get_child_index, which returns a None instead of an int, but i suppose it's probably better to change that to return -1 instead as that's what other providers do to indicate an invalid value. mypy just sorta gives up on a lot of the lldb stuff with the error:

Skipping analyzing "lldb": module is installed, but missing library stubs or py.typed marker

The main benefit of type hints is the completions tbh. Most of the method names are pretty long and have C++ naming conventions.

from typing import Dict, List, Optional work down to 3.5 to clarify some of the collection and optional types.

How important is backwards compatibility to 3.5? Since this is a debugger visualizer script, afaik it'll always be run through the python distribution that's bundled with LLDB. I checked winlibs and the oldest distributed version (LLVM 14.0.0) was built with python 3.9. I wasn't able to get anything older than LLVM14 on ubuntu either.

tgross35 · 2024-12-15T06:41:45Z

from typing import Dict, List, Optional work down to 3.5 to clarify some of the collection and optional types.

How important is backwards compatibility to 3.5? Since this is a debugger visualizer script, afaik it'll always be run through the python distribution that's bundled with LLDB. I checked winlibs and the oldest distributed version (LLVM 14.0.0) was built with python 3.9. I wasn't able to get anything older than LLVM14 on ubuntu either.

I didn't mean that 3.5 should be our supported version; just that if it is helpful, there is a way to specify content types or nullability that works with older versions. Since the SomeType | None syntax only came out in 3.10, and lowercase list[str] is 3.9+.

Your version analysis seems correct to me. Maybe we should aim to support down to 3.8 if that is LLVM's policy?

Walnut356 · 2024-12-16T00:15:56Z

Ah yeah i forgot about that. typing's history is such a mess lol

Mark-Simulacrum · 2024-12-26T23:12:20Z

r? tgross35

r=me if we're good on versions etc.

tgross35

Suggested a few things that could be more specific but I think most of this looks good. Thanks for putting up this change, type hints make Python so much nicer to work with.

Please squash once you update.

@rustbot author

tgross35 · 2024-12-31T02:59:30Z

src/etc/lldb_lookup.py

@@ -19,8 +19,7 @@ def classify_rust_type(type):
    return RustType.OTHER


-def summary_lookup(valobj, dict):
-    # type: (SBValue, dict) -> str
+def summary_lookup(valobj: lldb.SBValue, dict) -> str:


Can the dict arguments get typed as Dict? There are a few instances of this. They could probably also be renamed to _dict or _llvm_dict since AIUI they're supposed to be opaque and unused, any idea why LLVM passes them?

I chose not to since there's no reason to ever use it - anything to dissuade people from trying seemed like a good idea. If you'd like i can add them though. Allegedly LLDB uses them for bookkeeping, but I haven't dug around the source code enough to know how/when they're used

Actually wait, I checked and dict isn't actually a python dictionary

Huh, that is interesting. I wasn't sure what it would be but LLVM actually uses : dict in their repo a few times, thanks for double checking.

Maybe we could do a bit of both - what about something like LldbOpaque = object and then using LldbOpaque as the type hint? Then at least we have something there that also indicates it probably shouldn't be used.

tgross35 · 2024-12-31T03:00:41Z

src/etc/lldb_lookup.py

@@ -5,11 +5,11 @@


 # BACKCOMPAT: rust 1.35
-def is_hashbrown_hashmap(hash_map):
+def is_hashbrown_hashmap(hash_map) -> bool:


I think this could be:

def is_hashbrown_hashmap(hash_map: SBValue) -> bool:

tgross35 · 2024-12-31T03:07:02Z

src/etc/rust_types.py

@@ -111,7 +111,7 @@ def classify_struct(name, fields):
    return RustType.STRUCT


-def classify_union(fields):
+def classify_union(fields: List) -> str:


Can the List type be narrowed in this file? Per https://lldb.llvm.org/python_api/lldb.SBType.html#lldb.SBType.fields I think this could be List[SBTypeMember].

In this file they can't be, as the functions are shared between LLDB and GDB.

tgross35 · 2024-12-31T04:24:30Z

src/etc/lldb_providers.py

+class LLDBOpaque:
+    """
+    A type used by LLDB internally. Values of this type should never be used except when passing as
+    an argument to an LLDB function.
+    """
+
+    pass


Nit, you don't need pass since there is a docstring.

Also I think that technically a global LLDBOpaque = object would be more accurate since the real type will be an object but will it will never be a class LLDBOpaque, but that doesn't matter much (If you leave the class, I would just make it clear in the docstring that this is strictly for typing).

tgross35

Looks great, thanks for the changes!

tgross35 · 2024-12-31T07:18:12Z

@bors r+ rollup

bors · 2024-12-31T07:18:14Z

📌 Commit 693a0d5 has been approved by tgross35

It is now in the queue for this repository.

Rollup of 3 pull requests Successful merges: - rust-lang#134291 (Use python built in type annotations in LLDB visualizer scripts) - rust-lang#134857 (Unsafe binder support in rustdoc) - rust-lang#134957 (chore: fix some typos) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134291 - Walnut356:type_annots, r=tgross35 Use python built in type annotations in LLDB visualizer scripts Replaces type annotation comments with python's built-in type annotations. Built-in type annotations were added in python 3.5. LLDB [currently recommends (and as of LLVM 21, will enforce)](llvm/llvm-project#114807) a minimum python version of 3.8. Rust's test suite also requires python 3.10.

rustbot assigned Mark-Simulacrum Dec 14, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 14, 2024

This comment has been minimized.

Sign in to view

rustbot assigned tgross35 and unassigned Mark-Simulacrum Dec 26, 2024

tgross35 reviewed Dec 31, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 31, 2024

Walnut356 force-pushed the type_annots branch 2 times, most recently from f032edd to 4b00d02 Compare December 31, 2024 04:12

tgross35 reviewed Dec 31, 2024

View reviewed changes

use python built in type annotations

693a0d5

Walnut356 force-pushed the type_annots branch from 4b00d02 to 693a0d5 Compare December 31, 2024 06:54

tgross35 approved these changes Dec 31, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2024

jhpratt mentioned this pull request Dec 31, 2024

Rollup of 3 pull requests #134959

Merged

bors merged commit f3d404a into rust-lang:master Dec 31, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 31, 2024

Walnut356 deleted the type_annots branch January 10, 2025 06:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use python built in type annotations in LLDB visualizer scripts #134291

Use python built in type annotations in LLDB visualizer scripts #134291

Walnut356 commented Dec 14, 2024

rustbot commented Dec 14, 2024

This comment has been minimized.

tgross35 commented Dec 15, 2024

Walnut356 commented Dec 15, 2024 •

edited

Loading

tgross35 commented Dec 15, 2024

Walnut356 commented Dec 16, 2024

Mark-Simulacrum commented Dec 26, 2024

tgross35 left a comment •

edited

Loading

tgross35 Dec 31, 2024

Walnut356 Dec 31, 2024

Walnut356 Dec 31, 2024

tgross35 Dec 31, 2024

tgross35 Dec 31, 2024

tgross35 Dec 31, 2024

Walnut356 Dec 31, 2024

tgross35 Dec 31, 2024

tgross35 left a comment

tgross35 commented Dec 31, 2024

bors commented Dec 31, 2024

Use python built in type annotations in LLDB visualizer scripts #134291

Use python built in type annotations in LLDB visualizer scripts #134291

Conversation

Walnut356 commented Dec 14, 2024

rustbot commented Dec 14, 2024

This comment has been minimized.

tgross35 commented Dec 15, 2024

Walnut356 commented Dec 15, 2024 • edited Loading

tgross35 commented Dec 15, 2024

Walnut356 commented Dec 16, 2024

Mark-Simulacrum commented Dec 26, 2024

tgross35 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgross35 left a comment

Choose a reason for hiding this comment

tgross35 commented Dec 31, 2024

bors commented Dec 31, 2024

Walnut356 commented Dec 15, 2024 •

edited

Loading

tgross35 left a comment •

edited

Loading