On Mon, 15 Apr 2024 11:46:25 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> Should also remove the `#pragma alloca` in os_aix.cpp. > > It was too bad that I did not see and review this change in the makefiles. :-( > > While you guys could have gone either way, I strongly dislike the choice to > include a redefinition in the makefiles. If this really should be done, we > should introduced a new variable to carry such changes, instead of > piggybacking it with the OS defines. :-( But, I don't think it should be done > at all. > > There are several reasons why this is a inferior solution: > > 1) It does not follow prior examples. We have tried hard before not do things > like this, but rather pass flags as defines (e.g. `-DREDEFINE_ALLOCA` had > been better) > 2) It does not scale. If we start in effect allowing code in the command > line, there is no clear limit anymore what should be placed in the source > code files and what should be placed on the command line. > 3) It messes up command lines. Keeping command lines as short as reasonable > possible is a goal we try to strive for. In this case, there is also the `'` > inside them (which I don't understand why), which is just begging for > quoting/escaping problems, making command lines hard to copy/paste, send to > different systems (like logging) etc. > > I'd really like to see a follow-up PR that moves this away from the command > line define and into a source code file instead. Can we unconditionally `#include <alloca.h>` in all files which use `alloca`? Or does that disturb any platform? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1565770190
