jingham added inline comments.
================
Comment at: lldb/include/lldb/Symbol/CompilerDeclContext.h:77
/// in a struct, union or class.
- bool IsClassMethod(lldb::LanguageType *language_ptr,
- bool *is_instance_method_ptr,
- ConstString *language_object_name_ptr);
+ bool IsClassMethod(ConstString *instance_var_name_ptr = nullptr,
+ bool *instance_is_pointer_ptr = nullptr);
----------------
kastiglione wrote:
> teemperor wrote:
> > kastiglione wrote:
> > > shafik wrote:
> > > > If we are going to refactor this, this change does not feel very C++y
> > > > passing around pointers. I know we want a way to call this w/o any
> > > > arguments but perhaps we can write an overload for that case?
> > > >
> > > > Does `instance_var_name_ptr` need to be a string? Maybe we can encode
> > > > it using an enum, we don't have a lot of cases `this`, `self`, maybe
> > > > even not a pointer as well and get ride of `instance_is_pointer_ptr`.
> > > Something like?
> > >
> > > ```
> > > enum InstanceVariable {
> > > ThisPointer,
> > > SelfPointer,
> > > Self,
> > > };
> > > ```
> > We could also make this function that is something like
> > `llvm::Optional<SelfRef> GetCurrentObjectRef` and `SelfRef` is just
> > ConstString + enum if it's a pointer/ref/whatever.
> >
> > FWIW, encoding the string inside an enum doesn't seem to fit with the idea
> > that the TypeSystem interface just needs to be implemented (but not
> > extended) when adding a new language plugin (if the language uses a
> > different name like `$this` or `Self` then the enum needs to be expanded).
> > Also not sure what use this has to the caller (I don't see how the callers
> > do anything else with this enum then translating it to the actual string
> > and checking if it's a pointer, both are more complicated with an enum).
> I like this approach. Before I make the change, some questions/thoughts.
>
> I'm thinking the second field will be a bool, ex: `is_pointer`. The reason
> for bool and not enum is that I don't know if it's worth the complexity of
> trying to distinguish between reference and value. In Swift, the `self`
> variable could be reference (`class`) or value (`struct`, `enum`…).
>
> Instead of `SelfRef` I'm thinking `{This,Self,Instance}Variable`, since it's
> info about the variable (name, pointer-ness).
>
> Do we need to return a `ConstString`, or can it be `const char *` and let the
> caller do what it wants. It seems it will always be a string literal, and
> `const char *` is a lower common denominator. I guess I'm ultimately unclear
> on when, if ever, to not use `ConstString`?
ConstString's main purpose is to hold strings we're likely to compare against a
lot. For instance, if you take a symbol name and you are going to look it up
everywhere, it's appropriate for that to be a ConstString since we're going to
turn it into that anyway to do the searches.
Since a caller is likely to turn around and look up "this" having gotten that
name back, a ConstString seems an okay choice. Another way to do this would be
to make function statics with ConstStrings for "this" and "self". When you
make a ConstString from a c-string we have to hash it look for it in the string
pool. Copying a ConstString is just copying a pointer. So if you have just a
couple of options, making static ConstStrings makes returning the result cheap.
And since ConstString's are all null-terminated C-strings, ConstString ->
cstring is cheap.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98653/new/
https://reviews.llvm.org/D98653
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
