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

<atomic>: Building a header unit with /ZI emits warning C5260 #3287

Closed
orikama opened this issue Dec 14, 2022 · 6 comments
Closed

<atomic>: Building a header unit with /ZI emits warning C5260 #3287

orikama opened this issue Dec 14, 2022 · 6 comments
Labels
bug Something isn't working compiler Compiler work involved fixed Something works now, yay!

Comments

@orikama
Copy link

orikama commented Dec 14, 2022

C:\Test>cl /std:c++20 /EHsc /W4 /WX /ZI /Od /exportHeader /headerName:angle atomic
Microsoft (R) C/C++ Optimizing Compiler Version 19.35.32124 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

atomic
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32124\include\atomic(325): error C2220: the following warning is treated as an error
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32124\include\atomic(325): warning C5260: the constant variable 'std::memory_order const (* `std::memory_order __cdecl std::_Combine_cas_memory_orders(std::memory_order,std::memory_order)'::`2'::_Combined_memory_orders)[6]' has internal linkage in an included header file context, but external linkage in imported header unit context; consider declaring it 'inline' as well if it will be shared across translation units, or 'static' to express intent to use it local to this translation unit
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32124\include\atomic(325): note: to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings

Noticed this warning while reporting some other bugs in VS 2022 17.5.0 Preview 2

@CaseyCarter CaseyCarter added bug Something isn't working compiler Compiler work involved labels Dec 14, 2022
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Dec 14, 2022

Line 325 defines the function-local static constexpr array of memory_order _Combined_memory_orders in an inline function:

STL/stl/inc/atomic

Lines 312 to 342 in 4483e87

_NODISCARD inline memory_order _Combine_cas_memory_orders(
const memory_order _Success, const memory_order _Failure) noexcept {
// Finds upper bound of a compare/exchange memory order
// pair, according to the following partial order:
// seq_cst
// |
// acq_rel
// / \
// acquire release
// | |
// consume |
// \ /
// relaxed
static constexpr memory_order _Combined_memory_orders[6][6] = {// combined upper bounds
{memory_order_relaxed, memory_order_consume, memory_order_acquire, memory_order_release, memory_order_acq_rel,
memory_order_seq_cst},
{memory_order_consume, memory_order_consume, memory_order_acquire, memory_order_acq_rel, memory_order_acq_rel,
memory_order_seq_cst},
{memory_order_acquire, memory_order_acquire, memory_order_acquire, memory_order_acq_rel, memory_order_acq_rel,
memory_order_seq_cst},
{memory_order_release, memory_order_acq_rel, memory_order_acq_rel, memory_order_release, memory_order_acq_rel,
memory_order_seq_cst},
{memory_order_acq_rel, memory_order_acq_rel, memory_order_acq_rel, memory_order_acq_rel, memory_order_acq_rel,
memory_order_seq_cst},
{memory_order_seq_cst, memory_order_seq_cst, memory_order_seq_cst, memory_order_seq_cst, memory_order_seq_cst,
memory_order_seq_cst}};
_Check_memory_order(static_cast<unsigned int>(_Success));
_Check_load_memory_order(_Failure);
return _Combined_memory_orders[static_cast<int>(_Success)][static_cast<int>(_Failure)];
}

the name of this variable should have no linkage. I smell a compiler bug. @cdacamar, do you agree?

@cdacamar
Copy link
Contributor

I concur, the compiler should not be complaining here. Put it on my tab 😉.

@StephanTLavavej StephanTLavavej changed the title <atomic>: Importing as a header unit emits warning C5260 <atomic>: Building a header unit with /ZI emits warning C5260 Dec 15, 2022
@StephanTLavavej
Copy link
Member

Thanks, I've updated the title to note (1) this occurs when building the header unit (not importing it) and (2) this is specific to /ZI (debug info for edit and continue). I've filed VSO-1706387 internally for @cdacamar to keep track of, and I'll record this in the tracking issue #1694.

@StephanTLavavej
Copy link
Member

Fixed by internal MSVC-PR-442426 for VS 2022 17.6 Preview 1. Thanks again for the report!

@luusl
Copy link

luusl commented Mar 1, 2023

Is there any chance to merge the fix to 17.5? We have "Treat warnings as errors" active on all of our projects, which is causing the projects to stop compiling.

@StephanTLavavej
Copy link
Member

@luusl As this is only a warning, I doubt it would meet the servicing bar. (There's a pretty high requirement to backport compiler changes, since every backport risks destabilizing something else.) I recommend disabling this warning as a temporary workaround until 17.6 ships; it's a very new warning so you aren't giving up any other coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler Compiler work involved fixed Something works now, yay!
Projects
None yet
Development

No branches or pull requests

5 participants