haskell-suite / haskell-src-exts

Manipulating Haskell source: abstract syntax, lexer, parser, and pretty-printer
Other
193 stars 95 forks source link

Control fixity application #75

Open UnkindPartition opened 10 years ago

UnkindPartition commented 10 years ago

Currently the fixities field of ParseMode has type Maybe [Fixity]. When it's Nothing, the fixities from Prelude get applied.

This is a quick and dirty approach to get things done.

But when used for something serious, it is wrong in many ways.

  1. It doesn't take scoping into account (qualified Prelude import, local bindings that shadow the Prelude ones).
  2. Often Prelude fixities are not enough, and one gets an Ambiguous infix expression error (see #72)

Furthermore, even the idea of supplying complete fixities to the parsing function is wrong:

  1. We don't have access to the toplevel fixities defined in the module itself before we parse it
  2. We don't have any means at all to handle locally defined fixities for local operators.

Thus, for anything serious the fixities field of ParseMode is useless. The correct way to handle fixities is:

  1. Parse modules
  2. Perform name resolution
  3. Traverse the trees and apply fixities manually (this will eventually be a feature of haskell-names)

The problem is, there's currently no way to disable application of fixities. There's a workaround of supplying fixites = Just [], but it's not obvious that makes applyFixities an identity function, and it would still burn a few CPU cycles, it seems.

So, there should be a way of disabling automatic fixity application. In order to stay beginner-friendly and backwards-compatible, it's probably best to introduce a new field to ParseMode, something like applyFixities :: Bool, and make it True in defaultParseMode.

niklasbroberg commented 10 years ago

This sounds like a bug to me, if indeed it works as you say. The intention is that fixities = Nothing will indeed turn off the fixity resolution altogether, not use the prelude fixities, and that's how it's implemented in InternalParser.ly:

 applyFixities' :: (AppFixity a) => Maybe [Fixity] -> a L -> ParseResult (a L)
 applyFixities' Nothing ast = return ast
 applyFixities' (Just fixs) ast = applyFixities fixs ast

Browsing the code, it appears it will only default to using the prelude fixities if no explicit ParseMode is given. What case are you seeing?

UnkindPartition commented 10 years ago

Ah yes, you're right. But this behavior is even worse than the one I assumed — it means that parseModule and parseModuleWithMode defaultParseMode behave differently! No-one would expect that.