Menu
▾
▴
mesa3d-dev
|
From: Karl S. <kar...@gm...> - 2010-03-11 15:51:11
|
Am getting these warnings in the Windows build:
2>Compiling...
2>lex.yy.c
2>program_lexer.l(327) : warning C4244: '=' : conversion from 'double' to
'float', possible loss of data
2>program_lexer.l(331) : warning C4244: '=' : conversion from 'double' to
'float', possible loss of data
2>program_lexer.l(335) : warning C4244: '=' : conversion from 'double' to
'float', possible loss of data
2>program_lexer.l(339) : warning C4244: '=' : conversion from 'double' to
'float', possible loss of data
The corresponding lines in program_lexer.l are:
{num}?{frac}{exp}? {
yylval->real = _mesa_strtod(yytext, NULL);
return REAL;
}
{num}"."/[^.] {
yylval->real = _mesa_strtod(yytext, NULL);
return REAL;
}
{num}{exp} {
yylval->real = _mesa_strtod(yytext, NULL);
return REAL;
}
{num}"."{exp} {
yylval->real = _mesa_strtod(yytext, NULL);
return REAL;
}
I think that we need to add a cast to these four places, e.g.:
yylval->real = (float) _mesa_strtod(yytext, NULL);
Changing progam_lexer.l requires that lex.yy.c (and others) be regenerated
with flex/bison and the generated files committed as well, right? Is there
a git trigger that does this, which would be really cool? Or does someone
need to run flex/bison manually and commit all the resulting files?
If the latter, I don't know the exact process and I'd guess that someone
routinely does this.
If it is not a huge hassle, can someone add these casts to 7.8 and master?
The warnings are no big deal, but the casts do have some value in
documenting the implicit conversion.
Thanks,
Karl
|
|
From: Brian P. <br...@vm...> - 2010-03-11 16:04:52
|
Karl Schultz wrote:
> Am getting these warnings in the Windows build:
>
> 2>Compiling...
> 2>lex.yy.c
> 2>program_lexer.l(327) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(331) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(335) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(339) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
>
> The corresponding lines in program_lexer.l are:
>
> {num}?{frac}{exp}? {
> yylval->real = _mesa_strtod(yytext, NULL);
> return REAL;
> }
> {num}"."/[^.] {
> yylval->real = _mesa_strtod(yytext, NULL);
> return REAL;
> }
> {num}{exp} {
> yylval->real = _mesa_strtod(yytext, NULL);
> return REAL;
> }
> {num}"."{exp} {
> yylval->real = _mesa_strtod(yytext, NULL);
> return REAL;
> }
>
> I think that we need to add a cast to these four places, e.g.:
>
> yylval->real = (float) _mesa_strtod(yytext, NULL);
>
> Changing progam_lexer.l requires that lex.yy.c (and others) be
> regenerated with flex/bison and the generated files committed as well,
> right? Is there a git trigger that does this, which would be really
> cool? Or does someone need to run flex/bison manually and commit all
> the resulting files?
>
> If the latter, I don't know the exact process and I'd guess that someone
> routinely does this.
>
> If it is not a huge hassle, can someone add these casts to 7.8 and
> master? The warnings are no big deal, but the casts do have some value
> in documenting the implicit conversion.
I'll take care of it. It's up to the developer to regenerate the
files and commit them (to avoid everyone needing flex/bison).
-Brian
|
|
From: Ian R. <id...@fr...> - 2010-03-12 20:08:59
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Karl Schultz wrote: > Am getting these warnings in the Windows build: > > 2>Compiling... > 2>lex.yy.c > 2>program_lexer.l(327) : warning C4244: '=' : conversion from 'double' > to 'float', possible loss of data > 2>program_lexer.l(331) : warning C4244: '=' : conversion from 'double' > to 'float', possible loss of data > 2>program_lexer.l(335) : warning C4244: '=' : conversion from 'double' > to 'float', possible loss of data > 2>program_lexer.l(339) : warning C4244: '=' : conversion from 'double' > to 'float', possible loss of data [snip] > If it is not a huge hassle, can someone add these casts to 7.8 and > master? The warnings are no big deal, but the casts do have some value > in documenting the implicit conversion. Absolutely not. All the casts do is clutter the code. This is a completely worthless warning from Visual Studio. It generates only noise. I have never seen a case where it was actually useful. There are pragmas and command line options to disable specific warnings. You should do that. Brian, could you please revert those patches. I don't want that crap in code that I maintain. Seriously. Thanks. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkuanyEACgkQX1gOwKyEAw/PJwCgjZDqwdLrkC5hZ28hCgLpdmvH anYAnAm2rMb2fSgnOjsqRSybuhQ1ZmOA =cRwl -----END PGP SIGNATURE----- |
|
From: Gonsolo <go...@gm...> - 2010-03-12 20:44:34
|
> Absolutely not. All the casts do is clutter the code. This is a
> completely worthless warning from Visual Studio. It generates only
> noise. I have never seen a case where it was actually useful. There
> are pragmas and command line options to disable specific warnings. You
> should do that.
>
> Brian, could you please revert those patches. I don't want that crap in
> code that I maintain. Seriously.
What about this patch:
From 40136f460562d49a21877fdb52d9ddbf8ae6f7b9 Mon Sep 17 00:00:00 2001
From: Gon Solo <go...@gm...>
Date: Fri, 12 Mar 2010 21:39:59 +0100
Subject: [PATCH] Use strof. C99 is 11 years now.
---
src/mesa/main/imports.c | 7 +++++++
src/mesa/main/imports.h | 3 +++
src/mesa/shader/lex.yy.c | 8 ++++----
src/mesa/shader/program_lexer.l | 8 ++++----
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 56e8195..d77b36c 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -810,6 +810,13 @@ _mesa_strtod( const char *s, char **end )
#endif
}
+/** Wrapper around strtof() */
+float
+_mesa_strtof( const char *s, char **end )
+{
+ return strtof(s, end);
+}
+
/** Compute simple checksum/hash for a string */
unsigned int
_mesa_str_checksum(const char *str)
diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h
index fb4a00e..c30be11 100644
--- a/src/mesa/main/imports.h
+++ b/src/mesa/main/imports.h
@@ -578,6 +578,9 @@ _mesa_strdup( const char *s );
extern double
_mesa_strtod( const char *s, char **end );
+extern float
+_mesa_strtof( const char *s, char **end );
+
extern unsigned int
_mesa_str_checksum(const char *str);
diff --git a/src/mesa/shader/lex.yy.c b/src/mesa/shader/lex.yy.c
index d1af35f..6d09d1d 100644
--- a/src/mesa/shader/lex.yy.c
+++ b/src/mesa/shader/lex.yy.c
@@ -2212,7 +2212,7 @@ case 142:
YY_RULE_SETUP
#line 326 "program_lexer.l"
{
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
YY_BREAK
@@ -2224,7 +2224,7 @@ YY_DO_BEFORE_ACTION; /* set up yytext again */
YY_RULE_SETUP
#line 330 "program_lexer.l"
{
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
YY_BREAK
@@ -2232,7 +2232,7 @@ case 144:
YY_RULE_SETUP
#line 334 "program_lexer.l"
{
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
YY_BREAK
@@ -2240,7 +2240,7 @@ case 145:
YY_RULE_SETUP
#line 338 "program_lexer.l"
{
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
YY_BREAK
diff --git a/src/mesa/shader/program_lexer.l
b/src/mesa/shader/program_lexer.l
index 83bc508..fe18272 100644
--- a/src/mesa/shader/program_lexer.l
+++ b/src/mesa/shader/program_lexer.l
@@ -324,19 +324,19 @@ ARRAYSHADOW2D {
return_token_or_IDENTIFIER(require_ARB_fp && require
return INTEGER;
}
{num}?{frac}{exp}? {
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
{num}"."/[^.] {
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
{num}{exp} {
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
{num}"."{exp} {
- yylval->real = _mesa_strtod(yytext, NULL);
+ yylval->real = _mesa_strtof(yytext, NULL);
return REAL;
}
--
1.6.3.3
|
|
From: Ian R. <id...@fr...> - 2010-03-12 23:45:17
|
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Gonsolo wrote:
> diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
> index 56e8195..d77b36c 100644
> --- a/src/mesa/main/imports.c
> +++ b/src/mesa/main/imports.c
> @@ -810,6 +810,13 @@ _mesa_strtod( const char *s, char **end )
> #endif
> }
>
> +/** Wrapper around strtof() */
> +float
> +_mesa_strtof( const char *s, char **end )
> +{
> + return strtof(s, end);
> +}
> +
We can't use strtof for the same reason we can't (directly) use strtod.
It uses the locale, so that can make it parse numbers incorrectly. All
of the users of _mesa_strtod actually want _mesa_strtof, so the right
fix is to change the existing _mesa_strtod to _mesa_strtof.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkua0dEACgkQX1gOwKyEAw9kNgCffpzl4Zxk2JjOwAjIhR2gpiEp
Y8IAoIs8IjQdhm7RdOmmYqWk/RylSmYK
=0Ep5
-----END PGP SIGNATURE-----
|
|
From: Gonsolo <go...@gm...> - 2010-03-13 00:45:08
|
> We can't use strtof for the same reason we can't (directly) use strtod. > It uses the locale, so that can make it parse numbers incorrectly. All > of the users of _mesa_strtod actually want _mesa_strtof, so the right > fix is to change the existing _mesa_strtod to _mesa_strtof. Some remarks from a beginner: 1. Isn't strtod meant to be a very basic function? Making it dependent on a locale isn't helpful. 2. I see strtod_l is defined in /usr/include/stdlib.h which is part of the package libc6-dev on my Ubuntu system, yet there is no manpage for it. 3. strtod is defined in /usr/include/stdlib.h which is in package libc6-dev, the manpage is in manpages-dev. From my limited view, every function should be defined in some_package, it should be declared in some_package-dev and documented in some_package-doc. Is there any reason for this mess except being "historical"? (Please excuse my ignorance, just a beginner :) ) g |
|
From: Karl S. <kar...@gm...> - 2010-03-13 01:12:44
|
On Fri, Mar 12, 2010 at 5:44 PM, Gonsolo <go...@gm...> wrote:
> We can't use strtof for the same reason we can't (directly) use strtod.
>> It uses the locale, so that can make it parse numbers incorrectly. All
>> of the users of _mesa_strtod actually want _mesa_strtof, so the right
>> fix is to change the existing _mesa_strtod to _mesa_strtof.
>>
>
> Some remarks from a beginner:
>
> 1. Isn't strtod meant to be a very basic function? Making it dependent on a
> locale isn't helpful.
>
> 2. I see strtod_l is defined in /usr/include/stdlib.h which is part of the
> package libc6-dev on my Ubuntu system, yet there is no manpage for it.
>
> 3. strtod is defined in /usr/include/stdlib.h which is in package
> libc6-dev, the manpage is in manpages-dev.
>
> From my limited view, every function should be defined in some_package, it
> should be declared in some_package-dev and documented in some_package-doc.
>
> Is there any reason for this mess except being "historical"?
>
> (Please excuse my ignorance, just a beginner :) )
>
> g
>
The code in question is:
/** Wrapper around strtod() */
double
_mesa_strtod( const char *s, char **end )
{
#ifdef _GNU_SOURCE
static locale_t loc = NULL;
if (!loc) {
loc = newlocale(LC_CTYPE_MASK, "C", NULL);
}
return strtod_l(s, end, loc);
#else
return strtod(s, end);
#endif
}
In some locales, a comma can be used as a decimal place. That is, 5,2 is a
number between 5 and 6. (I think I have that right) I would guess that the
shader language, like C, wouldn't allow this form in code. So, it makes
sense to force the C locale when parsing numbers from shader source code, as
the code does above.
strtof doesn't show up until C99 and not all compilers support it, including
the MSFT Windows compilers. Ian says that all usages of this function want
a float anyway, so we may end up with something like:
float
_mesa_strtof( const char *s, char **end )
{
#ifdef _GNU_SOURCE
static locale_t loc = NULL;
if (!loc) {
loc = newlocale(LC_CTYPE_MASK, "C", NULL);
}
return (float) strtod_l(s, end, loc);
#else
return (float) strtod(s, end);
#endif
}
And then change all _mesa_strtod to _mesa_strtof.
If Ian doesn't care for the casts here, then I'm fine with silencing
warnings in the Studio with a compiler option.
|
|
From: Marcin B. <ma...@gm...> - 2010-03-13 13:35:52
|
2010/3/13 Karl Schultz <kar...@gm...>:
> In some locales, a comma can be used as a decimal place. That is, 5,2 is a
> number between 5 and 6. (I think I have that right) I would guess that the
> shader language, like C, wouldn't allow this form in code. So, it makes
> sense to force the C locale when parsing numbers from shader source code, as
> the code does above.
>
> strtof doesn't show up until C99 and not all compilers support it, including
> the MSFT Windows compilers. Ian says that all usages of this function want
> a float anyway, so we may end up with something like:
>
> float
> _mesa_strtof( const char *s, char **end )
> {
> #ifdef _GNU_SOURCE
> static locale_t loc = NULL;
> if (!loc) {
> loc = newlocale(LC_CTYPE_MASK, "C", NULL);
> }
> return (float) strtod_l(s, end, loc);
> #else
> return (float) strtod(s, end);
> #endif
> }
>
> And then change all _mesa_strtod to _mesa_strtof.
>
> If Ian doesn't care for the casts here, then I'm fine with silencing
> warnings in the Studio with a compiler option.
Attached patch uses strtof when it is available.
|
|
From: Ian R. <id...@fr...> - 2010-03-15 15:47:09
|
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Marcin Baczyński wrote:
> 2010/3/13 Karl Schultz <kar...@gm...>:
>> In some locales, a comma can be used as a decimal place. That is, 5,2 is a
>> number between 5 and 6. (I think I have that right) I would guess that the
>> shader language, like C, wouldn't allow this form in code. So, it makes
>> sense to force the C locale when parsing numbers from shader source code, as
>> the code does above.
>>
>> strtof doesn't show up until C99 and not all compilers support it, including
>> the MSFT Windows compilers. Ian says that all usages of this function want
>> a float anyway, so we may end up with something like:
>>
>> float
>> _mesa_strtof( const char *s, char **end )
>> {
>> #ifdef _GNU_SOURCE
>> static locale_t loc = NULL;
>> if (!loc) {
>> loc = newlocale(LC_CTYPE_MASK, "C", NULL);
>> }
>> return (float) strtod_l(s, end, loc);
>> #else
>> return (float) strtod(s, end);
>> #endif
>> }
>>
>> And then change all _mesa_strtod to _mesa_strtof.
>>
>> If Ian doesn't care for the casts here, then I'm fine with silencing
>> warnings in the Studio with a compiler option.
>
> Attached patch uses strtof when it is available.
Applied. Thanks.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkueVj8ACgkQX1gOwKyEAw/aTQCfRrPt+MDFkh2g87pQKShWSpvR
m9gAnjcl8jsgxfJJJW9j7zCxxhv3LpN9
=mmpj
-----END PGP SIGNATURE-----
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X