public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/4] Use const for some function arguments.
  2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
  2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
  2020-02-06 11:06 ` [PATCH 1/4] Remove 2 dead variables in bid_internal.h Martin Liska
@ 2020-02-06 11:06 ` Martin Liska
  2020-02-07 17:22   ` Segher Boessenkool
  2020-05-05 15:17   ` Jeff Law
  2020-02-06 11:06 ` [PATCH 3/4] Put index check before use Martin Liska
  2020-02-06 11:18 ` [PATCH 0/4] Fix various minor issues seen with cppcheck Richard Biener
  4 siblings, 2 replies; 15+ messages in thread
From: Martin Liska @ 2020-02-06 11:06 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]


gcc/ChangeLog:

2020-02-04  Martin Liska  <mliska@suse.cz>

	PR c/92472.
	* alloc-pool.h: Use const for some arguments.
	* bitmap.h: Likewise.
	* mem-stats.h: Likewise.
	* sese.h (get_entry_bb): Likewise.
	(get_exit_bb): Likewise.
---
 gcc/alloc-pool.h | 2 +-
 gcc/bitmap.h     | 2 +-
 gcc/mem-stats.h  | 4 ++--
 gcc/sese.h       | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Use-const-for-some-function-arguments.patch --]
[-- Type: text/x-patch; name="0002-Use-const-for-some-function-arguments.patch", Size: 2030 bytes --]

diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index 1686a8b5f91..fd7194bfea4 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -60,7 +60,7 @@ public:
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void
-  dump (mem_location *loc, mem_usage &total) const
+  dump (mem_location *loc, const mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
diff --git a/gcc/bitmap.h b/gcc/bitmap.h
index d52fd5bb905..b481f4b2606 100644
--- a/gcc/bitmap.h
+++ b/gcc/bitmap.h
@@ -237,7 +237,7 @@ public:
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void
-  dump (mem_location *loc, mem_usage &total) const
+  dump (mem_location *loc, const mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 21d038bb370..4a3177dd4fc 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -70,7 +70,7 @@ public:
 
   /* Return true if the memory location is equal to OTHER.  */
   int
-  equal (mem_location &other)
+  equal (const mem_location &other)
   {
     return m_filename == other.m_filename && m_function == other.m_function
       && m_line == other.m_line;
@@ -203,7 +203,7 @@ public:
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void
-  dump (mem_location *loc, mem_usage &total) const
+  dump (mem_location *loc, const mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
diff --git a/gcc/sese.h b/gcc/sese.h
index 8afea28b07e..74d3fe3cd8a 100644
--- a/gcc/sese.h
+++ b/gcc/sese.h
@@ -45,7 +45,7 @@ void dump_sese (const sese_l &);
 /* Get the entry of an sese S.  */
 
 static inline basic_block
-get_entry_bb (sese_l &s)
+get_entry_bb (const sese_l &s)
 {
   return s.entry->dest;
 }
@@ -53,7 +53,7 @@ get_entry_bb (sese_l &s)
 /* Get the exit of an sese S.  */
 
 static inline basic_block
-get_exit_bb (sese_l &s)
+get_exit_bb (const sese_l &s)
 {
   return s.exit->src;
 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/4] Use const for template argument.
  2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
@ 2020-02-06 11:06 ` Martin Liska
  2020-05-05 15:17   ` Jeff Law
  2020-05-06 10:01   ` Jonathan Wakely
  2020-02-06 11:06 ` [PATCH 1/4] Remove 2 dead variables in bid_internal.h Martin Liska
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Martin Liska @ 2020-02-06 11:06 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]


libstdc++-v3/ChangeLog:

2020-02-04  Martin Liska  <mliska@suse.cz>

	PR c/92472.
	* include/parallel/multiway_merge.h:
	Use const for _Compare template argument.
---
 libstdc++-v3/include/parallel/multiway_merge.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-Use-const-for-template-argument.patch --]
[-- Type: text/x-patch; name="0004-Use-const-for-template-argument.patch", Size: 995 bytes --]

diff --git a/libstdc++-v3/include/parallel/multiway_merge.h b/libstdc++-v3/include/parallel/multiway_merge.h
index 983c7b2bd9a..97a9ce0feb7 100644
--- a/libstdc++-v3/include/parallel/multiway_merge.h
+++ b/libstdc++-v3/include/parallel/multiway_merge.h
@@ -118,7 +118,7 @@ namespace __gnu_parallel
        *  @return @c true if less. */
       friend bool
       operator<(_GuardedIterator<_RAIter, _Compare>& __bi1,
-		_GuardedIterator<_RAIter, _Compare>& __bi2)
+		_GuardedIterator<_RAIter, const _Compare>& __bi2)
       {
 	if (__bi1._M_current == __bi1._M_end)       // __bi1 is sup
 	  return __bi2._M_current == __bi2._M_end;  // __bi2 is not sup
@@ -188,7 +188,7 @@ namespace __gnu_parallel
        *  @return @c true if less. */
       friend bool
       operator<(_UnguardedIterator<_RAIter, _Compare>& __bi1,
-		_UnguardedIterator<_RAIter, _Compare>& __bi2)
+		_UnguardedIterator<_RAIter, const _Compare>& __bi2)
       {
 	// Normal compare.
 	return (__bi1.__comp)(*__bi1, *__bi2);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/4] Fix various minor issues seen with cppcheck
@ 2020-02-06 11:06 Martin Liska
  2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Martin Liska @ 2020-02-06 11:06 UTC (permalink / raw)
  To: gcc-patches

Hi.

The series is about small issues that were spotted with cppcheck
and where David Binderman suggested a patch.

It's probably a stage1 material?

Martin

Martin Liska (4):
  Remove 2 dead variables in bid_internal.h.
  Use const for some function arguments.
  Put index check before use.
  Use const for template argument.

 gcc/alloc-pool.h                               | 2 +-
 gcc/bitmap.h                                   | 2 +-
 gcc/mem-stats.h                                | 4 ++--
 gcc/sese.h                                     | 4 ++--
 libgcc/config/libbid/bid_internal.h            | 4 ----
 liboffloadmic/runtime/offload_target.cpp       | 2 +-
 libstdc++-v3/include/parallel/multiway_merge.h | 4 ++--
 7 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.25.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/4] Put index check before use.
  2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
                   ` (2 preceding siblings ...)
  2020-02-06 11:06 ` [PATCH 2/4] Use const for some function arguments Martin Liska
@ 2020-02-06 11:06 ` Martin Liska
  2020-05-05 15:18   ` Jeff Law
  2020-02-06 11:18 ` [PATCH 0/4] Fix various minor issues seen with cppcheck Richard Biener
  4 siblings, 1 reply; 15+ messages in thread
From: Martin Liska @ 2020-02-06 11:06 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]


liboffloadmic/ChangeLog:

2020-02-04  Martin Liska  <mliska@suse.cz>

	PR other/89860.
	* runtime/offload_target.cpp: Put index check
	before its use.
---
 liboffloadmic/runtime/offload_target.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Put-index-check-before-use.patch --]
[-- Type: text/x-patch; name="0003-Put-index-check-before-use.patch", Size: 759 bytes --]

diff --git a/liboffloadmic/runtime/offload_target.cpp b/liboffloadmic/runtime/offload_target.cpp
index 8273faac13b..16ba4a32991 100644
--- a/liboffloadmic/runtime/offload_target.cpp
+++ b/liboffloadmic/runtime/offload_target.cpp
@@ -329,7 +329,7 @@ void OffloadDescriptor::merge_var_descs(
                 }
             }
             // instead of m_vars[i].type.dst we will use m_vars_extra[i].type_dst
-            if (m_vars[i].type.dst == c_extended_type && i < vars_total) {
+            if (i < vars_total && m_vars[i].type.dst == c_extended_type) {
                 VarDescExtendedType *etype =
                     reinterpret_cast<VarDescExtendedType*>(vars[i].into);
                 m_vars_extra[i].type_dst = etype->extended_type;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] Remove 2 dead variables in bid_internal.h.
  2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
  2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
@ 2020-02-06 11:06 ` Martin Liska
  2020-05-05 15:16   ` Jeff Law
  2020-02-06 11:06 ` [PATCH 2/4] Use const for some function arguments Martin Liska
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Martin Liska @ 2020-02-06 11:06 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]


