Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-29 Thread Zdenek Kotala

Tom Lane píše v čt 28. 05. 2009 v 11:42 -0400:
 Zdenek Kotala zdenek.kot...@sun.com writes:
  I attached another cleanup patch which fixes following warnings reported
  by Sun Studio:
 
 I'm not too impressed with any of these.  The proposed added
 initializers just increase future maintenance effort without solving
 any real problem (since the variables are required by C standard to
 initialize to zero). 

Agree.

  The proposed signature change on psql_completion
 is going to replace a warning on your system with outright failures on
 other people's.

I check readline and definition is still same at least from 5.0 version.
I'm not still understand why it should failure on other systems. I
looked on revision 1.30 of the file and there is only readline-4.2
support mentioned. Is readline 4.2 the problem?

Zdenek


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-29 Thread Zdenek Kotala

Tom Lane píše v čt 28. 05. 2009 v 11:57 -0400:
 ).
 
 AFAICS, Sun's compiler is just too stupid and shouldn't be emitting
 this warning.  Perhaps the right response is to file a bug report
 against the compiler.

I checked it and it is already know bug. It is new lint style check in
Sun Studio 12. Unfortunately, problem is that current workflow does not
allow to detect if code is dead or not in the verification phase. Next
sun studio release could fix it.

Zdenek


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-29 Thread Tom Lane
Zdenek Kotala zdenek.kot...@sun.com writes:
 Tom Lane píše v čt 28. 05. 2009 v 11:42 -0400:
 The proposed signature change on psql_completion
 is going to replace a warning on your system with outright failures on
 other people's.

 I check readline and definition is still same at least from 5.0 version.
 I'm not still understand why it should failure on other systems. I
 looked on revision 1.30 of the file and there is only readline-4.2
 support mentioned. Is readline 4.2 the problem?

[ pokes around... ]  Actually I think the reason it's like this is that
very old versions of readline have completion_matches() rather than
rl_completion_matches(), and the former is declared to take char * not
const char *.  So it still would compile, you'd just get cast-away-const
warnings.  Which is probably okay considering that hardly anyone is
likely to still be using such old readline libs anyway.

We could try experimenting with that after we branch for 8.5.  I'm not
eager to fool with it in late beta, as we'd be invalidating any port
testing already done.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Michael Meskes
On Thu, May 28, 2009 at 11:11:20AM +0200, Zdenek Kotala wrote:
 I attached another cleanup patch which fixes following warnings reported
 by Sun Studio:
 ...
 preproc.c, line 39569: warning: pointer expression or its operand do not 
 point to the same object yyerror_range, result is undefined and non-portable 
 ...
 Following list is still unfixed plus see my comments:
 
 gram.c, line 28487: warning: pointer expression or its operand do not point 
 to the same object yyerror_range, result is undefined and non-portable 
 ...

These two should be the same, both coming from bison. Both files are
auto-generated, thus it might be bison that has to be fixed to remove this
warning. Given that I didn't find any mentioning of preproc in your patch I
suppose it just hit the wrong list though.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Zdenek Kotala

Michael Meskes píše v čt 28. 05. 2009 v 13:33 +0200:
 On Thu, May 28, 2009 at 11:11:20AM +0200, Zdenek Kotala wrote:
  I attached another cleanup patch which fixes following warnings reported
  by Sun Studio:
  ...
  preproc.c, line 39569: warning: pointer expression or its operand do not 
  point to the same object yyerror_range, result is undefined and 
  non-portable 
  ...
  Following list is still unfixed plus see my comments:
  
  gram.c, line 28487: warning: pointer expression or its operand do not 
  point to the same object yyerror_range, result is undefined and 
  non-portable 
  ...
 
 These two should be the same, both coming from bison. Both files are
 auto-generated, thus it might be bison that has to be fixed to remove this
 warning. 

yeah it is generated, but question is if generated code is valid or it
is bug in bison. If it bison bug we need to care about it. There is the
code:

  yyerror_range[1] = yylloc;
  /* Using YYLLOC is tempting, but would change the location of
 the look-ahead.  YYLOC is available though.  */
  YYLLOC_DEFAULT (yyloc, (yyerror_range - 1), 2);
  *++yylsp = yyloc;

Problem is with YYLLOC_DEFAULT. When I look on macro definition 

#define YYLLOC_DEFAULT(Current, Rhs, N)  \
  Current.first_line   = Rhs[1].first_line;  \
  Current.first_column = Rhs[1].first_column;\
  Current.last_line= Rhs[N].last_line;   \
  Current.last_column  = Rhs[N].last_column;

It seems to me that it is OK, because 1 is used as a index which finally
point on yyerror_range[0]. 


 Given that I didn't find any mentioning of preproc in your patch I
 suppose it just hit the wrong list though.

I'm sorry copy paste error. Yeah, I did not fix preproc too.

Zdenek



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Michael Meskes
On Thu, May 28, 2009 at 01:51:07PM +0200, Zdenek Kotala wrote:
 Problem is with YYLLOC_DEFAULT. When I look on macro definition 
 
 #define YYLLOC_DEFAULT(Current, Rhs, N)  \
   Current.first_line   = Rhs[1].first_line;  \
   Current.first_column = Rhs[1].first_column;\
   Current.last_line= Rhs[N].last_line;   \
   Current.last_column  = Rhs[N].last_column;
 
 It seems to me that it is OK, because 1 is used as a index which finally
 point on yyerror_range[0]. 

Wait, this is the bison definition. Well to be more precise the bison
definition in your bison version. Mine is different:

# define YYLLOC_DEFAULT(Current, Rhs, N)\
do  \
  if (YYID (N))\
{   \
  (Current).first_line   = YYRHSLOC (Rhs, 1).first_line;\
  (Current).first_column = YYRHSLOC (Rhs, 1).first_column;  \
  (Current).last_line= YYRHSLOC (Rhs, N).last_line; \
  (Current).last_column  = YYRHSLOC (Rhs, N).last_column;   \
}   \
  else  \
{   \
  (Current).first_line   = (Current).last_line   =  \
YYRHSLOC (Rhs, 0).last_line;\
  (Current).first_column = (Current).last_column =  \
YYRHSLOC (Rhs, 0).last_column;  \
   }   \
while (YYID (0))

Having said that, it doesn't really matter as we redefine the macro:

#define YYLLOC_DEFAULT(Current, Rhs, N) \
do { \
if (N) \
(Current) = (Rhs)[1]; \
else \
(Current) = (Rhs)[0]; \
} while (0)

I have to admit that those version look strikingly unsimilar to me. There is no
reference to Rhs[N] in our macro at all. But then I have no idea whether this
is needed.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: mes...@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Zdenek Kotala

Michael Meskes píše v čt 28. 05. 2009 v 14:47 +0200:
 On Thu, May 28, 2009 at 01:51:07PM +0200, Zdenek Kotala wrote:
  Problem is with YYLLOC_DEFAULT. When I look on macro definition 
  
  #define YYLLOC_DEFAULT(Current, Rhs, N)  \
Current.first_line   = Rhs[1].first_line;  \
Current.first_column = Rhs[1].first_column;\
Current.last_line= Rhs[N].last_line;   \
Current.last_column  = Rhs[N].last_column;
  
  It seems to me that it is OK, because 1 is used as a index which finally
  point on yyerror_range[0]. 
 
 Wait, this is the bison definition. Well to be more precise the bison
 definition in your bison version. Mine is different:

I took it from documentation. I have same as you in generated code.

 # define YYLLOC_DEFAULT(Current, Rhs, N)\
 do  \
   if (YYID (N))\
 {   \
   (Current).first_line   = YYRHSLOC (Rhs, 1).first_line;\
   (Current).first_column = YYRHSLOC (Rhs, 1).first_column;  \
   (Current).last_line= YYRHSLOC (Rhs, N).last_line; \
   (Current).last_column  = YYRHSLOC (Rhs, N).last_column;   \
 }   \
   else  \
 {   \
   (Current).first_line   = (Current).last_line   =  \
 YYRHSLOC (Rhs, 0).last_line;\
   (Current).first_column = (Current).last_column =  \
 YYRHSLOC (Rhs, 0).last_column;  \
}   \
 while (YYID (0))
 
 Having said that, it doesn't really matter as we redefine the macro:
 
 #define YYLLOC_DEFAULT(Current, Rhs, N) \
 do { \
 if (N) \
 (Current) = (Rhs)[1]; \
 else \
 (Current) = (Rhs)[0]; \
 } while (0)
 
 I have to admit that those version look strikingly unsimilar to me. There is 
 no
 reference to Rhs[N] in our macro at all. But then I have no idea whether this
 is needed.

Current is only int. See gramparse.h. I think we could rewrite it this
way:

#define YYLLOC_DEFAULT(Current, Rhs, N) \
do { \
if (N) \
(Current) = (Rhs)[1]; \
else \
(Current) = (Rhs)[N]; \
} while (0)

It is same result and compiler is quite.

Zdenek






-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Tom Lane
Zdenek Kotala zdenek.kot...@sun.com writes:
 I attached another cleanup patch which fixes following warnings reported
 by Sun Studio:

I'm not too impressed with any of these.  The proposed added
initializers just increase future maintenance effort without solving
any real problem (since the variables are required by C standard to
initialize to zero).  The proposed signature change on psql_completion
is going to replace a warning on your system with outright failures on
other people's.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Compiler warning cleanup - unitilized const variables, pointer type mismatch

2009-05-28 Thread Tom Lane
Michael Meskes mes...@postgresql.org writes:
 I have to admit that those version look strikingly unsimilar to me. There is 
 no
 reference to Rhs[N] in our macro at all. But then I have no idea whether this
 is needed.

The default version of the macro is intended to track both the starting
and ending locations of every construct.  Our simplified version only
tracks the starting locations.  The inputs are RHS[k], the location
values for the constituent elements of the current production, and
the output is the location value for the construct being formed.
In the default version, you naturally want to copy the start of
RHS[1] and the end of RHS[N], where N is the number of production
elements.  In ours, we just copy the location of RHS[1].  However,
both macros need a special case for empty productions (with N = 0).

AFAICS, Sun's compiler is just too stupid and shouldn't be emitting
this warning.  Perhaps the right response is to file a bug report
against the compiler.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers