* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
@ 2022-05-03 23:41 ` mpolacek at gcc dot gnu.org
2022-05-03 23:49 ` pinskia at gcc dot gnu.org
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-05-03 23:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
Marek Polacek <mpolacek at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mpolacek at gcc dot gnu.org
--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Can you please provide a stand-alone test case?
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
2022-05-03 23:41 ` [Bug c++/105470] " mpolacek at gcc dot gnu.org
@ 2022-05-03 23:49 ` pinskia at gcc dot gnu.org
2022-05-04 0:14 ` redi at gcc dot gnu.org
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-05-03 23:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2022-05-03
Status|UNCONFIRMED |WAITING
Ever confirmed|0 |1
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Add -save-temps and attach the .ii file that is produced.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
2022-05-03 23:41 ` [Bug c++/105470] " mpolacek at gcc dot gnu.org
2022-05-03 23:49 ` pinskia at gcc dot gnu.org
@ 2022-05-04 0:14 ` redi at gcc dot gnu.org
2022-05-04 0:18 ` redi at gcc dot gnu.org
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-04 0:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It looks more like you're using -Werror and a new warning in the new GCC is
being turned into an error, because you asked for it.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (2 preceding siblings ...)
2022-05-04 0:14 ` redi at gcc dot gnu.org
@ 2022-05-04 0:18 ` redi at gcc dot gnu.org
2022-05-04 0:21 ` redi at gcc dot gnu.org
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-04 0:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|WAITING |RESOLVED
Resolution|--- |INVALID
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Roland Hughes from comment #0)
> If you look at the error message, this version of the compiler demands the &
> be against > instead of out with the variable.
No it doesn't.
> This appears to be a whitespace parsing bug.
I don't think so.
The warning is completely correct, and the code should be fixed.
for ( const std::pair<KeyModifiers, Scintilla::Message> &it : someMap )
This iterates over a map, with values of type:
std::pair<const KeyModifiers, Scintilla::Message>
But the loop is using a reference to a different type:
std::pair<KeyModifiers, Scintilla::Message>
That causes a temporary to be created on every iteration of the loop, and the
const-refernece binds to the temporary. The new GCC is smart enough to tell you
this, and you've asked for that warning to be a fatal error.
Either don't enable errors for that warning, or fix the code to avoid the
unnecessary temporaries being created.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (3 preceding siblings ...)
2022-05-04 0:18 ` redi at gcc dot gnu.org
@ 2022-05-04 0:21 ` redi at gcc dot gnu.org
2022-05-04 0:32 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-04 0:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
GCC even tells you exactly how to fix the code.
Here's the GCC error reformatted to make it a bit easier to read:
error: loop variable it of type
const std::pair<Scintilla::Internal::KeyModifiers, Scintilla::Message>&
binds to a temporary constructed from type
const std::pair<const Scintilla::Internal::KeyModifiers, Scintilla::Message>
[-Werror=range-loop-construct]
note: use non-reference type
const std::pair<Scintilla::Internal::KeyModifiers, Scintilla::Message>
to make the copy explicit or
const std::pair<const Scintilla::Internal::KeyModifiers, Scintilla::Message>&
to prevent copying
cc1plus: all warnings being treated as errors
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (4 preceding siblings ...)
2022-05-04 0:21 ` redi at gcc dot gnu.org
@ 2022-05-04 0:32 ` pinskia at gcc dot gnu.org
2022-05-04 8:07 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-05-04 0:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> GCC even tells you exactly how to fix the code.
>
> Here's the GCC error reformatted to make it a bit easier to read:
>
> error: loop variable it of type
> const std::pair<Scintilla::Internal::KeyModifiers, Scintilla::Message>&
> binds to a temporary constructed from type
> const std::pair<const Scintilla::Internal::KeyModifiers, Scintilla::Message>
> [-Werror=range-loop-construct]
>
> note: use non-reference type
> const std::pair<Scintilla::Internal::KeyModifiers, Scintilla::Message>
> to make the copy explicit or
> const std::pair<const Scintilla::Internal::KeyModifiers, Scintilla::Message>&
> to prevent copying
>
> cc1plus: all warnings being treated as errors
auto comes in handy here so you could also just do:
for ( const auto &it : sqt->kmap.GetKeyMap() )
And not worry about the type matches exactly.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (5 preceding siblings ...)
2022-05-04 0:32 ` pinskia at gcc dot gnu.org
@ 2022-05-04 8:07 ` redi at gcc dot gnu.org
2022-05-04 14:33 ` roland at logikalsolutions dot com
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-04 8:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, or use TheMapType::const_iterator. If you're going to name the type
explicitly, you need to be sure to use the correct type.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (6 preceding siblings ...)
2022-05-04 8:07 ` redi at gcc dot gnu.org
@ 2022-05-04 14:33 ` roland at logikalsolutions dot com
2022-05-04 14:39 ` mpolacek at gcc dot gnu.org
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: roland at logikalsolutions dot com @ 2022-05-04 14:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #8 from Roland Hughes <roland at logikalsolutions dot com> ---
(In reply to Marek Polacek from comment #1)
> Can you please provide a stand-alone test case?
I will work on this over the weekend.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (7 preceding siblings ...)
2022-05-04 14:33 ` roland at logikalsolutions dot com
@ 2022-05-04 14:39 ` mpolacek at gcc dot gnu.org
2022-05-04 14:40 ` roland at logikalsolutions dot com
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-05-04 14:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Roland Hughes from comment #8)
> (In reply to Marek Polacek from comment #1)
> > Can you please provide a stand-alone test case?
>
> I will work on this over the weekend.
Looks like that's not necessary anymore. Jon already explained why.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (8 preceding siblings ...)
2022-05-04 14:39 ` mpolacek at gcc dot gnu.org
@ 2022-05-04 14:40 ` roland at logikalsolutions dot com
2022-05-04 14:43 ` roland at logikalsolutions dot com
2022-05-04 15:31 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: roland at logikalsolutions dot com @ 2022-05-04 14:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #10 from Roland Hughes <roland at logikalsolutions dot com> ---
(In reply to Marek Polacek from comment #1)
>
> The warning is completely correct, and the code should be fixed.
>
> for ( const std::pair<KeyModifiers, Scintilla::Message> &it : someMap )
>
> This iterates over a map, with values of type:
>
> std::pair<const KeyModifiers, Scintilla::Message>
>
No, it doesn't.
[code]
roland@roland-HP-EliteDesk-800-G2-SFF:~/sf_projects/roland_hughes-csscintilla$
grep -irn GetKeyMap *
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:1031: // the fact
GetKeyMap() returns const means we can't use an iterator
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:1034: for ( const
std::pair<KeyModifiers, Scintilla::Message> &it : sqt->kmap.GetKeyMap() )
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:1059: auto
findResult = std::find_if( std::begin( sqt->kmap.GetKeyMap() ),
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:1060:
std::end( sqt->kmap.GetKeyMap() ),
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:1066: if (
findResult != std::end( sqt->kmap.GetKeyMap() ) )
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:4314:const
std::map<Scintilla::Internal::KeyModifiers, Scintilla::Message>
&CsScintillaEditBase::GetKeyMap() const noexcept
copperspice/CsScintillaEditBase/CsScintillaEditBase.cpp:4316: return
sqt->kmap.GetKeyMap();
copperspice/CsScintillaEditBase/CsScintillaEditBase.h:79: const
std::map<Scintilla::Internal::KeyModifiers, Scintilla::Message> &GetKeyMap()
const noexcept;
src/KeyMap.h:60: const std::map<KeyModifiers, Scintilla::Message>
&GetKeyMap() const noexcept;
src/KeyMap.cxx:50:const std::map<KeyModifiers, Message> &KeyMap::GetKeyMap()
const noexcept {
[/code]
There is no definition of that map anywhere in the codebase where KeyModifiers
is declared const.
The method being called returns a const & to the entire map, but there is no
const declared within the map.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (9 preceding siblings ...)
2022-05-04 14:40 ` roland at logikalsolutions dot com
@ 2022-05-04 14:43 ` roland at logikalsolutions dot com
2022-05-04 15:31 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: roland at logikalsolutions dot com @ 2022-05-04 14:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #11 from Roland Hughes <roland at logikalsolutions dot com> ---
(In reply to Marek Polacek from comment #9)
> (In reply to Roland Hughes from comment #8)
> > (In reply to Marek Polacek from comment #1)
> > > Can you please provide a stand-alone test case?
> >
> > I will work on this over the weekend.
>
> Looks like that's not necessary anymore. Jon already explained why.
It is actually necessary, see my other comment.
There is no declaration anywhere in the code base where the map contains a
constant value.
This is not resolved.
^ permalink raw reply [flat|nested] 13+ messages in thread* [Bug c++/105470] ranged for loop whitespace parsing
2022-05-03 23:28 [Bug c++/105470] New: ranged for loop whitespace parsing roland at logikalsolutions dot com
` (10 preceding siblings ...)
2022-05-04 14:43 ` roland at logikalsolutions dot com
@ 2022-05-04 15:31 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-04 15:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105470
--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Roland Hughes from comment #10)
> There is no definition of that map anywhere in the codebase where
> KeyModifiers is declared const.
That's just how std::map works.
std::map<A, B>::value_type is std::pair<const A, B>.
See the definition of value_type at
https://en.cppreference.com/w/cpp/container/map
So when you iterate over a map<A, B> the values you get back are pair<const A,
B>.
> The method being called returns a const & to the entire map, but there is no
> const declared within the map.
There is in the map's value_type though, and that's what you iterate over with
the range-based for loop.
^ permalink raw reply [flat|nested] 13+ messages in thread