RFR: 8301244: Replace legacy pragma implementations in HotSpot's compilerWarnings.hpp [v7]

[Kim Barrett]

On Tue, 31 Jan 2023 09:06:09 GMT, Julian Waters <jwaters at openjdk.org> wrote:

> The aim was to introduce a way to easily use pragmas inside macro implementations in HotSpot which behave identically to if one had used `#pragma`, regardless of any quirks on any platform/compiler, competing with the regular `#pragma` preprocessor directive is otherwise not the goal of this change. Defining differing pragmas which are compiler specific based on `#ifdef` blocks are a different issue too, all this proposal intends to do is make it easy for defining pragmas inside macros, as mentioned above. The vast majority of the compilers that can be used to compile HotSpot (that is to say, all of them except for Visual C++) don't have an easy way to do this that doesn't involve the hassle of defining one-off helper macros directly where they're used to expand the input to properly feed into `_Pragma` (see compilerWarnings_gcc.hpp for instance). I get that this might sound slightly petty, but I'd prefer if macros defined for the sole purpose of solving a local issue were somewhat
  kept to a minimum in HotSpot if they could be avoided. This can also be especially helpful in shared code where a one-for-all macro that contains a pragma is needed, where __pragma wouldn't be accepted by other compilers anyway. Regarding Kim's concerns, I could indeed make the implementation use __pragma if `#ifdef _MSC_VER` is true, but I don't really see the need for doing that, since Visual C++ pretty much treats __pragma and `_Pragma` identically (to the point that they share the same bugs), and doing so would just be extra code which would behave the same
> 
> I forgot to mention `PRAGMA_UTIL` was just `PRAGMA` earlier on and in macros.hpp, I changed it after hearing Kim's concerns, but now after looking back at it I think I should've kept it for review

The existing one-off helper macro (PRAGMA_DISABLE_GCC_WARNING_AUX) exists in
its current form because there is exactly one use of that behavior, by the
associated no-_AUX macro. If there were multiple uses then there would be
reason for a common helper macro, and such would likely already exist. That
hasn't happened yet. If it did, I'd probably call it PRAGMA_STRINGIZE or
something like that. (Coming up with a good name for that utility is another
reason why we have the existing _AUX macro.) But until there is such a need, I
don't see the point. It's just premature generalization.

To reiterate David, is there some new use-case that you have in mind? Maybe we
need to discuss that first.

I also don't see the point of making MSVC jump through that extra hoop. The
pragma tokens being used for a given PRAGMA_ macro are likely always going to
be different between MSVC and gcc (or any other compiler), so letting
MSVC-specific code use an MSVC-specific extension doesn't bother me at all.

And because the needed tokens for a particular effect are always different for
different compilers, the idea of a one-for-all macro containing a "bare"
pragma that works for all platforms is just not a thing. (There are a small
number of standard pragmas, where one would likely just directly use _Pragma
with the appropriate string on all platforms.)

And what MSVC bugs in __pragma/_Pragma are you referring to?  If it's the
macro expansion of the pragma tokens, that's not a bug, it's an intentional
feature that is permitted by the Standard(s).  It's implementation-defined
whether those tokens are subject to expansion.

-------------

PR: https://git.openjdk.org/jdk/pull/12255