refactor(list): replace panic with abort

[illusory0x0]

No description provided.

[peter-jerry-ye-code-review]

Inconsistent error message format across unreachable branches

Category
Maintainability
Code Snippet
Empty, _ => abort("unreachable")
Recommendation
Consider using a more descriptive error message that includes the function name, e.g. abort("unreachable branch in map function")
Reasoning
While the change from panic() to abort() is good, having function-specific error messages would make debugging easier by immediately identifying which function encountered the unreachable condition

Some unreachable branches might actually be reachable in edge cases

Category
Correctness
Code Snippet
Empty, _, _ => abort("unreachable")
Recommendation
Consider adding assertions or validations before these branches to ensure preconditions are met, e.g. checking list length or index bounds
Reasoning
What makes these branches unreachable isn't immediately clear from the code. Adding explicit checks would make the code more robust and self-documenting

Repetitive error handling code across functions

Category
Maintainability
Code Snippet
Multiple instances of Empty, _ => abort("unreachable")
Recommendation
Consider creating a helper function like fn unreachable(fn_name: String) { abort("unreachable branch in " + fn_name) }
Reasoning
This would reduce code duplication and make it easier to modify the error handling behavior across all functions in the future

[coveralls]

Pull Request Test Coverage Report for Build 6734

Details

• 0 of 16 (0.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.02%) to 92.427%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
list/list.mbt	0	16	0.0%

Files with Coverage Reduction	New Missed Lines	%
bench/stats.mbt	1	76.32%

Totals
Change from base Build 6733:	-0.02%
Covered Lines:	6139
Relevant Lines:	6642

---

💛 - Coveralls

[peter-jerry-ye]

Replacing with unreachable is meaning less here.

[illusory0x0]

Replacing with unreachable is meaning less here.

Meaning

Using abort internally improves code readability, is self-documenting, and can tell the user that some errors are internal to the library rather than the user violating the invariant.

binary bloat (debug string)

The binary bloat caused by using the debug string can generally be optimized away by compilers in other languages.
But moonc is not optimized as such. e.g. godbolt

As you said in this #1866 (comment), These things should be done by improving the compiler and not overthinking it at coding time.

More often than not, resolving this [moonc] generate duplicate code when type variable without constraint. #696
can drastically increase compilation speed and reduce binary size.

[peter-jerry-ye]

Meaning

Using abort internally improves code readability, is self-documenting, and can tell the user that some errors are internal to the library rather than the user violating the invariant.

Yes, but abort("unreachable") is equivalent to panic(). It doesn't add any useful information.

binary bloat (debug string)

The binary bloat caused by using the debug string can generally be optimized away by compilers in other languages. But moonc is not optimized as such. e.g. godbolt

As you said in this #1866 (comment), These things should be done by improving the compiler and not overthinking it at coding time.

Yes, and I do believe that the compiler will be able to handle this. However, there's a subtle difference : the places where you are adding the abort are unreachable code. They are not some preconditions to be checked, such as #2110, but are some runtime invariant that will never be broken as long as they are properly implemented. Even if the compiler can do that, they shouldn't exist in the first place.