Add missing nullptr checks in atomlist

[frmdstryr]

No description provided.

[MatthieuDartiailh]

Thanks can you rebase to get the formatter happy.

[sccolbert]

why are we doingAddModule here instead of Import? If we need a builtin function from globals, that should be pre-fetched on module initialization.

[MatthieuDartiailh]

I do not remember the details but I likely followed an example. I would like to fully move away from global static and properly use module state (and we could store super there) but I have no idea if I will find the time to do so.

[sccolbert]

But hold on here. We seem to have deviated quite a bit from the original code, by calling a builtin and returning before the observer code. What exactly is going on, and why have we deviated so much from the original code?

[MatthieuDartiailh]

Once again I do not have the whole history in mind but I think we were relying on a private function to sort the list that disappeared. However I do not remember why I went with super rather than looking up the method directly on list. It may have just been simpler.

[MatthieuDartiailh]

I will be worth revisiting this in the future to see if we can improve but the changes here make the code more correct and there is no reason to delay them so I will merge once the CI is green.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.67%. Comparing base (10447ae) to head (f006704).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #220   +/-   ##
=======================================
  Coverage   97.67%   97.67%           
=======================================
  Files          24       24           
  Lines        1074     1074           
  Branches      162      162           
=======================================
  Hits         1049     1049           
  Misses         12       12           
  Partials       13       13           

[sccolbert]

Now that this is merged, I think we need to avoid going thru super to call List.sort. We should be able to access that method directly via the C-api and short-circuit all of the super logic. atomlist is not designed for multiple-inheritance, so this should not be an issue.