[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

[Aaron Ballman via cfe-commits]

Somehow this entire thread seems to have dropped the mailing lists and
the review in question, so top-posting to ensure the discussion at
least makes it to the mailing lists.

~Aaron

On Mon, Apr 11, 2022 at 9:10 AM Aaron Ballman <aaron.ballman at gmail.com> wrote:
>
> On Mon, Apr 11, 2022 at 8:25 AM Joerg Sonnenberger <joerg at bec.de> wrote:
> >
> > Am Mon, Apr 11, 2022 at 07:42:15AM -0400 schrieb Aaron Ballman:
> > > On Mon, Apr 11, 2022 at 7:21 AM Joerg Sonnenberger <joerg at bec.de> wrote:
> > > > You don't see the value of being able to instrument std::move? I don't
> > > > even mean in the sense of the operator new overload. Being able to patch
> > > > libc++ to add a tracking feature seems like an entirely sensible thing
> > > > to support.
> > >
> > > I'd want to hear from STL maintainers whether *they* think it's
> > > reasonable. Every STL maintainer I've ever talked to seems to think
> > > letting users add their own symbols to 'std' is highly problematic, so
> > > I'm not certain that it's a good idea to make that easier and give the
> > > appearance that we bless such a thing as a toolchain. (This goes
> > > beyond libc++ given that Clang also supports other STL implementations
> > > like libstdc++.)
> >
> > Again, this is not about adding symbols to 'std'. You make it impossible
> > to instrument std::move and friends in the name of performance for
> > unoptimized code. Without any mechanism to disable it. If I want to
> > create my own fork of libc++ or libstdc++ or whatever to have such
> > functionality, it should be possible.
>
> Should it? The STL is part of the implementation in C++, so I think
> it's perfectly reasonable to assume the burden is on you to fork the
> compiler as well if you have to.
>
> The root of my concern is: I don't want to expose an option that gives
> the impression it's okay to replace STL functions (especially in *all
> language modes*) given that it is explicitly UB and WG21 has a
> standing document on the subject (https://wg21.link/sd-8). That's not
> an interface I think we should support *unless STL maintainers
> explicitly agree that this is a reasonable policy in general*. (If you
> wanted this to be a CC1 option, that's a bit different to me because
> then it's not giving the same "this is normal, go right ahead and do
> it" feeling with it. My concerns are only with a user-facing option
> and are specifically because the situation you want to support is
> explicitly undefined behavior.)
>
> > I'm not even saying that the
> > semantic analysis here should detect that the std::move implementation
> > is actually std::move, which would side step this question as well.
> > We have a long history of optimization-by-symbol-name breaking
> > reasonable real world code. That's where -fno-builtin-* came from and
> > checks e.g. to allow implementing calloc in C without the compiler
> > turning the code of malloc+memset into a recursive call. I don't see
> > this change here being any different to the nature of those checks.
>
> I see that as different -- those interfaces are in the global
> namespace and thus available for the user to reuse in a freestanding
> environment with different semantics. Giving users a knob to support
> that situation seems eminently reasonable because it's not UB to
> replace those functions.
>
> That said, I'm happy to leave the decision to Richard as code owner.
> If he thinks this is worth maintaining a flag for, I can hold my nose.
>
> ~Aaron
>
> >
> > Joerg