Hmm… so you're worried about people setting the definition to 0 for disabled and nonzero || present for enabled? - I suppose all the `#define`s in PostgreSQL could be guarded and checked for both presence and either empty value or value. But maybe you just don't want to touch this side of the codebase?
https://gcc.gnu.org/onlinedocs/cpp/If.html https://learn.microsoft.com/en-us/cpp/preprocessor/hash-if-hash-elif-hash-else-and-hash-endif-directives-c-cpp?view=msvc-170 (assuming similar on clang and other compilers; I'm sure this behaviour predated C89) It would be odd for people to depend on behaviour like not defining CLOBBER_FREED_MEMORY at the CPPFLAGS level. Anyway, I assume it's your call, so should I withdraw this PATCH then? - Or make a more comprehensive PATCH checking for definition and (emptiness or nonzero)? PS: I did send through a PR to that build-PostgreSQL-extensions-with-Rust project https://github.com/pgcentralfoundation/pgrx/pull/1826 Samuel Marks, PhD https://linkedin.com/in/samuelmarks On Fri, Aug 23, 2024 at 4:39 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Samuel Marks <samuelma...@gmail.com> writes: > > It will resolve the large number of these warnings from > > https://github.com/pgcentralfoundation/pgrx/blob/6dfb9d1/cargo-pgrx/src/command/init.rs#L411-L412: > > Hmm, that seems like their problem not ours. It's not very clear > to me why they'd want to force these flags from the compiler > command line in the first place, but if they do they should be > consistent with the more usual ways to set them. > > > and yes will be sending them a patch also. But there's no harm in not > > redefining symbols, so not sure why this is a controversial patch. > > The reason I'm resistant to changing it is that the code you want > to touch has been unchanged since 2003 in the first case, and 2013 > in the second. It's fairly unclear what external code might have > grown dependencies on the current behavior, but with that much > history I'm not eager to bet that the answer is "none". Also, > the present setup makes it clear that you are supposed to test > "#ifdef CLOBBER_FREED_MEMORY" not "#if CLOBBER_FREED_MEMORY". > If we stop locking down the expected contents of the macro, bugs > of that sort could sneak in. > > regards, tom lane
