error C2589: '(' : illegal token on right side of '::'

SPlatten

I'm working on porting an existing project which was originally developed for Qt 4.8. I have porting this to Qt 5.8 and updating various other libraries to later revisions.

I'm getting the error C2589 during compile on:

QList<double>& groupList = m_sampleHistory[group];
if(groupList.size() == 0) {
    groupList.push_back(0.0);
}
groupList.back() = std::max(groupList.back(), sample);

The error is shown on the last line above. I didn't write this code and it doesn't look correct as back() returns a 'const double&'. The headers that are include are:

#include "win.h"
#include <algorithm>
#include <iostream>
#include <string>

#include "QVariant"
#include "QMap"
#include "QList"
#include "QString"
#include "boost/array.hpp"

win.h is a header I created which contains:

#ifndef WIN_H
    #define WIN_H

    #ifndef AF_IPX
        #ifndef WIN32_LEAN_AND_MEAN
           #define WIN32_LEAN_AND_MEAN 1
        #endif
        #ifndef WIN32
            #define WIN32
        #endif
        #include <stdio.h>
        #include <stdlib.h>
        #include <WinSock2.h>
        #include <Windows.h>
    #endif
#endif

SPlatten

@sierdzio , also just tried your example and it won't allow it:

const double existing = groupList.takeLast();
groupList.append(std::max(existing, sample));

The last line has exclamantion mark on it , the errors are:

error: C2589: '(' : illegal token on right side of '::'    
error: C2143: syntax error : missing ')' before '::'
error: C2661: 'QList<double>::append' : no overloaded function takes 0 arguments
error: C2059: syntax error : ')'

All of those on that line. Having just looked at the details on QList, I can see that the const version of back is an overloaded version, there is also an identical version without const.
In the end, this works:

double last = groupList.takeLast();

if ( sample > last )
    last = sample;
groupList.last() = last;

sierdzio

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

groupList.back()

Try this:

const double existing = groupList.takeLast();
groupList.append(std::max(existing, sample));

although it returns the same value.

It's not as efficient, perhaps, but more readable. And should compile ;-)

SPlatten

@sierdzio , thanks , to me the logic just doesn't look right I have it compiling now, somehow its assigning a value to a const double reference.

The error has now changed, now I'm getting:

ocidl.h(45) : error C2146: syntax error : missing ';' before identifier

sierdzio

Is sample also a double? Perhaps std::max gets confused.

SPlatten

@sierdzio , yes, passed in as const double.

SPlatten

@sierdzio , also just tried your example and it won't allow it:

const double existing = groupList.takeLast();
groupList.append(std::max(existing, sample));

The last line has exclamantion mark on it , the errors are:

error: C2589: '(' : illegal token on right side of '::'    
error: C2143: syntax error : missing ')' before '::'
error: C2661: 'QList<double>::append' : no overloaded function takes 0 arguments
error: C2059: syntax error : ')'

All of those on that line. Having just looked at the details on QList, I can see that the const version of back is an overloaded version, there is also an identical version without const.
In the end, this works:

double last = groupList.takeLast();

if ( sample > last )
    last = sample;
groupList.last() = last;

KroMignon

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

double last = groupList.takeLast();

if ( sample > last )
last = sample;
groupList.last() = last;

I am not sure this is the same as groupList.back() = std::max(groupList.back(), sample);

I think your should change to

double last = groupList.takeLast();

if ( sample > last )
     last = sample;
groupList.insert(last);

or

groupList.insert(std::max(groupList.takeLast(), double(sample)));

SPlatten

@KroMignon , the original code was setting last in the same way, I've just kept it the same.

KroMignon

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

, I've just kept it the same.

No, you don't.
You take the last element from the list with QList::takeLast(). So your list will be 1 item shorter as begin.

SPlatten

@KroMignon , the original code, which would compile:

groupList.back() = std::max(groupList.back(), sample);

The replacement:

double last = groupList.takeLast();

if ( sample > last )
    last = sample;
groupList.last() = last;

How is the new code different from the original, except now it compiles?

jsulm

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

How is the new code different from the original, except now it compiles?

You now overwrite the element before the last element: say you have 3 elements [A, B, C].
After takeLast() you have [A, B].
If you now execute groupList.last() = last; you will overwrite B.

SPlatten

@jsulm , thank you, and the original just overrides the last item without removing it?

So if I just change it to:

double last = groupList.last();

if ( sample > last )
    last = sample;
groupList.last() = last;

jsulm

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

and the original just overrides the last item without removing it?

Yes.
With your code you can use append() instead of groupList.last() = last; to get same behaviour.

SPlatten

@jsulm , but I'm not sure thats what the original was trying to do either. I think the original code would just update the last item.

jsulm

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

I think the original code would just update the last item

Yes, it updates the last element. Your code now removes the last element and updates the one before last (the new last element). So, to get same behaviour using takeLast() you have to append the element which takeLast REMOVES from the list:

[A, B, C]
after takeLast()
[A, B]
after append()
[A, B, C*]
C* is the updated C

SPlatten

@jsulm , I think you missed one of my replies where I showed a replacement, changing the takeLast and replacing with just last.

jsulm

@SPlatten The one using last() looks fine. Somehow overlooked it :-)

KroMignon

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

thank you, and the original just overrides the last item without removing it?

Yes, that is the point:

QList::takeLast() will return last element from QList and remove it from the list
QList::last() will only return a reference to last element from QList and not change list size.

SPlatten

@KroMignon, still no idea why the compiler was complaining about the original?

groupList.back() = std::max(groupList.back(), sample);

KroMignon

@SPlatten said in error C2589: '(' : illegal token on right side of '::':

still no idea why the compiler was complaining about the original?

I guess sampleand groupList.back() do not have same type and std::max() template will not be able to match them.
Perhaps using QList::constLast()?

// I prefere using Last() which is more clear as back()
groupList.last() = std::max(groupList.constLast(), sample);