Simplify 'ExtBits' in the lexer
ClosedPublic

Authored by harpocrates on Nov 12 2018, 8:13 PM.

Details

Summary

The main change is to export 'ExtBits' instead of defining/exporting a
bunch of boilerplate functions that test for a particular 'ExtBits'.
In the process, I also

  • cleaned up an unneeded special case for 'ITstatic'
  • made 'UsePosPrags' another variant of 'ExtBits'
  • made the logic in 'reservedSymsFM' match that of 'reservedWordsFM'
Test Plan

make test

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.
harpocrates created this revision.Nov 12 2018, 8:13 PM

These are just some cleanup changes I wanted to do in D5269, but thought would distract from the main purpose of the diff. None of the stuff here is essential or fixes any bugs, so I'm willing to just abandon if folks think this is too much code churn for too little benefit.

bgamari accepted this revision.Thu, Nov 22, 1:35 PM

Looks reasonable to me.

This revision is now accepted and ready to land.Thu, Nov 22, 1:35 PM
bgamari requested changes to this revision.Thu, Nov 22, 1:35 PM

Except for the fact that it doesn't cleanly apply.

Do you think you could rebase, @harpocrates?

This revision now requires changes to proceed.Thu, Nov 22, 1:35 PM

Except for the fact that it doesn't cleanly apply.

Do you think you could rebase, @harpocrates?

I'll take a look.

On the off chance this is the problem, note you do need to land D5269 first... (and that would make @sjakobi happy too 😄)

bgamari accepted this revision.Thu, Nov 22, 1:40 PM

Except for the fact that it doesn't cleanly apply.

Do you think you could rebase, @harpocrates?

I'll take a look.

On the off chance this is the problem, note you do need to land D5269 first... (and that would make @sjakobi happy too 😄)

Ahh, indeed that was the problem. Forget I said anything.

Thanks @harpocrates!

This revision is now accepted and ready to land.Thu, Nov 22, 1:40 PM
This revision was automatically updated to reflect the committed changes.

Unfortunately either this or D5269 ended up regressing; I have reverted both. @harpocrates, could you have a look?

Unfortunately either this or D5269 ended up regressing; I have reverted both. @harpocrates, could you have a look?

Sorry! I'll take a look.

I think I've figured out why tests were failing and the issue is entirely in this diff, not D5269.

How do I proceed? Do I open a new diff that is an updated version of this existing diff or can I still update this diff? Do I also need to re-open something for D5269?

I think I've figured out why tests were failing and the issue is entirely in this diff, not D5269.

How do I proceed? Do I open a new diff that is an updated version of this existing diff or can I still update this diff? Do I also need to re-open something for D5269?

You can't push a new version of a closed diff. Please do just push a new differential.

I think I've figured out why tests were failing and the issue is entirely in this diff, not D5269.

How do I proceed? Do I open a new diff that is an updated version of this existing diff or can I still update this diff? Do I also need to re-open something for D5269?

You can't push a new version of a closed diff. Please do just push a new differential.

Yeah, I opened https://phabricator.haskell.org/D5405.