JDevlieghere added inline comments.

================
Comment at: lldb/examples/python/crashlog.py:515
     def parse_images(self, json_images):
-        idx = 0
-        for json_image in json_images:
+        for idx, json_image in enumerate(json_images):
             img_uuid = uuid.UUID(json_image['uuid'])
----------------
mib wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > mib wrote:
> > > > What do we use `idx` for ?
> > > You're right, this isn't necessary anymore.
> > I'm really not a big fan of having very similar image lists ... may be we 
> > could use the from the crashlog object and skip the first entry (since we 
> > know it's the main executable).
> > What do you think ?
> Otherwise, we could hoist the main executable image from the image list and 
> handle it separately.
I understand the concern. To be fair, I didn't check whether the main 
executable coming first is something we rely on, but I'm pretty sure we are: 
we'll need it to create the target. I didn't want to mess with that and risk 
introducing a bug that way. It took me quite some time to figure out this was 
an issue when parsing the symbol data. If we don't want to break that 
assumption, there's nothing more efficient than keeping a second list of 
references. I also think it makes sense to keep that in the JSON parser, 
because the index of (parsed) image is only something that makes sense for that 
format because it cross references images based on their index. That's not the 
case in the textual or parser crashlogs.  

FWIW this is the code that moves the main image to the top, invalidating the 
image indexes of every image before it:

```
    def set_main_image(self, identifier):
        for i, image in enumerate(self.images):
            if image.identifier == identifier:
                self.images.insert(0, self.images.pop(i))
                break
```



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

https://reviews.llvm.org/D148172

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

Reply via email to