WIP: Don't treat use of DEPRECATED APIs as errors under -Werror.
Summary
Trac: https://ghc.haskell.org/trac/ghc/ticket/16163
Differentiate between DEPRECATED and WARNING pragmas at the warning flag level, permitting us to change the behaviour of -Werror to continue to treat WARNINGs as fatal errors, but leave use of DEPRECATED APIs as warnings (unless explicitly requested to be treated as fatal errors via -Werror=deprecations).
Motivation
As it stands today, two major releases are required to deprecate (and remove) part of a public API due to the wide usage of -Werror which causes build failures when a new DEPRECATED pragma is added. The first major release adds the DEPRECATED pragma and the second removes the symbol or module. This discourages deprecating old APIs, introduces unnecessary version churn, and makes it more difficult to communicate to consumers of a library that an upcoming release will remove a feature.
As a specific example, when attempting to align the public APIs of containers and unordered-containers (renaming HashMap.lookupDefault to HashMap.findWithDefault to match the API of containers) the largest issue was how to actually deprecate the old function. The unordered-containers API is relatively stable and it wouldn’t warrant a new major version just to mark a function as DEPRECATED. You can see the discussion here.
Change in Semantics
This results in a change to what type of warnings are by default fatal errors under -Werror, and a different warning flag value is attributed to uses of DEPRECATED symbols or modules. Previously, if you were used a DEPRECATED symbol then passing -Wdeprecations or -Wwarnings-deprecations would both recognize and flag this usage, and based on which flag you used it would be reported under that flag.
If you specified both warning flags then uses of BOTH WARNING and DEPRECATED are attributed to -Wdeprecations. Confusingly, if you set -Werror=warnings-deprecations then uses of these APIs will cause errors, but will report the errors under -Wdeprecations which was never specified (which is counterintuitive). This change also fixes this inconsistency.
Treating uses of DEPRECATED symbols as an error (the old behaviour)
The old behaviour of treating use of deprecated symbols as a fatal error can be achieved by explicitly specifying -Werror=deprecations.
Overview of Code Changes
Conveniently, whether a WarnDecl came from a WARNING or DEPRECATED pragma is stored in the WarningTxt so I didn’t need to thread new info through from the parser to the error/warning handling code.
- Differentiates between
WARNINGandDEPRECATEDpragmas at the warning flag level (addDynFlags.hs>| Opt_WarnDeprecations). - The
-Wdeprecationsflag uses the newOpt_WarnDeprecationsflag value instead of theOpt_WarnWarningsDeprecationsvalue used by the-Wwarnings-deprecationsflag. - The
Opt_WarnDeprecationsflag is removed from the set of flags that are treated as fatal errors by default under-Werror(seeDynFlags.hs>make_ord_flag defFlag "Werror"). The other changes use the information available in theWarningTxtto add the correct warning flag based on whether theDEPRECATEDorWARNINGpragma was used to annotate the symbol or module.
Testing
I’ve manually tested these changes on a collection of .hs files with various combinations of warnings and deprecations on individual symbols and top level modules.
Unit testing: TODO, looking into the best way to unit test this, sending out for review in the meantime to get feedback. Will eventually write tests for the warningTextToWarningFlag
Possible Follow-up Up: Rename -Wwarnings-deprecations to -Wwarnings
The -Wwarnings-deprecations flag was added when the WARNING pragma was added and as a consequence of the implementation captured both WARNING and DEPRECATED usage. Now that uses of deprecated APIs are always captured under -Wdeprecations the other flag (which captures only WARNING usage) could be renamed. The -Wwarnings flag is available currently.
TODO before merging
- Update GHC docs with new meaning of
-Wdeprecationsflag and new-Werrorbehaviour. - More code comments.