mib marked 3 inline comments as done.
mib added inline comments.

================
Comment at: lldb/examples/python/crashlog.py:434
         except CrashLogFormatException:
-            return TextCrashLogParser(debugger, path, verbose).parse()
+            return  object().__new__(TextCrashLogParser)
 
----------------
kastiglione wrote:
> I have not seen the `object().__new__(SomeClass)` syntax. Why is it being 
> used for `TextCrashLogParser` but not `JSONCrashLogParser`? Also, `__new__` 
> is a static method, could it be `object.__new__(...)`? Or is there a subtly 
> that requires an `object` instance? Somewhat related, would it be better to 
> say `super().__new__(...)`?
> 
> Also: one class construction explicitly forwards the arguments, the other 
> does not. Is there a reason both aren't implicit (or both explicit)?
As you know, python class are implicitly derived from the `object` type, making 
`object.__new__` and `super().__new__` pretty much the same thing.

In this specific case, both the `TextCrashLogParser` and `JSONCrashLogParser` 
inherits from the `CrashLogParser` class, so `JSONCrashLogParser` will just 
inherits `CrashLogParser.__new__` implementation if we don't override it, which 
creates a recursive loop.
That's why I'm calling the `__new__` method specifying the class.


================
Comment at: lldb/examples/python/crashlog.py:447
+class JSONCrashLogParser(CrashLogParser):
+    def __new__(cls, debugger, path, verbose):
+        with open(path, 'r') as f:
----------------
kastiglione wrote:
> can this be an `__init__`?
No unfortunately because at the `__init__` is called, the type of the object is 
already set, but we actually want to fallback to the `TextCrashLogParser` class 
if this one fails.

`__new__`: takes arguments and returns an instance
`__init__`: takes instance and arguments (from `__new__`) and returns nothing


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131085/new/

https://reviews.llvm.org/D131085

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to