[SERVER-28890] Workaround for Visual C++ standard library under /permissive- Created: 20/Apr/17  Updated: 30/Oct/23  Resolved: 24/Apr/17

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: None
Fix Version/s: 3.5.7

Type: Bug Priority: Trivial - P5
Reporter: Yuriy Solodkyy Assignee: ADAM Martin (Inactive)
Resolution: Fixed Votes: 0
Labels: todo_in_code
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-43423 Use ternary more naturally for C++, w... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Steps To Reproduce:

This is only reproducible with MS Visual C++ that is not yet released.

Participants:

 Description   

Visual C++ compiler team is improving conformance of conditional operator for the upcoming VS2017 Update 1 release (15.3). The improvements will be available under a switch, but they will also be implied by the /permissive- switch. Our QA team currently builds regularly MongoDB as a part of RWC suite and so far MongoDB been clean under /permissive-. There is one place right now that will fail to compile under upcoming /Zc:ternary:

mongo\src\mongo\util\lru_cache.h(134)

const_iterator cfind(const K& key) const

{ auto it = this->_map.find(key); return (it == this->_map.end()) ? this->end() : it->second; // << Line 134 }

The problem is not so much in the code itself as other compilers are able to compile it without problems, but in our STL implementation that currently uses inheritance between mutable and const iterators. Stephan acknowledged the library team is planning to get rid of this trick and replace it with conformant implementation in the next major breaking release, but it's not going to happen anytime soon, while as is the above statement is ambiguous (according to the standard; note other compilers would have complained as well should they've been using our STL implementation).

We would like to ask you guys to patch the above code with an explicit cast to resolve the ambiguity caused by our current iterator implementation as following (pull request is following):

const_iterator cfind(const K& key) const

{ auto it = this->_map.find(key); return (it == this->_map.end()) ? this->end() : const_iterator(it->second); // << Line 134 }

This would allow us to keep building MongoDB clean under /permissive- for validating compiler changes.

Thank you!
Yuriy



 Comments   
Comment by Githook User [ 24/Apr/17 ]

Author:

{u'username': u'solodon4', u'name': u'Yuriy Solodkyy', u'email': u'yuriysol@microsoft.com'}

Message: SERVER-28890 Workaround for upcoming `/Zc:ternary`

This workaround allows building MongoDB under Microsoft's upcoming
compiler, with the `/Zc:ternary` option enabled. This option would
be implied by the `/permissive-` option. The code fails to compile
under this mode when using Microsoft's existing `std::list`
implementation. This explicit cast can be removed once Microsoft's
`std::list` stops using inheritance between const and modifiable
iterators.

The issue arises because the `?:` ternary operator requires an
impossible conversion from a `const iterator &` to a
`const_iterator &&` or a `const_iterator &`. This occurs because
Microsoft's `std::list< ... >::iterator` inherits from
`std::list< ... >::const_iterator`. Were the `const iterator &`
non-constant (`iterator &`), this would work. Additionally, were the
`const_iterator &` constant (`const const_iterator &`), this would
also work.

Closes #1147
Branch: master
https://github.com/mongodb/mongo/commit/918ac217df662fcb0865dba278df9151c3665fbc

Comment by Yuriy Solodkyy [ 20/Apr/17 ]

Hi Thomas,

Done.

Thank you!
Yuriy

Comment by Kelsey Schubert [ 20/Apr/17 ]

Hi solodon,

Thank you for the report. Would you please sign the contributor agreement so we can consider your pull request?

Thanks again,
Thomas

Comment by Yuriy Solodkyy [ 20/Apr/17 ]

Created pull request: https://github.com/mongodb/mongo/pull/1147

Generated at Thu Feb 08 04:19:20 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.