Modify

Ticket #6186 (closed Bugs: fixed)

Opened 2 years ago

Last modified 2 years ago

lexical_cast on vs2005 with wchar_t-

Reported by: anonymous Owned by: apolukhin
Milestone: To Be Determined Component: lexical_cast
Version: Boost 1.48.0 Severity: Problem
Keywords: Cc:

Description

In VisualStudio? 2005 set /Zc:wchar_t-

long long ll = lexical_cast< long long >( L"1000" );

error C2535: 'bool boost::detail::lexical_stream_limited_src<CharT,Traits,RequiresStringbuffer?>::operator >>(unsigned short &)' : member function already defined or declared d:\sdk\boost\1.48_v2\boost\lexical_cast.hpp 1454

Attachments

Change History

comment:1 Changed 2 years ago by apolukhin

  • Owner changed from nasonov to apolukhin
  • Status changed from new to assigned

comment:2 Changed 2 years ago by apolukhin

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [75864]) Fixes #6186 (lexical_cast compliation error fixed, when wchar_t is a typedef for unsigned short. Test added)

comment:3 Changed 2 years ago by apolukhin

(In [75921]) Fixes #6186 test failures

comment:4 follow-up: ↓ 5 Changed 2 years ago by cheng.yang <luckyangcheng@…>

On line 1.48_v2\boost\lexical_cast.hpp 1454

The code is like this:

bool operator>>(CharT& output) { return shr_xchar(output); }

The line should be changed to:

#ifndef BOOST_NO_INTRINSIC_WCHAR_T

bool operator>>(wchar_t& output) { return shr_xchar(output); }

#endif

Because there is already operator>> for short, char, and unnecessary for CharT overload. Compared with the operator<<, we can easy get to know that it should be prepared for wchar_t, and should be encapsulated by BOOST_NO_INTRINSIC_WCHAR_T macro.

comment:5 in reply to: ↑ 4 Changed 2 years ago by apolukhin

Replying to cheng.yang <luckyangcheng@…>:

On line 1.48_v2\boost\lexical_cast.hpp 1454

The code is like this:

bool operator>>(CharT& output) { return shr_xchar(output); }

The line should be changed to:

#ifndef BOOST_NO_INTRINSIC_WCHAR_T

bool operator>>(wchar_t& output) { return shr_xchar(output); }

#endif

Because there is already operator>> for short, char, and unnecessary for CharT overload. Compared with the operator<<, we can easy get to know that it should be prepared for wchar_t, and should be encapsulated by BOOST_NO_INTRINSIC_WCHAR_T macro.

Your solution is good, but it will break the behavior. When BOOST_NO_INTRINSIC_WCHAR_T is defined, unsigned short must behave as wchar_t, as a lexical character. Your solution will leave unsigned int integral (not lexical) behavior.

Bugfix is already in trunk, you can try it. As soon as it rolls through regression tests, it will be merged to trunk. After that (and other bugfixes), some refactoring will be done and better support for char16_t and char32_t types will be added to lexical_cast library.

comment:6 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Hi, apolukhin

I have gone through your fix in trunk and find there still has the problem.

If BOOST_NO_INTRINSIC_WCHAR_T is defined, then wchar_t will be defined as unsigned short.

And now, ushort_ambiguity_workaround will be deduced as type_used_as_workaround_for_typedefed_wchar_ts, so in the class, it add an overload function bool operator>>(type_used_as_workaround_for_typedefed_wchar_ts& output) which will call a private function.

But, the overload function will never be called, because no one will generate such a object do lexical_cast. If user call operator>>(wchar_t) will use the function operator>>(unsigned short&). So your fix has no difference with mine.

If the wchar_t type cannot be treated as unsigned short, the fix should as following:

#ifndef BOOST_NO_INTRINSIC_WCHAR_T

bool operator>>(unsigned short& output) { return shr_unsigned(output); } bool operator>>(wchar_t& output) { return shr_xchar(output); }

#else

bool operator>>(wchar_t& output) { return shr_xchar(output); }

#endif at the same time, remove line 1440.

bool operator>>(unsigned short& output) { return shr_unsigned(output); }

comment:7 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Totally, there are 3 cases when wchar_t is defined as unsigned short:

Case 1) User do lexical_cast for unsigned short, here no wchar_t defined. Case 2) User do lexical_cast for unsigned short, but wchar_t defined. Case 3) User do lexical_cast for wchar_t.

For all the cases, you can only cast unsigned short to one of the type, wchar_t or a number. It's just like: You can't have your cake and eat it too.

comment:8 Changed 2 years ago by apolukhin

Can you give an example when your solution and mine give different results?

But, the overload function will never be called, because no one will generate such a object do lexical_cast.

That's the point. Works just like your fix, but with less preprocessor macro.

comment:9 follow-up: ↓ 11 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Yes, there is the difference.

When wchar_t is defined as unsigned short, your solution still treat wchar_t as unsigned short. But my solution will treat it as the wchar_t, notice that I will call bool operator>>(wchar_t& output) { return shr_xchar(output); } in the case.

comment:10 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Also, I think the preprocessor macro is better than a series of deduce. We know that if we introduce boost, the compile time is fairly long. Why we not reduce the useless deduce?

comment:11 in reply to: ↑ 9 Changed 2 years ago by steven_watanabe

Replying to cheng.yang <luckyangcheng@…>:

Yes, there is the difference.

When wchar_t is defined as unsigned short, your solution still treat wchar_t as unsigned short.

As it should be.

But my solution will treat it as the wchar_t, notice that I will call bool operator>>(wchar_t& output) { return shr_xchar(output); } in the case.

