Vladislav Zavialov pushed to branch wip/int-index/enforce-namespaces at Glasgow Haskell Compiler / GHC Commits: b313d5f4 by Vladislav Zavialov at 2025-05-08T01:34:50+03:00 use NonEmpty, add comments, test cases - - - - - 20 changed files: - compiler/GHC/Rename/Names.hs - compiler/GHC/Tc/Errors/Ppr.hs - compiler/GHC/Tc/Errors/Types.hs - + testsuite/tests/rename/should_compile/T25983a.hs - testsuite/tests/rename/should_compile/T25983.stderr → testsuite/tests/rename/should_compile/T25983a.stderr - + testsuite/tests/rename/should_compile/T25983b.hs - + testsuite/tests/rename/should_compile/T25983b.stderr - + testsuite/tests/rename/should_compile/T25983c.hs - + testsuite/tests/rename/should_compile/T25983c.stderr - testsuite/tests/rename/should_compile/T25983.hs → testsuite/tests/rename/should_compile/T25983d.hs - + testsuite/tests/rename/should_compile/T25983d.stderr - + testsuite/tests/rename/should_compile/T25983e.hs - + testsuite/tests/rename/should_compile/T25983e.stderr - + testsuite/tests/rename/should_compile/T25983f.hs - + testsuite/tests/rename/should_compile/T25983f.stderr - + testsuite/tests/rename/should_compile/T25983g.hs - + testsuite/tests/rename/should_compile/T25983g.stderr - testsuite/tests/rename/should_compile/T25984a.stderr - testsuite/tests/rename/should_compile/all.T - testsuite/tests/rename/should_fail/T9006.stderr Changes: ===================================== compiler/GHC/Rename/Names.hs ===================================== @@ -1268,7 +1268,7 @@ filterImports hsc_env iface decl_spec (Just (want_hiding, L l import_items)) warning_msg (BadImportW ie sub) = do -- 'BadImportW' is only constructed below in 'bad_import_w', in -- the 'EverythingBut' case, so here we assume a 'hiding' clause. - (reason : _) <- badImportItemErr iface decl_spec ie sub all_avails + (reason :| _) <- badImportItemErr iface decl_spec ie sub all_avails pure (TcRnDodgyImports (DodgyImportsHiding reason)) warning_msg (DeprecatedExport n w) = pure $ TcRnPragmaWarning @@ -1281,14 +1281,14 @@ filterImports hsc_env iface decl_spec (Just (want_hiding, L l import_items)) run_lookup def m = case m of Failed err -> do msgs <- lookup_err_msgs err - mapM_ addErr [TcRnImportLookup msg | msg <- msgs] + forM_ msgs $ \msg -> addErr (TcRnImportLookup msg) return def Succeeded a -> return a lookup_err_msgs err = case err of BadImport ie sub -> badImportItemErr iface decl_spec ie sub all_avails - IllegalImport -> pure [ImportLookupIllegal] - QualImportError rdr -> pure [ImportLookupQualified rdr] + IllegalImport -> pure $ NE.singleton ImportLookupIllegal + QualImportError rdr -> pure $ NE.singleton (ImportLookupQualified rdr) -- For each import item, we convert its RdrNames to Names, -- and at the same time compute all the GlobalRdrElt corresponding @@ -1642,7 +1642,7 @@ lookupChildren :: [GlobalRdrElt] -> ( [LookupChildError] -- The ones for which the lookup failed , [LocatedA GlobalRdrElt] ) -- (lookupChildren all_kids rdr_items) maps each rdr_item to its --- corresponding Name all_kids, if the former exists +-- corresponding Name in all_kids, if the former exists -- The matching is done by FastString, not OccName, so that -- Cls( meth, AssocTy ) -- will correctly find AssocTy among the all_kids of Cls, even though @@ -1652,20 +1652,34 @@ lookupChildren all_kids rdr_items = (fails, successes) where mb_xs = map do_one rdr_items fails = [ err | Failed err <- mb_xs ] - successes = [ ok | Succeeded oks <- mb_xs, ok <- oks ] + successes = [ ok | Succeeded oks <- mb_xs, ok <- NE.toList oks ] - do_one :: LIEWrappedName GhcPs -> MaybeErr LookupChildError [LocatedA GlobalRdrElt] + do_one :: LIEWrappedName GhcPs -> MaybeErr LookupChildError (NonEmpty (LocatedA GlobalRdrElt)) do_one item@(L l r) = case r of - IEName{} | notNull val_gs -> Succeeded [L l g | g <- val_gs] - IEName{} | notNull typ_gs -> Succeeded [L l g | g <- typ_gs] - IEType{} | notNull typ_gs -> Succeeded [L l g | g <- typ_gs] - IEType{} | (g:_) <- val_gs -> Failed $ LookupChildNonType item g - _ -> Failed $ LookupChildNotFound item + IEName{} + -- IEName (unadorned name) places no restriction on the namespace of + -- the imported entity, so we look in both `val_gres` and `typ_gres`. + -- In case of conflict (punning), the value namespace takes priority. + -- See Note [Prioritise the value namespace in subordinate import lists] + | (gre:gres) <- val_gres -> Succeeded $ fmap (L l) (gre:|gres) + | (gre:gres) <- typ_gres -> Succeeded $ fmap (L l) (gre:|gres) + | otherwise -> Failed $ LookupChildNotFound item + + IEType{} + -- IEType ('type' namespace specifier) restricts the lookup to the + -- type namespace, i.e. to `typ_gres`. In case of failure, we check + -- `val_gres` to produce a more helpful error message. + | (gre:gres) <- typ_gres -> Succeeded $ fmap (L l) (gre:|gres) + | (gre:_) <- val_gres -> Failed $ LookupChildNonType item gre + | otherwise -> Failed $ LookupChildNotFound item + + IEPattern{} -> panic "lookupChildren: IEPattern" -- Never happens (invalid syntax) + IEDefault{} -> panic "lookupChildren: IEDefault" -- Never happens (invalid syntax) where fs = (occNameFS . rdrNameOcc . ieWrappedName) r - gs = fromMaybe [] (lookupFsEnv kid_env fs) - (val_gs, typ_gs) = partition (isValNameSpace . greNameSpace) gs + gres = fromMaybe [] (lookupFsEnv kid_env fs) + (val_gres, typ_gres) = partition (isValNameSpace . greNameSpace) gres -- See Note [Children for duplicate record fields] kid_env :: FastStringEnv [GlobalRdrElt] @@ -1673,6 +1687,38 @@ lookupChildren all_kids rdr_items = (fails, successes) [(occNameFS (occName x), [x]) | x <- all_kids] +{- Note [Prioritise the value namespace in subordinate import lists] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Consider this program that defines a class that has both an associated type +named (#) and a method named (#) + + module M_assoc where + class C a b where + type a # b + (#) :: a -> b -> () + module N_assoc where + import M_assoc( C((#)) ) + +In the import declaration, when we see the unadorned name (#) in the subordinate +import list of C, which children should we bring into scope? Our options are: + + a) only the method (#) + b) only the associated type (#) + c) both the method and the associated type + +To follow the precedent established by top-level items, we go with option (a). +Indeed, consider a slightly different program + + module M_top where + type family a # b + a # b = () + module N_top where + import M_top( (#) ) + +Here the import brings only the function (#) into scope, and one has to say +`type (#)` to get the type family. +-} + ------------------------------- {- @@ -2331,27 +2377,31 @@ badImportItemErr :: ModIface -> ImpDeclSpec -> IE GhcPs -> IsSubordinateError -> [AvailInfo] - -> TcRn [ImportLookupReason] -- non-empty + -> TcRn (NonEmpty ImportLookupReason) badImportItemErr iface decl_spec ie sub avails = do patsyns_enabled <- xoptM LangExt.PatternSynonyms expl_ns_enabled <- xoptM LangExt.ExplicitNamespaces + let import_lookup_bad :: BadImportKind -> ImportLookupReason + import_lookup_bad k = ImportLookupBad k iface decl_spec ie patsyns_enabled dflags <- getDynFlags hsc_env <- getTopEnv let rdr_env = mkGlobalRdrEnv $ gresFromAvails hsc_env (Just imp_spec) all_avails - pure [ImportLookupBad k iface decl_spec ie patsyns_enabled | k <- importErrorKind dflags rdr_env expl_ns_enabled ] + pure $ fmap import_lookup_bad (importErrorKind dflags rdr_env expl_ns_enabled) where + importErrorKind :: DynFlags -> GlobalRdrEnv -> Bool -> NonEmpty BadImportKind importErrorKind dflags rdr_env expl_ns_enabled | any checkIfTyCon avails = case sub of - IsNotSubordinate -> [BadImportAvailTyCon expl_ns_enabled] + IsNotSubordinate -> NE.singleton (BadImportAvailTyCon expl_ns_enabled) IsSubordinateError { subordinate_err_parent = gre , subordinate_err_unavailable = unavailable , subordinate_err_nontype = nontype } - -> [BadImportNotExportedSubordinates gre unavailable | notNull unavailable] ++ - [BadImportNonTypeSubordinates gre nontype | notNull nontype ] - | any checkIfVarName avails = [BadImportAvailVar] - | Just con <- find checkIfDataCon avails = [BadImportAvailDataCon (availOccName con)] - | otherwise = [BadImportNotExported suggs] + -> NE.fromList $ catMaybes $ + [ fmap (BadImportNotExportedSubordinates gre) (NE.nonEmpty unavailable) + , fmap (BadImportNonTypeSubordinates gre) (NE.nonEmpty nontype) ] + | any checkIfVarName avails = NE.singleton BadImportAvailVar + | Just con <- find checkIfDataCon avails = NE.singleton (BadImportAvailDataCon (availOccName con)) + | otherwise = NE.singleton (BadImportNotExported suggs) where suggs = similar_suggs ++ fieldSelectorSuggestions rdr_env rdr what_look = case sub of ===================================== compiler/GHC/Tc/Errors/Ppr.hs ===================================== @@ -119,6 +119,7 @@ import GHC.Data.List.SetOps ( nubOrdBy ) import GHC.Data.Maybe import GHC.Data.Pair import GHC.Settings.Constants (mAX_TUPLE_SIZE, mAX_CTUPLE_SIZE) +import GHC.Utils.Lexeme import GHC.Utils.Misc import GHC.Utils.Outputable import GHC.Utils.Panic @@ -5886,24 +5887,35 @@ pprImportLookup = \case where tycon_occ = rdrNameOcc $ ieName ie tycon = parenSymOcc tycon_occ (ppr tycon_occ) - BadImportNotExportedSubordinates gre unavailable -> + BadImportNotExportedSubordinates gre unavailable1 -> withContext [ what <+> text "called" <+> parent_name <+> text "is exported, but it does not export" , text "any" <+> what_children <+> text "called" <+> unavailable_names <> dot ] where + unavailable = NE.toList unavailable1 parent_name = (quotes . pprPrefixOcc . nameOccName . gre_name) gre unavailable_names = pprWithCommas (quotes . ppr) unavailable - (what, what_children) = case greInfo gre of - IAmTyCon ClassFlavour -> (text "a class", text "class methods or associated types") - IAmTyCon _ -> (text "a data type", text "constructors or record fields") - _ -> (text "an item", text "children") - BadImportNonTypeSubordinates gre nontype -> + any_names p = any (p . unpackFS) unavailable + what = case greInfo gre of + IAmTyCon ClassFlavour -> text "a class" + IAmTyCon _ -> text "a data type" + _ -> text "an item" + what_children = unquotedListWith "or" $ case greInfo gre of + IAmTyCon ClassFlavour -> + [text "class methods" | any_names okVarOcc ] ++ + [text "associated types" | any_names okTcOcc ] + IAmTyCon _ -> + [text "constructors" | any_names okConOcc ] ++ + [text "record fields" | any_names okVarOcc ] + _ -> [text "children"] + BadImportNonTypeSubordinates gre nontype1 -> withContext [ what <+> text "called" <+> parent_name <+> text "is exported," , sep [ text "but its subordinate" <+> "item" <> plural nontype <+> nontype_names , isOrAre nontype <+> "not in the type namespace." ] ] where + nontype = NE.toList nontype1 parent_name = (quotes . pprPrefixOcc . nameOccName . gre_name) gre nontype_names = pprWithCommas (quotes . pprPrefixOcc . nameOccName . gre_name) nontype what = case greInfo gre of ===================================== compiler/GHC/Tc/Errors/Types.hs ===================================== @@ -5837,9 +5837,9 @@ data BadImportKind -- @import Data.Maybe (Just)@ instead of @import Data.Maybe (Maybe(Just))@ | BadImportAvailDataCon OccName -- | The parent does not export the given children. - | BadImportNotExportedSubordinates !GlobalRdrElt [FastString] + | BadImportNotExportedSubordinates !GlobalRdrElt (NonEmpty FastString) -- | Incorrect @type@ keyword when importing subordinates that aren't types. - | BadImportNonTypeSubordinates !GlobalRdrElt [GlobalRdrElt] + | BadImportNonTypeSubordinates !GlobalRdrElt (NonEmpty GlobalRdrElt) -- | Incorrect @type@ keyword when importing something which isn't a type. | BadImportAvailVar deriving Generic ===================================== testsuite/tests/rename/should_compile/T25983a.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983a where + +import Prelude hiding (Bool(X,Y)) ===================================== testsuite/tests/rename/should_compile/T25983.stderr → testsuite/tests/rename/should_compile/T25983a.stderr ===================================== @@ -1,5 +1,5 @@ -T25983.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] +T25983a.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] In the import of ‘Prelude’: a data type called ‘Bool’ is exported, but it does not export - any constructors or record fields called ‘X’. + any constructors called ‘X’, ‘Y’. ===================================== testsuite/tests/rename/should_compile/T25983b.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983b where + +import Prelude hiding (Bool(fld1,fld2)) ===================================== testsuite/tests/rename/should_compile/T25983b.stderr ===================================== @@ -0,0 +1,5 @@ +T25983b.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a data type called ‘Bool’ is exported, but it does not export + any record fields called ‘fld1’, ‘fld2’. + ===================================== testsuite/tests/rename/should_compile/T25983c.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983c where + +import Prelude hiding (Bool(X,Y,fld1,fld2)) ===================================== testsuite/tests/rename/should_compile/T25983c.stderr ===================================== @@ -0,0 +1,5 @@ +T25983c.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a data type called ‘Bool’ is exported, but it does not export + any constructors or record fields called ‘X’, ‘Y’, ‘fld1’, ‘fld2’. + ===================================== testsuite/tests/rename/should_compile/T25983.hs → testsuite/tests/rename/should_compile/T25983d.hs ===================================== @@ -1,5 +1,5 @@ {-# OPTIONS -Wdodgy-imports #-} -module T25983 where +module T25983d where -import Prelude hiding (Bool(X)) +import Prelude hiding (Num((#))) ===================================== testsuite/tests/rename/should_compile/T25983d.stderr ===================================== @@ -0,0 +1,5 @@ +T25983d.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a class called ‘Num’ is exported, but it does not export + any class methods or associated types called ‘#’. + ===================================== testsuite/tests/rename/should_compile/T25983e.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983e where + +import Prelude hiding (Num(f,g)) ===================================== testsuite/tests/rename/should_compile/T25983e.stderr ===================================== @@ -0,0 +1,5 @@ +T25983e.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a class called ‘Num’ is exported, but it does not export + any class methods called ‘f’, ‘g’. + ===================================== testsuite/tests/rename/should_compile/T25983f.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983f where + +import Prelude hiding (Num(F,G)) ===================================== testsuite/tests/rename/should_compile/T25983f.stderr ===================================== @@ -0,0 +1,5 @@ +T25983f.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a class called ‘Num’ is exported, but it does not export + any associated types called ‘F’, ‘G’. + ===================================== testsuite/tests/rename/should_compile/T25983g.hs ===================================== @@ -0,0 +1,5 @@ +{-# OPTIONS -Wdodgy-imports #-} + +module T25983g where + +import Prelude hiding (Num(F,G,f,g)) ===================================== testsuite/tests/rename/should_compile/T25983g.stderr ===================================== @@ -0,0 +1,5 @@ +T25983g.hs:5:24: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] + In the import of ‘Prelude’: + a class called ‘Num’ is exported, but it does not export + any class methods or associated types called ‘F’, ‘G’, ‘f’, ‘g’. + ===================================== testsuite/tests/rename/should_compile/T25984a.stderr ===================================== @@ -1,5 +1,5 @@ T25984a.hs:5:31: warning: [GHC-10237] [-Wdodgy-imports (in -Wextra)] In the import of ‘T25984a_helper’: a data type called ‘H’ is exported, but it does not export - any constructors or record fields called ‘C’. + any constructors called ‘C’. ===================================== testsuite/tests/rename/should_compile/all.T ===================================== @@ -233,5 +233,11 @@ test('T25182', [extra_files(['ReExportTuples.hs'])], multimod_compile, ['T25182' test('T22581c', [extra_files(['T22581c_helper.hs'])], multimod_compile, ['T22581c', '-v0']) test('T22581d', combined_output, ghci_script, ['T22581d.script']) test('T25991a', [extra_files(['T25991a_helper.hs'])], multimod_compile, ['T25991a', '-v0']) -test('T25983', normal, compile, ['']) +test('T25983a', normal, compile, ['']) +test('T25983b', normal, compile, ['']) +test('T25983c', normal, compile, ['']) +test('T25983d', normal, compile, ['']) +test('T25983e', normal, compile, ['']) +test('T25983f', normal, compile, ['']) +test('T25983g', normal, compile, ['']) test('T25984a', [extra_files(['T25984a_helper.hs'])], multimod_compile, ['T25984a', '-v0']) ===================================== testsuite/tests/rename/should_fail/T9006.stderr ===================================== @@ -1,5 +1,5 @@ T9006.hs:3:16: error: [GHC-10237] In the import of ‘T9006a’: a data type called ‘T’ is exported, but it does not export - any constructors or record fields called ‘T’. + any constructors called ‘T’. View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/b313d5f4d1aba23af64be3fd3f34139e... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/b313d5f4d1aba23af64be3fd3f34139e... You're receiving this email because of your account on gitlab.haskell.org.