Skip to content

[#177] Walk through all modules and think carefully about SPECIALIZE/…#180

Merged
vrom911 merged 1 commit intomasterfrom
chshersh/177-Walk-through-all-modules-and
Aug 17, 2019
Merged

[#177] Walk through all modules and think carefully about SPECIALIZE/…#180
vrom911 merged 1 commit intomasterfrom
chshersh/177-Walk-through-all-modules-and

Conversation

@chshersh
Copy link
Copy Markdown
Contributor

…INLINABLE/INLINE pragmas

Resolves #177

I had a discussion recently about INLINE, SPECIALIZE and INLINEABLE pragmas. So I now understand these pragmas better. But the rules when to use them are still vague. So I tried to use my judgement to specify these pragmas where appropriately.

This PR does the following:

  1. Add missing pragmas in all modules
  2. Adds InstanceSigs and signatures in some places
  3. Update to modern .gitignore
  4. Move String type to Relude.String for consistency
  5. Minor stylistic changes

Checklist:

HLint

  • I've changed the exposed interface (add new reexports, remove reexports, rename reexported things, etc.).
    • I've updated hlint.dhall accordingly to my changes (add new rules for the new imports, remove old ones, when they are outdated, etc.).
    • I've generated the new .hlint.yaml file (see this instructions).

General

  • I've updated the CHANGELOG with the short description of my latest changes.
  • All new and existing tests pass.
  • I keep the code style used in the files I've changed (see style-guide for more details).
  • I've used the stylish-haskell file.
  • My change requires the documentation updates.
    • I've updated the documentation accordingly.
  • I've added the [ci skip] text to the docs-only related commit's name.

@chshersh chshersh requested a review from vrom911 August 16, 2019 11:45
@chshersh chshersh self-assigned this Aug 16, 2019
Copy link
Copy Markdown
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

Great work 👍 Thanks!

-- | Lifted version of 'System.Exit.exitWith'.
exitWith :: MonadIO m => ExitCode -> m a
exitWith a = liftIO (XIO.exitWith a)
{-# INLINE exitWith #-}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the reasoning behind this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reasoning is that if you call exit* functions, your program terminates and doesn't do anything else. So it doesn't make much sense to INLINE such functions, we can rely on GHC to do the right stuff. It's a minor micro-optimisation because abuse of INLINE can slow down compilation, but here I think we don't need it.

@vrom911 vrom911 merged commit 62203b1 into master Aug 17, 2019
@vrom911 vrom911 deleted the chshersh/177-Walk-through-all-modules-and branch August 17, 2019 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Walk through all modules and think carefully about SPECIALIZE/INLINABLE/INLINE pragmas

2 participants