-ddump-types vs -fprint-explicit-foralls, and symbol-ness of worker/wrapper names [Re: [commit: ghc] master: Pretty-print the following TyThings via their IfaceDecl counterpart: * AnId * ACoAxiom * AConLike (065c35a) (fwd)]

Dr. ERDI Gergo

17 Mar 2014 17 Mar '14

8:22 a.m.

I've just realized the original subject of this mail probably didn't properly highlight that this unearthed two issues that I'd like to hear opinions on, before going on to fix them. So here's my original mail: ---------- Forwarded message ---------- Date: Sun, 16 Mar 2014 00:50:19 +0800 (SGT) From: Dr. ERDI Gergo To: Joachim Breitner Cc: GHC Devs Subject: Re: [commit: ghc] master: Pretty-print the following TyThings via their IfaceDecl counterpart: * AnId * ACoAxiom * AConLike (065c35a) On Sat, 15 Mar 2014, Joachim Breitner wrote:

...

Unexpected failures: indexed-types/should_compile T3017 [stderr mismatch] (normal) roles/should_compile Roles1 [stderr mismatch] (normal) roles/should_compile Roles2 [stderr mismatch] (normal) simplCore/should_compile T4306 [bad exit code] (normal) th T8884 [stderr mismatch] (normal) typecheck/should_compile tc231 [stderr mismatch] (normal) https://s3.amazonaws.com/archive.travis-ci.org/jobs/20819762/log.txt

...

Are these related to your changes, and are pending fixing? (If not it might take too long until we notice, and it will be more difficult to find the cause.)

Hi, Yes, these are related. However, they all point to just a change in the output format of -ddump-types so that by default, foralls are not printed. The old output format can be restored by passing in an extra -fprint-explicit-foralls flag. I think this is actually an improvement, and thus my suggestion is to simply update the expected output of these tests. The one interesting case is T4306 which fails because the generated name $wupd is regarded as an infix name, and thus with my new code is rendered as ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# instead of the old $wupd :: GHC.Prim.Double# -> GHC.Prim.Double# I think this is actually a bug in isSymOcc, since I don't think the intention behind the generated name $wupd is to be regarded as an infix name. So we could change isLexVarSym so that if the first character is $, the rest is still checked for symbol-ness. If there's agreement on this, I'm happy to implement both changes. Bye, Gergo

Show replies by date

Simon Peyton Jones

17 Mar 17 Mar

9:45 a.m.

| Yes, these are related. However, they all point to just a change in the | output format of -ddump-types so that by default, foralls are not | printed. The old output format can be restored by passing in an extra - | fprint-explicit-foralls flag. I think this is actually an improvement, | and thus my suggestion is to simply update the expected output of these | tests. Yes I agree. Go ahead. | The one interesting case is T4306 which fails because the generated name | $wupd is regarded as an infix name, and thus with my new code is | rendered as | | ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# | | instead of the old | | $wupd :: GHC.Prim.Double# -> GHC.Prim.Double# I think it'd also be ok just to accept this output too. These "$wxx" names are generated by GHC and won't show up in user output. It doesn't much matter displaying them in parens. But changing isLexVarSym is probably equally fine too. I think (worth a check) that it's only called for display purposes, and not in any performance-critical parts. Whichever you choose, add a Note with isLexVarSym to explain the issue and the choice. | If there's agreement on this, I'm happy to implement both changes. Thanks. It would be good to nail this today; currently there are a bunch of consequential validate failures. Simon | | I think this is actually a bug in isSymOcc, since I don't think the | intention behind the generated name $wupd is to be regarded as an infix | name. So we could change isLexVarSym so that if the first character is | $, the rest is still checked for symbol-ness. | | | Bye, | Gergo | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs

Johan Tibell

10 a.m.

On Mon, Mar 17, 2014 at 2:45 PM, Simon Peyton Jones wrote:

...

| The one interesting case is T4306 which fails because the generated name | $wupd is regarded as an infix name, and thus with my new code is | rendered as | | ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# | | instead of the old | | $wupd :: GHC.Prim.Double# -> GHC.Prim.Double#

I think it'd also be ok just to accept this output too. These "$wxx" names are generated by GHC and won't show up in user output. It doesn't much matter displaying them in parens.

Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis.

...

But changing isLexVarSym is probably equally fine too. I think (worth a check) that it's only called for display purposes, and not in any performance-critical parts.

Whichever you choose, add a Note with isLexVarSym to explain the issue and the choice.

Does that mean that any operator that starts with $ will now not be considered infix for printing purposes? -- Johan

Simon Peyton Jones