libgcc/config/libbid/ChangeLog:

2020-02-04  Martin Liska  <mliska@suse.cz>

	PR libgcc/92565
	* bid_internal.h (handle_UF_128_rem): Remove unused variable.
	(handle_UF_128): Likewise.
---
 libgcc/config/libbid/bid_internal.h | 4 ----
 1 file changed, 4 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-2-dead-variables-in-bid_internal.h.patch --]
[-- Type: text/x-patch; name="0001-Remove-2-dead-variables-in-bid_internal.h.patch", Size: 687 bytes --]

diff --git a/libgcc/config/libbid/bid_internal.h b/libgcc/config/libbid/bid_internal.h
index cef36a9bb80..9baa098caac 100644
--- a/libgcc/config/libbid/bid_internal.h
+++ b/libgcc/config/libbid/bid_internal.h
@@ -1540,8 +1540,6 @@ handle_UF_128_rem (UINT128 * pres, UINT64 sgn, int expon, UINT128 CQ,
     __shr_128 (CQ, Qh, amount);
   }
 
-  expon = 0;
-
 #ifndef IEEE_ROUND_NEAREST_TIES_AWAY
 #ifndef IEEE_ROUND_NEAREST
   if (!(*prounding_mode))
@@ -1676,8 +1674,6 @@ handle_UF_128 (UINT128 * pres, UINT64 sgn, int expon, UINT128 CQ,
     __shr_128 (CQ, Qh, amount);
   }
 
-  expon = 0;
-
 #ifndef IEEE_ROUND_NEAREST_TIES_AWAY
 #ifndef IEEE_ROUND_NEAREST
   if (!(*prounding_mode))

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/4] Fix various minor issues seen with cppcheck
  2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
                   ` (3 preceding siblings ...)
  2020-02-06 11:06 ` [PATCH 3/4] Put index check before use Martin Liska
@ 2020-02-06 11:18 ` Richard Biener
  4 siblings, 0 replies; 15+ messages in thread
From: Richard Biener @ 2020-02-06 11:18 UTC (permalink / raw)
  To: Martin Liska; +Cc: GCC Patches

On Thu, Feb 6, 2020 at 12:07 PM Martin Liska <mliska@suse.cz> wrote:
>
> Hi.
>
> The series is about small issues that were spotted with cppcheck
> and where David Binderman suggested a patch.
>
> It's probably a stage1 material?

Yes.

> Martin
>
> Martin Liska (4):
>   Remove 2 dead variables in bid_internal.h.
>   Use const for some function arguments.
>   Put index check before use.
>   Use const for template argument.
>
>  gcc/alloc-pool.h                               | 2 +-
>  gcc/bitmap.h                                   | 2 +-
>  gcc/mem-stats.h                                | 4 ++--
>  gcc/sese.h                                     | 4 ++--
>  libgcc/config/libbid/bid_internal.h            | 4 ----
>  liboffloadmic/runtime/offload_target.cpp       | 2 +-
>  libstdc++-v3/include/parallel/multiway_merge.h | 4 ++--
>  7 files changed, 9 insertions(+), 13 deletions(-)
>
> --
> 2.25.0
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] Use const for some function arguments.
  2020-02-06 11:06 ` [PATCH 2/4] Use const for some function arguments Martin Liska
@ 2020-02-07 17:22   ` Segher Boessenkool
  2020-02-17 10:05     ` Martin Liška
  2020-05-05 15:17   ` Jeff Law
  1 sibling, 1 reply; 15+ messages in thread
From: Segher Boessenkool @ 2020-02-07 17:22 UTC (permalink / raw)
  To: Martin Liska; +Cc: gcc-patches

On Tue, Feb 04, 2020 at 02:55:14PM +0100, Martin Liska wrote:
> 
> gcc/ChangeLog:
> 
> 2020-02-04  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/92472.

That trailing dot should not be there (in some other patches as well).


Segher

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] Use const for some function arguments.
  2020-02-07 17:22   ` Segher Boessenkool
@ 2020-02-17 10:05     ` Martin Liška
  2020-02-17 10:39       ` Richard Biener
  0 siblings, 1 reply; 15+ messages in thread
From: Martin Liška @ 2020-02-17 10:05 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On 2/7/20 6:21 PM, Segher Boessenkool wrote:
> On Tue, Feb 04, 2020 at 02:55:14PM +0100, Martin Liska wrote:
>>
>> gcc/ChangeLog:
>>
>> 2020-02-04  Martin Liska  <mliska@suse.cz>
>>
>> 	PR c/92472.
> 
> That trailing dot should not be there (in some other patches as well).

Sure. I'll fix it.

@Richi: May I install the patch once stage1 opens, or will it require
another review?

Martin

> 
> 
> Segher
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] Use const for some function arguments.
  2020-02-17 10:05     ` Martin Liška
@ 2020-02-17 10:39       ` Richard Biener
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Biener @ 2020-02-17 10:39 UTC (permalink / raw)
  To: Martin Liška; +Cc: Segher Boessenkool, GCC Patches

On Mon, Feb 17, 2020 at 11:05 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 2/7/20 6:21 PM, Segher Boessenkool wrote:
> > On Tue, Feb 04, 2020 at 02:55:14PM +0100, Martin Liska wrote:
> >>
> >> gcc/ChangeLog:
> >>
> >> 2020-02-04  Martin Liska  <mliska@suse.cz>
> >>
> >>      PR c/92472.
> >
> > That trailing dot should not be there (in some other patches as well).
>
> Sure. I'll fix it.
>
> @Richi: May I install the patch once stage1 opens, or will it require
> another review?

Yes, OK for stage1.

> Martin
>
> >
> >
> > Segher
> >
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] Remove 2 dead variables in bid_internal.h.
  2020-02-06 11:06 ` [PATCH 1/4] Remove 2 dead variables in bid_internal.h Martin Liska
@ 2020-05-05 15:16   ` Jeff Law
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-05-05 15:16 UTC (permalink / raw)
  To: Martin Liska, gcc-patches

On Tue, 2020-02-04 at 14:54 +0100, Martin Liska wrote:
> libgcc/config/libbid/ChangeLog:
> 
> 2020-02-04  Martin Liska  <mliska@suse.cz>
> 
> 	PR libgcc/92565
> 	* bid_internal.h (handle_UF_128_rem): Remove unused variable.
> 	(handle_UF_128): Likewise.
OK for master.  And patches that drop unused variables should be considered
obvious and don't need a review cycle.

jeff


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] Use const for some function arguments.
  2020-02-06 11:06 ` [PATCH 2/4] Use const for some function arguments Martin Liska
  2020-02-07 17:22   ` Segher Boessenkool
@ 2020-05-05 15:17   ` Jeff Law
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-05-05 15:17 UTC (permalink / raw)
  To: Martin Liska, gcc-patches

On Tue, 2020-02-04 at 14:55 +0100, Martin Liska wrote:
> gcc/ChangeLog:
> 
> 2020-02-04  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/92472.
> 	* alloc-pool.h: Use const for some arguments.
> 	* bitmap.h: Likewise.
> 	* mem-stats.h: Likewise.
> 	* sese.h (get_entry_bb): Likewise.
> 	(get_exit_bb): Likewise.
OK.  And constification should be considered obvious and doesn't need a review
cycle.

jeff
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] Use const for template argument.
  2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
@ 2020-05-05 15:17   ` Jeff Law
  2020-05-06 10:01   ` Jonathan Wakely
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-05-05 15:17 UTC (permalink / raw)
  To: Martin Liska, gcc-patches

On Tue, 2020-02-04 at 14:55 +0100, Martin Liska wrote:
> libstdc++-v3/ChangeLog:
> 
> 2020-02-04  Martin Liska  <mliska@suse.cz>
> 
> 	PR c/92472.
> 	* include/parallel/multiway_merge.h:
> 	Use const for _Compare template argument.
OK
jeff
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] Put index check before use.
  2020-02-06 11:06 ` [PATCH 3/4] Put index check before use Martin Liska
