Page MenuHomePhabricator

[lldb] Dynamically generate enum names in lldbutil
ClosedPublic

Authored by kastiglione on Wed, Aug 3, 8:54 PM.

Details

Summary

Change the <enum>_to_str functions in lldbutil to be dynamic.

Instead of a manually maintained if/elif switch, the functions now perform
lookup in a dynamically generated dict. The names are determined by stripping
the enum's prefix, and then lowercasing the remaining suffix, ex:

eStateRunning -> "running"

Diff Detail

Event Timeline

kastiglione created this revision.Wed, Aug 3, 8:54 PM
Herald added a project: Restricted Project. ยท View Herald TranscriptWed, Aug 3, 8:54 PM
kastiglione requested review of this revision.Wed, Aug 3, 8:54 PM
Herald added a project: Restricted Project. ยท View Herald TranscriptWed, Aug 3, 8:54 PM
Herald added a subscriber: lldb-commits. ยท View Herald Transcript
JDevlieghere accepted this revision.Wed, Aug 3, 8:59 PM

Nice. Thanks for taking care of this!

This revision is now accepted and ready to land.Wed, Aug 3, 8:59 PM
mib added a comment.Wed, Aug 3, 9:07 PM

This is awesome ๐Ÿคฉ ! I was also thinking of changing the way enums are exposed to python: instead of having everything added to the lldb python module, we could create a class per enum and have static attributes for each enum value so we could do something like lldb.StopReason.Breakpoint. That static variable could be a pair with the value/string representation or maybe we could use the __str__ method to make it very pythonic. Just throwing some ideas here for later, but this LGTM ๐Ÿ˜Š !

mib accepted this revision.Wed, Aug 3, 9:08 PM

@mib I like that idea.

jingham added a subscriber: jingham.Fri, Aug 5, 9:59 AM

This is awesome ๐Ÿคฉ ! I was also thinking of changing the way enums are exposed to python: instead of having everything added to the lldb python module, we could create a class per enum and have static attributes for each enum value so we could do something like lldb.StopReason.Breakpoint. That static variable could be a pair with the value/string representation or maybe we could use the __str__ method to make it very pythonic. Just throwing some ideas here for later, but this LGTM ๐Ÿ˜Š !

I'm not sure you could do this w/o breaking binary compatibility, since we pass these enums to a bunch of the SB API's. But if you can make that work, this would be nice.

This is awesome ๐Ÿคฉ ! I was also thinking of changing the way enums are exposed to python: instead of having everything added to the lldb python module, we could create a class per enum and have static attributes for each enum value so we could do something like lldb.StopReason.Breakpoint. That static variable could be a pair with the value/string representation or maybe we could use the __str__ method to make it very pythonic. Just throwing some ideas here for later, but this LGTM ๐Ÿ˜Š !

I'm not sure you could do this w/o breaking binary compatibility, since we pass these enums to a bunch of the SB API's. But if you can make that work, this would be nice.

I don't think that's possible, at least not without having both, which then defeats the purpose. I suggested to Ismail that we should start a document to keep track of all the things we want to improve for a V2 of the SB API. These kind of ideas come up every once in a while and it would be good to have them tracked somewhere so they don't get lost with time if/when we ever decide to redo the stable API.

I wonder how much breakage there would be if you implemented a bunch of the dunder methods. Ex:

class StopReason:
    def __int__(self) -> int: ...
    def __eq__(self, other: StopReason | int) -> bool: ...
    # ...

This would cover many cases, but not all. For example 5 == StopReason.whatever would fail, even when StopReason.whatever == 5 passes.

Fix bug; Add type annotations

kastiglione requested review of this revision.Fri, Aug 5, 3:34 PM

@JDevlieghere @mib what do you think of introducing type annotations as code is being touched?

mib added a comment.Fri, Aug 5, 3:49 PM

@JDevlieghere @mib what do you think of introducing type annotations as code is being touched?

Since we're basically building an API, it makes a lot of sense of add type annotations. I'm strongly in favor of doing it.

This revision was not accepted when it landed; it landed in state Needs Review.Sun, Aug 7, 11:16 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.