kastiglione 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);
----------------
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`?


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

Reply via email to