@ 2020-05-05 15:18   ` Jeff Law
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-05-05 15:18 UTC (permalink / raw)
  To: Martin Liska, gcc-patches

On Tue, 2020-02-04 at 14:57 +0100, Martin Liska wrote:
> liboffloadmic/ChangeLog:
> 
> 2020-02-04  Martin Liska  <mliska@suse.cz>
> 
> 	PR other/89860.
> 	* runtime/offload_target.cpp: Put index check
> 	before its use.
OK
jeff
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] Use const for template argument.
  2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
  2020-05-05 15:17   ` Jeff Law
@ 2020-05-06 10:01   ` Jonathan Wakely
  2020-05-07 20:44     ` Jonathan Wakely
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2020-05-06 10:01 UTC (permalink / raw)
  To: Martin Liska; +Cc: gcc-patches, libstdc++

On 04/02/20 14:55 +0100, Martin Liska wrote:
>diff --git a/libstdc++-v3/include/parallel/multiway_merge.h b/libstdc++-v3/include/parallel/multiway_merge.h
>index 983c7b2bd9a..97a9ce0feb7 100644
>--- a/libstdc++-v3/include/parallel/multiway_merge.h
>+++ b/libstdc++-v3/include/parallel/multiway_merge.h
>@@ -118,7 +118,7 @@ namespace __gnu_parallel
>        *  @return @c true if less. */
>       friend bool
>       operator<(_GuardedIterator<_RAIter, _Compare>& __bi1,
>-               _GuardedIterator<_RAIter, _Compare>& __bi2)
>+               _GuardedIterator<_RAIter, const _Compare>& __bi2)
>       {
>        if (__bi1._M_current == __bi1._M_end)       // __bi1 is sup
>          return __bi2._M_current == __bi2._M_end;  // __bi2 is not sup
>@@ -188,7 +188,7 @@ namespace __gnu_parallel
>        *  @return @c true if less. */
>       friend bool
>       operator<(_UnguardedIterator<_RAIter, _Compare>& __bi1,
>-               _UnguardedIterator<_RAIter, _Compare>& __bi2)
>+               _UnguardedIterator<_RAIter, const _Compare>& __bi2)
>       {
>        // Normal compare.
>        return (__bi1.__comp)(*__bi1, *__bi2);


This is completely bogus, please revert.

The cppcheck warning is saying that it could be:

     const _UnguardedIterator<_RAIter, _Compare>&

which is completely different from:

     _UnguardedIterator<_RAIter, const _Compare>&

Also both parameters of operator< should have been changed, not just
one, and operator<= should have the same change.

But cppcheck is completely wrong anyway. The operator* member of
_GuardedIterator and _UnguardedIterator is not const, so trying to
dereference *__b1 and *__b2 would fail.

Nack nack nack.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] Use const for template argument.
  2020-05-06 10:01   ` Jonathan Wakely
@ 2020-05-07 20:44     ` Jonathan Wakely
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2020-05-07 20:44 UTC (permalink / raw)
  To: Martin Liska; +Cc: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

