LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 42027 - [Win] LNK2005: bool const std::_Is_integral<bool> already defined
Summary: [Win] LNK2005: bool const std::_Is_integral<bool> already defined
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: C++14 (show other bugs)
Version: unspecified
Hardware: PC Windows NT
: P enhancement
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-26 12:14 PDT by Mikhail Strelnikov
Modified: 2019-07-29 17:43 PDT (History)
7 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Strelnikov 2019-05-26 12:14:37 PDT
Self-hosting on Windows (https://github.com/llvm/llvm-project/blob/master/llvm/utils/release/build_llvm_package.bat) is broken by this commit:

https://github.com/llvm-mirror/clang/commit/4241f674da3531d69abc759d727fb0ae7b31535e

Another way to reproduce the bug:

// h.hpp
#pragma once

template<typename T>
constexpr bool v = false;

template <>
constexpr bool v<bool> = true;

// unit1.cpp
#include "h.hpp"

// unit2.cpp
#include "h.hpp"

int main()
{
	return v<bool>;
}

# clang++ -std=c++14 unit1.cpp unit2.cpp
error LNK2005: "bool const v<bool>" (??$v@_N@@3_NB) already defined

I've no idea if it is OK code for C++14, but this is what MS STL does in its <xtr1common> header.
Comment 1 Richard Smith 2019-05-26 12:25:22 PDT
This code is invalid, but it seems easy enough for us to support. We'll need to decide if we want to special-case the problematic declarations or give weak_odr linkage to all (const?) variable template explicit specialisations.
Comment 2 Mikhail Strelnikov 2019-05-26 14:00:35 PDT
Well, since it is invalid, maybe this can be treated as another non-standard Microsoft extension? But then I'd prefer to see a warning from clang so that Microsoft people can fix their STL (and compiler).
(but my main concern is this self-hosting issue)
Comment 3 Richard Smith 2019-05-26 14:30:49 PDT
Given that this only causes a failure at link time, it's unlikely we can warn on it. (We could warn if we find an strong external definition in a header file, but that's likely to have false positives for .def-style #includes and the like.)
Comment 4 Reid Kleckner 2019-06-10 18:30:37 PDT
Can you elaborate on the self-hosting issue? When I look in xtr1common I don't see anything that would be problematic. All the fully specialized variable templates delegate to class templates, which use static data members, which clang marks selectany.
Comment 5 Mikhail Strelnikov 2019-06-11 05:13:33 PDT
(In reply to Reid Kleckner from comment #4)

I'm using VS 2019 Preview (16.2.0 Preview 1.0) (14.22.27706).

Here is aforementioned _Is_integral:

template <class _Ty>
_INLINE_VAR constexpr bool _Is_integral = // determine whether cv-unqualified type _Ty is integral
    _Is_any_of_v<_Ty, bool, char, signed char, unsigned char, wchar_t,
#if _HAS_CHAR8_T
        char8_t,
#endif // _HAS_CHAR8_T
        char16_t, char32_t, short, unsigned short, int, unsigned int, long, unsigned long, long long,
        unsigned long long>;
#else // ^^^ workaround / no workaround vvv
template <class>
_INLINE_VAR constexpr bool _Is_integral = false; // determine whether cv-unqualified type argument is integral

template <>
_INLINE_VAR constexpr bool _Is_integral<bool> = true;

template <>
_INLINE_VAR constexpr bool _Is_integral<char> = true;

template <>
_INLINE_VAR constexpr bool _Is_integral<signed char> = true;

... (similar to this random file on the Internet: https://github.com/Chuyu-Team/VC-LTL/blob/master/VC/14.21.27702/include/xtr1common#L166)



In C++14 mode _INLINE_VAR (defined in <yvals_core.h>) expands to nothing:

#if _HAS_CXX17
#define _INLINE_VAR inline
#else // _HAS_CXX17
#define _INLINE_VAR
#endif // _HAS_CXX17

(https://github.com/Chuyu-Team/VC-LTL/blob/master/VC/14.21.27702/include/yvals_core.h#L386)

The error: LNK2005: "bool const std::_Is_integral<bool>" (??$_Is_integral@_N@std@@3_NB) already defined

My understanding is that this is not clang problem (_INLINE_VAR should probably expand to __declspec(selectany) in this case, idk), so please feel free to close this bug.
Comment 6 Reid Kleckner 2019-06-11 11:32:56 PDT
It happens that I was building locally with VC version 14.20.27508, which defines _Is_integral as a class template. You said you were using the preview version of VS 2019, so I figured this was a bug I could report back to the VC team before they release it. But first, I wanted to check if I had the most recently released version of 2019 to confirm the issue wasn't there. Of course, now that I've run the updater, it has the problem, and my local build is completely broken. :-[

I guess I'll go back to 2017, which I happen to still have installed, until I fix this bug.
Comment 7 Billy O'Neal 2019-06-11 11:58:11 PDT
Does clang support inline constexpr in this form in C++14 mode (presumably with a warning we can suppress) so we could turn inline on for clang unconditionally?

Is this some recent change after clang 8 to break this? (That's what we currently test with)

Billy O'Neal
Visual C++ Libraries
Comment 8 Reid Kleckner 2019-06-11 13:40:32 PDT
(In reply to Billy O'Neal from comment #7)
> Does clang support inline constexpr in this form in C++14 mode (presumably
> with a warning we can suppress) so we could turn inline on for clang
> unconditionally?

It looks like clang accepts inline variables as an extension under -Wc++17-extensions.

I tried using __declspec(selectany) as an alternative, but it doesn't seem to work with constexpr. Clang doesn't seem to consider selectany globals to be constant expressions, even if they are marked constexpr.

> Is this some recent change after clang 8 to break this? (That's what we
> currently test with)

Yes, r359260. Looking at that again, are we sure we want to give variable templates external linkage in C++14, where inline variables aren't available? MSVC doesn't implement this DR yet, so if we want to be compatible, we have to go back to our previous behavior.
Comment 9 Richard Smith 2019-06-11 14:05:03 PDT
> I tried using __declspec(selectany) as an alternative, but it doesn't seem
> to work with constexpr. Clang doesn't seem to consider selectany globals to
> be constant expressions, even if they are marked constexpr.

Yeah, if we're not sure which definition of a variable will be selected, we don't allow its value to be read from within the constant evaluator. That was intended to apply to __attribute__((weak)), but seems to catch this case too. Maybe it shouldn't? Can a selectany constexpr variable be replaced by one with a different initializer at link time? (But this probably isn't a fruitful line of inquiry, since if we can add 'selectany', we can add 'inline' instead.)


> are we sure we want to give variable templates external linkage in C++14,
> where inline variables aren't available?

If we make C++14 and C++17 differ in this regard, then we'll effectively have an ABI difference between C++14 and C++17 forever. If we take the hit now, we only have pain in the short term. I think the short-term pain is worth it.

(Also of note: any odr-use of _Is_integral<bool> in an inline function will result in an ODR violation with the old model.)

So, degrading from 'external' to 'weak_odr' seems OK, but I wouldn't want to go all the way to 'internal'.
Comment 10 Billy O'Neal 2019-06-11 14:34:41 PDT
>Can a selectany constexpr variable be replaced by one with a different initializer at link time?

Yes, but under the usual inline penalty of ODR.
Comment 11 Reid Kleckner 2019-06-11 15:10:29 PDT
(In reply to Richard Smith from comment #9)
> > I tried using __declspec(selectany) as an alternative, but it doesn't seem
> > to work with constexpr. Clang doesn't seem to consider selectany globals to
> > be constant expressions, even if they are marked constexpr.
> 
> Yeah, if we're not sure which definition of a variable will be selected, we
> don't allow its value to be read from within the constant evaluator. That
> was intended to apply to __attribute__((weak)), but seems to catch this case
> too. Maybe it shouldn't? Can a selectany constexpr variable be replaced by
> one with a different initializer at link time? (But this probably isn't a
> fruitful line of inquiry, since if we can add 'selectany', we can add
> 'inline' instead.)

We give __declspec(selectany) things _odr linkage, unlike __attribute__((weak)) things, so I think we can allow them in constexpr contexts. I'll file something for that.

I believe the name 'selectany' is a reference to 'IMAGE_COMDAT_SELECT_ANY', which means the linker should pick any definition, because they should all be functionally equivalent.

> 
> > are we sure we want to give variable templates external linkage in C++14,
> > where inline variables aren't available?
> 
> If we make C++14 and C++17 differ in this regard, then we'll effectively
> have an ABI difference between C++14 and C++17 forever. If we take the hit
> now, we only have pain in the short term. I think the short-term pain is
> worth it.
> 
> (Also of note: any odr-use of _Is_integral<bool> in an inline function will
> result in an ODR violation with the old model.)
> 
> So, degrading from 'external' to 'weak_odr' seems OK, but I wouldn't want to
> go all the way to 'internal'.

Should we make fully specialized variable templates weak_odr on all platforms or just on Windows? I can easily imagine that users will write this kind of code on Linux, compile it for C++14, and get link errors. What do you recommend that they write instead, assuming they don't have access to 'inline'? I suppose it's legal for them to add 'static' to explicitly opt into the old behavior, but it somehow feels wrong. I'm leaning towards making all fully specialized, constant, integer, variable templates weak_odr, everywhere, kind of like we do for explicit template instantiations.


---

BTW, I fixed my local build by hacking _INLINE_VAR to expand to 'inline'.
Comment 12 Reid Kleckner 2019-06-12 14:08:58 PDT
I made clang give constexpr explicit specializations of variable templates linkonce_odr linkage in r363191. This makes it so clang doesn't emit these _Is_integral symbols in every object file that transitively includes xtr1common, and if they are needed, they won't cause duplicate symbol errors.

This has essentially the same effect as implicitly considering all constexpr variable templates as being marked 'inline'. It isn't strictly conforming, because you could do this:

// tu1.cpp
template <class> const int foo;
extern template <> const int foo<int>;
const int *useit() { return &foo<int>; }
// tu2.cpp
template <class> const int foo;
template <> constexpr int foo<int> = 42;

With my change, clang will not emit foo<int> in tu2.cpp as written. But, for constexpr variable templates, I think this will be very uncommon. The point of constexpr is to be able to write constants and put them in headers to enable further optimization.
Comment 13 Reid Kleckner 2019-07-29 17:32:55 PDT
r363191 was reverted, but it seems this will be fixed in a soon-to-be-released version of the Visual C++ standard library.
Comment 14 Billy O'Neal 2019-07-29 17:43:32 PDT
(In reply to Reid Kleckner from comment #13)
> r363191 was reverted, but it seems this will be fixed in a
> soon-to-be-released version of the Visual C++ standard library.

Yes, Stephan fixed this one.