[Bug libstdc++/104602] New: std::source_location::current uses cast from void*

[Bug libstdc++/104602] New: std::source_location::current uses cast from void*

[foom at fuhm dot net]

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

            Bug ID: 104602
           Summary: std::source_location::current uses cast from void*
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: foom at fuhm dot net
  Target Milestone: ---

I'm working on implementing __builtin_source_location() in Clang
(https://reviews.llvm.org/D120159).

In testing it against the libstdc++ <source_location> header, I ran into a
minor issue.

"current()" in GNU libstdc++ is defined as so:

    static consteval source_location
    current(const void* __p = __builtin_source_location()) noexcept
    {
      source_location __ret;
      __ret._M_impl = static_cast <const __impl*>(__p);
      return __ret;
    }

But! A static_cast from a `const void*` parameter to `const __impl*` is not
permitted in constexpr evaluation:
"""
5. An expression E is a core constant expression unless the evaluation of E,
[...] would evaluate one of the following:
[...]
5.15. a conversion from type cv void* to a pointer-to-object type;"
"""
http://eel.is/c++draft/expr.const#5.15

Clang diagnoses this rule, but GCC apparently does not. (it's not really clear
to me why this rule really needs to exist in the standard -- why bother to
police which kinds of pointer casts you're allowed to do, instead of just
raising an error upon _access_ through the wrong type?)

Anyhow, to workaround this issue, I plan to simply hardcode an exception to the
check in Clang for casts which occur in a "std::source_location::current"
method. Yet, although it's perhaps too late to avoid this workaround, it'd be
nice if libstdc++ didn't require the use of an invalid cast.

In clang (in my proposed change), __builtin_source_location already returns the
expected `const __impl*` type, rather than `const void*` as it does in GCC. So,
the issue is only the cast TO `void*` and back again in libstdc++. ISTM this
would be fixed by moving the `static_cast <const __impl*>` into the default
parameter expression. That would then be a no-op cast on clang, and an (invalid
but undiagnosed) cast from void in GCC.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://gcc.gnu.org/pipermail/gcc-patches/2019-November/534374.html
Explains why it is currently this way.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-02-19

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
But we can certainly move the cast into the default argument.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Can't we make it
    static consteval source_location
    current(decltype (__builtin_source_location ()) __p
            = __builtin_source_location()) noexcept
?

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[foom at fuhm dot net]

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

--- Comment #4 from James Y Knight <foom at fuhm dot net> ---
Yea that should work. Or even just `auto`.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
No, with auto it doesn't work.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We can make that more readable:

--- a/libstdc++-v3/include/std/source_location
+++ b/libstdc++-v3/include/std/source_location
@@ -43,12 +43,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
   private:
     using uint_least32_t = __UINT_LEAST32_TYPE__;
+    using __builtin_ret_type = decltype(__builtin_source_location());

   public:

     // [support.srcloc.cons], creation
     static consteval source_location
-    current(const void* __p = __builtin_source_location()) noexcept
+    current(__builtin_ret_type __p = __builtin_source_location()) noexcept
     {
       source_location __ret;
       __ret._M_impl = static_cast <const __impl*>(__p);

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:41cbcf53dc60b7434b9ee059b3a734a47f5bf212

commit r12-7379-g41cbcf53dc60b7434b9ee059b3a734a47f5bf212
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Feb 24 21:33:44 2022 +0000

    libstdc++: Fix cast in source_location::current() [PR104602]

    This fixes a problem for Clang, which is going to return a non-void
    pointer from __builtin_source_location(). The current definition of
    std::source_location::current() converts that to void* and then has to
    cast it back again in the body (which makes it invalid in a constant
    expression). By using the actual type of the returned pointer, we avoid
    the problematic cast for Clang.

    libstdc++-v3/ChangeLog:

            PR libstdc++/104602
            * include/std/source_location (source_location::current): Use
            deduced type of __builtin_source_location().

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |12.0
         Resolution|---                         |FIXED

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I hope this works for you now, please reopen if not.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Hmm, we probably want to backport this, otherwise this check will start to pass
for future clang versions:
#if __cplusplus > 201703L && __has_builtin(__builtin_source_location)
but the code will be invalid.

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ed377f0fb3419fceed25ae378aca2efca1a6b79d

commit r11-10115-ged377f0fb3419fceed25ae378aca2efca1a6b79d
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Feb 24 21:33:44 2022 +0000

    libstdc++: Fix cast in source_location::current() [PR104602]

    This fixes a problem for Clang, which is going to return a non-void
    pointer from __builtin_source_location(). The current definition of
    std::source_location::current() converts that to void* and then has to
    cast it back again in the body (which makes it invalid in a constant
    expression). By using the actual type of the returned pointer, we avoid
    the problematic cast for Clang.

    libstdc++-v3/ChangeLog:

            PR libstdc++/104602
            * include/std/source_location (source_location::current): Use
            deduced type of __builtin_source_location().

    (cherry picked from commit 41cbcf53dc60b7434b9ee059b3a734a47f5bf212)

[Bug libstdc++/104602] std::source_location::current uses cast from void*

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.0                        |11.4

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Backported for 11.4