On 06/05/20 11:01 +0100, Jonathan Wakely wrote:
>On 04/02/20 14:55 +0100, Martin Liska wrote:
>>diff --git a/libstdc++-v3/include/parallel/multiway_merge.h b/libstdc++-v3/include/parallel/multiway_merge.h
>>index 983c7b2bd9a..97a9ce0feb7 100644
>>--- a/libstdc++-v3/include/parallel/multiway_merge.h
>>+++ b/libstdc++-v3/include/parallel/multiway_merge.h
>>@@ -118,7 +118,7 @@ namespace __gnu_parallel
>>       *  @return @c true if less. */
>>      friend bool
>>      operator<(_GuardedIterator<_RAIter, _Compare>& __bi1,
>>-               _GuardedIterator<_RAIter, _Compare>& __bi2)
>>+               _GuardedIterator<_RAIter, const _Compare>& __bi2)
>>      {
>>       if (__bi1._M_current == __bi1._M_end)       // __bi1 is sup
>>         return __bi2._M_current == __bi2._M_end;  // __bi2 is not sup
>>@@ -188,7 +188,7 @@ namespace __gnu_parallel
>>       *  @return @c true if less. */
>>      friend bool
>>      operator<(_UnguardedIterator<_RAIter, _Compare>& __bi1,
>>-               _UnguardedIterator<_RAIter, _Compare>& __bi2)
>>+               _UnguardedIterator<_RAIter, const _Compare>& __bi2)
>>      {
>>       // Normal compare.
>>       return (__bi1.__comp)(*__bi1, *__bi2);
>
>
>This is completely bogus, please revert.
>
>The cppcheck warning is saying that it could be:
>
>    const _UnguardedIterator<_RAIter, _Compare>&
>
>which is completely different from:
>
>    _UnguardedIterator<_RAIter, const _Compare>&
>
>Also both parameters of operator< should have been changed, not just
>one, and operator<= should have the same change.
>
>But cppcheck is completely wrong anyway. The operator* member of
>_GuardedIterator and _UnguardedIterator is not const, so trying to
>dereference *__b1 and *__b2 would fail.
>
>Nack nack nack.

Here's a correct fix for the cppcheck complaint.

Tested powerpc64le-linux, 'make check check-parallel', committed to
master.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4339 bytes --]

