[PATCH] D85937: [flang][msvc] Split class declaration and constexpr variable definition. NFC.

[Peter Klausler via Phabricator via llvm-commits]

klausler added a comment.

In D85937#2219052 <https://reviews.llvm.org/D85937#2219052>, @Meinersbur wrote:

> In D85937#2219028 <https://reviews.llvm.org/D85937#2219028>, @klausler wrote:
>
>> You might have to use conditional preprocessing to make this workaround specific to MSVC.
>
> The workaround is valid C++ and works fine with every compiler. So why add a #if maze?

Because you're working around a bug that might get fixed in MSVC, and then the conditional preprocessing could be removed.  And if you run into even one MSVC bug later that can't be worked around now, and we have to wait for a later MSVC version, this and other workarounds may no longer be necessary with that later compiler.

I ran into many bugs in GCC 7.2 when implementing the f18 parser.  Most of them I worked around when the replacement code was no less readable or maintainable; sometimes, I used conditional code when the replacements were not what I wanted to remain as permanent implementations.  It's always a case-by-case call.

In this specific case, the workaround doesn't look too bad.  But are there other workarounds?  Would adding "static" to the original "constexpr" suffice to avoid the bug?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85937/new/

https://reviews.llvm.org/D85937