Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loss of data conversion in Flex scanner. #73

Open
papaathome opened this issue Jan 24, 2021 · 5 comments
Open

Loss of data conversion in Flex scanner. #73

papaathome opened this issue Jan 24, 2021 · 5 comments
Labels
Flex Issues related to Flex upstream things that apply/should be done upstream

Comments

@papaathome
Copy link

papaathome commented Jan 24, 2021

In a test project I get the following warning: Details will follow below.

warning C4244: 'return': conversion from 'std::streamsize' to 'int', possible loss of data

Related code:
// YY_INTERACTIVE is not defined, refactored (by removing preprocessor directives) to make it shorter.
int yyFlexLexer::LexerInput( char* buf, int max_size )
{
if ( yyin.eof() || yyin.fail() ) return 0;
(void) yyin.read( buf, max_size );
if ( yyin.bad() ) return -1;
else return yyin.gcount(); // <-- This line is causing the warning
}

I am using Visual Studio 2017 with the latest version of win_bison_flex (win_flex_bison3-latest.zip)
As test project I use the source from Flex Bison C++ Example
There were a few minor things I had to fix but the code compiles and runs without apparent problems.

The scanner code generated by Flex is causing this warning.
I do not see a way to redefine or otherwise fix the definition of int yyFlexLexer::LexerInput(...).
I would like to change it to long long yyFlexLexer::LexerInput(...)

Does anybody have an advise on how to handle this problem?
When parsing large files this can become an issue and I don't like compiler warnings anyway.

Kind regards.

@lexxmark
Copy link
Owner

Could you please check that upstream bison doesn't produce such warning (under linux)?

If it doesn't produce a warning I should investigate that's the difference, otherwise please report that problem to upstream bison project to fix.

@papaathome
Copy link
Author

Sorry, I don't have a *nix development environment available at the moment.

It will take me some time to get one up (using VirtualBox) to do a quick test.
But when I find the time for it I will report all details to upstream bison and mention the results here.

Kind regards,
Andre.

@ArcaneTourist
Copy link

LexerInput() is a virtual function, right? So, you must have provided the code.

The return value for LexerInput is an int from 0..max_size. The way you coded your call to read() guarantees that the value from gcount() must be less than max_size and thus, must fit in an int. So, you should cast the result of gcount() to int and return an int as expected by the flex interface conventions.

This issue is due to the user defined code and should be closed.

@GitMensch
Copy link
Collaborator

LexerInput() is a virtual function, right? So, you must have provided the code.

No,that is generated by flex or in this case win_flex into scanner.cc and also prototyped in FlexLexer.h.

The referenced sample has those pre-generated / shipped and back then it all was of type int, too.

Current flex still has that as an int, and the current cpp skeleton also returns yyin.gcount(). So we are "identical to upstream.

There is currently no actual issue as the stream reads up to the max_size, which is also an int, the implementation would be completely broken if it returns anything bigger.

Depending on the actual environment std::istream::gcount() is defined differently because it is of type streamsize, MSVC defines that depending on the compilation of 32/64 bit:

#ifdef _WIN64
    typedef __int64 streamsize;
#else
    typedef int streamsize;
#endif

So the requested change for long long would be wrong for winflexbison.

But MSVC is not the only one that not always uses int, so an upstream change is reasonable.
I think the skeleton should be changed to explicit cast to the return code:

-		return yyin.gcount();
+		return (int)yyin.gcount();

Upstream merge-request: westes/flex#495.

@ArcaneTourist
Copy link

ArcaneTourist commented Jun 22, 2021

Thanks. The flex docs mentioned that it was a virtual function. I should have checked to see if any of the skeletons defined it.
Agreed that this is an upstream issue and that, as we both said, the appropriate fix is casting the gcount() result to int.

@GitMensch GitMensch added Flex Issues related to Flex upstream things that apply/should be done upstream labels Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Flex Issues related to Flex upstream things that apply/should be done upstream
Projects
None yet
Development

No branches or pull requests

4 participants