Re: [RFC C++ PATCH] __builtin_source_location ()

[Jonathan Wakely <jwakely at redhat dot com>]

On 14/11/19 20:34 +0100, Jakub Jelinek wrote:
> Hi!
>
> The following WIP patch implements __builtin_source_location (),
> which returns const void pointer to a std::source_location::__impl
> struct that is required to contain __file, __function, __line and __column
> fields, the first two with const char * type, the latter some integral type.
>
> I don't have testcase coverage yet and the hash map to allow sharing of
> VAR_DECLs with the same location is commented out both because it
> doesn't compile for some reason and because hashing on location_t
> is not enough, we probably need to hash on both location_t and fndecl,
> as the baz case in the following shows.
>
> Comments?
>
> namespace std {
>  struct source_location {
>    struct __impl {

Will this work if the library type is actually in an inline namespace,
such as std::__8::source_location::__impl (as for
--enable-symvers=gnu-versioned-namespace) or
std::__v1::source_location::__impl (as it probably would be in
libc++).

If I'm reading the patch correctly, it would work fine, because
qualified lookup for std::source_location would find that name even if
it's really in some inline namespace.


>      const char *__file;
>      const char *__function;
>      unsigned int __line, __column;
>    };
>    const void *__ptr;

If the magic type the compiler generates is declared in the header,
then this member might as well be 'const __impl*'.

>    constexpr source_location () : __ptr (nullptr) {}
>    static consteval source_location
>    current (const void *__p = __builtin_source_location ()) {
>      source_location __ret;
>      __ret.__ptr = __p;
>      return __ret;
>    }
>    constexpr const char *file () const {
>      return static_cast <const __impl *> (__ptr)->__file;

Not really relevant to your patch, but I'll say it here for the
benefit of others reading these mails ...

On IRC I suggested that the default constructor should set the __ptr
member to null, and these member functions should check for null, e.g.

 if (__ptr) [[likely]]
   return __ptr->__function;
 else
   return "";

The alternative is for the default constructor to call
__builtin_source_location() or refer to some static object in the
runtime library, but both options waste space. Adding a [[likely]]
branch to the accessors wastes no space and should only penalise users
who are misusing source_location by trying to get meaningful values
out of default constructed objects. If that's a bit slower I don't
care.