Andres Freund <and...@2ndquadrant.com> writes: > [ patch to mark elog() as non-returning if elevel >= ERROR ]
It strikes me that both in this patch, and in Peter's previous patch to do this for ereport(), there is an opportunity for improvement. Namely, that the added code only has any use if elevel is a constant, which in some places it isn't. We don't really need a call to abort() to be executed there, but the compiler knows no better and will have to generate code to test the value of elevel and call abort(). Which rather defeats the purpose of saving code; plus the compiler will still not have any knowledge that code after the ereport isn't reached. And another thing: what if the elevel argument isn't safe for multiple evaluation? No such hazard ever existed before these patches, so I'm not very comfortable with adding one. (Even if all our own code is safe, there's third-party code to worry about.) When we're using recent gcc, there is an easy fix, which is that the macros could do this: __builtin_constant_p(elevel) && (elevel) >= ERROR : pg_unreachable() : (void) 0 This gets rid of both useless code and the risk of undesirable multiple evaluation, while not giving up any optimization possibility that existed before. So I think we should add a configure test for __builtin_constant_p() and do it like that when possible. That still leaves the question of what to do when the compiler doesn't have __builtin_constant_p(). Should we use the coding that we have, or give up and not try to mark the macros as non-returning? I'm rather inclined to do the latter, because of the multiple-evaluation-risk argument. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers