diff mbox series

[committed] Fix compilation error in 2 1_neg.cc tests

Message ID 910dbf54-7e51-4c29-8a97-e4e348aae4f4@gmail.com
State New
Headers
Series [committed] Fix compilation error in 2 1_neg.cc tests |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 warning Patch is already merged

Commit Message

François Dumont July 31, 2024, 8:42 p.m. UTC 
  Committed as trivial.

Fix a compilation error that is not expected by the tests preserving
the expected ones.

The 'test' variable declaration is missing since commit
a9260b7eb688df43a724e25421ba40f35a89fee9 that removed the test global
variable in testsuite files.

libstdc++-v3/ChangeLog:

     * testsuite/23_containers/map/operators/1_neg.cc (test01): Add test 
variable
     declaration.
     * testsuite/23_containers/set/operators/1_neg.cc (test01): Likewise.

François

Comments

Jonathan Wakely July 31, 2024, 8:59 p.m. UTC | #1 
On Wed, 31 Jul 2024 at 21:44, François Dumont <frs.dumont@gmail.com> wrote:
>
> Committed as trivial.
>
> Fix a compilation error that is not expected by the tests preserving
> the expected ones.
>
> The 'test' variable declaration is missing since commit
> a9260b7eb688df43a724e25421ba40f35a89fee9 that removed the test global
> variable in testsuite files.

Oh good catch!

The problem is that the dg-error "no" is matching two errors on that
line, the "'test' was not declared" one and the "no match" one.

I think we should get rid of the 'test' variable again, and use a
better dg-error pattern. "no" is much too short and matches too much.

There's no need to assign the expressions to a variable, they're
ill-formed anyway.

So:

   itr != mapByName.end(); // { dg-error "no match" }
   itr == mapByName.end(); // { dg-error "no match" }


>
> libstdc++-v3/ChangeLog:
>
>      * testsuite/23_containers/map/operators/1_neg.cc (test01): Add test
> variable
>      declaration.
>      * testsuite/23_containers/set/operators/1_neg.cc (test01): Likewise.
>
> François
Jonathan Wakely July 31, 2024, 9:12 p.m. UTC | #2 
On Wed, 31 Jul 2024 at 21:59, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Wed, 31 Jul 2024 at 21:44, François Dumont <frs.dumont@gmail.com> wrote:
> >
> > Committed as trivial.
> >
> > Fix a compilation error that is not expected by the tests preserving
> > the expected ones.
> >
> > The 'test' variable declaration is missing since commit
> > a9260b7eb688df43a724e25421ba40f35a89fee9 that removed the test global
> > variable in testsuite files.
>
> Oh good catch!
>
> The problem is that the dg-error "no" is matching two errors on that
> line, the "'test' was not declared" one and the "no match" one.
>
> I think we should get rid of the 'test' variable again, and use a
> better dg-error pattern. "no" is much too short and matches too much.

Those are the only tests we have with a two-letter dg-error pattern,
there are none with one letter or three letters.

There are lots with four-letter patterns, but they're all { dg-error
"here" } which is fine.

There are lots of decimal FP tests with { dg-error "error" } which is
silly, it might as well be "." and match anything. They should be
fixed, although everything for DFP is low priority.

>
> There's no need to assign the expressions to a variable, they're
> ill-formed anyway.
>
> So:
>
>    itr != mapByName.end(); // { dg-error "no match" }
>    itr == mapByName.end(); // { dg-error "no match" }
>
>
> >
> > libstdc++-v3/ChangeLog:
> >
> >      * testsuite/23_containers/map/operators/1_neg.cc (test01): Add test
> > variable
> >      declaration.
> >      * testsuite/23_containers/set/operators/1_neg.cc (test01): Likewise.
> >
> > François
François Dumont Aug. 1, 2024, 5:09 a.m. UTC | #3 
Do you want me to take care of those 2 tests ?

You seem to have started something on the review of dg-error patterns in 
use.

Here I kept the test variable because I fear to potentially have a 
diagnostic about unused returned value.

François

