Skip to content

NOperatorDef lacks precedence & so OperatorInfo was invented because of that #982

Open
@Anton-Latukha

Description

@Anton-Latukha

It is strange that data type for operator definition has everything, except avoids having predefined absolute precedence for the operator.

hnix/src/Nix/Parser.hs

Lines 598 to 604 in 57a55fd

data NOperatorDef
= NUnaryDef Text NUnaryOp
| NBinaryDef Text NBinaryOp NAssoc
| NSpecialDef Text NSpecialOp NAssoc
deriving (Eq, Ord, Generic, Typeable, Data, Show, NFData)

I am certain - the OperatorInfo was invented as a side-car for it:

hnix/src/Nix/Parser.hs

Lines 718 to 724 in 57a55fd

data OperatorInfo = OperatorInfo
{ precedence :: Int
, associativity :: NAssoc
, operatorName :: Text
} deriving (Eq, Ord, Generic, Typeable, Data, Show)

If we look at what is special (tail) between repetitive functions:

hnix/src/Nix/Parser.hs

Lines 724 to 775 in 57a55fd

getUnaryOperator :: NUnaryOp -> OperatorInfo
getUnaryOperator = (m Map.!)
where
m =
Map.fromList $
concat $
zipWith
buildEntry
[1 ..]
(nixOperators $ fail "unused")
buildEntry i =
concatMap $
\case
(NUnaryDef name op, _) -> [(op, OperatorInfo i NAssocNone name)]
_ -> mempty
getBinaryOperator :: NBinaryOp -> OperatorInfo
getBinaryOperator = (m Map.!)
where
m =
Map.fromList $
concat $
zipWith
buildEntry
[1 ..]
(nixOperators $ fail "unused")
buildEntry i =
concatMap $
\case
(NBinaryDef name op assoc, _) -> [(op, OperatorInfo i assoc name)]
_ -> mempty
getSpecialOperator :: NSpecialOp -> OperatorInfo
getSpecialOperator NSelectOp = OperatorInfo 1 NAssocLeft "."
getSpecialOperator o = m Map.! o
where
m =
Map.fromList $
concat $
zipWith
buildEntry
[1 ..]
(nixOperators $ fail "unused")
buildEntry i =
concatMap $
\case
(NSpecialDef name op assoc, _) -> [(op, OperatorInfo i assoc name)]
_ -> mempty

Also, since precedence is removed from the operation definition field - functions implicitly establish precedence by ordering pattern matches/constructors.

And if we de-shotgun-surgery mentioned above functions:

getOperatorInfo :: _ -> OperatorInfo
getOperatorInfo = mmap spec
 where
  mmap = (m Map.!)
   where
    m =
      Map.fromList $
        concat $
          zipWith
            (concatMap . spec)
            [1 ..]
            l2
     where
      l2 :: [[(NOperatorDef, Operator Parser NExprLoc)]]
      l2 = nixOperators $ fail "unused"
  spec i =
    \case
      (NUnaryDef op name, _) -> [(op, OperatorInfo i NAssocNone name)]
      (NBinaryDef assoc op name, _) -> [(op, OperatorInfo i assoc name)]
      (NSpecialDef assoc op name, _) -> [(op, OperatorInfo i assoc name)]

All this 3 functions are there & all they do. They take an implicitly aligned nixOperators list, and (implicitly) treat its index as operator precedence. So function just takes the NOperatorDef, implicitly figures-outs the precedence from the Haskell hardcoded list & combines that info, with a loss of the info of the type of the operation.
Also (closing eyes for lazy getter nixOperators $ fail "unused") the (NUnaryDef op name, _) -> [(op, OperatorInfo i NAssocNone name)] is quite nasty, unary operators do not have associativity, & the NAssocNone name - can't be (associativity is the minimal requirement to operations in regular algebras, if there would be AssocNone far-reaching consequences on the whole algebra) NAssocNone "really" means (gets used as) "Associative" (both left & right), which also requires to be fixed.

Adding the absolute precedence to the definitions - would complete the definition, remove the OperatorInfo, reduce get{Unary,Binary,Special}Operation (which anyway need to tell what they really do), it would change Pretty module a bit - that would be pretty much it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions