[Bug c++/102350] New: __builtin_source_location not available in earlier language modes

[Bug c++/102350] New: __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

            Bug ID: 102350
           Summary: __builtin_source_location not available in earlier
                    language modes
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pdimov at gmail dot com
  Target Milestone: ---

Currently, `__builtin_source_location` requires (a) `<source_location>` to be
included and (b) -std=c++20. Are there good reasons for these restrictions? The
builtin would still be extremely valuable in earlier language modes.

Libraries that still support 03/11/14/17 (Boost.System, for instance) could
transparently supply source locations by using a default `void const* loc =
__builtin_source_location()` argument.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
__builtin_source_location doesn't require -std=c++20, but indeed does require
<source_location> or some compatible definition of std::source_location::__impl
class, and as it doesn't have hardcoded layout of that structure but instead
matches whatever the source header provides (looks up fields it needs in there
and uses whatever types and layout they have), there is no way around that.
Of course, without -std=c++20 the usefulness of it is limited because it relies
on consteval that is only supported in C++20 or later.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #2 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jakub Jelinek from comment #1)
> __builtin_source_location doesn't require -std=c++20, but indeed does
> require <source_location> or some compatible definition of
> std::source_location::__impl class, and as it doesn't have hardcoded layout
> of that structure but instead matches whatever the source header provides
> (looks up fields it needs in there and uses whatever types and layout they
> have), there is no way around that.

That was my guess. I suppose that's inevitable then, and there's nothing to ask
for on the compiler side, <source_location> just needs to not disable itself
completely for pre-C++20 as it does now. (It's not going to work, but it can
still provide __impl so that the builtin can see it.)

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
That would be non-conforming though, because source_location isn't a reserved
name in C++17, so programs can use that as a (really dumb) macro name, which
would then break the header.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #4 from Peter Dimov <pdimov at gmail dot com> ---
On the surface this looks not hard to fix - use ::__source_location_impl (or
std::__source_location_impl) instead of std::source_location:__impl as the
layout struct - but I'm not sure whether this would pose some further problems.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Sure. It's just a question of whether we're trying to provide a general purpose
extension available for users, or an internal helper for the std::lib. IIRC we
explicitly decided we only cared about supporting the latter.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, having the struct somewhere else isn't that useful unless you know
exactly how its non-static data members are named and what they mean, so
ideally a class with accessor methods, which is what std::source_location
provides currently.

At least during the development of __builtin_source_location, the exact naming
conventions of the members was left to the library (whether it wants _M_ or __
prefixes of the member names), the final implementation uses the libstdc++
naming conventions, but I bet if e.g. libc++ implements <source_location> we
could add support even for __ prefixes.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #7 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jonathan Wakely from comment #5)
> Sure. It's just a question of whether we're trying to provide a general
> purpose extension available for users, or an internal helper for the
> std::lib. IIRC we explicitly decided we only cared about supporting the
> latter.

Yes, of course. It's just that __builtin_source_location is so painfully close
to exactly what I want - it gives a single pointer representing the location -
that it would be a pity not being able to use it without -std=c++20.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #8 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jakub Jelinek from comment #6)
> Note, having the struct somewhere else isn't that useful unless you know
> exactly how its non-static data members are named and what they mean, so
> ideally a class with accessor methods, which is what std::source_location
> provides currently.

I was going to undefined-behavior my way to victory by making
boost::source_location layout-compatible with the internal struct, and just
casting the result of __builtin_source_location to boost::source_location
const*. I think this works under the GCC object model?

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That would be an aliasing violation.
The artificial vars created by __builtin_source_location have the
std::source_location::__impl type, so accessing those using some other dynamic
type is invalid.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #10 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jakub Jelinek from comment #9)
> That would be an aliasing violation.
> The artificial vars created by __builtin_source_location have the
> std::source_location::__impl type, so accessing those using some other
> dynamic type is invalid.

In that case, the only valid way to use the result of __builtin_source_location
would just be std::source_location itself. :-/

I wonder whether there's a conformance problem in making it available. It's
true that the identifier `source_location` isn't reserved, but only programs
that include `<source_location>` can tell the difference, and these programs
(assuming they existed and worked) will probably be broken anyway because now
they'll be including the standard header instead of their own.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That is not possible, because std::source_location::current() should be
consteval and it can't be in C++ < 20, and without consteval it will behave
significantly differently.  Note, the C++ FE knows
std::source_location::current() and has magic behavior for it, just consteval
semantics is not enough.
And by providing std::source_location without the current method for C++ < 20
there would be no way to initialize it to __builtin_source_location (),
std::source_location::__impl is private...

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We could have added std::__source_location_impl as the type that the built-in
returns and used that instead of making it a private member of
std::source_location. That would also have allowed it to be used by
std::experimental::source_location, and by other users outside the std::lib.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It wouldn't work correctly in all cases, as Jakub points out, because
std::source_location::current() is part of the magic.

And I'm not convinced we want/need to support those uses.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
But we haven't done that that way and how would headers know if the
__builtin_source_location that is available is the old or new one?
As for std::experimental::source_location, could we change ABI of those?

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #15 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jonathan Wakely from comment #13)
> It wouldn't work correctly in all cases, as Jakub points out, because
> std::source_location::current() is part of the magic.
> 
> And I'm not convinced we want/need to support those uses.

I think that users of __builtin_source_location will be content with the subset
of uses it supports. :-)

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #16 from Peter Dimov <pdimov at gmail dot com> ---
(In reply to Jakub Jelinek from comment #14)
> But we haven't done that that way and how would headers know if the
> __builtin_source_location that is available is the old or new one?

The header could do

namespace std {

  struct __source_location_impl { ... };

  class source_location {

    using __impl = __source_location_impl;

    // ...

  };
}

unless the compiler looks specifically for a nested struct, in which case

  class source_location {

    struct __impl: __source_location_impl {};

    // ...

  };

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
You'd need a different builtin (so that you know the presence of the builtin
means the new behavior), ideally tell the builtin some way the type it should
construct objects in (as opposed to std::source_location::__impl), because
otherwise you don't know how __builtin_source_location will behave (except for
libstdc++ where for GCC we expect to support only the same GCC/libstdc++
combo).
But more importantly, how would you use the builtin?  I expect not using the
builtin directly, so through a macro?  Otherwise, if it is some inline
function, you'd most likely get the location of the inline function and not
that of the caller.  That is part of the magic std::source_location::current
behavior where exactly takes the location from for the builtin...

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[pdimov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #18 from Peter Dimov <pdimov at gmail dot com> ---
I would use it like this: https://godbolt.org/z/1eqEx6678

#include <source_location>

struct error_category
{
};

error_category const& system_category();

struct error_code
{
    error_code( int v, error_category const& cat, void const* loc =
__builtin_source_location() );
};

int main()
{
    error_code ec( 5, system_category() );
}

provided, of course, I have some not-undefined way to interpret its result.

[Bug c++/102350] __builtin_source_location not available in earlier language modes

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102350

--- Comment #19 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #14)
> As for std::experimental::source_location, could we change ABI of those?

Yes, we don't promise stability between major releases for the experimental
stuff (although in practice it doesn't change).