clayborg added a comment.
The main issue I have with this is the name of the function we are adding to
LanguageRuntime. See inlined comments.
================
Comment at: include/lldb/Target/ObjCLanguageRuntime.h:253
+ CompilerType CalculateCompleteType(CompilerType base_type) override;
+
----------------
Is this named correctly? Maybe this should be named "CompilerType
GetRuntimeType(CompilerType base_type) override;"?
What this function does is gets the real definition from the objective C
runtime at the moment. This name would better reflect what is going on and
would be something we might ask of a runtime.
================
Comment at: source/Target/ObjCLanguageRuntime.cpp:403
+CompilerType
+ObjCLanguageRuntime::CalculateCompleteType(CompilerType base_type) {
+ CompilerType type_to_return;
----------------
So a main question for ObjC here: do we always want to show the runtime type?
Should we not check if the class inside of "base_type" is the one true
definition and skip grabbing the runtime type here and return {}?
================
Comment at: source/Target/ObjCLanguageRuntime.cpp:404
+ObjCLanguageRuntime::CalculateCompleteType(CompilerType base_type) {
+ CompilerType type_to_return;
+
----------------
JDevlieghere wrote:
> I would move this down.
Remove this variable and just return {} everywhere
================
Comment at: source/Target/ObjCLanguageRuntime.cpp:430-433
+ if (is_pointer_type)
+ type_to_return = complete_class.GetPointerType();
+ else
+ type_to_return = complete_class;
----------------
```
if (is_pointer_type)
return complete_class.GetPointerType();
else
return complete_class;
```
================
Comment at: source/Target/ObjCLanguageRuntime.cpp:436
+
+ return type_to_return;
+}
----------------
```
return {};
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64159/new/
https://reviews.llvm.org/D64159
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
