This morning I've been busy tracking a bug in my code, for which I have no explanation as to why it is a bug. The situation is I have an if-else test where, in case of a match, the if-block gets executed, but in case of a non-match, the else-block does NOT get executed. If I turn the whole test around by adding a NOT to the test, and swapping the if and else blocks, everything works.
This is the code which has the bug:
string key_name = "IssueID_shortname"; array qresult = myDB->Query("SHOW keys FROM dbFinalPublicationPassive.tbIssueDetails"); if (qresult && has_value(qresult->Key_name || ({}), key_name)) do_log("Index %O was already set", key_name); else { do_log("Adding index %O", key_name); myDB->Query("ALTER TABLE dbFinalPublicationPassive.tbIssueDetails" " ADD INDEX IssueID_shortname (IssueID, ShortName)"); } This code does not have the bug:
string key_name = "IssueID_shortname"; array qresult = myDB->Query("SHOW keys FROM dbFinalPublicationPassive.tbIssueDetails"); if (!(qresult && has_value(qresult->Key_name || ({}), key_name))) { do_log("Adding index %O", key_name); myDB->Query("ALTER TABLE dbFinalPublicationPassive.tbIssueDetails" " ADD INDEX IssueID_shortname (IssueID, ShortName)"); } else do_log("Index %O was already set", key_name);
Anyone has a clue? (Pike 7.4.464)
Regards,
Arjan
__________________________________________________________ Deze e-mail en de inhoud is vertrouwelijk en uitsluitend bestemd voor de geadresseerde(n). Indien u niet de geadresseerde bent van deze e-mail verzoeken wij u dit direct door te geven aan de verzender door middel van een reply e-mail en de ontvangen e-mail uit uw systemen te verwijderen. Als u geen geadresseerde bent, is het niet toegestaan om kennis te nemen van de inhoud, deze te kopieren, te verspreiden, bekend te maken aan derden noch anderszins te gebruiken.
The information contained in this e-mail is confidential and may be legally privileged. It is intended solely for the addressee. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful. Please notify us immediately if you have received it in error by reply e-mail and then delete this message from your system. __________________________________________________________
It appears the problem occurs because do_log has been implemented as a single-line macro, which in itself has an unbracketed if-statement, without else.
#define do_log(X, Y...) if ( !(< DBG, TME >)[T] ) low_log(X, Y)
The original 'qresult-else' will most likely be seen as an else for the macro definition after compilation, causing it to be behaving different than I expected.
Now I wonder how I should rewrite the above macro definition so I can be sure it checks it's own 'if' and never unexpectedly interferes which other else statements... I don't know how...
Regards,
Arjan
-----Oorspronkelijk bericht----- Van: pike-devel-bounces@lists.lysator.liu.se [mailto:pike-devel-bounces@lists.lysator.liu.se] Namens Arjan van Staalduijnen Verzonden: Thursday, July 31, 2008 12:05 PM Aan: pike-devel@lists.lysator.liu.se Onderwerp: 'if' with failing 'else' block
This morning I've been busy tracking a bug in my code, for which I have no explanation as to why it is a bug. The situation is I have an if-else test where, in case of a match, the if-block gets executed, but in case of a non-match, the else-block does NOT get executed. If I turn the whole test around by adding a NOT to the test, and swapping the if and else blocks, everything works.
This is the code which has the bug:
string key_name = "IssueID_shortname"; array qresult = myDB->Query("SHOW keys FROM dbFinalPublicationPassive.tbIssueDetails"); if (qresult && has_value(qresult->Key_name || ({}), key_name)) do_log("Index %O was already set", key_name); else { do_log("Adding index %O", key_name); myDB->Query("ALTER TABLE dbFinalPublicationPassive.tbIssueDetails" " ADD INDEX IssueID_shortname (IssueID, ShortName)"); } This code does not have the bug:
string key_name = "IssueID_shortname"; array qresult = myDB->Query("SHOW keys FROM dbFinalPublicationPassive.tbIssueDetails"); if (!(qresult && has_value(qresult->Key_name || ({}), key_name))) { do_log("Adding index %O", key_name); myDB->Query("ALTER TABLE dbFinalPublicationPassive.tbIssueDetails" " ADD INDEX IssueID_shortname (IssueID, ShortName)"); } else do_log("Index %O was already set", key_name);
Anyone has a clue? (Pike 7.4.464)
Regards,
Arjan
__________________________________________________________ Deze e-mail en de inhoud is vertrouwelijk en uitsluitend bestemd voor de geadresseerde(n). Indien u niet de geadresseerde bent van deze e-mail verzoeken wij u dit direct door te geven aan de verzender door middel van een reply e-mail en de ontvangen e-mail uit uw systemen te verwijderen. Als u geen geadresseerde bent, is het niet toegestaan om kennis te nemen van de inhoud, deze te kopieren, te verspreiden, bekend te maken aan derden noch anderszins te gebruiken.
The information contained in this e-mail is confidential and may be legally privileged. It is intended solely for the addressee. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful. Please notify us immediately if you have received it in error by reply e-mail and then delete this message from your system. __________________________________________________________
__________________________________________________________ Deze e-mail en de inhoud is vertrouwelijk en uitsluitend bestemd voor de geadresseerde(n). Indien u niet de geadresseerde bent van deze e-mail verzoeken wij u dit direct door te geven aan de verzender door middel van een reply e-mail en de ontvangen e-mail uit uw systemen te verwijderen. Als u geen geadresseerde bent, is het niet toegestaan om kennis te nemen van de inhoud, deze te kopieren, te verspreiden, bekend te maken aan derden noch anderszins te gebruiken.
The information contained in this e-mail is confidential and may be legally privileged. It is intended solely for the addressee. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful. Please notify us immediately if you have received it in error by reply e-mail and then delete this message from your system. __________________________________________________________
#define do_log(X, Y...) if ( !(< DBG, TME >)[T] ) low_log(X, Y)
[...]
Now I wonder how I should rewrite the above macro definition so I can be sure it checks it's own 'if' and never unexpectedly interferes which other else statements... I don't know how...
#define do_log(X, Y...) do { if ( !(< DBG, TME >)[T] ) low_log(X, Y); } while(0)
A classic.
Now I wonder how I should rewrite the above macro definition so I can be sure it checks it's own 'if' and never unexpectedly interferes which other else statements... I don't know how...
I think using brackets around the if in the code where the makro is used should fix it.
Like this:
if (qresult && has_value(qresult->Key_name || ({}), key_name)) { do_log("Index %O was already set", key_name); } else { do_log("Adding index %O", key_name); myDB->Query("ALTER TABLE dbFinalPublicationPassive.tbIssueDetails" " ADD INDEX IssueID_shortname (IssueID, ShortName)");
Marc
i don't think there is a safe way to do this with a macro. you are taking a statement and make it look like an expression, yet it really isn't. there will always be cases where this can break.
you could do something like:
#define do_log(X, Y...) { if ( !(< DBG, TME >)[T] ) low_log(X, Y); }
then you would get an error if you place a ; after do_log() before an else (unless you have another block of {} around it.
an error is better than a random hard to see unusual behaviour.
i'd use a regular function instead of a macro though.
greetings, martin.
I was trying to implement a change to the macro which would make it safe, because we're using the call to this macro in lots of places and it would be hard to pinpoint all the problematic places.
I made the change like you describe earlier and figured I had to be doing something wrong, because it resulted in so many compile errors (macro is defined in an included file). Reading your explanation it showed the change was okay, but it had to be something else. Now making the change again I was able to trace all those compile errors back to the places where we wwere using the macro inside if-else-statements without brackets, being the places where things were silently going wrong.
Thanks!
Arjan
-----Oorspronkelijk bericht----- Van: Martin Bähr [mailto:mbaehr@email.archlab.tuwien.ac.at] Verzonden: Thursday, July 31, 2008 3:06 PM Aan: Arjan van Staalduijnen CC: pike-devel@lists.lysator.liu.se Onderwerp: Re: 'if' with failing 'else' block
i don't think there is a safe way to do this with a macro. you are taking a statement and make it look like an expression, yet it really isn't. there will always be cases where this can break.
you could do something like:
#define do_log(X, Y...) { if ( !(< DBG, TME >)[T] ) low_log(X, Y); }
then you would get an error if you place a ; after do_log() before an else (unless you have another block of {} around it.
an error is better than a random hard to see unusual behaviour.
i'd use a regular function instead of a macro though.
greetings, martin.
__________________________________________________________ Deze e-mail en de inhoud is vertrouwelijk en uitsluitend bestemd voor de geadresseerde(n). Indien u niet de geadresseerde bent van deze e-mail verzoeken wij u dit direct door te geven aan de verzender door middel van een reply e-mail en de ontvangen e-mail uit uw systemen te verwijderen. Als u geen geadresseerde bent, is het niet toegestaan om kennis te nemen van de inhoud, deze te kopieren, te verspreiden, bekend te maken aan derden noch anderszins te gebruiken.
The information contained in this e-mail is confidential and may be legally privileged. It is intended solely for the addressee. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful. Please notify us immediately if you have received it in error by reply e-mail and then delete this message from your system. __________________________________________________________
Arjan van Staalduijnen wrote:
I was trying to implement a change to the macro which would make it safe, because we're using the call to this macro in lots of places and it would be hard to pinpoint all the problematic places.
Do yourself a favour and use the:
do { ...; } while(0)
construct as suggested earlier. Using the merely braced version introduces other problems (sometimes).
I never said I wouldn't use that code, but I can decide for myself which version to use best where. There were reasons why it wasn't used as a function in the first place. In any case that might still be the best code to switch back to.
Regards,
Arjan
-----Oorspronkelijk bericht----- Van: pike-devel-bounces@lists.lysator.liu.se [mailto:pike-devel-bounces@lists.lysator.liu.se] Namens Stephen R. van den Berg Verzonden: Thursday, July 31, 2008 5:55 PM Aan: Arjan van Staalduijnen CC: pike-devel@lists.lysator.liu.se; Martin B?hr Onderwerp: Re: 'if' with failing 'else' block
Arjan van Staalduijnen wrote:
I was trying to implement a change to the macro which would make it
safe, because we're using the call to this macro in lots of places and it would be hard to pinpoint all the problematic places.
Do yourself a favour and use the:
do { ...; } while(0)
construct as suggested earlier. Using the merely braced version introduces other problems (sometimes).
pike-devel@lists.lysator.liu.se
-
Arjan van Staalduijnen -
Marc Dirix -
Marcus Comstedt (ACROSS) (Hail Ilpalazzo!) ＠ Pike (-) developers forum -
Martin Bähr -
Stephen R. van den Berg