[PATCH] libstdc++: Refactor implementation of operator+ for std::string
Sorry for the delay. Tested on x86-64 Linux.
-->8--
After consultation with Jonathan, it seemed like a good idea to create a
single function that performed one-allocation string concatenation that
could be used by various different version of operator+. This patch adds
such a function and calls it from the relevant implementations.
libstdc++-v3/ChangeLog:
* include/bits/basic_string.h:
Add common function that performs single-allocation string
concatenation. (__str_cat)
Use __str_cat to perform optimized operator+, where relevant.
* include/bits/basic_string.tcc::
Remove single-allocation implementation of operator+.
Signed-off-by: Will Hawkins
---
libstdc++-v3/include/bits/basic_string.h | 66 --
libstdc++-v3/include/bits/basic_string.tcc | 41 --
2 files changed, 49 insertions(+), 58 deletions(-)
diff --git a/libstdc++-v3/include/bits/basic_string.h
b/libstdc++-v3/include/bits/basic_string.h
index cd244191df4..9c2b57f5a1d 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_GLIBCXX_END_NAMESPACE_CXX11
#endif
+ template
+_GLIBCXX20_CONSTEXPR
+inline _Str
+__str_concat(typename _Str::value_type const* __lhs,
+typename _Str::size_type __lhs_len,
+typename _Str::value_type const* __rhs,
+typename _Str::size_type __rhs_len,
+typename _Str::allocator_type const& __a)
+{
+ typedef typename _Str::allocator_type allocator_type;
+ typedef __gnu_cxx::__alloc_traits _Alloc_traits;
+ _Str __str(_Alloc_traits::_S_select_on_copy(__a));
+ __str.reserve(__lhs_len + __rhs_len);
+ __str.append(__lhs, __lhs_len);
+ __str.append(__rhs, __rhs_len);
+ return __str;
+}
+
// operator+
/**
* @brief Concatenate two strings.
@@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT, _Traits, _Alloc>
+inline basic_string<_CharT, _Traits, _Alloc>
operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
const basic_string<_CharT, _Traits, _Alloc>& __rhs)
{
- basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
- __str.append(__rhs);
- return __str;
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+__rhs.c_str(), __rhs.size(),
+__lhs.get_allocator());
}
/**
@@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
+inline basic_string<_CharT,_Traits,_Alloc>
operator+(const _CharT* __lhs,
- const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+ const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+ __glibcxx_requires_string(__lhs);
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
/**
* @brief Concatenate character and string.
@@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
-operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+inline basic_string<_CharT,_Traits,_Alloc>
+operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
/**
* @brief Concatenate string and C string.
@@ -3538,11 +3570,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
const _CharT* __rhs)
{
- basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
- __str.append(__rhs);
- return __str;
+ __glibcxx_requires_string(__rhs);
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+__rhs, _Traits::length(__rhs),
+__lhs.get_allocator());
}
-
/**
* @brief Concatenate string and character.
* @param __lhs First string.
@@ -3554,11 +3587,10 @@ _GLIBCXX_END_NAMESPAC
Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
Just wanted to see if there was anything else I can do to help move
this over the finish line! Thanks for all the work that you all do!
Sincerely,
Will
On Wed, Oct 19, 2022 at 8:06 PM Will Hawkins wrote:
>
> Sorry for the delay. Tested on x86-64 Linux.
>
> -->8--
>
> After consultation with Jonathan, it seemed like a good idea to create a
> single function that performed one-allocation string concatenation that
> could be used by various different version of operator+. This patch adds
> such a function and calls it from the relevant implementations.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/basic_string.h:
> Add common function that performs single-allocation string
> concatenation. (__str_cat)
> Use __str_cat to perform optimized operator+, where relevant.
> * include/bits/basic_string.tcc::
> Remove single-allocation implementation of operator+.
>
> Signed-off-by: Will Hawkins
> ---
> libstdc++-v3/include/bits/basic_string.h | 66 --
> libstdc++-v3/include/bits/basic_string.tcc | 41 --
> 2 files changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> index cd244191df4..9c2b57f5a1d 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3485,6 +3485,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> _GLIBCXX_END_NAMESPACE_CXX11
> #endif
>
> + template
> +_GLIBCXX20_CONSTEXPR
> +inline _Str
> +__str_concat(typename _Str::value_type const* __lhs,
> +typename _Str::size_type __lhs_len,
> +typename _Str::value_type const* __rhs,
> +typename _Str::size_type __rhs_len,
> +typename _Str::allocator_type const& __a)
> +{
> + typedef typename _Str::allocator_type allocator_type;
> + typedef __gnu_cxx::__alloc_traits _Alloc_traits;
> + _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> + __str.reserve(__lhs_len + __rhs_len);
> + __str.append(__lhs, __lhs_len);
> + __str.append(__rhs, __rhs_len);
> + return __str;
> +}
> +
>// operator+
>/**
> * @brief Concatenate two strings.
> @@ -3494,13 +3512,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
>template
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -basic_string<_CharT, _Traits, _Alloc>
> +inline basic_string<_CharT, _Traits, _Alloc>
> operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> {
> - basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> - __str.append(__rhs);
> - return __str;
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> +__rhs.c_str(), __rhs.size(),
> +__lhs.get_allocator());
> }
>
>/**
> @@ -3511,9 +3530,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
>template
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -basic_string<_CharT,_Traits,_Alloc>
> +inline basic_string<_CharT,_Traits,_Alloc>
> operator+(const _CharT* __lhs,
> - const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> + const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +{
> + __glibcxx_requires_string(__lhs);
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> +__rhs.c_str(), __rhs.size(),
> +__rhs.get_allocator());
> +}
>
>/**
> * @brief Concatenate character and string.
> @@ -3523,8 +3549,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> */
>template
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> -basic_string<_CharT,_Traits,_Alloc>
> -operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>&
> __rhs);
> +inline basic_string<_CharT,_Traits,_Alloc>
> +operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> +{
> + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> + return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
> +__rhs.c_str(), __rhs.size(),
> +__rhs.get_allocator());
> +}
>
>/**
> * @brief Concaten
Re: [PATCH] libstdc++: Optimize operator+(string/char*, string/char*) equally
On Tue, Aug 23, 2022 at 12:33 PM Jonathan Wakely wrote:
>
> On Mon, 22 Aug 2022 at 19:15, Will Hawkins wrote:
> >
> > Until now operator+(char*, string) and operator+(string, char*) had
> > different performance characteristics. The former required a single
> > memory allocation and the latter required two. This patch makes the
> > performance equal.
>
> If you don't have a GCC copyright assignment on file with the FSF then
> please follow the procedure described at https://gcc.gnu.org/dco.html
Thank you.
>
>
> > libstdc++-v3/ChangeLog:
> > * libstdc++-v3/include/bits/basic_string.h (operator+(string,
> > char*)):
> > Remove naive implementation.
> > * libstdc++-v3/include/bits/basic_string.tcc (operator+(string,
> > char*)):
> > Add single-allocation implementation.
> > ---
> > libstdc++-v3/include/bits/basic_string.h | 7 +--
> > libstdc++-v3/include/bits/basic_string.tcc | 21 +
> > 2 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/basic_string.h
> > b/libstdc++-v3/include/bits/basic_string.h
> > index b04fba95678..bc048fc0689 100644
> > --- a/libstdc++-v3/include/bits/basic_string.h
> > +++ b/libstdc++-v3/include/bits/basic_string.h
> > @@ -3523,12 +3523,7 @@ _GLIBCXX_END_NAMESPACE_CXX11
> > _GLIBCXX20_CONSTEXPR
> > inline basic_string<_CharT, _Traits, _Alloc>
>
> Please remove the 'inline' specifier here, since you're moving the
> definition into the non-inline .tcc file.
>
> There's a separate discussion to be had about whether these operator+
> overloads *should* be inline. But for the purposes of this change, we
> want these two operator+ overloads to be consistent, and so they
> should both be non-inline.
Thank you for the feedback. I sent out a v2 of the patch. Again, I
hope that I followed the proper procedure by having my mailer put the
patch in reply to my previous message.
Thank you again!
Will
>
> > operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> > - const _CharT* __rhs)
> > -{
> > - basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> > - __str.append(__rhs);
> > - return __str;
> > -}
> > + const _CharT* __rhs);
> >
> >/**
> > * @brief Concatenate string and character.
> > diff --git a/libstdc++-v3/include/bits/basic_string.tcc
> > b/libstdc++-v3/include/bits/basic_string.tcc
> > index 4563c61429a..95ba8e503e9 100644
> > --- a/libstdc++-v3/include/bits/basic_string.tcc
> > +++ b/libstdc++-v3/include/bits/basic_string.tcc
> > @@ -640,6 +640,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >return __str;
> > }
> >
> > + template
> > +_GLIBCXX20_CONSTEXPR
> > +basic_string<_CharT, _Traits, _Alloc>
> > +operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> > + const _CharT* __rhs)
> > +{
> > + __glibcxx_requires_string(__rhs);
> > + typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
> > + typedef typename __string_type::size_type __size_type;
> > + typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
> > + rebind<_CharT>::other _Char_alloc_type;
> > + typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
> > + const __size_type __len = _Traits::length(__rhs);
> > + __string_type __str(_Alloc_traits::_S_select_on_copy(
> > + __lhs.get_allocator()));
> > + __str.reserve(__len + __lhs.size());
> > + __str.append(__lhs);
> > + __str.append(__rhs, __len);
> > + return __str;
> > +}
> > +
> >template
> > _GLIBCXX_STRING_CONSTEXPR
> > typename basic_string<_CharT, _Traits, _Alloc>::size_type
> > --
> > 2.34.1
> >
>
Re: [PATCH] libstdc++: Optimize operator+(string/char*, string/char*) equally
On Wed, Aug 24, 2022 at 7:03 PM Jonathan Wakely wrote: > > On Wed, 24 Aug 2022 at 23:47, Jonathan Wakely wrote: > > > > On Wed, 24 Aug 2022 at 23:39, Alexandre Oliva wrote: > > > > > > On Aug 24, 2022, Jonathan Wakely via Gcc-patches > > > wrote: > > > > > > >* include/bits/basic_string.h (operator+(const string&, > > > > const char*)): > > > >Remove naive implementation. > > > >* include/bits/basic_string.tcc (operator+(const string&, > > > > const char*)): > > > >Add single-allocation implementation. > > > > > > ISTM this requires the following additional tweak: > > > > > > diff --git a/libstdc++-v3/src/c++11/string-inst.cc > > > b/libstdc++-v3/src/c++11/string-inst.cc > > > index bfae6d902a1dd..2ec0e9d85f947 100644 > > > --- a/libstdc++-v3/src/c++11/string-inst.cc > > > +++ b/libstdc++-v3/src/c++11/string-inst.cc > > > @@ -58,6 +58,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > >template class basic_string; > > >template S operator+(const C*, const S&); > > > + template S operator+(const S&, const C*); > > >template S operator+(C, const S&); > > >template S operator+(const S&, const S&); > > > I realize that I should have noticed that asymmetry as well. Sorry! > > > > > > Without this, I'm getting undefined references to this specialization in > > > libstdc++.so, that I tracked down to a std::system_error ctor in > > > cxx11-ios_failure.o. I got this while testing another patch that might > > > be the reason why the template instantiation doesn't get inlined, but... > > > we can't depend on its being inlined, can we? > > > > Right. But adding that will cause another symbol to be exported, > > probably with the wrong symbol version. > > Oh, but those symbols aren't exported ... they're just needed because > we compile with -fno-implicit-templatesand other symbols in > libstdc++.so require them. > > So that probably is the right fix. I have another change for operator+ > in mind now anyway, which optimizes operator(const string&, char) the > same way, and removes the duplication in those five operator+ > overloads. Let me know if/how I can help. Will > > > > > To fix https://gcc.gnu.org/PR106735 I'm just going to revert it for > > now, and revisit in the morning. >
[PATCH] libstdc++: Refactor implementation of operator+ for std::string
Based on Jonathan's work, here is a patch for the implementation of operator+
on std::string that makes sure we always use the best allocation strategy.
I have attempted to learn from all the feedback that I got on a previous
submission -- I hope I did the right thing.
Passes abi and conformance testing on x86-64 trunk.
Sincerely,
Will
-- >8 --
Create a single function that performs one-allocation string concatenation
that can be used by various different version of operator+.
libstdc++-v3/ChangeLog:
* include/bits/basic_string.h:
Add common function that performs single-allocation string
concatenation. (__str_cat)
Use __str_cat to perform optimized operator+, where relevant.
* include/bits/basic_string.tcc::
Remove single-allocation implementation of operator+.
Signed-off-by: Will Hawkins
---
libstdc++-v3/include/bits/basic_string.h | 66 --
libstdc++-v3/include/bits/basic_string.tcc | 41 --
2 files changed, 49 insertions(+), 58 deletions(-)
diff --git a/libstdc++-v3/include/bits/basic_string.h
b/libstdc++-v3/include/bits/basic_string.h
index 0df64ea98ca..4078651fadb 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_GLIBCXX_END_NAMESPACE_CXX11
#endif
+ template
+_GLIBCXX20_CONSTEXPR
+inline _Str
+__str_concat(typename _Str::value_type const* __lhs,
+typename _Str::size_type __lhs_len,
+typename _Str::value_type const* __rhs,
+typename _Str::size_type __rhs_len,
+typename _Str::allocator_type const& __a)
+{
+ typedef typename _Str::allocator_type allocator_type;
+ typedef __gnu_cxx::__alloc_traits _Alloc_traits;
+ _Str __str(_Alloc_traits::_S_select_on_copy(__a));
+ __str.reserve(__lhs_len + __rhs_len);
+ __str.append(__lhs, __lhs_len);
+ __str.append(__rhs, __rhs_len);
+ return __str;
+}
+
// operator+
/**
* @brief Concatenate two strings.
@@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT, _Traits, _Alloc>
+inline basic_string<_CharT, _Traits, _Alloc>
operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
const basic_string<_CharT, _Traits, _Alloc>& __rhs)
{
- basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
- __str.append(__rhs);
- return __str;
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+__rhs.c_str(), __rhs.size(),
+__lhs.get_allocator());
}
/**
@@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
+inline basic_string<_CharT,_Traits,_Alloc>
operator+(const _CharT* __lhs,
- const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+ const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+ __glibcxx_requires_string(__lhs);
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
/**
* @brief Concatenate character and string.
@@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
*/
template
_GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
-operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+inline basic_string<_CharT,_Traits,_Alloc>
+operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
/**
* @brief Concatenate string and C string.
@@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
const _CharT* __rhs)
{
- basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
- __str.append(__rhs);
- return __str;
+ __glibcxx_requires_string(__rhs);
+ typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+ return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+__rhs, _Traits::length(__rhs),
+__lhs.get_
Re: [PATCH 1/2] libstdc++: Fix pretty printer tests of tuple indexes
Confirming that patch 1 of 2 *does* fix the failing tests here (x86-64).
Will
PS: Please tell me if emails like this are helpful or not. Just trying to help!
On Sun, Sep 4, 2022 at 2:48 PM Philipp Fent via Libstdc++
wrote:
>
> Signed-off-by: Philipp Fent
> ---
> libstdc++-v3/testsuite/libstdc++-prettyprinters/48362.cc | 2 +-
> libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/48362.cc
> b/libstdc++-v3/testsuite/libstdc++-prettyprinters/48362.cc
> index cc91803e247..af335d0d3c7 100644
> --- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/48362.cc
> +++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/48362.cc
> @@ -29,7 +29,7 @@ main()
> // { dg-final { note-test t1 {empty std::tuple} } }
>
>std::tuple> t2{ "Johnny", 5, {} };
> -// { dg-final { regexp-test t2 {std::tuple containing = {\[1\] = "Johnny",
> \[2\] = 5, \[3\] = empty std::tuple}} } }
> +// { dg-final { regexp-test t2 {std::tuple containing = {\[0\] = "Johnny",
> \[1\] = 5, \[2\] = empty std::tuple}} } }
>
>std::cout << "\n";
>return 0; // Mark SPOT
> diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
> b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
> index f97640a0189..bc5978ee69d 100644
> --- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
> +++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
> @@ -166,9 +166,9 @@ main()
> // { dg-final { note-test runiq_ptr {std::unique_ptr = {get() = 0x0}} }
> }
>
>ExTuple tpl(6,7);
> -// { dg-final { note-test tpl {std::tuple containing = {[1] = 6, [2] = 7}} }
> }
> +// { dg-final { note-test tpl {std::tuple containing = {[0] = 6, [1] = 7}} }
> }
>ExTuple &rtpl = tpl;
> -// { dg-final { note-test rtpl {std::tuple containing = {[1] = 6, [2] = 7}}
> } }
> +// { dg-final { note-test rtpl {std::tuple containing = {[0] = 6, [1] = 7}}
> } }
>
>std::error_code e0;
>// { dg-final { note-test e0 {std::error_code = { }} } }
> --
> 2.37.3
>
Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string
On Thu, Sep 8, 2022 at 2:05 PM Jonathan Wakely wrote:
>
>
>
> On Thu, 8 Sep 2022, 18:51 François Dumont via Libstdc++,
> wrote:
>>
>> On 05/09/22 20:30, Will Hawkins wrote:
>> > Based on Jonathan's work, here is a patch for the implementation of
>> > operator+
>> > on std::string that makes sure we always use the best allocation strategy.
>> >
>> > I have attempted to learn from all the feedback that I got on a previous
>> > submission -- I hope I did the right thing.
>> >
>> > Passes abi and conformance testing on x86-64 trunk.
>> >
>> > Sincerely,
>> > Will
>> >
>> > -- >8 --
>> >
>> > Create a single function that performs one-allocation string concatenation
>> > that can be used by various different version of operator+.
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> > * include/bits/basic_string.h:
>> > Add common function that performs single-allocation string
>> > concatenation. (__str_cat)
>> > Use __str_cat to perform optimized operator+, where relevant.
>> > * include/bits/basic_string.tcc::
>> > Remove single-allocation implementation of operator+.
>> >
>> > Signed-off-by: Will Hawkins
>> > ---
>> > libstdc++-v3/include/bits/basic_string.h | 66 --
>> > libstdc++-v3/include/bits/basic_string.tcc | 41 --
>> > 2 files changed, 49 insertions(+), 58 deletions(-)
>> >
>> > diff --git a/libstdc++-v3/include/bits/basic_string.h
>> > b/libstdc++-v3/include/bits/basic_string.h
>> > index 0df64ea98ca..4078651fadb 100644
>> > --- a/libstdc++-v3/include/bits/basic_string.h
>> > +++ b/libstdc++-v3/include/bits/basic_string.h
>> > @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>> > _GLIBCXX_END_NAMESPACE_CXX11
>> > #endif
>> >
>> > + template
>> > +_GLIBCXX20_CONSTEXPR
>> > +inline _Str
>> > +__str_concat(typename _Str::value_type const* __lhs,
>> > + typename _Str::size_type __lhs_len,
>> > + typename _Str::value_type const* __rhs,
>> > + typename _Str::size_type __rhs_len,
>> > + typename _Str::allocator_type const& __a)
>> > +{
>> > + typedef typename _Str::allocator_type allocator_type;
>> > + typedef __gnu_cxx::__alloc_traits _Alloc_traits;
>> > + _Str __str(_Alloc_traits::_S_select_on_copy(__a));
>> > + __str.reserve(__lhs_len + __rhs_len);
>> > + __str.append(__lhs, __lhs_len);
>> > + __str.append(__rhs, __rhs_len);
>> > + return __str;
>> > +}
>> > +
>> > // operator+
>> > /**
>> > * @brief Concatenate two strings.
>> > @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> > */
>> > template
>> > _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>> > -basic_string<_CharT, _Traits, _Alloc>
>> > +inline basic_string<_CharT, _Traits, _Alloc>
>> > operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
>> > const basic_string<_CharT, _Traits, _Alloc>& __rhs)
>> > {
>> > - basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
>> > - __str.append(__rhs);
>> > - return __str;
>> > + typedef basic_string<_CharT, _Traits, _Alloc> _Str;
>> > + return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
>> > + __rhs.c_str(), __rhs.size(),
>>
>> You should use data() rather than c_str() here and all other operators.
>>
>> It is currently the same but is more accurate in your context. Maybe one
>> day it will make a difference.
>
>
>
> I don't think so, that would be a major breaking change, for no benefit. I
> think it's safe to assume they will always stay equivalent now.
Happy to make any changes to the patch that the group thinks are necessary!
Will
>
>
>>
>> > + __lhs.get_allocator());
>> > }
>> >
>> > /**
>> > @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
>> > */
>> > template
>> > _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
>> > -basic_string<_CharT,_Traits,_Alloc>
>> > +inline basic_string<_Cha
[PATCH] bpf: Add documentation for the -mcpu option
Add documentation describing the meaning and values for the -mcpu command-line option. Tested for bpf-unknown-none on x86_64-linux-gnu host. gcc/ChangeLog: * config/bpf/bpf.opt: Add help information for -mcpu. Signed-off-by: Will Hawkins --- gcc/config/bpf/bpf.opt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt index bc5b2220116..acfddebdad7 100644 --- a/gcc/config/bpf/bpf.opt +++ b/gcc/config/bpf/bpf.opt @@ -77,9 +77,11 @@ Enable signed move and memory load instructions. mcpu= Target RejectNegative Joined Var(bpf_isa) Enum(bpf_isa) Init(ISA_V4) +Select the eBPF ISA version to target in code generation. Enum Name(bpf_isa) Type(enum bpf_isa_version) +Valid ISA versions (for use with the -mcpu= option) EnumValue Enum(bpf_isa) String(v1) Value(ISA_V1) -- 2.43.0
Re: [PATCH] bpf: Add documentation for the -mcpu option
On Tue, Feb 20, 2024 at 7:35 AM Jose E. Marchesi wrote: > > > Hello Will. > > Thanks for the patch. > I just installed it on your behalf. Thank you! > > > Add documentation describing the meaning and values for the -mcpu > > command-line option. > > > > Tested for bpf-unknown-none on x86_64-linux-gnu host. > > > > gcc/ChangeLog: > > > > * config/bpf/bpf.opt: Add help information for -mcpu. > > > > Signed-off-by: Will Hawkins > > --- > > gcc/config/bpf/bpf.opt | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt > > index bc5b2220116..acfddebdad7 100644 > > --- a/gcc/config/bpf/bpf.opt > > +++ b/gcc/config/bpf/bpf.opt > > @@ -77,9 +77,11 @@ Enable signed move and memory load instructions. > > > > mcpu= > > Target RejectNegative Joined Var(bpf_isa) Enum(bpf_isa) Init(ISA_V4) > > +Select the eBPF ISA version to target in code generation. > > > > Enum > > Name(bpf_isa) Type(enum bpf_isa_version) > > +Valid ISA versions (for use with the -mcpu= option) > > > > EnumValue > > Enum(bpf_isa) String(v1) Value(ISA_V1)
Re: [PATCH v2 2/2] libstdc++: implement std::generator
On Thu, Dec 21, 2023 at 4:26 PM Arsen Arsenović wrote:
>
> libstdc++-v3/ChangeLog:
>
... snip ...
> + void
> + _M_jump_in(_Coro_handle __rest, _Coro_handle __new) noexcept
> + {
> + __glibcxx_assert(&__new.promise()._M_nest == this);
> + __glibcxx_assert(this->_M_is_bottom());
> + // We're bottom. We're also top of top is unset (note that this is
Should this read:
// We're bottom. We're also top if top is unset (note that this is
?
Very impressive work -- I learned a ton by reading your implementation.
Will
[PATCH] libstdc++: Add test for LWG Issue 3897
Hello!
Thank you, as always, for the great work that you do on libstdc++. The
inout_ptr implementation properly handles the issue raised in LWG 3897
but it seems like having an explicit test might be a good idea.
I hope that this helps!
Will
-- >8 --
Add a test to verify that the implementation of inout_ptr is not
vulnerable to LWG Issue 3897.
libstdc++-v3/ChangeLog:
* testsuite/20_util/smartptr.adapt/inout_ptr/3.cc: New test
for LWG Issue 3897.
Signed-off-by: Will Hawkins
---
.../20_util/smartptr.adapt/inout_ptr/3.cc | 17 +
1 file changed, 17 insertions(+)
create mode 100644 libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
diff --git a/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
new file mode 100644
index 000..f9114dc57b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
@@ -0,0 +1,17 @@
+// { dg-do run { target c++23 } }
+
+#include
+#include
+
+// C++23 [inout.ptr.t] Class template inout_ptr_t
+// Verify that implementation handles LWG Issue 3897
+void nuller(int **p) {
+ *p = nullptr;
+}
+
+int main(int, char **) {
+ int *i = new int{5};
+ nuller(std::inout_ptr(i));
+
+ VERIFY(i == nullptr);
+}
--
2.41.0
Re: [PATCH] libstdc++: Add test for LWG Issue 3897
On Tue, Dec 5, 2023 at 10:46 AM Jonathan Wakely wrote: > > On Mon, 4 Dec 2023 at 16:42, Will Hawkins wrote: > > > > Hello! > > > > Thank you, as always, for the great work that you do on libstdc++. The > > inout_ptr implementation properly handles the issue raised in LWG 3897 > > but it seems like having an explicit test might be a good idea. > > Thanks, Will, we should definitely have a test for this. > > I've tweaked it a bit to avoid leaking the pointer (in case anybody > runs the tests under valgrind or ASan) and to add your new test to the Of course ... how could I forget to delete the pointer? I'm a goofball. > existing file (to avoid the overhead of a separate test just for this > one check). Makes perfect sense. I wasn't sure how you typically handle that. I will know for the future. > > See attached ... Thank you for the feedback! I look forward to the next time I can help! Will
[PATCH] btf: Protect BTF_KIND_INFO against invalid kind
If the user provides a kind value that is more than 5 bits, the BTF_KIND_INFO macro would emit incorrect values for info (by clobbering values of the kind flag). Tested on x86_64-redhat-linux. include/ChangeLog: * btf.h (BTF_TYPE_INFO): Protect against user providing invalid kind. Signed-off-by: Will Hawkins --- Notes: I have a small out-of-tree test but was not sure whether a) it should be included and/or b) where it should be included. If you would like me to include it, please just let me know where it should go! include/btf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/btf.h b/include/btf.h index 3f45ffb0b6b..0c3e1a1cf51 100644 --- a/include/btf.h +++ b/include/btf.h @@ -82,7 +82,7 @@ struct btf_type }; }; -/* The folloing macros access the information encoded in btf_type.info. */ +/* The following macros access the information encoded in btf_type.info. */ /* Type kind. See below. */ #define BTF_INFO_KIND(info)(((info) >> 24) & 0x1f) /* Number of entries of variable length data following certain type kinds. @@ -95,7 +95,7 @@ struct btf_type /* Encoding for struct btf_type.info. */ #define BTF_TYPE_INFO(kind, kflag, vlen) \ - kflag) ? 1 : 0 ) << 31) | ((kind) << 24) | ((vlen) & 0x)) + kflag) ? 1 : 0 ) << 31) | ((kind & 0x1f) << 24) | ((vlen) & 0x)) #define BTF_KIND_UNKN 0 /* Unknown or invalid. */ #define BTF_KIND_INT 1 /* Integer. */ -- 2.45.2
Re: [PATCH] btf: Protect BTF_KIND_INFO against invalid kind
On Mon, Jul 29, 2024 at 2:14 PM David Faust wrote: > > > On 7/29/24 07:42, Will Hawkins wrote: > > If the user provides a kind value that is more than 5 bits, the > > BTF_KIND_INFO macro would emit incorrect values for info (by clobbering > > values of the kind flag). > > > > Tested on x86_64-redhat-linux. > > OK, thanks. Just let me know if there is anything else that you need from me! Will > > > > > include/ChangeLog: > > > > * btf.h (BTF_TYPE_INFO): Protect against user providing invalid > > kind. > > > > Signed-off-by: Will Hawkins > > --- > > > > Notes: > > I have a small out-of-tree test but was not sure whether a) it should > >be included and/or b) where it should be included. If you > > would > >like me to include it, please just let me know where it > > should go! > > > > include/btf.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/btf.h b/include/btf.h > > index 3f45ffb0b6b..0c3e1a1cf51 100644 > > --- a/include/btf.h > > +++ b/include/btf.h > > @@ -82,7 +82,7 @@ struct btf_type > >}; > > }; > > > > -/* The folloing macros access the information encoded in btf_type.info. */ > > +/* The following macros access the information encoded in btf_type.info. > > */ > > /* Type kind. See below. */ > > #define BTF_INFO_KIND(info) (((info) >> 24) & 0x1f) > > /* Number of entries of variable length data following certain type kinds. > > @@ -95,7 +95,7 @@ struct btf_type > > > > /* Encoding for struct btf_type.info. */ > > #define BTF_TYPE_INFO(kind, kflag, vlen) \ > > - kflag) ? 1 : 0 ) << 31) | ((kind) << 24) | ((vlen) & 0x)) > > + kflag) ? 1 : 0 ) << 31) | ((kind & 0x1f) << 24) | ((vlen) & 0x)) > > > > #define BTF_KIND_UNKN0 /* Unknown or invalid. */ > > #define BTF_KIND_INT 1 /* Integer. */
Re: [PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.3)
Hello everyone!
I know every member of the community is very busy, but I am following
up on this patch to conform to the 'ping' etiquette.
Please let me know what comments you have about this patch and how I
can modify it to make sure that it meets standards.
Thanks for everything that you all do to make GCC the best compiler out there.
Will
On Fri, May 18, 2018 at 5:34 PM, Will Hawkins wrote:
> Hello again!
>
> Thanks to the feedback of Mr. Myers and those on the PR, I have
> created a version 3 of this patch. This version introduces a new
> warning flag (enabled at Wall) -Wignored-asm-name that will flag cases
> where the user specifies an ASM name that the compiler ignores.
>
> Test cases included. Results from make bootstrap and/or make -k check
> are available upon request.
>
> Please let me know what I can do to make this better and bring it up
> to the standards of the community! Thanks again for the feedback on
> this patch during the previous two revisions!
>
> Sincerely,
> Will Hawkins
>
>
> 2018-05-18 Will Hawkins
>
> PR c,c++/85444
> * gcc/c/c-decl.c: Warn about ignored asm label for
> typedef declaration
> * gcc/cp/decl.c: Warn about ignored asm label for
> typedef declaration
> * gcc/testsuite/gcc.dg/asm-pr85444.c: c testcase.
> * gcc/testsuite/g++.dg/asm-pr85444.C: c++ testcase.
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index c48d6dc..ab3a9af 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -595,6 +595,10 @@ Wignored-attributes
> C C++ Var(warn_ignored_attributes) Init(1) Warning
> Warn whenever attributes are ignored.
>
> +Wignored-asm-name
> +C C++ Var(warn_ignored_asm_name) Warning LangEnabledBy(C C++,Wall)
> +Warn whenever assembler names are specified but ignored.
> +
> Wincompatible-pointer-types
> C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
> Warn when there is a conversion between pointers that have incompatible
> types.
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 3c4b18e..5a1ecd7 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -5177,7 +5177,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
>if (!DECL_FILE_SCOPE_P (decl)
>&& variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
> add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
> -
> + if (asmspec_tree != NULL_TREE)
> +{
> + warning (OPT_Wignored_asm_name, "asm-specifier is ignored in "
> + "typedef declaration");
> +}
>rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
> }
>
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 10e3079..4c3ee36 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6981,6 +6981,11 @@ cp_finish_decl (tree decl, tree init, bool
> init_const_expr_p,
>/* Take care of TYPE_DECLs up front. */
>if (TREE_CODE (decl) == TYPE_DECL)
> {
> + if (asmspec_tree != NULL_TREE)
> +{
> + warning (OPT_Wignored_asm_name, "asm-specifier is ignored for "
> + "typedef declarations");
> +}
>if (type != error_mark_node
>&& MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
> {
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ca3772b..63f81f4 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -286,7 +286,8 @@ Objective-C and Objective-C++ Dialects}.
> -Wformat-y2k -Wframe-address @gol
> -Wframe-larger-than=@var{len} -Wno-free-nonheap-object
> -Wjump-misses-init @gol
> -Wif-not-aligned @gol
> --Wignored-qualifiers -Wignored-attributes -Wincompatible-pointer-types @gol
> +-Wignored-qualifiers -Wignored-attributes -Wignored-asm-name @gol
> +-Wincompatible-pointer-types @gol
> -Wimplicit -Wimplicit-fallthrough -Wimplicit-fallthrough=@var{n} @gol
> -Wimplicit-function-declaration -Wimplicit-int @gol
> -Winit-self -Winline -Wno-int-conversion -Wint-in-bool-context @gol
> @@ -4523,6 +4524,14 @@ Warn when an attribute is ignored. This is
> different from the
> to drop an attribute, not that the attribute is either unknown, used in a
> wrong place, etc. This warning is enabled by default.
>
> +@item -Wignored-asm-name @r{(C and C++ only)}
> +@opindex Wignored-asm-name
> +@opindex Wno-ignored-asm-name
> +Warn when an assembler name is given but ignored. For C and C++, this
> +happens when a @code{typdef} declaration is given an assembler name.
> +
> +This warning is also enabled by @option{-Wall}.
> +
> @item -Wmain
> @opindex Wmain
> @opindex Wno-main
> diff --git a/gcc/tes
[PATCH] Warn for ignored ASM labels on typdef declarations
Hello everyone!
First, let me ask for your patience -- this is the first patch I am
submitting to gcc. I tried very hard to follow the proper procedure,
but I am sure that I've missed something.
According to https://gcc.gnu.org/onlinedocs/gcc/Asm-Labels.html#Asm-Labels
asm labels have no effect on anything other than variable and function
declarations. When an asm label is included on a typedef, the compiler
happily goes along but does not indicate anything to the user.
For instance, when parsing
typedef struct {} x asm ("X");
the compiler silently discards the asm label. This patch will generate
a warning (at -Wpedantic) for such a situation. This is in Bugzilla as
85444.
Again, I hope that I have done all the proper formatting so that it is
inline with all the contribution guidelines. Thank you for your
patience!
Will
2018-04-18 Will Hawkins
* gcc/c/c-decl.c: Warn about ignored asm label for
typedef declaration
* gcc/cp/decl.c: Warn about ignored asm label for
typedef declaration
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index f0198ec..e9c0a72 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5166,7 +5166,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
if (!DECL_FILE_SCOPE_P (decl)
&& variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wpedantic, "asm-specifier is ignored in "
+ "typedef declaration");
+}
rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
}
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 44a152b..88b4b94 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7069,6 +7069,11 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
/* Take care of TYPE_DECLs up front. */
if (TREE_CODE (decl) == TYPE_DECL)
{
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wpedantic, "asm-specifier is ignored for "
+ "typedef declarations");
+}
if (type != error_mark_node
&& MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
{
Re: [PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.2)
Thanks to Mr. Meyers for his comments. Here is an updated version of
the patch. Test cases are included this time.
Tested with 'make bootstrap' on x86_64-pc-linux-gnu. Results from make
-k check available upon request.
I hope that this one is better! Thanks again for everyone's patience!
Will
2018-04-18 Will Hawkins
PR c,c++/85444
* gcc/c/c-decl.c: Warn about ignored asm label for
typedef declaration
* gcc/cp/decl.c: Warn about ignored asm label for
typedef declaration
* gcc/testsuite/gcc.dg/asm-pr85444.c: c testcase.
* gcc/testsuite/g++.dg/asm-pr85444.C: c++ testcase.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index f0198ec..f18bb7d 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5166,7 +5166,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
if (!DECL_FILE_SCOPE_P (decl)
&& variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wignored_qualifiers, "asm-specifier is ignored in "
+ "typedef declaration");
+}
rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
}
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 44a152b..32c2dfa 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7069,6 +7069,11 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
/* Take care of TYPE_DECLs up front. */
if (TREE_CODE (decl) == TYPE_DECL)
{
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wignored_qualifiers, "asm-specifier is ignored for "
+ "typedef declarations");
+}
if (type != error_mark_node
&& MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
{
diff --git a/gcc/testsuite/g++.dg/asm-pr85444.C
b/gcc/testsuite/g++.dg/asm-pr85444.C
new file mode 100644
index 000..9e4b6c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-pr85444.C
@@ -0,0 +1,13 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+ { dg-do compile }
+ { dg-options "-Wignored-qualifiers" } */
+
+typedef struct
+{
+ int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/asm-pr85444.c
b/gcc/testsuite/gcc.dg/asm-pr85444.c
new file mode 100644
index 000..9e4b6c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-pr85444.c
@@ -0,0 +1,13 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+ { dg-do compile }
+ { dg-options "-Wignored-qualifiers" } */
+
+typedef struct
+{
+ int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+ return 0;
+}
Re: [PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.2)
On Mon, Apr 30, 2018 at 7:51 AM, Joseph Myers wrote:
> On Sat, 28 Apr 2018, Will Hawkins wrote:
>
>> +{
>> + warning (OPT_Wignored_qualifiers, "asm-specifier is ignored in "
>> + "typedef declaration");
>
> This does not match the documented semantics of -Wignored-qualifiers. I
> don't think it's appropriate to expand those semantics to include this
> warning either.
>
I agree! It was, however, the closest of all the categories that I
could find that seemed to match the warning that I am trying to emit.
I will go back and review the categories and see if there is something
that I missed.
I am certainly not asking you to "do my homework" for me, but does
anyone have suggestions for a category that might house this warning?
Thanks!
Will
> --
> Joseph S. Myers
> jos...@codesourcery.com
Re: [PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.3)
Hello again!
Thanks to the feedback of Mr. Myers and those on the PR, I have
created a version 3 of this patch. This version introduces a new
warning flag (enabled at Wall) -Wignored-asm-name that will flag cases
where the user specifies an ASM name that the compiler ignores.
Test cases included. Results from make bootstrap and/or make -k check
are available upon request.
Please let me know what I can do to make this better and bring it up
to the standards of the community! Thanks again for the feedback on
this patch during the previous two revisions!
Sincerely,
Will Hawkins
2018-05-18 Will Hawkins
PR c,c++/85444
* gcc/c/c-decl.c: Warn about ignored asm label for
typedef declaration
* gcc/cp/decl.c: Warn about ignored asm label for
typedef declaration
* gcc/testsuite/gcc.dg/asm-pr85444.c: c testcase.
* gcc/testsuite/g++.dg/asm-pr85444.C: c++ testcase.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c48d6dc..ab3a9af 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -595,6 +595,10 @@ Wignored-attributes
C C++ Var(warn_ignored_attributes) Init(1) Warning
Warn whenever attributes are ignored.
+Wignored-asm-name
+C C++ Var(warn_ignored_asm_name) Warning LangEnabledBy(C C++,Wall)
+Warn whenever assembler names are specified but ignored.
+
Wincompatible-pointer-types
C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
Warn when there is a conversion between pointers that have incompatible types.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 3c4b18e..5a1ecd7 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5177,7 +5177,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
if (!DECL_FILE_SCOPE_P (decl)
&& variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wignored_asm_name, "asm-specifier is ignored in "
+ "typedef declaration");
+}
rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
}
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 10e3079..4c3ee36 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6981,6 +6981,11 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
/* Take care of TYPE_DECLs up front. */
if (TREE_CODE (decl) == TYPE_DECL)
{
+ if (asmspec_tree != NULL_TREE)
+{
+ warning (OPT_Wignored_asm_name, "asm-specifier is ignored for "
+ "typedef declarations");
+}
if (type != error_mark_node
&& MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
{
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ca3772b..63f81f4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -286,7 +286,8 @@ Objective-C and Objective-C++ Dialects}.
-Wformat-y2k -Wframe-address @gol
-Wframe-larger-than=@var{len} -Wno-free-nonheap-object
-Wjump-misses-init @gol
-Wif-not-aligned @gol
--Wignored-qualifiers -Wignored-attributes -Wincompatible-pointer-types @gol
+-Wignored-qualifiers -Wignored-attributes -Wignored-asm-name @gol
+-Wincompatible-pointer-types @gol
-Wimplicit -Wimplicit-fallthrough -Wimplicit-fallthrough=@var{n} @gol
-Wimplicit-function-declaration -Wimplicit-int @gol
-Winit-self -Winline -Wno-int-conversion -Wint-in-bool-context @gol
@@ -4523,6 +4524,14 @@ Warn when an attribute is ignored. This is
different from the
to drop an attribute, not that the attribute is either unknown, used in a
wrong place, etc. This warning is enabled by default.
+@item -Wignored-asm-name @r{(C and C++ only)}
+@opindex Wignored-asm-name
+@opindex Wno-ignored-asm-name
+Warn when an assembler name is given but ignored. For C and C++, this
+happens when a @code{typdef} declaration is given an assembler name.
+
+This warning is also enabled by @option{-Wall}.
+
@item -Wmain
@opindex Wmain
@opindex Wno-main
diff --git a/gcc/testsuite/g++.dg/asm-pr85444.C
b/gcc/testsuite/g++.dg/asm-pr85444.C
new file mode 100644
index 000..f1f8f61
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-pr85444.C
@@ -0,0 +1,13 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+ { dg-do compile }
+ { dg-options "-Wignored-asm-name" } */
+
+typedef struct
+{
+ int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/asm-pr85444.c
b/gcc/testsuite/gcc.dg/asm-pr85444.c
new file mode 100644
index 000..73ccea0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-pr85444.c
@@ -0,0 +1,14 @@
+/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
+ { dg-do compile }
+ { dg-options "-Wignored-asm-name" } */
+
+typedef struct
+{
+ int a;
+} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
+
+int main()
+{
+ retur
