[v1,2/2] test: controls: control_info_map: Fix libc++ warning
diff mbox series

Message ID 20260428084550.91669-2-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • [v1,1/2] libcamera: base: regex: Fix preprocessor check
Related show

Commit Message

Barnabás Pőcze April 28, 2026, 8:45 a.m. UTC
Since llvm 22, libc++ has made `std::unordered_map::at()` have the `nodiscard`
attribute, leading to warnings, fix that with `std::ignore`.

Link: https://github.com/llvm/llvm-project/commit/9a03a30706cca40b93146f379fb5faa75d417af5
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 test/controls/control_info_map.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham April 30, 2026, 10:54 a.m. UTC | #1
Quoting Barnabás Pőcze (2026-04-28 09:45:50)
> Since llvm 22, libc++ has made `std::unordered_map::at()` have the `nodiscard`
> attribute, leading to warnings, fix that with `std::ignore`.
> 
> Link: https://github.com/llvm/llvm-project/commit/9a03a30706cca40b93146f379fb5faa75d417af5
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  test/controls/control_info_map.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
> index b0be14b58..6c20ee8d0 100644
> --- a/test/controls/control_info_map.cpp
> +++ b/test/controls/control_info_map.cpp
> @@ -49,7 +49,7 @@ protected:
>                         return TestFail;
>                 }
>  
> -               infoMap.at(&controls::Brightness);
> +               std::ignore = infoMap.at(&controls::Brightness);

I presume the test is looking for the fact that this will compile and
not cause an assertion rather than actually doing anything with the data
?

So ignoreing it is the same as what we have, so at least in that
instance for this patch:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
>                 /* Test looking up a valid control by numerical ID. */
>                 if (infoMap.count(controls::Brightness.id()) != 1) {
> -- 
> 2.54.0
>
Barnabás Pőcze April 30, 2026, 10:57 a.m. UTC | #2
2026. 04. 30. 12:54 keltezéssel, Kieran Bingham írta:
> Quoting Barnabás Pőcze (2026-04-28 09:45:50)
>> Since llvm 22, libc++ has made `std::unordered_map::at()` have the `nodiscard`
>> attribute, leading to warnings, fix that with `std::ignore`.
>>
>> Link: https://github.com/llvm/llvm-project/commit/9a03a30706cca40b93146f379fb5faa75d417af5
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   test/controls/control_info_map.cpp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
>> index b0be14b58..6c20ee8d0 100644
>> --- a/test/controls/control_info_map.cpp
>> +++ b/test/controls/control_info_map.cpp
>> @@ -49,7 +49,7 @@ protected:
>>                          return TestFail;
>>                  }
>>   
>> -               infoMap.at(&controls::Brightness);
>> +               std::ignore = infoMap.at(&controls::Brightness);
> 
> I presume the test is looking for the fact that this will compile and
> not cause an assertion rather than actually doing anything with the data
> ?

Yes, it throws an assertion if not found, which would abort the process
since there are no handlers, leading to the test failure.


> 
> So ignoreing it is the same as what we have, so at least in that
> instance for this patch:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>>   
>>                  /* Test looking up a valid control by numerical ID. */
>>                  if (infoMap.count(controls::Brightness.id()) != 1) {
>> -- 
>> 2.54.0
>>
Laurent Pinchart June 11, 2026, 3:57 p.m. UTC | #3
On Thu, Apr 30, 2026 at 12:57:19PM +0200, Barnabás Pőcze wrote:
> 2026. 04. 30. 12:54 keltezéssel, Kieran Bingham írta:
> > Quoting Barnabás Pőcze (2026-04-28 09:45:50)
> >> Since llvm 22, libc++ has made `std::unordered_map::at()` have the `nodiscard`
> >> attribute, leading to warnings, fix that with `std::ignore`.
> >>
> >> Link: https://github.com/llvm/llvm-project/commit/9a03a30706cca40b93146f379fb5faa75d417af5
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >> ---
> >>   test/controls/control_info_map.cpp | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
> >> index b0be14b58..6c20ee8d0 100644
> >> --- a/test/controls/control_info_map.cpp
> >> +++ b/test/controls/control_info_map.cpp
> >> @@ -49,7 +49,7 @@ protected:
> >>                          return TestFail;
> >>                  }
> >>   
> >> -               infoMap.at(&controls::Brightness);
> >> +               std::ignore = infoMap.at(&controls::Brightness);
> > 
> > I presume the test is looking for the fact that this will compile and
> > not cause an assertion rather than actually doing anything with the data
> > ?
> 
> Yes, it throws an assertion if not found, which would abort the process
> since there are no handlers, leading to the test failure.

I'd rather write

		if (!infoMap.contains(&controls::Brightness)) {
			std::cout << ...;
			return TestFail;
		}

That's more explicit.

> > So ignoreing it is the same as what we have, so at least in that
> > instance for this patch:
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> >>   
> >>                  /* Test looking up a valid control by numerical ID. */
> >>                  if (infoMap.count(controls::Brightness.id()) != 1) {
Barnabás Pőcze June 15, 2026, 8:32 a.m. UTC | #4
2026. 06. 11. 17:57 keltezéssel, Laurent Pinchart írta:
> On Thu, Apr 30, 2026 at 12:57:19PM +0200, Barnabás Pőcze wrote:
>> 2026. 04. 30. 12:54 keltezéssel, Kieran Bingham írta:
>>> Quoting Barnabás Pőcze (2026-04-28 09:45:50)
>>>> Since llvm 22, libc++ has made `std::unordered_map::at()` have the `nodiscard`
>>>> attribute, leading to warnings, fix that with `std::ignore`.
>>>>
>>>> Link: https://github.com/llvm/llvm-project/commit/9a03a30706cca40b93146f379fb5faa75d417af5
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>    test/controls/control_info_map.cpp | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
>>>> index b0be14b58..6c20ee8d0 100644
>>>> --- a/test/controls/control_info_map.cpp
>>>> +++ b/test/controls/control_info_map.cpp
>>>> @@ -49,7 +49,7 @@ protected:
>>>>                           return TestFail;
>>>>                   }
>>>>    
>>>> -               infoMap.at(&controls::Brightness);
>>>> +               std::ignore = infoMap.at(&controls::Brightness);
>>>
>>> I presume the test is looking for the fact that this will compile and
>>> not cause an assertion rather than actually doing anything with the data
>>> ?
>>
>> Yes, it throws an assertion if not found, which would abort the process

assertion -> exception

>> since there are no handlers, leading to the test failure.
> 
> I'd rather write
> 
> 		if (!infoMap.contains(&controls::Brightness)) {
> 			std::cout << ...;
> 			return TestFail;
> 		}
> 
> That's more explicit.

Already merged, I'm afraid.


> 
>>> So ignoreing it is the same as what we have, so at least in that
>>> instance for this patch:
>>>
>>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>
>>>>    
>>>>                   /* Test looking up a valid control by numerical ID. */
>>>>                   if (infoMap.count(controls::Brightness.id()) != 1) {
>

Patch
diff mbox series

diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
index b0be14b58..6c20ee8d0 100644
--- a/test/controls/control_info_map.cpp
+++ b/test/controls/control_info_map.cpp
@@ -49,7 +49,7 @@  protected:
 			return TestFail;
 		}
 
-		infoMap.at(&controls::Brightness);
+		std::ignore = infoMap.at(&controls::Brightness);
 
 		/* Test looking up a valid control by numerical ID. */
 		if (infoMap.count(controls::Brightness.id()) != 1) {