10:08 a.m.

Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis. I don't think so (because -ddump-simpl doesn't print *any* operators in parens) but I could be wrong, and I agree that would be bad. Does that mean that any operator that starts with $ will now not be considered infix for printing purposes? No, I believe that Gergo's suggestion is that a function be considered infix operator (for display purposes) only if all its characters are operator chars, rather than just the first one. Simon From: Johan Tibell [mailto:johan.tibell@gmail.com] Sent: 17 March 2014 14:00 To: Simon Peyton Jones Cc: Dr. ERDI Gergo; GHC Devs Subject: Re: -ddump-types vs -fprint-explicit-foralls, and symbol-ness of worker/wrapper names [Re: [commit: ghc] master: Pretty-print the following TyThings via their IfaceDecl counterpart: * AnId * ACoAxiom * AConLike (065c35a) (fwd)] On Mon, Mar 17, 2014 at 2:45 PM, Simon Peyton Jones mailto:simonpj@microsoft.com> wrote: | The one interesting case is T4306 which fails because the generated name | $wupd is regarded as an infix name, and thus with my new code is | rendered as | | ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# | | instead of the old | | $wupd :: GHC.Prim.Double# -> GHC.Prim.Double# I think it'd also be ok just to accept this output too. These "$wxx" names are generated by GHC and won't show up in user output. It doesn't much matter displaying them in parens. Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis. But changing isLexVarSym is probably equally fine too. I think (worth a check) that it's only called for display purposes, and not in any performance-critical parts. Whichever you choose, add a Note with isLexVarSym to explain the issue and the choice. Does that mean that any operator that starts with $ will now not be considered infix for printing purposes? -- Johan

Dr. ÉRDI Gergő

10:23 a.m.

Exactly, the problem is precisely that $foo is regarded as an infix operator in that code path, so with my change, it would be classified as prefix. On Mar 17, 2014 10:10 PM, "Simon Peyton Jones" wrote:

...

Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis.

I don't think so (because -ddump-simpl doesn't print **any** operators in parens) but I could be wrong, and I agree that would be bad.

Does that mean that any operator that starts with $ will now not be considered infix for printing purposes?

No, I believe that Gergo's suggestion is that a function be considered infix operator (for display purposes) only if all its characters are operator chars, rather than just the first one.

Simon

*From:* Johan Tibell [mailto:johan.tibell@gmail.com] *Sent:* 17 March 2014 14:00 *To:* Simon Peyton Jones *Cc:* Dr. ERDI Gergo; GHC Devs *Subject:* Re: -ddump-types vs -fprint-explicit-foralls, and symbol-ness of worker/wrapper names [Re: [commit: ghc] master: Pretty-print the following TyThings via their IfaceDecl counterpart: * AnId * ACoAxiom * AConLike (065c35a) (fwd)]

On Mon, Mar 17, 2014 at 2:45 PM, Simon Peyton Jones wrote:

| The one interesting case is T4306 which fails because the generated name

| $wupd is regarded as an infix name, and thus with my new code is | rendered as | | ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# | | instead of the old | | $wupd :: GHC.Prim.Double# -> GHC.Prim.Double#

I think it'd also be ok just to accept this output too. These "$wxx" names are generated by GHC and won't show up in user output. It doesn't much matter displaying them in parens.

Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis.

But changing isLexVarSym is probably equally fine too. I think (worth a check) that it's only called for display purposes, and not in any performance-critical parts.

Whichever you choose, add a Note with isLexVarSym to explain the issue and the choice.

Does that mean that any operator that starts with $ will now not be considered infix for printing purposes?

-- Johan

Johan Tibell

11:25 a.m.

Sounds good to me. On Mon, Mar 17, 2014 at 3:23 PM, Dr. ÉRDI Gergő wrote:

...

Exactly, the problem is precisely that $foo is regarded as an infix operator in that code path, so with my change, it would be classified as prefix. On Mar 17, 2014 10:10 PM, "Simon Peyton Jones" wrote:

...
Do they show up in –ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis.

I don’t think so (because –ddump-simpl doesn’t print **any** operators in parens) but I could be wrong, and I agree that would be bad.

Does that mean that any operator that starts with $ will now not be considered infix for printing purposes?

No, I believe that Gergo’s suggestion is that a function be considered infix operator (for display purposes) only if all its characters are operator chars, rather than just the first one.

Simon

*From:* Johan Tibell [mailto:johan.tibell@gmail.com] *Sent:* 17 March 2014 14:00 *To:* Simon Peyton Jones *Cc:* Dr. ERDI Gergo; GHC Devs *Subject:* Re: -ddump-types vs -fprint-explicit-foralls, and symbol-ness of worker/wrapper names [Re: [commit: ghc] master: Pretty-print the following TyThings via their IfaceDecl counterpart: * AnId * ACoAxiom * AConLike (065c35a) (fwd)]

On Mon, Mar 17, 2014 at 2:45 PM, Simon Peyton Jones < simonpj@microsoft.com> wrote:

| The one interesting case is T4306 which fails because the generated name

| $wupd is regarded as an infix name, and thus with my new code is | rendered as | | ($wupd) :: GHC.Prim.Double# -> GHC.Prim.Double# | | instead of the old | | $wupd :: GHC.Prim.Double# -> GHC.Prim.Double#

I think it'd also be ok just to accept this output too. These "$wxx" names are generated by GHC and won't show up in user output. It doesn't much matter displaying them in parens.

Do they show up in -ddump-simpl? It would be nice to keep that output as readable as possible, as there are quite a few of us that read it on a regular basis.

But changing isLexVarSym is probably equally fine too. I think (worth a check) that it's only called for display purposes, and not in any performance-critical parts.

Whichever you choose, add a Note with isLexVarSym to explain the issue and the choice.

Does that mean that any operator that starts with $ will now not be considered infix for printing purposes?

-- Johan

4208

Age (days ago)

4208

Last active (days ago)

List overview

Download

5 comments

4 participants

participants (4)

Dr. ERDI Gergo
Dr. ÉRDI Gergő
Johan Tibell
Simon Peyton Jones