Implement -Wambiguous-fields

Fixes #18966. Adds a new warning -Wambiguous-fields for uses of field selectors
or record updates that will be rejected in the future, when the DuplicateRecordFields
extension is simplified per https://github.com/ghc-proposals/ghc-proposals/pull/366.

13 files changed, 140 insertions, 9 deletions

diff --git a/compiler/GHC/Driver/Flags.hs b/compiler/GHC/Driver/Flags.hs
index 3d0908caa0..7867d4bd04 100644
--- a/compiler/GHC/Driver/Flags.hs
+++ b/compiler/GHC/Driver/Flags.hs
@@ -505,6 +505,7 @@ data WarningFlag =
    | Opt_WarnInvalidHaddock               -- Since 8.12
    | Opt_WarnOperatorWhitespaceExtConflict  -- Since 9.2
    | Opt_WarnOperatorWhitespace             -- Since 9.2
+   | Opt_WarnAmbiguousFields                -- Since 9.2
    deriving (Eq, Show, Enum)
 
 -- | Used when outputting warnings: if a reason is given, it is
diff --git a/compiler/GHC/Driver/Session.hs b/compiler/GHC/Driver/Session.hs
index 7afcf7309c..a0a6153b0b 100644
--- a/compiler/GHC/Driver/Session.hs
+++ b/compiler/GHC/Driver/Session.hs
@@ -3055,6 +3055,7 @@ wWarningFlagsDeps = [
 -- Please keep the list of flags below sorted alphabetically
   flagSpec "alternative-layout-rule-transitional"
                                       Opt_WarnAlternativeLayoutRuleTransitional,
+  flagSpec "ambiguous-fields"            Opt_WarnAmbiguousFields,
   depFlagSpec "auto-orphans"             Opt_WarnAutoOrphans
     "it has no effect",
   flagSpec "cpp-undef"                   Opt_WarnCPPUndef,
@@ -3909,6 +3910,7 @@ standardWarnings -- see Note [Documenting warning flags]
         Opt_WarnDerivingDefaults,
         Opt_WarnOverflowedLiterals,
         Opt_WarnEmptyEnumerations,
+        Opt_WarnAmbiguousFields,
         Opt_WarnMissingFields,
         Opt_WarnMissingMethods,
         Opt_WarnWrongDoBind,
diff --git a/compiler/GHC/Tc/Gen/Expr.hs b/compiler/GHC/Tc/Gen/Expr.hs
index 8ad1e59796..dfb9d6abd9 100644
--- a/compiler/GHC/Tc/Gen/Expr.hs
+++ b/compiler/GHC/Tc/Gen/Expr.hs
@@ -1216,13 +1216,16 @@ disambiguateRecordBinds record_expr record_rho rbnds res_ty
 
         -- Multiple possible parents: try harder to disambiguate
         -- Can we get a parent TyCon from the pushed-in type?
-        _:_ | Just p <- tyConOfET fam_inst_envs res_ty -> return (RecSelData p)
+        _:_ | Just p <- tyConOfET fam_inst_envs res_ty ->
+              do { reportAmbiguousField p
+                 ; return (RecSelData p) }
 
         -- Does the expression being updated have a type signature?
         -- If so, try to extract a parent TyCon from it
             | Just {} <- obviousSig (unLoc record_expr)
             , Just tc <- tyConOf fam_inst_envs record_rho
-            -> return (RecSelData tc)
+            -> do { reportAmbiguousField tc
+                  ; return (RecSelData tc) }
 
         -- Nothing else we can try...
         _ -> failWithTc badOverloadedUpdate
@@ -1263,6 +1266,18 @@ disambiguateRecordBinds record_expr record_rho rbnds res_ty
            ; return $ L l upd { hsRecFieldLbl
                                   = L loc (Unambiguous i (L loc lbl)) } }
 
