Skip to content
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

Move deprecated modules to undeprecated hidden modules #213

Merged
merged 2 commits into from
Dec 3, 2023

Conversation

tbidne
Copy link

@tbidne tbidne commented Dec 3, 2023

Resolves #209, obviates #211.

Problem

Given the modules:

module X {-# DEPRECATED "message" #-} (foo) where

foo :: ()
module Y (foo) where

import X (foo)

We want the following to succeed without warnings:

module Z where

import Y (foo)

Unfortunately we receive a deprecation warning for the usage in Z even though we imported a(n ostensibly non-deprecated) symbol from a non-deprecated symbol. The problem is that any symbol (foo) defined in a deprecated module (X) inherits the deprecation.

Solution

The strategy here is outlined in this comment: #209 (comment)

We move X's implementation into the non-deprecated but hidden X.Hidden.

module X.Hidden (foo)

-- foo no longer inherits the deprecation
foo :: ()

X retains its deprecation, merely re-exporting X.Hidden.

-- importing this still warns
module X {-# DEPRECATED "message" #-} (module X.Hidden) where

import X.Hidden
module Y (foo) where

import X (foo)

Now, this succeeds as we would like:

module Z where

import Y (foo)

And this fails:

module W where

import X (foo)

This PR also changes internal usage of deprecated modules to use the new hidden ones. This removes the need to litter the codebase with {-# OPTIONS_GHC -Wno-deprecations #-}, to avoid internal warnings.

It is recommended to review the commits individually, as the overall diff is large. The first commit merely renames modules X -> X.Hidden. The second adds back X with deprecation message, and removes the deprecation from X.Hidden.

Deprecating the OsString modules in 1.4.200.0 had an unfortunate
side-effect: every symbol defined in such a module inherited the
deprecation status, even if the symbol itself was intended to be
stable.

To work around this, we move the definitions to new, unexposed, and
non-deprecated .Hidden modules. These definitions are then re-exported
from the original deprecated modules. This achieves the desired effect
of:

- Using a deprecated module results in a warning.
- Using a non-deprecated symbol originally defined in a deprecated
  module does __not__ receive a deprecation warning, provided it is
  imported from a non-deprecated module.

For example, the type EncodingException was originally defined in the
deprecated module System.OsPath.Encoding.Internal. This meant that
users would receive a deprecation warning for using it, even if they
imported it from a non-deprecated module e.g. System.OsPath.Encoding.

System.OsPath.Encoding.Internal has been renamed to the unexposed and
non-deprecated module System.OsPath.Encoding.Internal.Hidden. The
old System.OsPath.Encoding.Internal module retains its deprecation
warning and re-exports System.OsPath.Encoding.Internal.Hidden.

Thus:

- Importing EncodingException from System.OsPath.Encoding incurs no
  warnings.
- Importing System.OsPath.Encoding.Internal retains its deprecation.

The implementation strategy is, for each deprecated module X:

1. Rename X -> X.Hidden, removing the deprecation.
2. X now re-exports X.Hidden, adding the deprecation.
@hasufell
Copy link
Member

hasufell commented Dec 3, 2023

I tested this locally with the https://github.com/tbidne/osstr-deprecated package and GHC 8.6 to 9.8. It seems to work as expected.

Copy link
Contributor

@Bodigrim Bodigrim left a comment

Choose a reason for hiding this comment

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

LGTM!

@hasufell hasufell merged commit 6662633 into haskell:1.4 Dec 3, 2023
@tbidne
Copy link
Author

tbidne commented Dec 3, 2023

Thanks everyone!

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.

None yet

3 participants