What are you supposed to do about -Wambiguous-fields?

You don’t necessarily have to put each type in a different module, because you can do something like

import qualified Mod (Foo(..)) as Foo
import qualified Mod (Bar(..)) as Bar

and use Foo.foo and Bar.foo unambiguously. See here:

3 Likes

Ah ha! Okay.

In my case, I have two internal helper types that share a record field name.

I structured the types this way because with

  • DuplicateRecordFields
  • NoFieldSelectors
  • OverloadedRecordDot

the getter story is actually rather nice! foo.bar syntax just works, and I don’t have to worry about bar polluting the namespace.

… but it seems updating is still somewhat fraught! So, perhaps generic-lens is the cleanest solution for this problem today.

Also be aware of proposal #537, which is still open. I wouldn’t expect to see type-directed disambiguation go away for some time, because there are still a few things to work out.

No, optics is. I’ve explained why way too many times though, time to give up I guess. Enjoy shitty long compilation times and bad error messages.

EDIT: @mitchellwrosen btw, nothing personal man, I’m just frustrated because:

  1. A long time ago I tried to improve generic-lens-core etc. and my PRs were ignored because the repo was abandoned.
  2. I optimized the shit out of generic optics wrt. compilation times, runtime efficiency and user experience and integrated them into optics proper.
  3. I wrote a lot of documentation in optics for this.
  4. Over the years in multiple places I explained that optics has generic optics integrated and they behave better than generic-lens on all fronts.
  5. People still mention generic-lens most of the time as their first choice when it comes to this.

So you just triggered the bomb. Now however I can say that I learned my lesson and I’m officially done with the point (4).

7 Likes

Okay, good to know!

To be quite honest (as you can probably tell by this thread) my general Haskell dependency and language knowledge is probably 5-10 years out of date. So I can’t help but feel you have taken the wrong lesson here regarding your point (4)… you’ve got at least one new (potential) user as of today!

4 Likes

I thought we were supposed to migrate to OverloadedRecordUpdate. At least, that’s what I was planning on doing. What’s wrong with this? It seems much better than module-qualifying field names.

