Support re-export deprecations (re #4879)
Needs RevisionPublic

Authored by hvr on Jan 29 2015, 6:38 AM.

Details

Reviewers
Mikolaj
alanz
simonpj
ekmett
austin
Trac Issues
#4879
Summary

This is basically the patch originally implemented by Ian Lynagh
manually forward-ported to GHC 7.10/11 (the patch wasn't mergeable anymore)

WARNING: this is mostly untested... it just compiles... that's it; look out for XXX-markers

Original comment from Ian regarding this patch from Trac #4878:

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.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
master
Lint
Lint WarningsExcuse: don't care about aesthetics till the code is correct
SeverityLocationCodeMessage
Warningcompiler/deSugar/DsMonad.hs:261TXT3Line Too Long
Warningcompiler/main/InteractiveEval.hs:858TXT3Line Too Long
Warningcompiler/rename/RnEnv.hs:908TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:655TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:670TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:800TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:888TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:914TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:1081TXT3Line Too Long
Warningcompiler/rename/RnNames.hs:1192TXT3Line Too Long
Warningcompiler/typecheck/TcRnDriver.hs:171TXT3Line Too Long
Unit
No Unit Test Coverage
Build Status
Buildable 3036
Build 3063: GHC Patch Validation (amd64/Linux)
hvr updated this revision to Diff 2177.Jan 29 2015, 6:38 AM
hvr retitled this revision from to Support re-export deprecations (re #4879).
hvr updated this object.
hvr edited the test plan for this revision. (Show Details)
hvr added reviewers: simonpj, austin, ekmett.
hvr updated the Trac tickets for this revision.Jan 29 2015, 6:39 AM
hvr added a reviewer: alanz.Jan 31 2015, 6:28 AM
hvr added a subscriber: alanz.

@alanz I think you have gained quite some experience with the renamer, and also while forward-porting I stumbled over a few places where the recent annotation-changes caused merge conflicts...

hvr updated this object.Jan 31 2015, 6:31 AM
alanz edited edge metadata.Jan 31 2015, 8:13 AM

I have commented on the Api Annotations part.

If you want, I can make my proposed change and submit a patch, let me know.

compiler/parser/Parser.y
585

For API Annotaions there needs to be a single Located item to hang them off.

So we would need something like

data IEWarnings = IEWarnings SourceText [LIeWarning]

This is modelled on WarnDecls

simonpj requested changes to this revision.Feb 9 2015, 8:28 AM
simonpj edited edge metadata.

I don't like attaching a Maybe WarningTxt to every Name in every Avail. That's a LOT of Nothings!

I think it'd be better instead to have an OccEnv WarningTxt that travels with a [AvailInfo] giving the deprecations on exports (and I guess imports). This is a pretty simple refactoring; and it helps to keep the issues separate. AvailInfo is used quite widely, whereas this is very specific to deprecations.

The patch even generates such a mapping, in ExportAccum.

Adding a is_warning field to the ImportSpec in a GRE looks right to me.

I would much prefer it if all of these things were called "deprecations" not "warnings". I know that's an unforced refactoring, but it would be a helpful one.

Simon

This revision now requires changes to proceed.Feb 9 2015, 8:28 AM
austin resigned from this revision.Nov 6 2017, 10:08 PM