presumed wrong function signature

[massimomorara]

Hi.

I've tried to compile your code with "-Wall -Wextra -ansi -pedantic" flags enabled. I got a lot of warning that (I suppose) are harmless.

With an exception:

XCSP3Code.cc:132:52: warning: parameter ‘v’ set but not used [-Wunused-but-set-parameter]
     bool XCSP3Core::isVariable(XEntity *xe, XVariable *v) {

The point is that in this function

bool XCSP3Core::isVariable(XEntity *xe, XVariable *v) {
    XVariable *x;
    if((x = dynamic_cast<XVariable *>(xe)) != NULL) {
        v = x;
        return true;
    }
    return false;

}

the value assigned to v (with v = x) is lost at the end of the function.

I don't know your code but I suppose that the intention was to copy the XVariable pointed by x in the XVariable pointed by v.

So (if an operator=() for XVariable is available)

bool XCSP3Core::isVariable (XEntity * xe, XVariable & v) {
    XVariable * x;
    if ( (x = dynamic_cast<XVariable *>(xe)) != nullptr) {
        v = *x;
        return true;
    }
    return false;
}

otherwise

bool XCSP3Core::isVariable (XEntity * xe, XVariable * & v) {
    XVariable * x;
    if ((x = dynamic_cast<XVariable *>(xe)) != nullptr) {
        v = x;
        return true;
    }
    return false;
}

p.s.: if you want, I can add a patch file for the other 1233 harmless warnings

[xcsp3team]

Dear Massimo, thanks a lot for your report. I am going to take a loot at this issue asap.

Best.

Gilles Audemard.

[xcsp3team]

I fix the bug. Thanks.

Gilles.

PS : a patch for warning is welcome!

[massimomorara]

First of all, sorry but I strongly suggest the systematic use of "-Wall -Wextra -ansi -pedantic".

In this way I very often find a lot of errors in my code.

Anyway, I'm not sure to remember all but... the most of the warning I've corrected with attached path was of type "unused parameter".

By example, in a method like

virtual void beginInstance(InstanceType type) {}

the parameter type is unused; in C++ the warning can be avoided simply deleting the name of the variable, so

virtual void beginInstance(InstanceType) {}

I've done a lot of this type of deletion, in the patch, but to maintain the name of the parameter (that can be a sort of documentation), I've maintained it in a preceding comment, something like

// InstanceType type
virtual void beginInstance(InstanceType) {}

Another type of usual warnings is regarding the "comparison between signed and unsigned integer"

By example, in

for(int i = 0; i < list.size() - 1; i++)

i, signed, is compared with a size(), that is unsigned.

In all cases I've seen that there is no reason to not define i as unsigned, so I've modified the code as follows

for(auto i = 0U; i < list.size() - 1U; i++)

If you don't like auto (but in C++11 is incredibly useful), I suggest the explicit use of std::size_t.

A strange warning (related with namespaces but that I don't understand; I've opened a question in Stack Overflow but nobody give me a useful answer) is in cases like the following

ostream &XCSP3Core::operator<<(ostream &O, const XIntegerEntity &ie) {
    ie.print(O);
    return O;
}

I've modified the code making explicit the namespace, as follows

namespace XCSP3Core {
ostream & operator<<(ostream &O, const XIntegerEntity &ie) {
    ie.print(O);
    return O;
}
}

Not exactly a warning removing but in the patch I've modified as const some methods in XDomainInteger; by example, I've modified

const int nbValues() {
    return size;
}

(where, IMHO, there is no reason to define const the type of returned value) as follows

int nbValues() const {
    return size;
}

Obviously, if you want to be able to modify the value inside the object using the value returned by the method, you should return a reference; so you could develop a couple of method as follows

int const & nbValues() const 
 { return size; }

int & nbValues()
 { return size; }

patch.txt

[xcsp3team]

Thanks for this issue.

[xcsp3team]

I add again the unused parameters in XCSP3CoreCallbacks.h since there are very important to understand the callback. I added a documentation too.