diff mbox series

verbs: fix compilation warning with C++20

Message ID 20230609153147.667674-1-neelx@redhat.com (mailing list archive)
State Superseded
Headers show
Series verbs: fix compilation warning with C++20 | expand

Commit Message

Daniel Vacek June 9, 2023, 3:31 p.m. UTC
Our customer reported the below warning whe using Clang v16.0.4 and C++20,
on a code that includes the header "/usr/include/infiniband/verbs.h":

error: bitwise operation between different enumeration types ('ibv_access_flags' and
'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
                mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
                             ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
                              ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

According to the article "Clang 11 warning: Bitwise operation between different
enumeration types is deprecated":

C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.

Reported-by: Rogerio Moraes <rogerio@cadence.com>
Signed-off-by: Daniel Vacek <neelx@redhat.com>
---
 libibverbs/verbs.h     | 4 +++-
 libibverbs/verbs_api.h | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Jason Gunthorpe June 9, 2023, 4 p.m. UTC | #1
On Fri, Jun 09, 2023 at 05:31:47PM +0200, Daniel Vacek wrote:
> Our customer reported the below warning whe using Clang v16.0.4 and C++20,
> on a code that includes the header "/usr/include/infiniband/verbs.h":
> 
> error: bitwise operation between different enumeration types ('ibv_access_flags' and
> 'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
>                 mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
>                              ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
>                               ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> 
> According to the article "Clang 11 warning: Bitwise operation between different
> enumeration types is deprecated":
> 
> C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
> likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.

There should be a cast to an integer in the macro, we can't know what
the user will pass in there and it may not be that enum.

Jason
Daniel Vacek June 9, 2023, 4:15 p.m. UTC | #2
On Fri, Jun 9, 2023 at 6:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Fri, Jun 09, 2023 at 05:31:47PM +0200, Daniel Vacek wrote:
> > Our customer reported the below warning whe using Clang v16.0.4 and C++20,
> > on a code that includes the header "/usr/include/infiniband/verbs.h":
> >
> > error: bitwise operation between different enumeration types ('ibv_access_flags' and
> > 'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
> >                 mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
> >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
> >                              ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
> >                               ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> >
> > According to the article "Clang 11 warning: Bitwise operation between different
> > enumeration types is deprecated":
> >
> > C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
> > likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.
>
> There should be a cast to an integer in the macro, we can't know what
> the user will pass in there and it may not be that enum.

Hmm, if the user passes a definition from the header files at least we
should be consistent I'd say, which is this case. No one was passing
any custom values here.
If you cast to an integer here you may start silently hiding possible
errors. If the user passes any custom value, IMO, it's his
responsibility to make it right.

I forgot to explicitly mention before that this change was tested and
it addresses the warning successfully.

--nX

> Jason
>
Jason Gunthorpe June 9, 2023, 4:18 p.m. UTC | #3
On Fri, Jun 09, 2023 at 06:15:44PM +0200, Daniel Vacek wrote:
> On Fri, Jun 9, 2023 at 6:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Fri, Jun 09, 2023 at 05:31:47PM +0200, Daniel Vacek wrote:
> > > Our customer reported the below warning whe using Clang v16.0.4 and C++20,
> > > on a code that includes the header "/usr/include/infiniband/verbs.h":
> > >
> > > error: bitwise operation between different enumeration types ('ibv_access_flags' and
> > > 'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
> > >                 mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
> > >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
> > >                              ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
> > >                               ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 1 error generated.
> > >
> > > According to the article "Clang 11 warning: Bitwise operation between different
> > > enumeration types is deprecated":
> > >
> > > C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
> > > likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.
> >
> > There should be a cast to an integer in the macro, we can't know what
> > the user will pass in there and it may not be that enum.
> 
> Hmm, if the user passes a definition from the header files at least we
> should be consistent I'd say, which is this case. No one was passing
> any custom values here.
> If you cast to an integer here you may start silently hiding possible
> errors. If the user passes any custom value, IMO, it's his
> responsibility to make it right.

The signature of the API is to accept an int, we cannot demand any
more of that from the user. The macro wiped out the type cast to an
int, it should put it back.

Jason
Daniel Vacek June 9, 2023, 4:29 p.m. UTC | #4
On Fri, Jun 9, 2023 at 6:18 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Fri, Jun 09, 2023 at 06:15:44PM +0200, Daniel Vacek wrote:
> > On Fri, Jun 9, 2023 at 6:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > On Fri, Jun 09, 2023 at 05:31:47PM +0200, Daniel Vacek wrote:
> > > > Our customer reported the below warning whe using Clang v16.0.4 and C++20,
> > > > on a code that includes the header "/usr/include/infiniband/verbs.h":
> > > >
> > > > error: bitwise operation between different enumeration types ('ibv_access_flags' and
> > > > 'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
> > > >                 mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
> > > >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
> > > >                              ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
> > > >                               ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > 1 error generated.
> > > >
> > > > According to the article "Clang 11 warning: Bitwise operation between different
> > > > enumeration types is deprecated":
> > > >
> > > > C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
> > > > likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.
> > >
> > > There should be a cast to an integer in the macro, we can't know what
> > > the user will pass in there and it may not be that enum.
> >
> > Hmm, if the user passes a definition from the header files at least we
> > should be consistent I'd say, which is this case. No one was passing
> > any custom values here.
> > If you cast to an integer here you may start silently hiding possible
> > errors. If the user passes any custom value, IMO, it's his
> > responsibility to make it right.
>
> The signature of the API is to accept an int, we cannot demand any
> more of that from the user. The macro wiped out the type cast to an
> int, it should put it back.

Oh, I see. In that case shall we still keep this patch or just do the cast?

--nX

> Jason
>
Jason Gunthorpe June 9, 2023, 4:34 p.m. UTC | #5
On Fri, Jun 09, 2023 at 06:29:55PM +0200, Daniel Vacek wrote:
> On Fri, Jun 9, 2023 at 6:18 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Fri, Jun 09, 2023 at 06:15:44PM +0200, Daniel Vacek wrote:
> > > On Fri, Jun 9, 2023 at 6:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > > On Fri, Jun 09, 2023 at 05:31:47PM +0200, Daniel Vacek wrote:
> > > > > Our customer reported the below warning whe using Clang v16.0.4 and C++20,
> > > > > on a code that includes the header "/usr/include/infiniband/verbs.h":
> > > > >
> > > > > error: bitwise operation between different enumeration types ('ibv_access_flags' and
> > > > > 'ib_uverbs_access_flags') is deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
> > > > >                 mem->mr = ibv_reg_mr(dev->pd, (void*)start, len, IBV_ACCESS_LOCAL_WRITE);
> > > > >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > /usr/include/infiniband/verbs.h:2514:19: note: expanded from macro 'ibv_reg_mr'
> > > > >                              ((access) & IBV_ACCESS_OPTIONAL_RANGE) == 0))
> > > > >                               ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > 1 error generated.
> > > > >
> > > > > According to the article "Clang 11 warning: Bitwise operation between different
> > > > > enumeration types is deprecated":
> > > > >
> > > > > C++20's P1120R0 deprecated bitwise operations between different enums. Such code is
> > > > > likely to become ill-formed in C++23. Clang 11 warns about such cases. It should be fixed.
> > > >
> > > > There should be a cast to an integer in the macro, we can't know what
> > > > the user will pass in there and it may not be that enum.
> > >
> > > Hmm, if the user passes a definition from the header files at least we
> > > should be consistent I'd say, which is this case. No one was passing
> > > any custom values here.
> > > If you cast to an integer here you may start silently hiding possible
> > > errors. If the user passes any custom value, IMO, it's his
> > > responsibility to make it right.
> >
> > The signature of the API is to accept an int, we cannot demand any
> > more of that from the user. The macro wiped out the type cast to an
> > int, it should put it back.
> 
> Oh, I see. In that case shall we still keep this patch or just do
> the cast?

Just do the cast

Jason
diff mbox series

Patch

diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 03a7a2a7..85995abe 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -617,7 +617,9 @@  enum ibv_access_flags {
 	IBV_ACCESS_HUGETLB		= (1<<7),
 	IBV_ACCESS_FLUSH_GLOBAL		= (1 << 8),
 	IBV_ACCESS_FLUSH_PERSISTENT	= (1 << 9),
-	IBV_ACCESS_RELAXED_ORDERING	= IBV_ACCESS_OPTIONAL_FIRST,
+
+	IBV_ACCESS_RELAXED_ORDERING	= IBV_ACCESS_OPTIONAL_FIRST,		// bit 20
+	IBV_ACCESS_OPTIONAL_RANGE	= IB_UVERBS_ACCESS_OPTIONAL_RANGE	// mask of bits 20-29
 };
 
 struct ibv_mw_bind_info {
diff --git a/libibverbs/verbs_api.h b/libibverbs/verbs_api.h
index 309f6fba..7a5f0cdf 100644
--- a/libibverbs/verbs_api.h
+++ b/libibverbs/verbs_api.h
@@ -94,7 +94,6 @@ 
 
 #define IBV_QPF_GRH_REQUIRED				IB_UVERBS_QPF_GRH_REQUIRED
 
-#define IBV_ACCESS_OPTIONAL_RANGE			IB_UVERBS_ACCESS_OPTIONAL_RANGE
 #define IBV_ACCESS_OPTIONAL_FIRST			IB_UVERBS_ACCESS_OPTIONAL_FIRST
 #endif