Fix MS VC 2019 build

[mkhon]

C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\xmemory(714): error C2280: 'std::pair<const libMesh::FEType,std::unique_ptr<libMesh::FEAbstract,std::default_deletelibMesh::FEAbstract>>::pair(const std::pair<const libMesh::FEType,std::unique_ptr<libMesh::FEAbstract,std::default_deletelibMesh::FEAbstract>> &)': attempting to reference a deleted function

[jwpeterson]

We can't delete this code because it's a workaround for another compiler (Intel). In addition this potentially changes the behavior of the code because init_internal_data() could theoretically be called multiple times (not sure if it actually is) and we would need _element_fe, _side_fe, etc. to be cleared each time.

[mkhon]

1. How can I test ICC build (e.g on Linux)?

2. Currently init_internal_data() is private and is only called from the constructor.

3. The problem with MS VC 2019 is that neither of this works:

    std::vector<std::map<int, std::unique_ptr<const char *>>> v1;
    v1.resize(1); // error: attempting to reference a deleted function
    v1.push_back(std::map<int, std::unique_ptr<const char *>>()); // DITTO
    v1.push_back(std::move(std::map<int, std::unique_ptr<const char *>>())); // DITTO
    v1.emplace_back(std::map<int, std::unique_ptr<const char *>>()); // DITTO
    v1.emplace_back(std::move(std::map<int, std::unique_ptr<const char *>>())); // DITTO

[jwpeterson]

Maybe try emplace_back()?

How can I test ICC build (e.g on Linux)?

We no longer have CI testing of the Intel compiler, unfortunately. If you can find some other workaround that fixes MSVC then we could consider ifdef'ing based on the compiler. Honestly I find it surprising that two different compilers have issues with the same (otherwise innocuous looking) lines of code. I'm pretty sure the code is standards conforming and it's the compilers that are wrong.

[mkhon]

I think the problem is not in the compilers but in their STL implementations.

The problem with this piece of code is that std::unique_ptr<> is non-copyable (it has copy constructor explicitly deleted), but std::vector::push_back() may involve copying.

The same about std::vector::resize() which may re-allocate memory and copy objects from old location to new.

resize() and push_back() implementations can use move constructors, but likely ICC 19 and MS VC 2019 got it wrong.

[mkhon]

@jwpeterson yes, I also tried emplace_back() - produces the same error with MS VC 2019

[mkhon]

I submitted a less intrusive change, but I don't think that different codepaths and #ifdef-fu is a good thing.

I am almost sure construction of vector of specified size will work in older ICC.

[jwpeterson]

I am almost sure construction of vector of specified size will work in older ICC

Yes, it's possible you are correct. Looking back to the original change (7e6abb4) the only offending lines were the resize() calls, no compiler that I'm aware of has complained about the constructor initialization syntax. This raises the question of whether MSVC/ICC19 would accept the syntax

_element_fe = std::vector<std::map<FEType, std::unique_ptr<FEAbstract>>>(4);

for reinitializing the vector with four empty maps.

[mkhon]

@jwpeterson bingo! I tested the fix and updated the PR

[moosebuild]

Job Coverage on 62914ad wanted to post the following:

Coverage

	d9f0f7	#3137 62914a
	Total	Total	+/-	New
Rate	56.36%	56.36%	-0.00%	100.00%
Hits	44049	44045	-4	2
Misses	34106	34106	-	0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

[roystgnr]

Rather than clearing the vector of maps and then restoring empty maps, would it work to loop over the maps and clear them individually?

[jwpeterson]

Latest diff looks good to me. I will just throw in a comment that these two lines are a resize() workaround for the Intel 19 and MSVC STL implementations before merging (once the CI all passes again).