From fb33decf09ae9f53327d5e43e09c3aebfefa5eea Mon Sep 17 00:00:00 2001 From: Martin Huschenbett Date: Thu, 15 Oct 2020 19:40:13 +0200 Subject: [PATCH] Warn on potentially un-upgradable language extensions (#7662) * Warn on potentially un-upgradable language extensions We issue a warning whenever a module is compiled with a language extension enabled for which we are not certain that it works with data-dependencies. Currently, we consider all extensions except for the ones enables by default, `ApplicativeDo` and `PartialTypeSignatures` problematic in this regard. The list of extensions that work fine with data-dependencies might extend over time but we need to do further research and add further tests before we add any extensions. The warning is always put at the location of the module name regardless of where the `{-# LANGUAGE ... #-}` pragma is actually used. This is due to the fact that the `ParsedModule` we get from GHC does not contain the location information of these pragmas. We should probably improve this over time but I think it is acceptable for now. CHANGELOG_BEGIN [DAML Compiler] Warn when a module uses a language extension that might not work with data-dependencies. CHANGELOG_END * Make the warning less polite :) Co-authored-by: Moritz Kiefer Co-authored-by: Moritz Kiefer --- .../daml-opts/daml-opts/DA/Daml/Options.hs | 25 +++++++++++++++-- .../src/DA/Daml/Preprocessor.hs | 28 +++++++++++++++++-- .../daml-test-files/BadExtensionOption.daml | 8 ++++++ .../daml-test-files/BadExtensionPragma.daml | 7 +++++ .../ConstrainedClassMethod.daml | 1 + .../tests/daml-test-files/Constraint.daml | 2 +- .../tests/daml-test-files/Existential.daml | 2 +- .../tests/daml-test-files/ExistentialSum.daml | 2 +- .../damlc/tests/daml-test-files/Generics.daml | 2 +- .../daml-test-files/GoodExtensionOption.daml | 5 ++++ .../daml-test-files/GoodExtensionPragma.daml | 4 +++ .../daml-test-files/GoodExtensionUseless.daml | 4 +++ .../daml-test-files/PatternSynonyms.daml | 1 + .../damlc/tests/daml-test-files/Records.daml | 1 - .../tests/daml-test-files/TypeFamily.daml | 4 +-- 15 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 compiler/damlc/tests/daml-test-files/BadExtensionOption.daml create mode 100644 compiler/damlc/tests/daml-test-files/BadExtensionPragma.daml create mode 100644 compiler/damlc/tests/daml-test-files/GoodExtensionOption.daml create mode 100644 compiler/damlc/tests/daml-test-files/GoodExtensionPragma.daml create mode 100644 compiler/damlc/tests/daml-test-files/GoodExtensionUseless.daml diff --git a/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs b/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs index 41a026e90b..4cba6d0102 100644 --- a/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs +++ b/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs @@ -20,6 +20,7 @@ module DA.Daml.Options , setupDamlGHC , toCompileOpts , PackageDynFlags(..) + , dataDependableExtensions ) where import Control.Exception @@ -31,6 +32,7 @@ import Data.IORef import Data.List.Extra import Data.Maybe (fromMaybe) import DynFlags (parseDynamicFilePragma) +import qualified EnumSet as ES import qualified Data.Map.Strict as Map import qualified Data.Text as T import Config (cProjectVersion) @@ -64,7 +66,7 @@ import SdkVersion (damlStdlib) toCompileOpts :: Options -> Ghcide.IdeReportProgress -> Ghcide.IdeOptions toCompileOpts options@Options{..} reportProgress = Ghcide.IdeOptions - { optPreprocessor = if optIsGenerated then generatedPreprocessor else damlPreprocessor (optUnitId options) + { optPreprocessor = if optIsGenerated then generatedPreprocessor else damlPreprocessor dataDependableExtensions (optUnitId options) , optGhcSession = getDamlGhcSession options , optPkgLocationOpts = Ghcide.IdePkgLocationOptions { optLocateHieFile = locateInPkgDb "hie" @@ -239,7 +241,12 @@ runGhcFast act = do -- | Language options enabled in the DAML-1.2 compilation xExtensionsSet :: [Extension] xExtensionsSet = - [ -- syntactic convenience + [ -- Haskell 2010 extensions which are enabled by default (we would need to + -- list them for `dataDependableExtensions` below anyway, so let's make + -- them explicit here) + ImplicitPrelude, StarIsType, MonomorphismRestriction, TraditionalRecordSyntax + , EmptyDataDecls, PatternGuards, DoAndIfThenElse, RelaxedPolyRec, NondecreasingIndentation + , -- syntactic convenience RecordPuns, RecordWildCards, LambdaCase, TupleSections, BlockArguments, ViewPatterns, NumericUnderscores -- records @@ -265,10 +272,22 @@ xExtensionsSet = , DamlSyntax ] +-- | Extensions which we support with data-dependencies. +dataDependableExtensions :: ES.EnumSet Extension +dataDependableExtensions = ES.fromList $ xExtensionsSet ++ + [ -- useful for beginners to learn about type inference + PartialTypeSignatures + -- needed for script and triggers + , ApplicativeDo + ] -- | Language settings _disabled_ ($-XNo...$) in the DAML-1.2 compilation xExtensionsUnset :: [Extension] -xExtensionsUnset = [ ] +xExtensionsUnset = + [ -- This is part of Haskell 2010 and would hence be enabled by default, + -- which makes zero sense for DAML. + ForeignFunctionInterface + ] -- | Flags set for DAML-1.2 compilation xFlagsSet :: Options -> [GeneralFlag] diff --git a/compiler/damlc/daml-preprocessor/src/DA/Daml/Preprocessor.hs b/compiler/damlc/daml-preprocessor/src/DA/Daml/Preprocessor.hs index 03c5be2de2..e23f6677e9 100644 --- a/compiler/damlc/daml-preprocessor/src/DA/Daml/Preprocessor.hs +++ b/compiler/damlc/daml-preprocessor/src/DA/Daml/Preprocessor.hs @@ -14,11 +14,13 @@ import DA.Daml.Preprocessor.EnumType import Development.IDE.Types.Options import qualified "ghc-lib" GHC +import qualified "ghc-lib-parser" EnumSet as ES import qualified "ghc-lib-parser" SrcLoc as GHC import qualified "ghc-lib-parser" Module as GHC import qualified "ghc-lib-parser" RdrName as GHC import qualified "ghc-lib-parser" OccName as GHC import qualified "ghc-lib-parser" FastString as GHC +import qualified "ghc-lib-parser" GHC.LanguageExtensions.Type as GHC import Outputable import Control.Monad.Extra @@ -59,11 +61,11 @@ mayImportInternal = ] -- | Apply all necessary preprocessors -damlPreprocessor :: Maybe GHC.UnitId -> GHC.DynFlags -> GHC.ParsedSource -> IdePreprocessedSource -damlPreprocessor mbUnitId dflags x +damlPreprocessor :: ES.EnumSet GHC.Extension -> Maybe GHC.UnitId -> GHC.DynFlags -> GHC.ParsedSource -> IdePreprocessedSource +damlPreprocessor dataDependableExtensions mbUnitId dflags x | maybe False (isInternal ||^ (`elem` mayImportInternal)) name = noPreprocessor dflags x | otherwise = IdePreprocessedSource - { preprocWarnings = checkDamlHeader x ++ checkVariantUnitConstructors x + { preprocWarnings = checkDamlHeader x ++ checkVariantUnitConstructors x ++ checkLanguageExtensions dataDependableExtensions dflags x , preprocErrors = checkImports x ++ checkDataTypes x ++ checkModuleDefinition x ++ checkRecordConstructor x ++ checkModuleName x , preprocSource = recordDotPreprocessor $ importDamlPreprocessor $ genericsPreprocessor mbUnitId $ enumTypePreprocessor "GHC.Types" x } @@ -244,6 +246,26 @@ checkModuleDefinition x ] | otherwise = [] +checkLanguageExtensions :: ES.EnumSet GHC.Extension -> GHC.DynFlags -> GHC.ParsedSource -> [(GHC.SrcSpan, String)] +checkLanguageExtensions dataDependableExtensions dflags x = + let exts = ES.toList (GHC.extensionFlags dflags) + badExts = filter (\ext -> not (ext `ES.member` dataDependableExtensions)) exts + in + [ (modNameLoc, warning ext) | ext <- badExts ] + where + warning ext = unlines + [ "Modules compiled with the " ++ show ext ++ " language extension" + , "might not work properly with data-dependencies. This might stop the" + , "whole package from being extensible or upgradable using other versions" + , "of the SDK. Use this language extension at your own risk." + ] + -- NOTE(MH): Neither the `DynFlags` nor the `ParsedSource` contain + -- information about where a `{-# LANGUAGE ... #-}` pragma has been used. + -- In fact, there might not even be such a pragma if a `-X...` flag has been + -- used on the command line. Thus, we always put the warning at the location + -- of the module name. + modNameLoc = maybe GHC.noSrcSpan GHC.getLoc (GHC.hsmodName (GHC.unLoc x)) + -- Extract all data constructors with their locations universeConDecl :: GHC.ParsedSource -> [GHC.LConDecl GHC.GhcPs] -- equivalent to universeBi, but specialised to be faster diff --git a/compiler/damlc/tests/daml-test-files/BadExtensionOption.daml b/compiler/damlc/tests/daml-test-files/BadExtensionOption.daml new file mode 100644 index 0000000000..a2afb58914 --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/BadExtensionOption.daml @@ -0,0 +1,8 @@ +-- Test that we get a warning when using a problematic extension. +{-# OPTIONS_GHC -XPatternSynonyms #-} +-- @WARN Modules compiled with the PatternSynonyms language extension might not work properly with data-dependencies. +-- @INFO Use LANGUAGE pragmas + +module BadExtensionOption where + +pattern Nil = [] diff --git a/compiler/damlc/tests/daml-test-files/BadExtensionPragma.daml b/compiler/damlc/tests/daml-test-files/BadExtensionPragma.daml new file mode 100644 index 0000000000..6adcc3f7f2 --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/BadExtensionPragma.daml @@ -0,0 +1,7 @@ +-- Test that we get a warning when using a problematic extension. +{-# LANGUAGE PatternSynonyms #-} +-- @WARN Modules compiled with the PatternSynonyms language extension might not work properly with data-dependencies. + +module BadExtensionPragma where + +pattern Nil = [] diff --git a/compiler/damlc/tests/daml-test-files/ConstrainedClassMethod.daml b/compiler/damlc/tests/daml-test-files/ConstrainedClassMethod.daml index 2545f4e81c..6471c27606 100644 --- a/compiler/damlc/tests/daml-test-files/ConstrainedClassMethod.daml +++ b/compiler/damlc/tests/daml-test-files/ConstrainedClassMethod.daml @@ -1,4 +1,5 @@ {-# LANGUAGE ConstrainedClassMethods #-} +-- @WARN Modules compiled with the ConstrainedClassMethods language extension might not work properly with data-dependencies. -- | This module tests the case where a class method contains a constraint -- not present in the class itself. diff --git a/compiler/damlc/tests/daml-test-files/Constraint.daml b/compiler/damlc/tests/daml-test-files/Constraint.daml index 1713176b79..4881a7d6d4 100644 --- a/compiler/damlc/tests/daml-test-files/Constraint.daml +++ b/compiler/damlc/tests/daml-test-files/Constraint.daml @@ -2,7 +2,7 @@ -- All rights reserved. {-# LANGUAGE ExistentialQuantification #-} - +-- @WARN Modules compiled with the ExistentialQuantification language extension might not work properly with data-dependencies. module Constraint where diff --git a/compiler/damlc/tests/daml-test-files/Existential.daml b/compiler/damlc/tests/daml-test-files/Existential.daml index e0678edd74..b7567520f9 100644 --- a/compiler/damlc/tests/daml-test-files/Existential.daml +++ b/compiler/damlc/tests/daml-test-files/Existential.daml @@ -3,8 +3,8 @@ {-# LANGUAGE ExistentialQuantification #-} +-- @WARN Modules compiled with the ExistentialQuantification language extension might not work properly with data-dependencies. -- @ ERROR range=15:0-15:6; Pattern match with existential type. --- @ TODO Existential quantification module Existential where diff --git a/compiler/damlc/tests/daml-test-files/ExistentialSum.daml b/compiler/damlc/tests/daml-test-files/ExistentialSum.daml index 821af9bdb7..8c6c8d08d2 100644 --- a/compiler/damlc/tests/daml-test-files/ExistentialSum.daml +++ b/compiler/damlc/tests/daml-test-files/ExistentialSum.daml @@ -3,8 +3,8 @@ {-# LANGUAGE ExistentialQuantification #-} +-- @WARN Modules compiled with the ExistentialQuantification language extension might not work properly with data-dependencies. -- @ ERROR range=17:0-17:6; Pattern match with existential type. --- @ TODO Existential quantification module ExistentialSum where diff --git a/compiler/damlc/tests/daml-test-files/Generics.daml b/compiler/damlc/tests/daml-test-files/Generics.daml index 44f197c6b0..1d856b2e5e 100644 --- a/compiler/damlc/tests/daml-test-files/Generics.daml +++ b/compiler/damlc/tests/daml-test-files/Generics.daml @@ -2,7 +2,7 @@ -- SPDX-License-Identifier: Apache-2.0 {-# LANGUAGE EmptyCase #-} - +-- @WARN Modules compiled with the EmptyCase language extension might not work properly with data-dependencies. module Generics where diff --git a/compiler/damlc/tests/daml-test-files/GoodExtensionOption.daml b/compiler/damlc/tests/daml-test-files/GoodExtensionOption.daml new file mode 100644 index 0000000000..b14e5c5066 --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/GoodExtensionOption.daml @@ -0,0 +1,5 @@ +-- Test that we don't get a warning when using an unproblematic extension. +{-# OPTIONS_GHC -XPartialTypeSignatures #-} +-- @INFO Use LANGUAGE pragmas + +module GoodExtensionOption where diff --git a/compiler/damlc/tests/daml-test-files/GoodExtensionPragma.daml b/compiler/damlc/tests/daml-test-files/GoodExtensionPragma.daml new file mode 100644 index 0000000000..70c76f01f6 --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/GoodExtensionPragma.daml @@ -0,0 +1,4 @@ +-- Test that we don't get a warning when using an unproblematic extension. +{-# LANGUAGE PartialTypeSignatures #-} + +module GoodExtensionPragma where diff --git a/compiler/damlc/tests/daml-test-files/GoodExtensionUseless.daml b/compiler/damlc/tests/daml-test-files/GoodExtensionUseless.daml new file mode 100644 index 0000000000..484f2eefea --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/GoodExtensionUseless.daml @@ -0,0 +1,4 @@ +-- Test that we don't get a warning when using a default extension. +{-# LANGUAGE ImplicitPrelude #-} + +module GoodExtensionUseless where diff --git a/compiler/damlc/tests/daml-test-files/PatternSynonyms.daml b/compiler/damlc/tests/daml-test-files/PatternSynonyms.daml index 3a9f65792e..65c6c81293 100644 --- a/compiler/damlc/tests/daml-test-files/PatternSynonyms.daml +++ b/compiler/damlc/tests/daml-test-files/PatternSynonyms.daml @@ -2,6 +2,7 @@ -- All rights reserved. {-# LANGUAGE PatternSynonyms #-} +-- @WARN Modules compiled with the PatternSynonyms language extension might not work properly with data-dependencies. module PatternSynonyms where diff --git a/compiler/damlc/tests/daml-test-files/Records.daml b/compiler/damlc/tests/daml-test-files/Records.daml index a99bfb005e..c585be668c 100644 --- a/compiler/damlc/tests/daml-test-files/Records.daml +++ b/compiler/damlc/tests/daml-test-files/Records.daml @@ -2,7 +2,6 @@ -- All rights reserved. -- @ ERROR B's Company is run by B and they are 3 years old -{-# LANGUAGE FlexibleContexts #-} module Records where diff --git a/compiler/damlc/tests/daml-test-files/TypeFamily.daml b/compiler/damlc/tests/daml-test-files/TypeFamily.daml index 13a7b04c96..ca0daaecdb 100644 --- a/compiler/damlc/tests/daml-test-files/TypeFamily.daml +++ b/compiler/damlc/tests/daml-test-files/TypeFamily.daml @@ -2,9 +2,9 @@ -- All rights reserved. {-# LANGUAGE TypeFamilies #-} - -- @ERROR range=11:0-11:15; Data definition, of type type family. - +-- @WARN Modules compiled with the TypeFamilies language extension might not work properly with data-dependencies. +-- @WARN Modules compiled with the ExplicitNamespaces language extension might not work properly with data-dependencies. module TypeFamily where