Opened 3 years ago

Last modified 3 months ago

#4879 new feature request

Deprecate exports

Reported by: basvandijk Owned by:
Priority: high Milestone: 7.8.3
Component: Compiler Version: 7.0.1
Keywords: Cc: roma@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

Motivation

During the library submission process there's sometimes the desire to have the ability to deprecate an export from a module.

For example during the discussing about ticket #4422, I would have liked the ability to deprecate the exports of the String functions: lines, words, unlines and unwords from Data.List in favour of importing them from Data.String. However I wasn't able to do so, so these exports remain.

Similarly, during the discussion about ticket #4865, Ian also desired to deprecate the export of catch from System.IO.Error but was unable to do so.

Syntax

To deprecate an export simply place a DEPRECATE pragma for the export inside the export list, as in:

module Data.List
  (  ...
  {-# DEPRECATE lines "Exported from Data.String instead" #-}
  , lines
  ...
  ) where
...

Another design might be to have a different pragma as in:

{-# DEPRECATE_EXPORT lines "Exported from Data.String instead" #-}

But I find the former much prettier and more obvious.

Semantics

If the lines export from Data.List is deprecated the following should raise deprecation warnings:

  • Directly importing a deprecated export:
    import Data.List (lines)
    
  • Referring to a deprecated export:
    import Data.List
    foo = lines
    

If you import the same symbol from different modules and only some of them are deprecated exports then referring to the symbol won't give a deprecation warning. For example the following should not give deprecation warnings:

import Data.List
import Data.String
foo = lines

What exports can be deprecated?

  • Functions.
  • Types.
  • Classes.
  • Constructors. Possible syntax:
    module A
      ( {-# DEPRECATE T(C1) "The export of C1 is deprecated" #-}
        T(C1, C2, C3)
      ) where
    
  • Modules. Possible syntax:
    module A
      ( {-# DEPRECATE module B "The export of module B is deprecated" #-}
        module B
      ) where
    

The semantics of deprecating a module export is that you get deprecation warnings for all symbols from module B that you refer to inside a module that imports A. (Does that make sense?)

Attachments (1)

0002-WIP-4879-support-for-deprecating-exports.patch (39.1 KB) - added by igloo 9 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 follow-up: Changed 3 years ago by simonpj

How does this differ from what we have now? You can say

  {-# DEPRECATE lines "Exported from Data.String instead" #-}

in Data.List and it will work just as you say. Maybe you can elaborate your proposal to say how it relates the current mechanisms?

comment:2 Changed 3 years ago by igloo

You can't do this:

module Q (foo) where

foo :: ()
foo = ()
module W (foo) where

import Q

{-# DEPRECATED foo "Some reason" #-}
$ ghc --make W.hs
[1 of 2] Compiling Q                ( Q.hs, Q.o )
[2 of 2] Compiling W                ( W.hs, W.o )

W.hs:6:16:
    The deprecation for `foo' lacks an accompanying binding
      (You cannot give a deprecation for an imported value)

comment:3 in reply to: ↑ 1 Changed 3 years ago by basvandijk

Replying to simonpj:

How does this differ from what we have now? You can say

  {-# DEPRECATE lines "Exported from Data.String instead" #-}

in Data.List and it will work just as you say. Maybe you can elaborate your proposal to say how it relates the current mechanisms?

The current mechanism allows you to deprecate a definition. I would like to deprecate an export. In the case of lines, I would like to move the definition of lines to Data.String and re-export it from Data.List so that user code doesn't break immediately.

However, I do want to mark the export of lines from Data.List as deprecated so that users are warned that in the next version of base they need to import it from Data.String:

module Data.String 
  ( ...
  , lines
    ... 
  ) where

lines = ...
module Data.List 
  ( ...
    {-# DEPRECATE lines "Exported from Data.String instead" #-}
  , lines
    ... 
  ) where

import Data.String ( lines )

We could keep the definition of lines in Data.List and deprecate it and define a new function lines in Data.String. However this has the disadvantage that users importing both Data.List and Data.String get an "ambiguous occurrence of lines" error unless they use qualified imports.

In the proposed mechanism we don't have this problem and there's no need for a duplicate definition.

comment:4 Changed 3 years ago by igloo

  • Milestone set to 7.2.1

comment:5 follow-up: Changed 3 years ago by simonpj

Ah, now I see. It's a bit like deprecating the "definition" of lines in Data.List, where in this case the "definition" is the import from Data.String.

But there are questions of course. What if I now say

module Foo( lines ) where
  import Data.List( lines )

Do I get a deprecation message from the mention of lines in the export list? What if there was no export list was moudle Foo( module Data.List )?

What if something is in scope more than one way, one deprecated and one not:

module Bar where
  import Data.List( lines )
  import Data.String( lines )
  f = lines

Do I get a deprecation warning? Or does the use "pick the best" import declaration?

What if something is imported two ways, and both are separately deprectated. Does it accumulate two deprecation warnings?

What if a moudule M imports Data.List(lines), and re-exports lines? Is an import from M deprecated? That is, are deprecations transitive?

Thinking about it, a possible view is this:

  • The existing deprecating mechanism attaches a deprecation to a definition, and complains at a use, but not at imports.
  • The new mechanism attaches a deprecation to an export (from module M, say), and complains at import (of module M only), but but not at uses.

Under this interpretation it's not clear whether plain import module M should do.

But regardless, there are lots of details to be worked out. Whether the pain is worth the gain is not clear to me.

Simon

comment:6 in reply to: ↑ 5 Changed 3 years ago by simonmar

We have another use for this: since Haskell 2010, Foreign.unsafePerformIO is now deprecated, you are supposed to get it from System.IO.Unsafe.unsafePerformIO. I didn't want to make it a distinct identifier, because that would cause name clashes, but I would like to deprecate uses of Foreign.unsafePerformIO.

In other words, I would like a warning to be emitted iff a compilation error would ensue if the deprecated export was removed.

So, if the user imports Foreign.unsafePerformIO by name, the warning would be generated.

If unsafePerformIO is in scope via two routes and is used somehow, that's ok - no warning needs to be generated. If it was in scope via multiple routes and all routes were deprecated, then I expect we should emit all the warnings.

comment:7 Changed 2 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Priority changed from normal to low

comment:8 Changed 22 months ago by simonmar

  • Difficulty set to Unknown
  • Priority changed from low to high

comment:9 Changed 20 months ago by igloo

  • Milestone changed from 7.6.1 to 7.8.1

comment:10 Changed 11 months ago by basvandijk

Another use-case for this came up today on the libraries list:

Edward Kmett:
"If it was possible to deprecate a re-export of a function from a module,
deprecating the re-export of the foldr-like functions from Data.List would
provide a path towards a more sane final state."

simonpj: to address the questions you raised two years ago:

What if I now say

module Foo( lines ) where
  import Data.List( lines )

Do I get a deprecation message from the mention of lines in the export list?

Yes and you should also get a warning on the explicit import of the deprecated lines.

What if there was no export list as in: module Foo( module Data.List )?

Since this re-exports all the symbols from Data.List including the deprecated ones it should give a warning.

The way to fix this is to hide lines when importing Data.List:

import Data.List hiding ( lines )

This does mean that when lines gets removed in the next release this code would become redundant (although it won't fail to compile, so I think it's fine).

What if something is in scope more than one way, one deprecated and one not:

module Bar where
  import Data.List( lines )
  import Data.String( lines )
  f = lines

Do I get a deprecation warning? Or does the use "pick the best" import declaration?

In this case, since you're importing lines explicitly from Data.List you should get a deprecation warning when importing it.

However if you didn't import it explicitly like:

module Bar where
  import Data.List
  import Data.String( lines )
  f = lines

It should "pick the best" import.

What if something is imported two ways, and both are separately deprecated. Does it accumulate two deprecation warnings?

Good question, I think it makes sense to accumulate the deprecation warnings in this case, accompanied by some good phrasing what and why this is happening.

What if a moudule M imports Data.List(lines), and re-exports lines? Is an import from M deprecated? That is, are deprecations transitive?

No. I think warnings should always be fixable in the module in which they originate since the offending module could be outside the control of the user.

Thinking about it, a possible view is this:

  1. The existing deprecating mechanism attaches a deprecation to a definition, and complains at a use, but not at imports.
  1. The new mechanism attaches a deprecation to an export (from module M, say), and complains at import (of module M only), but but not at uses. Under this interpretation it's not clear whether plain import module M should do.

I think it should be a combination of 1 and 2:

  1. The new mechanism attaches a deprecation to an export (from module M, say), and complains at use. "Use" means when referencing the symbol (both in code, import lists or export lists).

But regardless, there are lots of details to be worked out. Whether the pain is worth the gain is not clear to me.

What would be the best way to proceed?

Create a more detailed language proposal in the wiki? Or discuss the remaining details in this thread?

I could have a go at implementing this myself in my very limited spare time. I guess the main changes need to go in the Renamer.

comment:11 Changed 11 months ago by simonpj

Bas askes: What would be the best way to proceed? Create a more detailed language proposal in the wiki? Or discuss the remaining details in this thread?

Both, I think. The wiki is a good place to write a spec that represents the current proposal. The thread is a good place to discuss the proposal. Having only the thread is terrible, because to reconstruct the proposal you have to replay the thread in your head!

comment:12 Changed 9 months ago by igloo

I've added the beginnings of a patch for this. It needs some polishing, but basically works.

In essence, warnings are parsed in export lists, and kept in AvailInfo and then Provenance. When giving deprecation warnings, we check to see whether any provenance is warning-free, and if not then we print all the warnings.

I think we probably want to parameterise AvailInfo; in particular, rather than

addAvailInfoWarnings :: Map Name WarningTxt -> [AvailInfo] -> [AvailInfo]

we should have

addAvailInfoWarnings :: Map Name WarningTxt -> [AvailInfo Name] -> [AvailInfo NameWarn]

bestImport in extendImportMap should probably be trying to find an import without a warning, too.

comment:13 Changed 9 months ago by simonpj

Can someone perhaps write a wiki page that gives the specification? I'm uncomfortable with adopting an implementation when I don't know the spec, except by reading this entire ticket, which isn't really a good thing. Thanks!

Simon

comment:14 Changed 3 months ago by Feuerbach

  • Cc roma@… added
Note: See TracTickets for help on using tickets.