Skip to content

Desugar special-case applications of particular functions consistently (follow up to !12685)

The following discussion from !12685 (closed) Pmc: Improve implementation of -Wincomplete-record-selectors ( #24824 (closed), #24891 (closed))

  • @simonpj started a discussion: (+1 comment)

    Wow, this is brutal -- calling decomposeRecSelHead on every single application.

    Moreover, something rather similar happens immediately afterwards, in mkCoreAppsDs.

    I wonder if we could do something like:

    dsExpr e@(HsApp {}) = dsApp e []
    
    dsApp (HsApp fun arg) core_args
      = do { arg' <- dsLExpr arg
           ; dsApp fun (arg':core_args) }
    
    dsApp (HsVar fun) core_args
      | fun == seqId     = ...
      | fun == noInlinId = ...
      | isRecSel fun     = ...
      | fun = hasFieldId = ...

    We might need to duplicate a bit of dsHsWrapper, namely the cases for dictionary and type application, but it might be worth it. Basically, just accumulate the arguments, and dispatch on the function at the head.

    There is yet more stuff in dsHsWrapped which I can't figure out. But it calls warnAboutIdentities that would also fit nicely in the HsVar case of dsApp above. We seem to be doing something very similar (look at the function, do something based on that) in at least three different places.

    I suppose this is an unforced refactoring but the current story grates on me a bit. It seems bad to do the same thing in 3 different ways!

    Moreover the fancy switching on and off of flags to deal with the 1/2/3 ovderlap might not be needed any more: we just have three equations in the Var case of dsApp.

    The more I think about this the more I like it!

    and also:

    it has nothing to do with perf. It's the intellectual overhead of doing the same thing in three different ways!

Edited by Sebastian Graf
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information