Turn off BOOST_IOSTREAMS_HAS_DINKUMWARE_FPOS for MSVC++ by BillyONeal · Pull Request #57 · boostorg/iostreams

BillyONeal · 2018-04-16T21:32:53Z

Boost is attempting to use a function not in the standard, seekpos(),
guarded by this setting. Looks like Boost can use the standards
conforming interface for MSVC++ instead. It should work going
back effectively "forever" as far as MSVC++ is concerned, but this
guard applies the change only to 2017. It's possible there is a better
control in boost.config I missed.

Boost is attempting to use a function not in the standard, seekpos(), guarded by this setting. Looks like Boost can use the standards conforming interface for MSVC++ instead. It should work going back effectively "forever" as far as MSVC++ is concerned, but this guard applies the change only to 2017. It's possible there is a better control in boost.config I missed.

eldiener · 2018-04-18T00:16:48Z

The question is how we should be targeting vc++ 14.1 ? I think we should use BOOST_MSVC from config, which holds the value of vc++'s _MSC_VER. Using a recent macro to identify a version of the vc++'s standard library seems very chancy to me.

BillyONeal · 2018-04-18T00:45:14Z

_MSC_VER should be OK if you'd prefer it; we added _MSVC_STL_VERSION specifically at the request of Boost for separate version numbers between the compiler and library (as sometimes new compilers get used with old libraries).

eldiener · 2018-04-18T01:07:24Z

I would not use _MSC_VER directly in your PR but specifically BOOST_MSVC. This is because some other compilers, which are not vc++, also define _MSC_VER whereas BOOST_MSVC means that vc++ specifically is being used. Also are you really saying that newer versions of vc++ could be used with old versions of your C++ standard library ? I did not think that was possible in Microsoft distributions.

When you say that 'Boost can use the standards conforming interface for MSVC++ instead. It should work going back effectively "forever" as far as MSVC++ is concerned' do you mean that all older versions of VC++, with their distributed versions of the C++ standard library, have a standards conforming interface as far as this problem is concerned ? That would be strange as it would put in question the original reason for the vc++ specific code in this particular situation.

BillyONeal · 2018-04-18T01:36:51Z

Also are you really saying that newer versions of vc++ could be used with old versions of your C++ standard library ?

Yes, its a thing people do sometimes. Old libs with new compiler should work, old compiler with new libs is unsupported.

do you mean that all older versions of VC++, with their distributed versions of the C++ standard library, have a standards conforming interface as far as this problem is concerned ?

Modulo bugs, yes. I only tested with 2017 though which is why I so guarded it that way.

That would be strange as it would put in question the original reason for the vc++ specific code in this particular situation.

The Dinkumware code path is used for other Dinkumware licensees; they have this behavior of "call fsetpos followed by fseek" because the C standard says it's UB to call fsetpos on anything not returned by a call to fgetpos. (See C11, 7.21.9.3 "The fsetpos function"/2) But Dinkumware's caution there happens to be unnecessary for at least our current CRT implementation, where fsetpos just sets a 64 bit offset.

There are some bugs that happened with VS 2017 15.7 and earlier where directly comparing fpos instances with non-streamoff integer types created ambiguities (compiler didn't know whether to convert fpos into streamoff and widen other integer, or convert other integer into fpos and compare those) which I fixed in 15.8 by adding comparison operators between fpos and all other integral types. (Note that 15.8 isn't even in preview yet; I just fixed this in our sources last week and found out about Boost depending on this nonstandard seekoff() member I removed because we build Boost with the prerelease compiler every night)

BillyONeal · 2018-04-18T01:42:40Z

directly comparing fpos instances with non-streamoff integer types

Also note that the non-Dinkumware path in Boost already deals with this problem by directly casting the fpos to streamoff first.

eldiener · 2018-04-19T16:57:43Z

The original code path is for Dinkumware rather than for vc++ specifically. If you are saying that vc++ always worked correctly with Dinkumware and did not need this one-off code path we could remove it for any version of vc++ by just checking if BOOST_MSVC is defined. But to be more conservative than that, in order not to destroy the use of iostreams for past versions of vc++ other than the present one, I am fine with approving your PR as long as _MSVC_STL_VERSION is never used by any other compiler, which I assume and hope will be the case, without checking for BOOST_MSVC. To be really conservative though it might be best to check for both _MSVC_STL_VERSION and BOOST_MSVC as a reason to not use the code path. Can you change it appropriately to do that or do you want me to merge the PR and do it myself ? BOOST_MSVC is defined whenever vc++ is the compiler and not defined otherwise.

BillyONeal · 2018-04-19T21:55:07Z

I was just informed last night that there's a Boost.IOStreams test failing right now; let me investigate and I'll keep you posted. Thanks!

BillyONeal · 2018-04-20T02:45:07Z

Turns out the test failure is because I broke basic_stringbuf::seekoff last week :/, so not related to this change. I'll make the change you requested to test BOOST_MSVC first as soon as I hear I haven't broken boost.iostream any more. :)

BillyONeal · 2018-04-20T04:52:14Z

OK, confirmed that it's just because I made seekoff corrupt basic_stringbuf which I just fixed. (Yay, thank you boost iostreams tests for catching my mistake!) Updated PR with the additional test you requested.

MarcelRaad · 2018-07-03T08:15:57Z

Could this be merged to master please? This is needed for VS 2017 Update 8 as of Preview 3.0.

garethsb · 2018-07-09T09:11:02Z

As the original author of the workarounds in boost/iostreams/positioning.hpp, I can confirm they absolutely were necessary back when I produced them in 2005, but that's a long time ago now, even for Boost. ;-)

BillyONeal · 2018-07-09T18:51:41Z

I apologize on behalf of my predecessors? :P

alanbirtles · 2018-07-10T12:27:30Z

No so long ago, this was only fixed in VS 2012, see https://groups.google.com/forum/#!searchin/boost-developers-archive/_FPOSOFF%7Csort:date/boost-developers-archive/VT7UxdJwfn4/MXpi6gyoBAAJ for some more discussion

BillyONeal · 2018-07-10T22:44:35Z

Wait wait wait wait we cast it to long? WHY?!???! :(

alanbirtles · 2018-07-11T09:39:35Z

Before VS2010 lots of ugly hacks were required for seeking past 2GB in std::fstream for similar reasons.

…eekpos() with VS 15.8 Change taken from boostorg/iostreams#57, but this is not yet included in a boost release.

mnordie · 2018-10-16T09:34:11Z

Any idea when this will be released?
Doesn't seem to be in 1.68.

jeking3 · 2018-10-16T11:43:11Z

This should appear in 1.69.0 - I think it was supposed to be in 1.68.0 but wasn't merged in time. It is in the pipeline now.

mwpowellhtx · 2018-10-30T23:52:30Z

I'm receiving the same error in iostreams/positioning.hpp. Make sure y'all check and double check your regression before making that commit and/or merging, please.

BillyONeal · 2018-10-31T02:24:50Z

@mwpowellhtx That should be guarded by the ifdef here?

iostreams/include/boost/iostreams/positioning.hpp

Line 63 in a3e973f

# ifndef BOOST_IOSTREAMS_HAS_DINKUMWARE_FPOS

mwpowellhtx · 2018-10-31T03:31:28Z

@BillyONeal It's a good question, it is, but how do I know whether I have "Dinkumware fpos"?

Meanwhile, I am defining _SILENCE_FPOS_SEEKPOS_DEPRECATION_WARNING, but letting the deprecated (returning zero) seekpos() linger just seems like asking for regression trouble.

BillyONeal · 2018-11-01T01:38:44Z

@mwpowellhtx It would be more appropriate to call it "workaround broken Dinkumware behavior where they put file sizes into an int" :)

mwpowellhtx · 2018-11-01T01:45:57Z

@BillyONeal Copy that; I understand. 👍

I installed the patch, BTW, and it appears to be working. I still have to "acknowledge" the issue, so to speak, but at least I can proceed forward.

K-M-Ibrahim-Khalilullah · 2020-04-02T05:33:39Z

@BillyONeal
would tell me please how can I turn off Boost_IOstream_has_dinkkumware_fpos? I already install boost with pcl!

@std

Boost stopped using `_FPOSOFF` when boostorg/iostreams@7c2592c was merged on 2016-09-16. I've verified that this shipped in Boost 1.63.0 on 2016-12-26. We deprecated `std::fpos::seekpos()` with MSVC-PR-132953 merged on 2018-07-18. (We first noticed the problem when MSVC-PR-115404 was merged on 2018-04-11.) Boost stopped using it when boostorg/iostreams#57 was merged on 2018-04-20. I've verified that this shipped in Boost 1.69.0 on 2018-12-12. Note that while the `std::fpos` type appears in the parameter types and return types of dllexported functions, `std::fpos` is not dllexported itself, as indicated by the lack of explicit calling conventions in the source. Because `std::fpos::seekpos()` isn't dllexported, we can freely remove it. (The only mentions of `seekpos` in the dllexport surface are for the different `std::basic_streambuf::seekpos()`, which conveniently also demonstrates how it takes and returns `std::fpos`.) ``` D:\GitHub\STL\out\x64\out\bin\amd64>dumpbin /exports msvcp140d_oss.dll | rg "\bseekpos\b" 1206 4B5 00053D70 ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<char,struct std::char_traits<char> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1207 4B6 00053DB0 ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<unsigned short,struct std::char_traits<unsigned short> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1208 4B7 00053DF0 ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<wchar_t,struct std::char_traits<wchar_t> >::seekpos(class std::fpos<struct _Mbstatet>,int)) ```

@std

Boost stopped using `_FPOSOFF` when boostorg/iostreams@7c2592c was merged on 2016-09-16. I've verified that this shipped in Boost 1.63.0 on 2016-12-26. We deprecated `std::fpos::seekpos()` with MSVC-PR-132953 merged on 2018-07-18. (We first noticed the problem when MSVC-PR-115404 was merged on 2018-04-11.) Boost stopped using it when boostorg/iostreams#57 was merged on 2018-04-20. I've verified that this shipped in Boost 1.69.0 on 2018-12-12. Note that while the `std::fpos` type appears in the parameter types and return types of dllexported functions, `std::fpos` is not dllexported itself, as indicated by the lack of explicit calling conventions in the source. Because `std::fpos::seekpos()` isn't dllexported, we can freely remove it. (The only mentions of `seekpos` in the dllexport surface are for the different `std::basic_streambuf::seekpos()`, which conveniently also demonstrates how it takes and returns `std::fpos`.) ``` D:\GitHub\STL\out\x64\out\bin\amd64>dumpbin /exports msvcp140d_oss.dll | rg "\bseekpos\b" 1206 4B5 00053D70 ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<char,struct std::char_traits<char> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1207 4B6 00053DB0 ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<unsigned short,struct std::char_traits<unsigned short> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1208 4B7 00053DF0 ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<wchar_t,struct std::char_traits<wchar_t> >::seekpos(class std::fpos<struct _Mbstatet>,int)) ```

@std

Boost stopped using `_FPOSOFF` when boostorg/iostreams@7c2592c was merged on 2016-09-16. I've verified that this shipped in Boost 1.63.0 on 2016-12-26. We deprecated `std::fpos::seekpos()` with MSVC-PR-132953 merged on 2018-07-18. (We first noticed the problem when MSVC-PR-115404 was merged on 2018-04-11.) Boost stopped using it when boostorg/iostreams#57 was merged on 2018-04-20. I've verified that this shipped in Boost 1.69.0 on 2018-12-12. Note that while the `std::fpos` type appears in the parameter types and return types of dllexported functions, `std::fpos` is not dllexported itself, as indicated by the lack of explicit calling conventions in the source. Because `std::fpos::seekpos()` isn't dllexported, we can freely remove it. (The only mentions of `seekpos` in the dllexport surface are for the different `std::basic_streambuf::seekpos()`, which conveniently also demonstrates how it takes and returns `std::fpos`.) ``` D:\GitHub\STL\out\x64\out\bin\amd64>dumpbin /exports msvcp140d_oss.dll | rg "\bseekpos\b" 1206 4B5 00053D70 ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@DU?$char_traits@D@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<char,struct std::char_traits<char> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1207 4B6 00053DB0 ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@GU?$char_traits@G@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<unsigned short,struct std::char_traits<unsigned short> >::seekpos(class std::fpos<struct _Mbstatet>,int)) 1208 4B7 00053DF0 ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z = ?seekpos@?$basic_streambuf@_WU?$char_traits@_W@std@@@std@@Meaa?AV?$fpos@U_Mbstatet@@@2@V32@H@Z (protected: virtual class std::fpos<struct _Mbstatet> __cdecl std::basic_streambuf<wchar_t,struct std::char_traits<wchar_t> >::seekpos(class std::fpos<struct _Mbstatet>,int)) ```

Guard _MSVC_STL_VERSION test with BOOST_MSVC test.

15e7258

eldiener merged commit a0c32ba into boostorg:develop Apr 20, 2018

BillyONeal mentioned this pull request Jun 1, 2018

Remove call to nonexistent member seekpos() of std::fpos #58

Closed

Wedge009 mentioned this pull request Aug 17, 2018

Compile error in iostreams\detail\config\fpos.hpp because std::fpos<>::seekpos() is absent in VS 15.8 Preview 4 #60

Closed

Wedge009 added a commit to aquileia/external that referenced this pull request Aug 17, 2018

Resolve compilation warning/error on non-standard use of streampos::s…

31a7227

…eekpos() with VS 15.8 Change taken from boostorg/iostreams#57, but this is not yet included in a boost release.

gtritchie mentioned this pull request Sep 13, 2018

Warnings when building Windows desktop with newer MSVC and Boost rstudio/rstudio#3140

Closed

code-affinity mentioned this pull request Dec 17, 2018

The member std::fpos::seekpos() is non-Standard sergey-shandar/getboost#44

Closed

pdimov mentioned this pull request Jan 15, 2019

Turn off BOOST_IOSTREAMS_HAS_DINKUMWARE_FPOS for clang++ #77

Merged

StephanTLavavej mentioned this pull request Apr 19, 2024

Remove _FPOSOFF and non-Standard std::fpos::seekpos() microsoft/STL#4606

Merged

Conversation

BillyONeal commented Apr 16, 2018

Uh oh!

eldiener commented Apr 18, 2018

Uh oh!

BillyONeal commented Apr 18, 2018

Uh oh!

eldiener commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BillyONeal commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BillyONeal commented Apr 18, 2018

Uh oh!

eldiener commented Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BillyONeal commented Apr 19, 2018

Uh oh!

BillyONeal commented Apr 20, 2018

Uh oh!

BillyONeal commented Apr 20, 2018

Uh oh!

MarcelRaad commented Jul 3, 2018

Uh oh!

garethsb commented Jul 9, 2018

Uh oh!

BillyONeal commented Jul 9, 2018

Uh oh!

alanbirtles commented Jul 10, 2018

Uh oh!

BillyONeal commented Jul 10, 2018

Uh oh!

alanbirtles commented Jul 11, 2018

Uh oh!

mnordie commented Oct 16, 2018

Uh oh!

jeking3 commented Oct 16, 2018

Uh oh!

mwpowellhtx commented Oct 30, 2018

Uh oh!

BillyONeal commented Oct 31, 2018

Uh oh!

mwpowellhtx commented Oct 31, 2018

Uh oh!

BillyONeal commented Nov 1, 2018

Uh oh!

mwpowellhtx commented Nov 1, 2018

Uh oh!

K-M-Ibrahim-Khalilullah commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

eldiener commented Apr 18, 2018 •

edited

Loading

BillyONeal commented Apr 18, 2018 •

edited

Loading

eldiener commented Apr 19, 2018 •

edited

Loading