AST changes to prepare for API annotations, for #9628
ClosedPublic

Authored by alanz on Nov 3 2014, 11:42 AM.

Details

Summary

AST changes to prepare for API annotations

Add locations to parts of the AST so that API annotations can
then be added.

The outline of the whole process is captured here
https://ghc.haskell.org/trac/ghc/wiki/GhcAstAnnotations

Lint fixes

Merge remote-tracking branch 'alanz/wip/ast-prepare-annotations' into wip/ast-prepare-annotations

Removing junk

Test Plan

sh ./validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
alanz edited edge metadata.Nov 8 2014, 11:23 AM

Changed con_name in ConDecl from Located name to [Located name].

alanz updated this revision to Diff 1357.Nov 9 2014, 3:20 AM
alanz edited edge metadata.

Gratuitous push, build is failing

alanz added a comment.Nov 9 2014, 4:05 AM

I'm confused.

If I do

git checkout master
git pull
./sync-all get
arc patch D426
./validate

against HEAD e2769df912672d39346727616750ba8066e489f9 (current at the moment)

Then it applies and builds no problem, but harbourmaster fails.

thomie added a comment.Nov 9 2014, 5:26 AM

This Diff has D438 as a dependency. Either remove that Diff, or see if it validates on HEAD first. You can restart a Harbormaster build by going to its build log and clicking "Restart build" in the top right corner.

alanz added a comment.Nov 9 2014, 5:35 AM

That is what I thought too, but D438 depends on this one, it is not a
dependency. I tried adding a dependency and it came up in a different
section of the summary.

And I have restarted the build a number of times, it fails identically each
time.

If I build it locally against the github ghc HEAD it builds fine, just
doing it again against the actual arcpatch-D426 branch, and it is looking
good, has hit stage2

thomie added a comment.Nov 9 2014, 5:47 AM

Ok, dependency, not depends on.

If you haven't yet, you could try rebasing your commits for this Diff onto master (git checkout branch; git rebase master), and then update this Diff with 'arc diff'. Maybe that helps.

alanz updated this revision to Diff 1358.Nov 9 2014, 6:11 AM

Doing arc diff e2769df912672d39346727616750ba8066e489f9 to make sure
it applies aginst current head

alanz added a comment.Nov 9 2014, 6:18 AM

Ok, git rebase master shows up conflicts, will fix them.

Thanks.

alanz updated this revision to Diff 1359.Nov 9 2014, 7:35 AM
  • Fix rebase
alanz added a comment.Nov 9 2014, 8:04 AM

The build now fails with the single expected regression, feedback required as to wether this is ok

alanz updated this revision to Diff 1360.Nov 9 2014, 1:01 PM
  • AST changes to prepare for API annotations, for Trac #9628
  • In HsModule, make exports Located and imports not
  • Add annotations
  • Capture the end of file position in an annotation
  • Update for changes in the AST
alanz updated this revision to Diff 1361.Nov 9 2014, 1:05 PM

Make it relative to D426

alanz updated this revision to Diff 1363.Nov 9 2014, 1:18 PM
  • AST changes to prepare for API annotations, for Trac #9628
  • AST changes to prepare for API annotations
  • Lint fixes
  • In HsModule, make exports Located and imports not
  • WIP
  • All tests pass bar deriving/should_run T5712
  • Fix rebase
alanz updated this revision to Diff 1386.Nov 10 2014, 3:17 PM
  • Make WarningTxt Located in HsModule
In D426#11365, @alanz wrote:

I have worked it through and in looking at the test failures (19 occurred),
we have changes like

(:+:) 'x' ((:*:) 3 True)

instead of the expected

(:+:) 'x' (3 :*: True)

for deriving/should_run T5712

This seems to be a natural consequence of removing the InfixCon detection.
Is this acceptable?

It's a consequence, but not a natural one!

At issue is the setting of the dcInfix field of a data constructor (see module DataCon). This controls how the data constructor responds to dataConIsInfix, and that controls how the Read, Show (and I think Generic) instances are done.

Parsing GADTs as they are written does NOT mean that we need to change this logic. dcInfix is set when we call mkDataCon from buildDataCon. And that is called from TcTyClsDecls.tcConDecl, which in turns gets is_infix from its call to tcConArgs.

Now, tcConArgs bases its decision solely on the constructor of HsConDeclDetails. But in the PrefixCon case it could easily return True under the exact same circumstances that the current code in rnConResult does. (tcConArgs would need to be given the hs_res_ty; or you could move the is_infix calculation out of tcConArgs altogether into an another function)

Then there would be no change in behaviour

OK?

Simon

alanz updated this revision to Diff 1398.Nov 11 2014, 4:59 AM
  • Detect infix constructors for GADTs
simonpj accepted this revision.Nov 11 2014, 5:15 AM
simonpj edited edge metadata.

A few comments but generally looks good thanks

compiler/typecheck/TcTyClsDecls.lhs
1162

Surely "names" not "name"!

1209

I suggest you do the unLoc in buildOneDataCon, avoiding the map.

1225

Why is the Note in RnSource??? I suggest you must move it here.

Also we now understand better what the effect of the infix-thing is, so you might be able to clarify the Note

Simon

alanz updated this revision to Diff 1399.Nov 11 2014, 6:34 AM
alanz edited edge metadata.
  • Change con_name to con_names in ConDecl, and tidy up
alanz updated this revision to Diff 1404.Nov 11 2014, 1:28 PM
  • Add location to ideclHiding
alanz updated this revision to Diff 1419.Nov 12 2014, 2:12 PM
  • FixitySig has multiple names
alanz updated this revision to Diff 1420.Nov 12 2014, 4:06 PM
  • Update haddock
alanz updated this revision to Diff 1430.Nov 13 2014, 7:44 AM
  • Make dd_derivs located in HsDataDefn
alanz updated this revision to Diff 1505.Nov 18 2014, 11:43 AM
  • Final changes based on documentation update
  • Rebased against master 2014-11-18
alanz added a comment.Nov 20 2014, 9:43 AM

I will rebase this for the changes from the pattern synonyms, and update

compiler/typecheck/TcTyClsDecls.lhs
1225

I am not sure how the note would be clarified, except to perhaps include that the is_infix is used for derivations too

alanz updated this revision to Diff 1599.Nov 20 2014, 2:07 PM

Rebase for pattern synonym type signatures

alanz updated this revision to Diff 1602.Nov 20 2014, 2:40 PM

More rebase.

alanz updated this revision to Diff 1603.Nov 20 2014, 3:04 PM

Fix haddock rebase

Mikolaj accepted this revision.Nov 21 2014, 7:14 AM
Mikolaj edited edge metadata.

A very shallow review, mainly looking for typos and stray hunks from rebase. I've found no obvious problems.

alanz updated this revision to Diff 1628.Nov 21 2014, 10:05 AM
alanz edited edge metadata.

Rebasing

This revision was automatically updated to reflect the committed changes.