I'm not entirely sure how to add tests for this though, or for codegen code in general.

Feel free to extract and introduce the tests/purs/optimize machinery I built in #3915 for this sort of thing. I don't think that one's getting merged any time soon.

I haven't the time right this moment to consider a constructive alternative, but as initial feedback I think it's a little unfortunate to duplicate the $ inlining both here and in L.PS.CoreImp.Optimizer.Inliner downstream. The latter is much more extensive (in particular, it covers # as well), and ideally I think we'd want all of our inlining optimizations in one place?

So, for example, one thing I almost proposed to you now was to replace newtype constructors here with Control.Category.identity(Control.Category.categoryFn), and inline saturated calls to that identity in the L.PS.CoreImp.Optimizer.Inliner machinery. That would cover both MyNewtype $ x and x # MyNewtype, as well as some less probable combinations of constructors and inline-able operators. With only a tad more work we could probably cover >>> MyNewtype etc. as well.

The reason I'm not proposing precisely that is that it introduces a dependency on Control.Category, and thus implicitly purescript-prelude, in any project that uses a newtype constructor if that constructor isn't in a position that gets optimized away. Projects using an alternate prelude might break at run time.

As I said, I'd need more time than I have right this moment to come up with a better version of that idea, but maybe this inspires a good idea in you or someone else, or a counterargument to one of my thoughts.

I think we'd want all of our inlining optimizations in one place?

I agree. I had not considered the CoreImp inliner ahead of time because I'm still not really familiar with that part of the code base.

Though I figured the CoreFn would be another likely candidate to put this optimization into, it wouldn't really be the best move to make considering the reasons you've pointed out.

Thanks a lot, I'll take those pointers in mind.

With CoreImp though, I'm not sure if there's a simple way of determining whether or not some Var is a newtype function, unless I'm missing something... My best assumption at the moment is that newtype functions likely start with capitalized names, which is rather fragile.

I've tested out this approach on another branch PureFunctor@c828d35f0eb935b9b10d6567c90e909a0b59ca0e and it passes all tests, which at least gives me a bit more confidence with manually checking newtypes like this, but I still think something better could be done.

In particular, I sort of propose parameterizing the CoreImp type with some annotation a much like in CoreFn.Expr; this should allow for meta annotations like determining whether some Var is a newtype constructor or not. However, I don't feel like this optimization justifies this breaking change in particular, and I move that this be discussed in-depth in some other proposal.

I didn't see mention of the fact that non-newtype constructors are also compiled differently with $ just now (though I doubt it matters much).

testNonNt1 = NonNT 42
testNonNt2 = NonNT $ 42

var testNonNt2 = NonNT.create(42);
var testNonNt1 = new NonNT(42);

As the inlining of $ and # affect codegen, rather than having to care about it here and also in CoreImp optimisations, I'd suggest it might better be part of the CoreFn optimisation pass. I'm doing that in purerl as it affects codgen even more in my case

Yeah, I think I agree with you. Most of the stuff being inlined in CoreImp is inlined to somewhat backend-specific stuff, except for $ and #, which appear to be there as legacy from before there was a CoreFn pass. So I think you both are right about this being better as a CoreFn optimization; it probably does make sense to move (assuming no unexpected breaks) those specific inlinings out of CoreImp.Optimizer.Inliner and into CoreFn.Optimizer, which should be sufficient to fix #2866.

I think it'd be best to write up a proposal if we ought to move some CoreImp optimizations, given that feedback from downstream consumers of CoreFn API would be pretty invaluable. In the meantime, I think this PR can be put on hold until further feedback is received.

Likewise, I think this is one of the first few steps in creating a general optimization API for the compiler that isn't backend/codegen-specific, which would prove to be pretty helpful considering that the CoreFn is a rich format by itself.

I'm not sure what to do with the weeder errors here.

Anything else we want to do before merging this?