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

CWG-2387 cleanups: Remove inline/_INLINE_VAR from constexpr variable templates #4533

Closed
StephanTLavavej opened this issue Mar 26, 2024 · 0 comments · Fixed by #4546
Closed
Labels
enhancement Something can be improved fixed Something works now, yay!

Comments

@StephanTLavavej
Copy link
Member

VS 2022 17.10 Preview 1 fixed DevCom-10518531 "MSVC doesn't implement CWG-2387 (const-qualified variable templates have external linkage by default)". Clang 9 implemented this long ago (see llvm/llvm-project@bb75068).

This issue was requested by @CaseyCarter in #4116 (comment):

we should have an issue reminding us to audit and remove all the extraneous inlines when our supported compilers are all ready.

To resolve this, we should remove inline and _INLINE_VAR from all of our constexpr variable templates (both primary templates and partial specializations).

⚠️ However, explicit specializations of variable templates, and ordinary (non-template) variables, should NOT be modified. They still need inline/_INLINE_VAR.

This change will follow our general rationale of "don't help the compiler". Avoiding unnecessary code helps to emphasize the remaining necessary code. Also, this will make our conventions for constexpr variables nearly identical to our conventions for functions: primary templates and partial specializations don't need inline, while explicit specializations and non-templates do.

I'm less enthusiastic about cleaning up test code (we generally avoid intrusive changes to test code because they're low value; also, test code sometimes intentionally does wacky things, so cleanups have the potential to disrupt what they were testing). In this case, the change is semi-mechanical, the impact is well-understood and limited, and there aren't that many occurrences in test code, so I think it would be acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay!
Projects
None yet
1 participant