Sorry that this is a bikeshedding comment but it may be nicer to just import scala.util.chaining._, since the other examples of stdlib extension method imports do not use the Ops suffix.

import scala.collection.JavaConverters._
List(1).asJava
import scala.concurrent.duration._
1.second
import scala.util.chaining._
1.pipe(scala.math.abs)

Can we get a performance warning on these? Something like,

Note: `foo(x).pipe(bar)` may have a small amount of overhead at runtime compared to the
equivalent  `bar(foo(x))` or `{ val temp = foo(x); bar(temp) }`.

and then add inlining instructions to fix the overhead (if inlining will actually fix it)?

I continue to think it is a bad idea to include library features with the semantics of, say, match, but where there is a non-obvious-except-to-experts performance hit.

Can we get a performance warning on these?

In the Scaladoc, sure. Or in a linting rule included in a third-party performance-oriented linting ruleset, sure.

As a compiler warning, I feel strongly that no, that really wouldn't be appropriate. I think both Scala itself and the Scala standard library are already teeming with low-level performance gotchas like this — an extra method call, non-obvious boxing, allocating an extra short-lived object, that kind of thing. It's par for the course, not something the compiler constantly needs to call out.

I agree with @joshlemer's naming suggestion, any other opinions on that?

@SethTisue - I just mean in the scaladoc! As you say, there are lots of performance gotchas already, and many are worse than these.

Still, it's nice to know when two semantically identical things have significantly different performance characteristics. (This isn't even specialized, so it boxes primitives too!)

I also agree regarding naming the import chaining

I just mean in the scaladoc!

LOL, sorry for the overreaction then :-)

I should be more careful when using the term "warning" since it has a technical meaning when it applies to compiler messages.

Is there a reason not to make this specialized? (At least as specialized as Function1 is?)

Looks like all the feedback has been addressed.

According to some quick tests at least the implicit conversion should definitely be made @inline...

🤔

resolvers += "scala-pr" at "https://scala-ci.typesafe.com/artifactory/scala-integration/"

scalaVersion := "2.13.0-pre-79d5047"

Welcome to Scala 2.13.0-pre-79d5047 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> import scala.util.chaining._
import scala.util.chaining._

scala> "a".tap(x => x)
res0: _1.self.type forSome { val _1: scala.util.ChainingOps[String] } = a

scala> 42.tap(x => x)
scala.reflect.internal.Types$TypeError: type mismatch;
 found   : Int
 required: ?#self.type

That entry seems to have slain the compiler.  Shall I replay
your session? I can re-run each line except the last one.
[y/n]

compiler bug? (regression?) 🤔

Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> final class ChainingOps[A](val self: A) extends AnyVal {
     |   def tap[U](f: A => U): self.type = {
     |     f(self)
     |     self
     |   }
     | }
<console>:12: error: type mismatch;
 found   : ChainingOps.this.self.type (with underlying type A)
 required: AnyRef
Note that A is unbounded, which means AnyRef is not a known parent.
Such types can participate in value classes, but instances
cannot appear in singleton types or in reference comparisons.
         def tap[U](f: A => U): self.type = {
                                ^

Welcome to Scala 2.13.0-M4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> final class ChainingOps[A](val self: A) extends AnyVal {
     |   def tap[U](f: A => U): self.type = {
     |     f(self)
     |     self
     |   }
     | }
defined class ChainingOps

#7036

compiler bug? (regression?)

scala/bug#10768 ? or another issue?🤔

paging @milessabin in aisle literal types

scala/bug#10768 ? or another issue?🤔

Hmmm, maybe another issue 🤔?

• Deconst thoroughly #6452 , dae0dd8

$ sbt
> set resolvers += "pr" at "https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots/"
> set scalaVersion := "2.13.0-pre-dae0dd8-SNAPSHOT" // pull req #6452
> console

Welcome to Scala 2.13.0-20180806-115211-dae0dd8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> case class Foo[A](self: A) { def bar: self.type = self }
defined class Foo

scala> Foo(1).bar
warning: there was one feature warning; for details, enable `:setting -feature' or `:replay -feature'
scala.reflect.internal.Types$TypeError: type mismatch;
 found   : Int
 required: ?#self.type

That entry seems to have slain the compiler.  Shall I replay
your session? I can re-run each line except the last one.
[y/n]

I'll take a look.

Looks like a variant of scala/bug#10792 (which was fixed in #6454).

Looks like a variant of scala/bug#10792 (which was fixed in #6454).

My example ( 79d5047 ) contains #6454 commits but still fail.

Yeah, it's a variant that's not covered by that fix.

Re familiarity of |>, FWIW Javascript has a proposal to add it: https://github.com/tc39/proposal-pipeline-operator

For anyone doubting that tap might be used in places where performance matters and thus that a macro solution is not of high value, the first PR I know of that used it is:

https://github.com/scala/scala/pull/7063/files#diff-873c1cee0e89e8df586c53df0c303ceaR44

which could have a substantial performance impact on small byte arrays. Maybe someone wants to benchmark it both ways; it's more hassle to benchmark the difference than it's worth to avoid writing it a way that's assuredly fast. (One could counter: but nextBytes isn't all that fast anyway, and this is true, but in a widely-used library it's generally better to "prematurely" optimize to the extent consistent with what's already there. Un-optimized, which is really easy to test by copying the tap code it delivers a ~30% slowdown on an array of four bytes.)

I've just fixed the issue reported by @xuwei-k. With #7070 the result type of his example is now revealed to be a bit of a monstrosity,

scala> case class Foo[A](self: A) { def bar: self.type = self }
defined class Foo

scala> val res = Foo(1).bar
                        ^
       warning: inferred existential type _1.self.type forSome { val _1: Foo[Int] }, which cannot be expressed by wildcards, should be enabled
       by making the implicit value scala.language.existentials visible.
       ----
       This can be achieved by adding the import clause 'import scala.language.existentials'
       or by setting the compiler option -language:existentials.
       See the Scaladoc for value scala.language.existentials for a discussion
       why the feature should be explicitly enabled.
res: _1.self.type forSome { val _1: Foo[Int] } = 1

self.type is almost certainly not what you want, even now that it doesn't crash the compiler.

renewed discussion at #7326 of whether to have |> (instead of, or in addition to, pipe)