On 31/07/2024 23:12, Jonathan Wakely wrote:
> On Wed, 31 Jul 2024 at 21:59, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On Wed, 31 Jul 2024 at 21:44, François Dumont <frs.dumont@gmail.com> wrote:
>>> Committed as trivial.
>>>
>>> Fix a compilation error that is not expected by the tests preserving
>>> the expected ones.
>>>
>>> The 'test' variable declaration is missing since commit
>>> a9260b7eb688df43a724e25421ba40f35a89fee9 that removed the test global
>>> variable in testsuite files.
>> Oh good catch!
>>
>> The problem is that the dg-error "no" is matching two errors on that
>> line, the "'test' was not declared" one and the "no match" one.
>>
>> I think we should get rid of the 'test' variable again, and use a
>> better dg-error pattern. "no" is much too short and matches too much.
> Those are the only tests we have with a two-letter dg-error pattern,
> there are none with one letter or three letters.
>
> There are lots with four-letter patterns, but they're all { dg-error
> "here" } which is fine.
>
> There are lots of decimal FP tests with { dg-error "error" } which is
> silly, it might as well be "." and match anything. They should be
> fixed, although everything for DFP is low priority.
>
>> There's no need to assign the expressions to a variable, they're
>> ill-formed anyway.
>>
>> So:
>>
>>     itr != mapByName.end(); // { dg-error "no match" }
>>     itr == mapByName.end(); // { dg-error "no match" }
>>
>>
>>> libstdc++-v3/ChangeLog:
>>>
>>>       * testsuite/23_containers/map/operators/1_neg.cc (test01): Add test
>>> variable
>>>       declaration.
>>>       * testsuite/23_containers/set/operators/1_neg.cc (test01): Likewise.
>>>
>>> François
Jonathan Wakely Aug. 1, 2024, 7:26 a.m. UTC | #4 
On Thu, 1 Aug 2024 at 06:09, François Dumont <frs.dumont@gmail.com> wrote:
>
> Do you want me to take care of those 2 tests ?

Yes please.

>
> You seem to have started something on the review of dg-error patterns in
> use.
>
> Here I kept the test variable because I fear to potentially have a
> diagnostic about unused returned value.

I thought about that too, but you can't have an unused return value
from a function that doesn't compile. There's no return value, because
itr != mapByName.end() doesn't compile, so there's no function that
gets called, so no return type.
François Dumont Aug. 1, 2024, 4:57 p.m. UTC | #5 
libstdc++: Make dg-error pattern more accurate

     Remove useless test variable and use a more accurate dg-error 
pattern so
     that only the ill-formed expression compilation error is considered.

     libstdc++-v3/ChangeLog:

             * testsuite/23_containers/map/operators/1_neg.cc (test01): 
Remove test variable
             and use 'no match' dg-error patter.
             * testsuite/23_containers/set/operators/1_neg.cc (test01): 
Likewise.

Comitted with you as author.

François


On 01/08/2024 09:26, Jonathan Wakely wrote:
> On Thu, 1 Aug 2024 at 06:09, François Dumont <frs.dumont@gmail.com> wrote:
>> Do you want me to take care of those 2 tests ?
> Yes please.
>
>> You seem to have started something on the review of dg-error patterns in
>> use.
>>
>> Here I kept the test variable because I fear to potentially have a
>> diagnostic about unused returned value.
> I thought about that too, but you can't have an unused return value
> from a function that doesn't compile. There's no return value, because
> itr != mapByName.end() doesn't compile, so there's no function that
> gets called, so no return type.
>
Except in case of regression that would make the expression well-formed 
and then produce another compilation error. But with your modification 
of the dg-error pattern this cannot happen anymore so all good.
diff --git a/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc b/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
index 0eb1eee640b..6ce7b3249d8 100644
--- a/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
@@ -35,6 +35,6 @@ void test01()
   std::map<unsigned, int>::iterator itr(mapByIndex.begin());
 
   // NB: notice, it's not mapByIndex!!
