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