+    -- See Note [Deprecating ambiguous fields] in GHC.Tc.Gen.Head
+    reportAmbiguousField :: TyCon -> TcM ()
+    reportAmbiguousField parent_type =
+        setSrcSpan loc $ warnIfFlag Opt_WarnAmbiguousFields True $
+          vcat [ text "The record update" <+> ppr rupd
+                   <+> text "with type" <+> ppr parent_type
+                   <+> text "is ambiguous."
+               , text "This will not be supported by -XDuplicateRecordFields in future releases of GHC."
+               ]
+      where
+        rupd = RecordUpd { rupd_expr = record_expr, rupd_flds = rbnds, rupd_ext = noExtField }
+        loc  = getLoc (head rbnds)
 
 {-
 Game plan for record bindings
diff --git a/compiler/GHC/Tc/Gen/Head.hs b/compiler/GHC/Tc/Gen/Head.hs
index 7dc993d8cc..faf5e0f2f2 100644
--- a/compiler/GHC/Tc/Gen/Head.hs
+++ b/compiler/GHC/Tc/Gen/Head.hs
@@ -435,8 +435,25 @@ add_head_ctxt fun args thing_inside
 *                                                                      *
 ********************************************************************* -}
 
-{- Note [Disambiguating record fields]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+{-
+Note [Deprecating ambiguous fields]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+In the future, the -XDuplicateRecordFields extension will no longer support
+disambiguating record fields during type-checking (as described in Note
+[Disambiguating record fields]).  For now, the -Wambiguous-fields option will
+emit a warning whenever an ambiguous field is resolved using type information.
+In a subsequent GHC release, this functionality will be removed and the warning
+will turn into an ambiguity error in the renamer.
+
+For background information, see GHC proposal #366
+(https://github.com/ghc-proposals/ghc-proposals/pull/366).
+
+
+Note [Disambiguating record fields]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+NB. The following is going to be removed: see
+Note [Deprecating ambiguous fields].
+
 When the -XDuplicateRecordFields extension is used, and the renamer
 encounters a record selector or update that it cannot immediately
 disambiguate (because it involves fields that belong to multiple
@@ -601,6 +618,16 @@ finish_ambiguous_selector lr@(L _ rdr) parent_type
     -- See Note [Unused name reporting and HasField] in GHC.Tc.Instance.Class
     do { addUsedGRE True gre
        ; keepAlive (greMangledName gre)
+         -- See Note [Deprecating ambiguous fields]
+       ; warnIfFlag Opt_WarnAmbiguousFields True $
+          vcat [ text "The field" <+> quotes (ppr rdr)
+                   <+> text "belonging to type" <+> ppr parent_type
+                   <+> text "is ambiguous."
+               , text "This will not be supported by -XDuplicateRecordFields in future releases of GHC."
+               , if isLocalGRE gre
+                 then text "You can use explicit case analysis to resolve the ambiguity."
+                 else text "You can use a qualified import or explicit case analysis to resolve the ambiguity."
+               ]
        ; return (greMangledName gre) } } } } }
 
 -- This field name really is ambiguous, so add a suitable "ambiguous
diff --git a/docs/users_guide/exts/duplicate_record_fields.rst b/docs/users_guide/exts/duplicate_record_fields.rst
index d8abedaefa..8f0ace158b 100644
--- a/docs/users_guide/exts/duplicate_record_fields.rst
+++ b/docs/users_guide/exts/duplicate_record_fields.rst
@@ -45,6 +45,13 @@ is still not allowed if both ``S(x)`` and ``T(x)`` are in scope: ::
 
     bad r = x r
 
+**Warning**: the type-based disambiguation rules described in the remainder of
+this section are being removed (see the proposal `DuplicateRecordFields without
+ambiguous field access
+<https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0366-no-ambiguous-field-access.rst>`_).
+The :ghc-flag:`-Wambiguous-fields` option will warn about code that relies on
+these rules. In a future GHC release, such code will produce ambiguity errors.
+
 An ambiguous selector may be disambiguated by the type being "pushed down" to
 the occurrence of the selector (see :ref:`higher-rank-type-inference` for more
 details on what "pushed down" means). For example, the following are permitted: ::
@@ -86,14 +93,21 @@ definitions: ::
 
 Without :extension:`DuplicateRecordFields`, an update mentioning ``foo`` will always be
 ambiguous if all these definitions were in scope. When the extension is enabled,
-there are several options for disambiguating updates:
-
-- Check for types that have all the fields being updated. For example: ::
+and there is exactly one type that has all the fields being updated, that type will be used.
+For example: ::
 
       f x = x { foo = 3, bar = 2 }
 
-  Here ``f`` must be updating ``T`` because neither ``S`` nor ``U`` have both
-  fields.
+Here ``f`` must be updating ``T`` because neither ``S`` nor ``U`` have both
+fields.
+
+If there are multiple types with all the fields, type information may be used to
+disambiguate which record type is meant. **Warning**: the following rules are
+being removed (see the proposal `DuplicateRecordFields without ambiguous field
+access
+<https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0366-no-ambiguous-field-access.rst>`_).
+The :ghc-flag:`-Wambiguous-fields` option will warn about code that relies on
+these rules. In a future GHC release, such code will produce ambiguity errors.
 
 - Use the type being pushed in to the record update, as in the following: ::
 
diff --git a/docs/users_guide/using-warnings.rst b/docs/users_guide/using-warnings.rst
index 5c2c9b66e1..a9995268ea 100644
--- a/docs/users_guide/using-warnings.rst
+++ b/docs/users_guide/using-warnings.rst
@@ -52,6 +52,7 @@ To reverse ``-Werror``, which makes all warnings into errors, use ``-Wwarn``.
         * :ghc-flag:`-Winaccessible-code`
         * :ghc-flag:`-Wstar-binder`
         * :ghc-flag:`-Woperator-whitespace-ext-conflict`
+        * :ghc-flag:`-Wambiguous-fields`
 
 The following flags are simple ways to select standard "packages" of warnings:
 
@@ -1947,6 +1948,25 @@ of ``-W(no-)*``.
     Since GHC 7.10, ``Typeable`` is automatically derived for all types.
     Thus, deriving ``Typeable`` yourself is redundant.
 
+.. ghc-flag:: -Wambiguous-fields
+    :shortdesc: warn about ambiguous field selectors or updates
+    :type: dynamic
+    :category:
+
+    :since: 9.2
+
+    When :extension:`DuplicateRecordFields` is enabled, the option
+    :ghc-flag:`-Wambiguous-fields` warns about occurrences of fields in
+    selectors or updates that depend on the deprecated mechanism for
+    type-directed disambiguation.  This mechanism will be removed in a future
+    GHC release, at which point these occurrences will be rejected as ambiguous.
+    See the proposal `DuplicateRecordFields without ambiguous field access
+    <https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0366-no-ambiguous-field-access.rst>`_
+    and the documentation on :extension:`DuplicateRecordFields` for further details.
+
+    This warning has no effect when :extension:`DuplicateRecordFields` is
+    disabled.
+
 If you're feeling really paranoid, the :ghc-flag:`-dcore-lint` option is a good choice.
 It turns on heavyweight intra-pass sanity-checking within GHC. (It checks GHC's
 sanity, not yours.)
diff --git a/testsuite/tests/backpack/should_compile/T13323.stderr b/testsuite/tests/backpack/should_compile/T13323.stderr
index eb49bcbfab..7e637d9dd4 100644
--- a/testsuite/tests/backpack/should_compile/T13323.stderr
+++ b/testsuite/tests/backpack/should_compile/T13323.stderr
@@ -1,6 +1,11 @@
 [1 of 3] Processing p
   [1 of 2] Compiling A[sig]           ( p/A.hsig, nothing )
   [2 of 2] Compiling P                ( p/P.hs, nothing )
+
+T13323.bkp:9:13: warning: [-Wambiguous-fields (in -Wdefault)]
+    The field ‘foo’ belonging to type A is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use a qualified import or explicit case analysis to resolve the ambiguity.
 [2 of 3] Processing q
   Instantiating q
   [1 of 1] Compiling A                ( q/A.hs, T13323.out/q/A.o )
@@ -10,5 +15,10 @@
     Instantiating p[A=q:A]
     [1 of 2] Compiling A[sig]           ( p/A.hsig, T13323.out/p/p-HVmFlcYSefiK5n1aDP1v7x/A.o )
     [2 of 2] Compiling P                ( p/P.hs, T13323.out/p/p-HVmFlcYSefiK5n1aDP1v7x/P.o )
+
+T13323.bkp:9:13: warning: [-Wambiguous-fields (in -Wdefault)]
+    The field ‘foo’ belonging to type A is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use a qualified import or explicit case analysis to resolve the ambiguity.
   [1 of 2] Compiling R                ( r/R.hs, T13323.out/r/R.o )
   [2 of 2] Instantiating p