IMHO, it's much worse to treat unsigned short as wchar_t than to treat wchar_t as unsigned short. When wchar_t is not a first class citizen, it's impossible to avoid doing one or the other.

comment:12 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Please see your comment 6.

"Your solution is good, but it will break the behavior. When BOOST_NO_INTRINSIC_WCHAR_T is defined, unsigned short must behave as wchar_t, as a lexical character. Your solution will leave unsigned int integral (not lexical) behavior."

But why in comment 11, you say:

IMHO, it's much worse to treat unsigned short as wchar_t than to treat wchar_t as unsigned short. When wchar_t is not a first class citizen, it's impossible to avoid doing one or the other.

comment:13 Changed 2 years ago by cheng.yang <luckyangcheng@…>

And for the fix in the trunk, it cannot pass the compile for:

int test2 = boost::lexical_cast<int>("2");

Because, if CharT is not same as unsigned short, then operator>>(short&) will be defined twice.

comment:14 Changed 2 years ago by apolukhin

I made some more tests under VC with wchar_t- flag. Here is the test and output is shown in comments:

#include <iostream>
#include <sstream>

using namespace std;

int main() {
	cout << L'A' << endl;	// 65
	wcout << L'A' << endl;	// A
	cout << L"A" << endl;	// 00147800
	wcout << L"A" << endl;	// A

	wchar_t w;
	stringstream ss;
	ss << 65; 
	ss >> w;  
	wcout << w << endl;	// A

	wstringstream wss;
	wss << 65;
	wss >> w;  
	wcout << w << endl;	// 6

	return 0;
}

Exactly the same output without wchar_t- flag (+ compilation error on line "ss >> w;"). Behavior depends on stream type that we are using.

I`ll think about lexical_cast default behavior, shall it be treated as char stream by default or as wchar_t stream or shall it be something else. Current documentation does not reflect it. All the thoughts will be posted here and in documentation.

P.S.: I'm talking only about the situations, when wchar_t is not a native type. In other situations everything is clear, and behavior is documented.

Last edited 2 years ago by apolukhin (previous) (diff)

comment:15 Changed 2 years ago by cheng.yang <luckyangcheng@…>

And any conclusion?

As a user, what I most often do is to cast some string(wstring or string) into the number, float,double or int, and vise visa. I will not cast a unsigned int to int, double, etc. To treat unsigned short as wchar_t when wchar_t- flag is set should be ok to me.

The current fix in the trunk is not complete, and the ticket cannot be closed.

comment:16 Changed 2 years ago by apolukhin

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:17 Changed 2 years ago by apolukhin

  • Status changed from reopened to closed
  • Resolution set to fixed

(In [75937]) Fixes #6186 (treat conversions to/from single wchar_t character as conversions to/from unsigned short. Test added, documentation updated)

comment:18 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Test and find the solution works fine.

But find a compile warning.

converter.hpp(166) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)

comment:19 Changed 2 years ago by cheng.yang <luckyangcheng@…>

And, if I want to submit something to boost, can anyone let me know what shall I do?

For example, I write another set of bind, I call it bind2f. Current bind in boost will generate an object in boost::_mfi::mf2<R, type1, type2, ...> type, and it can be assigned to boost::function<R(t1,t2)>.

Some cases it will be a great difficult in the programming. For example: Class A {

void do_test(function<bool()> cmd); void do_test(function<bool(int)>cmd);

} A a; a.do_test(bind(...))compile error, don't know use what's the version for do_test.

So if we want to call a.do_test, we must write like this:

function<bool()>cmd = bind(.....); a.do_test(cmd);

And one day the function is refactored as function<int()>, all defines should also been changed. But indeed, the type after bind can be deduced to a function type.

The bind2f is prepared for this. bind2f will return the right function after the bind, it can support normal function, member function, even for irregular place holder order, it works right.(such as bind(&test,this,_9,_2). The missed parameters is set as int by default.

for example, for the function: void test_bind2f(char, string); bind2f(&test_bind2f,_4,_1) will be deduced as function<void(string,int,int,char)>.

Beside the bind2f, I have some other products that may fit for boost, how can I let you know about it?

comment:20 Changed 2 years ago by apolukhin

Read Boost Library Submission Process.
If you want to participate, I`m really exited about making boost::variant work with rvalues/boost::move.
Also. there is a heavy math challenge in lexical_cast (implementing fast conversions to long doubles and doubles without precision loss):

            /* We need a more accurate algorithm... We can not use current algorithm
             * with long doubles (and with doubles if sizeof(double)==sizeof(long double)).
             */
            long double result = std::pow(10.0L, pow_of_10) * mantissa;
            value = static_cast<T>( has_minus ? (boost::math::changesign)(result) : result);

If you want something more, read for a few weeks main developers mailing list and try asking for some work there.

By the way, can you give me some code example, when this warning appears "converter.hpp(166) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning) "?

Feel free to contact me and ask any questions. (My email can be found at  maintainers list near the "conversion" library)

comment:21 follow-up: ↓ 22 Changed 2 years ago by cheng.yang <luckyangcheng@…>

Thanks for your advice at first, and I will follow your suggestions. Maybe one morning you will find that your mailbox is full of my questions. :)

The code that may lead to warning is like this:

int val = boost::lexical_cast<int>(bool(true));

comment:22 in reply to: ↑ 21 Changed 2 years ago by apolukhin

Replying to cheng.yang <luckyangcheng@…>:

The code that may lead to warning is like this:

int val = boost::lexical_cast<int>(bool(true));

Created ticket #6297

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.