Skip to content

Improve correctness and function declarations #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swapneils
Copy link

@swapneils swapneils commented Apr 20, 2025

Correctness changes:

  • join now treats a nil separator as the empty string, rather than the string "nil", conforming to the semantics in the README of not converting nil to "nil".
  • containsp now automatically returns nil if either string input is nil.
  • alphanump and similar now use the correct regex checks for multi-line strings, rather than accepting an input if it has any line matching their specification.

Optimization changes:

  • Multiple non-recursive functions have been inlined
  • Multiple functions have ftype declarations
  • In local testing, there is a ~0.1s reduction from ~1.05s to ~0.95s in (time (dotimes (i 100) (with-output-to-string (throwaway) (let ((*standard-output* throwaway)) (asdf:test-system :str))))), despite the new tests in this version.
    • Note: Most of the tests use constantp inputs, so performance improvements here derive mainly from the inlining; they do not represent benefits from the ftype declarations in user-code.
    • Note: The comparison was specifically with the contents of Optimize str:repeat for large workloads #128, but that still shows an improvement over the original.

(length s)))
(let ((end (max (- len (length ellipsis))
0)))
(setf s (concat
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to convert this setf to a static dataflow so SBCL could type-check this function.

@vindarel
Copy link
Owner

excellent, thanks, it looks good, though I'll take more time to test in the wild, because merging bad type declarations can be very annoying (and imagine if Quicklisp releases in-between our quick fixes). I encourage anyone to try this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants