diff mbox series

[committed,_GLIBCXX_INLINE_VERSION] Fix constract violation

Message ID ebc5b615-b75a-443e-94be-7be27b95888d@gmail.com
State Accepted
Headers
Series [committed,_GLIBCXX_INLINE_VERSION] Fix constract violation |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

François Dumont Oct. 29, 2023, 9:11 p.m. UTC 
  This fixes handle_contract_violation under versioned namespace mode.

Tested under Linux x64 and confirmed to also fix Darwin build.

libstdc++: [_GLIBCXX_INLINE_VERSION] Provide handle_contract_violation 
symbol

libstdc++-v3/ChangeLog:

         * src/experimental/contract.cc
         [_GLIBCXX_INLINE_VERSION](handle_contract_violation): Provide 
symbol
         without version namespace decoration for gcc.

François

Comments

Jonathan Wakely Oct. 30, 2023, 1:45 p.m. UTC | #1 
On Sun, 29 Oct 2023 at 21:11, François Dumont <frs.dumont@gmail.com> wrote:
>
> This fixes handle_contract_violation under versioned namespace mode.
>
> Tested under Linux x64 and confirmed to also fix Darwin build.
>
> libstdc++: [_GLIBCXX_INLINE_VERSION] Provide handle_contract_violation
> symbol
>
> libstdc++-v3/ChangeLog:
>
>          * src/experimental/contract.cc
>          [_GLIBCXX_INLINE_VERSION](handle_contract_violation): Provide
> symbol
>          without version namespace decoration for gcc.

>+#if _GLIBCXX_INLINE_VERSION
>+// Provide symbol without version namespace decoration for gcc.

For the comment in the code, I think this would be better:

// The compiler expects the contract_violation class to be in an unversioned
// namespace, so provide a forwarding function with the expected symbol name.

Do we want the forwarding function to be a weak symbol? The main
handler function is weak because we want users to be able to override
it with their own handler. But for this new forwarding function, they
can't even declare it (because it has a reserved name that doesn't
demangle to a valid type for the versioned namespace build).
François Dumont Oct. 30, 2023, 6:31 p.m. UTC | #2 
On 30/10/2023 14:45, Jonathan Wakely wrote:
> On Sun, 29 Oct 2023 at 21:11, François Dumont <frs.dumont@gmail.com> wrote:
>> This fixes handle_contract_violation under versioned namespace mode.
>>
>> Tested under Linux x64 and confirmed to also fix Darwin build.
>>
>> libstdc++: [_GLIBCXX_INLINE_VERSION] Provide handle_contract_violation
>> symbol
>>
>> libstdc++-v3/ChangeLog:
>>
>>           * src/experimental/contract.cc
>>           [_GLIBCXX_INLINE_VERSION](handle_contract_violation): Provide
>> symbol
>>           without version namespace decoration for gcc.
>> +#if _GLIBCXX_INLINE_VERSION
>> +// Provide symbol without version namespace decoration for gcc.
> For the comment in the code, I think this would be better:
>
> // The compiler expects the contract_violation class to be in an unversioned
> // namespace, so provide a forwarding function with the expected symbol name.
Sure, I'll update it.
> Do we want the forwarding function to be a weak symbol? The main
> handler function is weak because we want users to be able to override
> it with their own handler. But for this new forwarding function, they
> can't even declare it (because it has a reserved name that doesn't
> demangle to a valid type for the versioned namespace build).
>
Good point, I see no reason neither so I'll remove it.
Jonathan Wakely Oct. 30, 2023, 7:14 p.m. UTC | #3 
On Mon, 30 Oct 2023, 18:31 François Dumont, <frs.dumont@gmail.com> wrote:

>
> On 30/10/2023 14:45, Jonathan Wakely wrote:
> > On Sun, 29 Oct 2023 at 21:11, François Dumont <frs.dumont@gmail.com>
> wrote:
> >> This fixes handle_contract_violation under versioned namespace mode.
> >>
> >> Tested under Linux x64 and confirmed to also fix Darwin build.
> >>
> >> libstdc++: [_GLIBCXX_INLINE_VERSION] Provide handle_contract_violation
> >> symbol
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >>           * src/experimental/contract.cc
> >>           [_GLIBCXX_INLINE_VERSION](handle_contract_violation): Provide
> >> symbol
> >>           without version namespace decoration for gcc.
> >> +#if _GLIBCXX_INLINE_VERSION
> >> +// Provide symbol without version namespace decoration for gcc.
> > For the comment in the code, I think this would be better:
> >
> > // The compiler expects the contract_violation class to be in an
> unversioned
> > // namespace, so provide a forwarding function with the expected symbol
> name.
> Sure, I'll update it.
> > Do we want the forwarding function to be a weak symbol? The main
> > handler function is weak because we want users to be able to override
> > it with their own handler. But for this new forwarding function, they
> > can't even declare it (because it has a reserved name that doesn't
> > demangle to a valid type for the versioned namespace build).
> >
> Good point, I see no reason neither so I'll remove it.
>

Thanks, looks good for trunk (and gcc-13 maybe?) with that change.

>
diff mbox series

Patch

diff --git a/libstdc++-v3/src/experimental/contract.cc b/libstdc++-v3/src/experimental/contract.cc
index 504a6c041f1..d550b49c4eb 100644
--- a/libstdc++-v3/src/experimental/contract.cc
+++ b/libstdc++-v3/src/experimental/contract.cc
@@ -67,3 +67,11 @@  handle_contract_violation (const std::experimental::contract_violation &violatio
   std::cerr << std::endl;
 #endif
 }
+
+#if _GLIBCXX_INLINE_VERSION
+// Provide symbol without version namespace decoration for gcc.
+extern "C" __attribute__ ((weak)) void
+_Z25handle_contract_violationRKNSt12experimental18contract_violationE
+(const std::experimental::contract_violation &violation)
+{ handle_contract_violation(violation); }
+#endif