commit 4cbc9d8b346b932f34828a51e8822881413951b7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 7 21:43:49 2020 +0100

    libstdc++: Make relational operators work with const guarded iterators (PR 92472)
    
    This is a correct fix for the incorrect cppcheck suggestion to make
    these parameters const. In order to that, the dereference operators need
    to be const. The conversions to the underlying iterator can be const
    too.
    
            PR c/92472
            * include/parallel/multiway_merge.h (_GuardedIterator::operator*)
            (_GuardedIterator::operator _RAIter, _UnguardedIterator::operator*)
            (_UnguardedIterator::operator _RAIter): Add const qualifier.
            (operator<(_GuardedIterator&, _GuardedIterator&)
            (operator<=(_GuardedIterator&, _GuardedIterator&)
            (operator<(_UnguardedIterator&, _UnguardedIterator&)
            (operator<=(_UnguardedIterator&, _UnguardedIterator&): Change
            parameters to const references.

diff --git a/libstdc++-v3/include/parallel/multiway_merge.h b/libstdc++-v3/include/parallel/multiway_merge.h
index 983c7b2bd9a..52a8b2ca9e7 100644
--- a/libstdc++-v3/include/parallel/multiway_merge.h
+++ b/libstdc++-v3/include/parallel/multiway_merge.h
@@ -104,12 +104,12 @@ namespace __gnu_parallel
       /** @brief Dereference operator.
       *  @return Referenced element. */
       typename std::iterator_traits<_RAIter>::value_type&
-      operator*()
+      operator*() const
       { return *_M_current; }
 
       /** @brief Convert to wrapped iterator.
       *  @return Wrapped iterator. */
-      operator _RAIter()
+      operator _RAIter() const
       { return _M_current; }
 
       /** @brief Compare two elements referenced by guarded iterators.
@@ -117,8 +117,8 @@ namespace __gnu_parallel
        *  @param __bi2 Second iterator.
        *  @return @c true if less. */
       friend bool
-      operator<(_GuardedIterator<_RAIter, _Compare>& __bi1,
-		_GuardedIterator<_RAIter, _Compare>& __bi2)
+      operator<(const _GuardedIterator<_RAIter, _Compare>& __bi1,
+		const _GuardedIterator<_RAIter, _Compare>& __bi2)
       {
 	if (__bi1._M_current == __bi1._M_end)       // __bi1 is sup
 	  return __bi2._M_current == __bi2._M_end;  // __bi2 is not sup
@@ -132,8 +132,8 @@ namespace __gnu_parallel
        *  @param __bi2 Second iterator.
        *  @return @c True if less equal. */
       friend bool
-      operator<=(_GuardedIterator<_RAIter, _Compare>& __bi1,
-		 _GuardedIterator<_RAIter, _Compare>& __bi2)
+      operator<=(const _GuardedIterator<_RAIter, _Compare>& __bi1,
+		 const _GuardedIterator<_RAIter, _Compare>& __bi2)
       {
 	if (__bi2._M_current == __bi2._M_end)       // __bi1 is sup
 	  return __bi1._M_current != __bi1._M_end;  // __bi2 is not sup
@@ -174,12 +174,12 @@ namespace __gnu_parallel
       /** @brief Dereference operator.
       *  @return Referenced element. */
       typename std::iterator_traits<_RAIter>::value_type&
-      operator*()
+      operator*() const
       { return *_M_current; }
 
       /** @brief Convert to wrapped iterator.
       *  @return Wrapped iterator. */
-      operator _RAIter()
+      operator _RAIter() const
       { return _M_current; }
 
       /** @brief Compare two elements referenced by unguarded iterators.
@@ -187,8 +187,8 @@ namespace __gnu_parallel
        *  @param __bi2 Second iterator.
        *  @return @c true if less. */
       friend bool
-      operator<(_UnguardedIterator<_RAIter, _Compare>& __bi1,
-		_UnguardedIterator<_RAIter, _Compare>& __bi2)
+      operator<(const _UnguardedIterator<_RAIter, _Compare>& __bi1,
+		const _UnguardedIterator<_RAIter, _Compare>& __bi2)
       {
 	// Normal compare.
 	return (__bi1.__comp)(*__bi1, *__bi2);
@@ -199,8 +199,8 @@ namespace __gnu_parallel
        *  @param __bi2 Second iterator.
        *  @return @c True if less equal. */
       friend bool
-      operator<=(_UnguardedIterator<_RAIter, _Compare>& __bi1,
-		 _UnguardedIterator<_RAIter, _Compare>& __bi2)
+      operator<=(const _UnguardedIterator<_RAIter, _Compare>& __bi1,
+		 const _UnguardedIterator<_RAIter, _Compare>& __bi2)
       {
 	// Normal compare.
 	return !(__bi1.__comp)(*__bi2, *__bi1);

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-05-07 20:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06 11:06 [PATCH 0/4] Fix various minor issues seen with cppcheck Martin Liska
2020-02-06 11:06 ` [PATCH 4/4] Use const for template argument Martin Liska
2020-05-05 15:17   ` Jeff Law
2020-05-06 10:01   ` Jonathan Wakely
2020-05-07 20:44     ` Jonathan Wakely
2020-02-06 11:06 ` [PATCH 1/4] Remove 2 dead variables in bid_internal.h Martin Liska
2020-05-05 15:16   ` Jeff Law
2020-02-06 11:06 ` [PATCH 2/4] Use const for some function arguments Martin Liska
2020-02-07 17:22   ` Segher Boessenkool
2020-02-17 10:05     ` Martin Liška
2020-02-17 10:39       ` Richard Biener
2020-05-05 15:17   ` Jeff Law
2020-02-06 11:06 ` [PATCH 3/4] Put index check before use Martin Liska
2020-05-05 15:18   ` Jeff Law
2020-02-06 11:18 ` [PATCH 0/4] Fix various minor issues seen with cppcheck Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).