Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initializers of static data members of class templates not instantiated with the class when targeting the MS ABI #48962

Open
zoecarver opened this issue Mar 17, 2021 · 2 comments
Labels
bugzilla Issues migrated from bugzilla c++ confirmed Verified by a second party

Comments

@zoecarver
Copy link
Contributor

Bugzilla Link 49618
Version trunk
OS All
CC @DougGregor,@zygoloid

Extended Description

I think this Godbolt demonstrates the problem pretty clearly: https://godbolt.org/z/Mv5osj

Basically, if there's an invalid decl (such as the var decl shown in the Godbolt), neither the decl nor its parent will be marked invalid on Windows. But, on all other platforms, they will (properly?) be marked as invalid. I ran into this problem while testing Swift's C++ interop; it was causing crashes because we had no way to differentiate valid and invalid decls on Windows, so invalid decls were slipping through.

I thought the MS compatibility features might have been the cause of this issue, but the problem persists when I pass -fno-delayed-template-parsing and -fno-ms-compatibility.

I'd like to understand why this is happening and if it's the intended behavior. I'd also like to find a good short-term solution so that I can stop Swift's C++ interop from crashing. (Whether that's a flag to disable it or another API that I can use to find out if the decl is actually invalid. Right now, I'm using "isInvalidDecl.")

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Mar 17, 2021

This looks like inline variable semantics leaking through...

MSVC treats an in-class definition of a static data member as being a definition. For example,

struct T { static const bool value = true; };
extern const bool &b = T::value;

does not require a separate definition of T::value under the MS ABI. Because this affects the ABI, Clang implements the same behavior.

However, this means that Clang thinks the initializer of the static data member in your testcase is part of the definition of the variable, not part of the declaration, and instantiating a class doesn't instantiate the definition portions of any static data members (http://eel.is/c++draft/temp.inst#3.1), so Clang doesn't instantiate the initializer with the class definition and instead delays the instantiation until the variable is used (like it would for an inline static data member).

Clang does diagnose if you later mention Invalid::value (more specifically, if you odr-use it or if it's needed for constant evaluation), because that triggers the instantiation of the definition.

Given that MSVC instantiates the initializer with the class declaration in this case, and doing so is more consistent with the language rules, we should probably do the same.

@zoecarver
Copy link
Contributor Author

Sorry for the delay in responding. Thanks for all that information, it was super helpful. I did some digging and it seems like this stems from Clang incorrectly marking constexpr var decls as implicitly inline on Windows. I think I'll be able to put up a fix for this later today.

Thanks again for the help and quick reply!

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla c++ confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

2 participants