-  bool __attribute__((unused)) test = itr != mapByName.end(); // { dg-error "no" }
-  test &= itr == mapByName.end(); // { dg-error "no" }
+  itr != mapByName.end(); // { dg-error "no match" }
+  itr == mapByName.end(); // { dg-error "no match" }
 }
diff --git a/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc b/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
index 28d08f308e1..b5f69ae920c 100644
--- a/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
@@ -32,6 +32,6 @@ void test01()
   std::set<unsigned int>::iterator itr(setByIndex.begin());
 
   // NB: it's not setByIndex!!
-  bool __attribute__((unused)) test = itr != setByName.end(); // { dg-error "no" }
-  test &= itr == setByName.end(); // { dg-error "no" }
+  itr != setByName.end(); // { dg-error "no match" }
+  itr == setByName.end(); // { dg-error "no match" }
 }
Jonathan Wakely Aug. 1, 2024, 5:45 p.m. UTC | #6 
On Thu, 1 Aug 2024 at 17:57, François Dumont <frs.dumont@gmail.com> wrote:
>
>      libstdc++: Make dg-error pattern more accurate
>
>      Remove useless test variable and use a more accurate dg-error
> pattern so
>      that only the ill-formed expression compilation error is considered.
>
>      libstdc++-v3/ChangeLog:
>
>              * testsuite/23_containers/map/operators/1_neg.cc (test01):
> Remove test variable
>              and use 'no match' dg-error patter.
>              * testsuite/23_containers/set/operators/1_neg.cc (test01):
> Likewise.
>
> Comitted with you as author.

Great, thanks.


> François
>
>
> On 01/08/2024 09:26, Jonathan Wakely wrote:
> > On Thu, 1 Aug 2024 at 06:09, François Dumont <frs.dumont@gmail.com> wrote:
> >> Do you want me to take care of those 2 tests ?
> > Yes please.
> >
> >> You seem to have started something on the review of dg-error patterns in
> >> use.
> >>
> >> Here I kept the test variable because I fear to potentially have a
> >> diagnostic about unused returned value.
> > I thought about that too, but you can't have an unused return value
> > from a function that doesn't compile. There's no return value, because
> > itr != mapByName.end() doesn't compile, so there's no function that
> > gets called, so no return type.
> >
> Except in case of regression that would make the expression well-formed
> and then produce another compilation error. But with your modification
> of the dg-error pattern this cannot happen anymore so all good.
>
diff mbox series

Patch

diff --git a/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc b/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
index 1cb08930690..0eb1eee640b 100644
--- a/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/map/operators/1_neg.cc
@@ -28,14 +28,13 @@  void test01()
 {
   std::map<unsigned int, int> mapByIndex;
   std::map<std::string, unsigned> mapByName;
-  
+
   mapByIndex.insert(std::pair<unsigned, int>(0, 1));
   mapByIndex.insert(std::pair<unsigned, int>(6, 5));
-  
+
   std::map<unsigned, int>::iterator itr(mapByIndex.begin());
 
   // NB: notice, it's not mapByIndex!!
-  test &= itr != mapByName.end(); // { dg-error "no" } 
-  test &= itr == mapByName.end(); // { dg-error "no" } 
+  bool __attribute__((unused)) test = itr != mapByName.end(); // { dg-error "no" }
+  test &= itr == mapByName.end(); // { dg-error "no" }
 }
-
diff --git a/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc b/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
index be90ba51cd4..28d08f308e1 100644
--- a/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/set/operators/1_neg.cc
@@ -28,11 +28,10 @@  void test01()
 {
   std::set<unsigned int> setByIndex;
   std::set<std::string> setByName;
-  
+
   std::set<unsigned int>::iterator itr(setByIndex.begin());
-  
+
   // NB: it's not setByIndex!!
-  test &= itr != setByName.end(); // { dg-error "no" } 
-  test &= itr == setByName.end(); // { dg-error "no" } 
+  bool __attribute__((unused)) test = itr != setByName.end(); // { dg-error "no" }
+  test &= itr == setByName.end(); // { dg-error "no" }
 }
-