- Notifications
You must be signed in to change notification settings - Fork4.9k
Commit1c27d16
committed
Revise tree-walk APIs to improve spec compliance & silence warnings.
expression_tree_walker and allied functions have traditionallydeclared their callback functions as, say, "bool (*walker) ()"to allow for variation in the declared types of the callbackfunctions' context argument. This is apparently going to beforbidden by the next version of the C standard, and the latestversion of clang warns about that. In any case it's alwaysbeen pretty poor for error-detection purposes, so fixing it isa good thing to do.What we want to do is change the callback argument declarations tobe like "bool (*walker) (Node *node, void *context)", which iscorrect so far as expression_tree_walker and friends are concerned,but not change the actual callback functions. Strict compliance withthe C standard would require changing them to declare their argumentsas "void *context" and then cast to the appropriate context structtype internally. That'd be very invasive and it would also introducea bunch of opportunities for future bugs, since we'd no longer haveany check that the correct sort of context object is passed by outsidecallers or internal recursion cases. Therefore, we're just goingto ignore the standard's position that "void *" isn't necessarilycompatible with struct pointers. No machine built in the last fortyor so years actually behaves that way, so it's not worth introducingbug hazards for compatibility with long-dead hardware.Therefore, to silence these compiler warnings, introduce a layer ofmacro wrappers that cast the supplied function name to the officialargument type. Thanks to our use of -Wcast-function-type, this willstill produce a warning if the supplied function is seriouslyincompatible with the required signature, without going as far asthe official spec restriction does.This method fixes the problem without any need for source code changesoutside nodeFuncs.h/.c. However, it is an ABI break because thephysically called functions now have names ending in "_impl". Hencewe can only fix it this way in HEAD. In the back branches, we'll haveto settle for disabling -Wdeprecated-non-prototype.Discussion:https://postgr.es/m/CA+hUKGKpHPDTv67Y+s6yiC8KH5OXeDg6a-twWo_xznKTcG0kSA@mail.gmail.com1 parenteccb607 commit1c27d16
2 files changed
+391
-334
lines changed0 commit comments
Comments
(0)