diff --git a/testsuite/tests/overloadedrecflds/should_fail/DRFUnused.stderr b/testsuite/tests/overloadedrecflds/should_fail/DRFUnused.stderr
index a9dbd2cdd5..7e75f5c8c7 100644
--- a/testsuite/tests/overloadedrecflds/should_fail/DRFUnused.stderr
+++ b/testsuite/tests/overloadedrecflds/should_fail/DRFUnused.stderr
@@ -1,3 +1,8 @@
 
 DRFUnused.hs:10:16: error: [-Wunused-top-binds (in -Wextra, -Wunused-binds), -Werror=unused-top-binds]
     Defined but not used: ‘foo’
+
+DRFUnused.hs:18:5: warning: [-Wambiguous-fields (in -Wdefault)]
+    The field ‘foo’ belonging to type U is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use explicit case analysis to resolve the ambiguity.
diff --git a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail06.stderr b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail06.stderr
index 3aae5c5061..254931a9bc 100644
--- a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail06.stderr
+++ b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail06.stderr
@@ -26,3 +26,11 @@ overloadedrecfldsfail06.hs:9:1: error: [-Wunused-imports (in -Wextra), -Werror=u
 overloadedrecfldsfail06.hs:10:1: error: [-Wunused-imports (in -Wextra), -Werror=unused-imports]
     The qualified import of ‘U(x), U’
     from module ‘OverloadedRecFldsFail06_A’ is redundant
+
+overloadedrecfldsfail06.hs:15:22: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
+    The record update u {x = True} with type U is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+
+overloadedrecfldsfail06.hs:18:28: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
+    The record update v {P.x = 3} with type V is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
diff --git a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail11.stderr b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail11.stderr
index 0aa41a2962..9599cddcba 100644
--- a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail11.stderr
+++ b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail11.stderr
@@ -4,3 +4,8 @@
 overloadedrecfldsfail11.hs:5:15: error: [-Wdeprecations (in -Wdefault), -Werror=deprecations]
     In the use of ‘foo’ (imported from OverloadedRecFldsFail11_A):
     "Warning on a record field"
+
+overloadedrecfldsfail11.hs:5:15: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
+    The field ‘foo’ belonging to type S is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use a qualified import or explicit case analysis to resolve the ambiguity.
diff --git a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.hs b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.hs
index 56092b6ce0..270c43e5d4 100644
--- a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.hs
+++ b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.hs
@@ -12,4 +12,7 @@ f e = e { foo = 3, bar = 3 }
 s :: T -> Int
 s = foo
 
+t :: S -> Bool
+t = foo
+
 main = return ()
diff --git a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.stderr b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.stderr
index e17c9f8573..fe8ac81ef9 100644
--- a/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.stderr
+++ b/testsuite/tests/overloadedrecflds/should_fail/overloadedrecfldsfail12.stderr
@@ -12,3 +12,13 @@ overloadedrecfldsfail12.hs:10:20: error: [-Wdeprecations (in -Wdefault), -Werror
 overloadedrecfldsfail12.hs:13:5: error: [-Wdeprecations (in -Wdefault), -Werror=deprecations]
     In the use of ‘foo’ (imported from OverloadedRecFldsFail12_A):
     "Deprecated foo"
+
+overloadedrecfldsfail12.hs:13:5: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
+    The field ‘foo’ belonging to type T is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use a qualified import or explicit case analysis to resolve the ambiguity.
+
+overloadedrecfldsfail12.hs:16:5: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
+    The field ‘foo’ belonging to type S is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use explicit case analysis to resolve the ambiguity.
diff --git a/testsuite/tests/rename/should_compile/T11167_ambig.stderr b/testsuite/tests/rename/should_compile/T11167_ambig.stderr
new file mode 100644
index 0000000000..5320b42149
--- /dev/null
+++ b/testsuite/tests/rename/should_compile/T11167_ambig.stderr
@@ -0,0 +1,11 @@
+
+T11167_ambig.hs:10:13: warning: [-Wambiguous-fields (in -Wdefault)]
+    The field ‘runContT’ belonging to type ContT r m a is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use explicit case analysis to resolve the ambiguity.
+
+T11167_ambig.hs:17:9: warning: [-Wambiguous-fields (in -Wdefault)]
+    The field ‘runContT’ belonging to type forall a.
+                                           ContT () IO a is ambiguous.
+    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
+    You can use explicit case analysis to resolve the ambiguity.