Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in / Register
  • GHC GHC
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 5,262
    • Issues 5,262
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
  • Merge requests 570
    • Merge requests 570
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Releases
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Insights
    • Issue
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Glasgow Haskell CompilerGlasgow Haskell Compiler
  • GHCGHC
  • Issues
  • #22424
Closed
Open
Issue created Nov 08, 2022 by sheaf@sheafMaintainer

Suspicious uses of lookupExactOrOrig_maybe wrt field selectors

The current implementation of rnExpr (HsVar ...) looks something like (slightly simplified):

rnExpr (HsVar _ (L l v))
  = do { mb_name <- lookupExprOccRn v
       ; case mb_name of
        { Nothing
          -> rnUnboundVar v
        ; Just (NormalGreName name)
          -> return (HsVar noExtField (L l name), unitFV name)
        ; Just (FieldGreName fl)
          -> do { let sel_name = flSelector fl
                ; return (HsRecSel noExtField (FieldOcc sel_name (L l v) ), unitFV sel_name) } }

My point here being that we do something different for field selectors, returning an HsRecSel instead of an HsVar.

However, if you look at the implementation of lookupExprOccRn, it ends up going through (among other things) lookupGlobalOccRn_overloaded:

lookupGlobalOccRn_overloaded :: DuplicateRecordFields -> FieldsOrSelectors -> RdrName
                             -> RnM (Maybe AmbiguousResult)
lookupGlobalOccRn_overloaded dup_fields_ok fos rdr_name =
  lookupExactOrOrig_maybe rdr_name (fmap (UnambiguousGre . NormalGreName)) $
    do { res <- lookupGreRn_helper fos rdr_name
       ; case res of
           GreNotFound -> fmap UnambiguousGre <$> lookupOneQualifiedNameGHCi fos rdr_name
           OneNameMatch gre -> return $ Just (UnambiguousGre (gre_name gre))
           MultipleNames gres
             | all isRecFldGRE gres
             , dup_fields_ok == DuplicateRecordFields -> return $ Just AmbiguousFields
             | otherwise -> do
                  addNameClashErrRn rdr_name gres
                  return (Just (UnambiguousGre (gre_name (NE.head gres)))) }

The above code shows that we first try to look up an Orig or Exact Name in which case we always return a non-field GreName, and if that fails we go to the normal path which might return a field GRE or not.

This seems completely wrong to me: what if we have the exact name of a field selector? We should then surely return a field GreName? If we don't, then rnExpr will return an HsVar instead of an HsRecSel.

I'm attempting to fix this in !8686. It seems that lookupExactOrOrig_maybe should not return just a Name, but something which has more information (in particular, enough information to distinguish fields from non-fields). I'm not sure how to engineer this however, especially with Orig names which just correspond to Names in the NameCache, and I don't know how we can extract more information out of them.

Edited Nov 08, 2022 by sheaf
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking