teemperor 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:
> 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).


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