{-# LANGUAGE GHC2021 #-}
{-# LANGUAGE AllowAmbiguousTypes #-}
{-# LANGUAGE DuplicateRecordFields #-}
{-# LANGUAGE OverloadedRecordUpdate #-}
{-# LANGUAGE RebindableSyntax #-}
{-# LANGUAGE FunctionalDependencies #-}

import Prelude

data Foo = Foo { foo :: Int }
data Bar = Bar { foo :: Int }

-- Of course this should ultimately be shipped with base, or at least
-- a library that comes with GHC.  So far we only have
--
-- https://www.stackage.org/haddock/lts-22.21/base-4.18.2.1/GHC-Records.html#t:HasField
class GetSetField fieldName t fieldType | t fieldName -> fieldType where
  getField :: t -> fieldType
  setField :: t -> fieldType -> t

-- Inferred type:
--
-- blah :: (GetSetField "foo" t fieldType, Num fieldType) => t -> t
--
-- (AllowAmbiguousTypes is needed for this type)
blah x = x { foo = 5 }

Please don’t give up! This is the first time I’ve heard that optics supports this functionality. I suspect a lot more people need to hear the message!

4 Likes

Great to see this question asked here as well. I’m working on a rather large code base that uses DuplicateRecordFields.

We’re getting warnings about ambiguous record fields now and we’re largely just disabling these. I know this is footgun because this is going to be an error but I really don’t know a way forward here.

Moreover, it is extremely unintuitive; The approach of disambiguating field names by type seems completely sensible to me in a language like Haskell, I would even go as far and say this is something where types are really unambiguously good and unproblematic, I know that the reason of introducing this is a technical reason, however, I don’t see how such a reason can be the cause of such an unintuitive and (I think it will be) disruptive change.

What I expect at least is a good way forward. Importing the own module qualified is not something I would count as a good way forward.

In retrospect I would’ve been happier if DuplicateRecordField was either never introduced or the type based disambiguating was kept.

For the future I hope there will be a real solution as soon as this becomes an error.

3 Likes

Them’s fighting words :smiley: Type-directed name resolution is controversial in languages with type inference because it generates an ugly chicken-egg problem: In order to generate constraints to find out the types of the program you have to know what the types of the identifiers (in this case, the record selectors) in your program are, but with type-directed name resolution you first have to know the types in order to find out what the identifier stands for, and what type signature it has. The old mechanism used an ad-hoc way of solving this problem; the main advantage of RecordDot syntax, in my opinion, is that it uses typeclasses for the name-resolution part, which is much better integrated into the type inference of GHC.

So I absolutely agree that we should use the types to infer the right record accessors, but we should use the better mechanism for it that GHC provides, i.e. typeclasses and the HasField typeclass.

3 Likes

that would be great, I hope that this mechanism will arrive in time before we remove support for ambiguous record fields. Otherwise I suspect that would cause a lot of trouble.

1 Like

I already completely switched several projects to record dot syntax for accessing record fields, together with NoFieldSelectors, and I really liked it. In my opinion it also makes the code more readable, because function application and field access are more visually distinct, but on that point opinions may vary. As soon as record update is properly stabilized I will no longer look back on the old record syntax and exclusively use the new one :slight_smile: But we should of course keep the old mechanism available for as long as necessary, or even indefinitely.

1 Like

But doesn’t the replacement already exist? That is, OverloadedRecordDot and OverloadedRecordUpdate? I still don’t understand what people think that they are lacking. See also What are you supposed to do about -Wambiguous-fields? - #8 by tomjaguarpaw.

EDIT: Corrected extension names

I didn’t know RecordUpdateSyntax, where would I be able to find information about it? Doesn’t look like it’s in GHC 9.10. (RebindableSyntax is not an option in the general case)

It also appears that the compiler doesn’t generated setField yet, does it?

https://ghc.gitlab.haskell.org/ghc/doc/users_guide/exts/overloaded_record_update.html

What I meant is the OverloadedRecordUpdate extension, which is still marked as experimental in the user guide.

Sorry, I got the extension names wrong: they should be OverloadedRecordDot and OverloadedRecordUpdate.

Right, and we have the guarantee that ambiguous field selector/update usage will not be removed until this replacement (or some other) stabilises:

In a subsequent GHC release, remove support for ambiguous field selector/update occurrences entirely and remove the warning. This step should not be taken until RecordDotSyntax or another generally-accepted mechanism for disambiguation is available, to provide users with a clear alternative.

Not as far as I know. It sounds like something that should be derived anyway, but I don’t know the best way of doing that.

The warning GHC-02256 also wasn’t documented yet, so I started a PR to document the warning in the error message index. I currently have one example which uses the solution that @adamgundry proposes, but I am happy to add other solutions as well :slight_smile:
The PR is here: Document GHC-02256 by BinderDavid · Pull Request #484 · haskellfoundation/error-message-index · GitHub

I would also be grateful for suggestions on how to improve the explanation of the warning.

2 Likes

for playing around:
https://bin.mangoiv.com/note?id=f876d8ce-3a7a-4acf-9ce7-adb348d44b5b

edit: yes I know it should be (name :: Symbol) cause otherwise we don’t get an instance but mmh this is hacked together

Thanks to @MangoIV and @DavidB I can provide some answers:

  • OverloadedRecordUpdate is considered experimental.
  • There is no definitive story for generating getField for OverloadedRecordDot.
  • There is not even a blessed class for setField, for OverloadedRecordUpdate.

Them’s fighting words :smiley: Type-directed name resolution is controversial in languages with type inference because it generates an ugly chicken-egg problem

One unintuitive thing for me: why is type inference involved at all? In my example, I should think that the type checker could use my user-written type signature as a constraint: bar : Bar.

blah :: Bar -> Bar
blah bar = bar { foo = 5 }

Rather, this ambiguous nugget seems to be considered in isolation at some point, generating the warning.

{ foo = 5 }

But I don’t need GHC to work so hard; I want to assert what type the record is!

The proposal DuplicateRecordFields without ambiguous field access by adamgundry · Pull Request #366 · ghc-proposals/ghc-proposals · GitHub contains a pretty good discussion about the design space. For example DuplicateRecordFields without ambiguous field access by adamgundry · Pull Request #366 · ghc-proposals/ghc-proposals · GitHub gives good arguments why type directed name resolution doesn’t work so well with the way GHC works.

As another example: There is a recent bug report #23992: `$` converts ambiguous record update error to a warning · Issues · Glasgow Haskell Compiler / GHC · GitLab which shows that your blah bar = bar { foo = 5 } currently works, blah bar = id bar { foo = 5 } does not work, but blah bar = id $ bar { foo = 5 } works again. It is just not very transparent and understandable when GHC can do the type-directed name resolution and when it can’t.

1 Like

I suspect that when the proposal to add the warning was written, it wasn’t anticipated that development of OverloadedRecordUpdate (then part of the proposed RecordDotSyntax along with what is now known as OverloadedRecordDot) would take so long.

This has led to an unfortunate state of affairs where we’re warned of the need to migrate before the ideal migration strategy actually exists, since OverloadedRecordUpdate is still experimental, and in practice basically unusable due to the need for manual setField instances.

So, like others, I’ve ended up just disabling the warning everywhere I’ve seen it so far, on the assumption that the DuplicateRecordFields behaviour won’t actually change before OverloadedRecordUpdate has been fully implemented. And I would strongly urge any GHC developers on this thread to ensure that’s the case, since anything else would be a very annoying regression.

I fear this whole situation is going to confuse everyone who hasn’t totally kept up with the status of the many record extensions. Which, anecdotally, seems to be almost everyone.

tl;dr I don’t think there’s currently a satisfying answer to the actual title of this thread. (EDIT: at least in the record-update case which is what we’re discussing - the same warning also covers ambiguous selection, but there are better workarounds for that)

1 Like