typedefs for indent

Andrew Dunstan wrote:

So I continue to think the best way to generate a list will be to
consolidate lists generated from the buildfarm which represents a wide
variety of build scenarios.

Is anyone else looking at GNU indent to see if it has improved enough to
meet our needs?

I am not going to be able to test GNU indent for a few months so I hope
someone else tests it sooner than that.

Just checking, but you remembered that GNU indent needs a typedef list
too, right? Also, there are scripts in pgindent surrounding the indent
binary that should also be reviewed.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Andrew Dunstan wrote:

So I continue to think the best way to generate a list will be to
consolidate lists generated from the buildfarm which represents a wide
variety of build scenarios.

Is anyone else looking at GNU indent to see if it has improved enough to
meet our needs?

I am not going to be able to test GNU indent for a few months so I hope
someone else tests it sooner than that.

Just checking, but you remembered that GNU indent needs a typedef list
too, right? Also, there are scripts in pgindent surrounding the indent
binary that should also be reviewed.

Right now I am addressing the very specific problem of coming up with a
better typedef list to use with whatever indenter we use, given that it
seems a task well suited to the buildfarm.

If I do get to looking at GNU indent it will be a way down the track for
me too. In fact, this seems a task that might be well suited to someone
who is not spending what time they have available for Postgres on
hacking (currently I'm trying to remove bitrot from the column level
privs patch).

cheers

andrew

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

I'm still trying to get a working objdump for OSX. Automating this is
difficult because we need to make sure we get all (or pretty close to
all) the typedefs we can get on each platform for various build
configurations.

cheers

andrew

Andrew Dunstan wrote:

Alvaro Herrera wrote:

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

I'm still trying to get a working objdump for OSX. Automating this is
difficult because we need to make sure we get all (or pretty close to
all) the typedefs we can get on each platform for various build
configurations.

At this point I would like to get a typedef list into CVS, even if it is
not perfect --- it is better than what we have now.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Andrew Dunstan wrote:

Alvaro Herrera wrote:

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

I'm still trying to get a working objdump for OSX. Automating this is
difficult because we need to make sure we get all (or pretty close to
all) the typedefs we can get on each platform for various build
configurations.

At this point I would like to get a typedef list into CVS, even if it is
not perfect --- it is better than what we have now.

Well, you can start with this one:

http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dungbeetle&amp;dt=2008-07-21%20204856&amp;stg=typedefs

After I have a number of buildfarm machines producing them, I'll work on
a stored proc to consolidate them and make them available, probably via
a SOAP call (c.f.
http://people.planetpostgresql.org/andrew/index.php?/archives/14-SOAP-server-for-Buildfarm-dashboard.html
)

cheers

andrew

Andrew Dunstan wrote:

Alvaro Herrera wrote:

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

I'm still trying to get a working objdump for OSX. Automating this is
difficult because we need to make sure we get all (or pretty close to
all) the typedefs we can get on each platform for various build
configurations.

I need to run pgindent in a few months. What typedef list am I going to
use?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Andrew Dunstan wrote:

Alvaro Herrera wrote:

Andrew Dunstan wrote:

OK, I have spent some time generating and filtering typdefs via objdump
on various platforms. I filtered them and Bruce's list to eliminate
items not actually found in the sources thus:

Did this go anywhere?

I'm still trying to get a working objdump for OSX. Automating this is
difficult because we need to make sure we get all (or pretty close to
all) the typedefs we can get on each platform for various build
configurations.

I need to run pgindent in a few months. What typedef list am I going to
use?

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Anyone running a buildfarm member should be able to do this and add to
the results, if they are up to date with release 3.2. I have my linux
crontab set up to do one typedefs run on the HEAD branch each day.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

Bruce Momjian wrote:

I need to run pgindent in a few months. What typedef list am I going to
use?

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Could we get diffs of the lists produced by those machines individually?
That would provide a bit of evidence about how severe the platform
dependence issue really is for this, and thereby help us guess how
urgent it is to gather more data.

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Bruce Momjian wrote:

I need to run pgindent in a few months. What typedef list am I going to
use?

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Could we get diffs of the lists produced by those machines individually?
That would provide a bit of evidence about how severe the platform
dependence issue really is for this, and thereby help us guess how
urgent it is to gather more data.

Yes. I'll set it up with a query.

For now, they are here:

Linux:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dungbeetle&amp;dt=2009-03-22%20064401&amp;stg=typedefs&gt;
Cygwin:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=brown_bat&amp;dt=2009-03-21%20130058&amp;stg=typedefs&gt;
MinGW:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dawn_bat&amp;dt=2009-03-21%20142340&amp;stg=typedefs&gt;

cheers

andrew

Andrew Dunstan wrote:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Bruce Momjian wrote:

I need to run pgindent in a few months. What typedef list am I
going to
use?

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three
fairly different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't
know how to extract the typedefs on OSX.

Could we get diffs of the lists produced by those machines individually?
That would provide a bit of evidence about how severe the platform
dependence issue really is for this, and thereby help us guess how
urgent it is to gather more data.

Yes. I'll set it up with a query.

For now, they are here:

Linux:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dungbeetle&amp;dt=2009-03-22%20064401&amp;stg=typedefs&gt;

Cygwin:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=brown_bat&amp;dt=2009-03-21%20130058&amp;stg=typedefs&gt;

MinGW:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dawn_bat&amp;dt=2009-03-21%20142340&amp;stg=typedefs&gt;

Or from now on, use this for the individual URL list:

<http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list=1&gt;

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

[ typedef lists ]

Hmm ... the windows members are claiming that "int", "char", "float",
"double" etc are typedefs, which doesn't exactly match up with my
mental model of C. On the other hand, dungbeetle is failing to report
a whole bunch of typedefs that it should report, for example
AfterTriggerEventDataOneCtid which comes from entirely non platform
specific code in commands/trigger.c.

In short, I don't think I trust this data at all...

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

[ typedef lists ]

Hmm ... the windows members are claiming that "int", "char", "float",
"double" etc are typedefs, which doesn't exactly match up with my
mental model of C. On the other hand, dungbeetle is failing to report
a whole bunch of typedefs that it should report, for example
AfterTriggerEventDataOneCtid which comes from entirely non platform
specific code in commands/trigger.c.

In short, I don't think I trust this data at all...

Well, the procedure for generating it is quite public.

The relevant piece of perl is this - feel free to suggest improvements:

if (@err == 1) # Linux
{
@dumpout = `objdump -W $bin 2>/dev/null | egrep -A3
'(DW_TAG_typedef|DW_TAG_structure_type|DW_TAG_union_type)' 2>/dev/null`;
foreach (@dumpout)
{
@flds = split;
next if ($flds[0] ne 'DW_AT_name' || $flds[-1] =~
/^DW_FORM_str/);
$syms{$flds[-1]} =1;
}
}
else
{
@dumpout = `objdump --stabs $bin 2>/dev/null`;
foreach (@dumpout)
{
@flds = split;
next if (@flds < 7);
next if ($flds[1] ne 'LSYM' || $flds[6] !~ /([^:]+):[tT]/);
$syms{$1} =1;
}
}

cheers

andrew

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

[ typedef lists ]

Hmm ... the windows members are claiming that "int", "char", "float",
"double" etc are typedefs, which doesn't exactly match up with my
mental model of C.

I don't think this is a problem, because the whole point is telling
indent what names should be considered type names, which of course those
should all be.

On the other hand, dungbeetle is failing to report
a whole bunch of typedefs that it should report, for example
AfterTriggerEventDataOneCtid which comes from entirely non platform
specific code in commands/trigger.c.

This was probably optimized out by the compiler.

I tend to think that having this list is much better than no list at all
(the current situation), and it's better than the old list we used to
have.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

On the other hand, dungbeetle is failing to report
a whole bunch of typedefs that it should report,

I tend to think that having this list is much better than no list at all
(the current situation), and it's better than the old list we used to
have.

Well, AIUI the old list was essentially the result of this code as run
on Bruce's BSD machine. We had supposed that the obvious omissions in
the old list were caused by platform-specific decisions to not build
code that referenced particular typedef names, but now I wonder. It
certainly appears that the technique has got some significant bug on
dungbeetle.

regards, tom lane

After doing some further digging I've concluded that the typedefs that
are missing from dungbeetle's list are those that are declared, but are
never used as the type of anything. For instance
AfterTriggerEventDataOneCtid is only used in a sizeof() computation,
and there are quite a few enums for which no variable is ever declared
as being of the enum type. So Alvaro seems to be right that these are
getting optimized out of the debug info. The good news is that as far
as I can think at the moment, that means we don't really care whether
pg_indent knows they are typedef names or not --- they aren't getting
used in any contexts where it would matter for indenting purposes.

However, this does complicate the matter of comparing the typedef
lists to identify how many symbols are platform-specific or
compile-option-specific typedefs.

BTW, is dungbeetle still running Fedora 6? On my F-10 machine the
output of objdump seems to be formatted differently than your script
is expecting, eg

$ objdump -W postgres | egrep -A3 '(DW_TAG_typedef|DW_TAG_structure_type|DW_TAG_union_type)' | grep AfterTriggerEvent
<19ac75> DW_AT_name : (indirect string, offset: 0x1c1ad): AfterTriggerEvent
<19ac87> DW_AT_name : (indirect string, offset: 0x1bffc): AfterTriggerEventData
<19acc2> DW_AT_name : (indirect string, offset: 0x1bffc): AfterTriggerEventData
<19acce> DW_AT_name : (indirect string, offset: 0x1c891): AfterTriggerEventChunk
<19ad1e> DW_AT_name : (indirect string, offset: 0x1c891): AfterTriggerEventChunk
<19ad2a> DW_AT_name : (indirect string, offset: 0x1c166): AfterTriggerEventList
<19ad6b> DW_AT_name : (indirect string, offset: 0x1c166): AfterTriggerEventList

regards, tom lane

Tom Lane wrote:

BTW, is dungbeetle still running Fedora 6?

yes. Upgrading it is on my long TODO list. I wish Fedora had a bit
longer release cycles.

On my F-10 machine the
output of objdump seems to be formatted differently than your script
is expecting

I guess that will make upgrading a bit more urgent ;-)

cheers

andrew

Andrew Dunstan wrote:

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Anyone running a buildfarm member should be able to do this and add to
the results, if they are up to date with release 3.2. I have my linux
crontab set up to do one typedefs run on the HEAD branch each day.

[ Discussion deleted.]

Andrew, this is disappointing news.  When you talked about generating an
typedef list from the buildfarm, you were saying how great it would be
--- now a year later you post:

It'd be nice to get that dealt with before we run pg_indent, but it's
not like we'd be any worse off than before if we don't. In any case it's
surely no blocker for 8.4beta.

We can't have the system-supplied typedef list changing from release to
release because that affects the indenting from release to release,
which affects backpatching and other stuff. And even if you get a more
complete list then we have used in the past, what are the odds you are
going to supply a typedef that is a typedef on some operating system
that matches an identifier in our code that _isn't_ used as a typedef by
us?

We only have a few weeks until I have to run pgindent so I would like
this resolved one way or another soon. Unless I hear otherwise I assume
we are going to just use the an updated list of our defined typedefs
that gets generated from our code, which includes my BSD typedefs.

One other approach would be to include in pg_indent a hard-coded list of
non-BSD system-defined typedefs that we reference from our code. One
way to find those would be to run pg_indent with and without Andrew's
list of typedefs and see how the formatting changes.

Or just use a Linux list of system typedefs from now on.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

bruce wrote:

Andrew Dunstan wrote:

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Anyone running a buildfarm member should be able to do this and add to
the results, if they are up to date with release 3.2. I have my linux
crontab set up to do one typedefs run on the HEAD branch each day.

[ Discussion deleted.]

Andrew, this is disappointing news.  When you talked about generating an
typedef list from the buildfarm, you were saying how great it would be
--- now a year later you post:

It'd be nice to get that dealt with before we run pg_indent, but it's
not like we'd be any worse off than before if we don't. In any case it's
surely no blocker for 8.4beta.

My apologies; the above are Tom's words, not Andrews.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

bruce wrote:

Andrew Dunstan wrote:

This URL <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt; gives a
typedef list that is (currently) the combined result from three fairly
different buildfarm members:

dungbeetle | 2009-03-22 06:44:01
brown_bat | 2009-03-21 13:00:58
dawn_bat | 2009-03-21 14:23:40

These are respectively my Linux, Cygwin and MinGW buildfarm members.

I don't have a BSD machine of any flavor to test on, and I don't know
how to extract the typedefs on OSX.

Anyone running a buildfarm member should be able to do this and add to
the results, if they are up to date with release 3.2. I have my linux
crontab set up to do one typedefs run on the HEAD branch each day.

[ Discussion deleted.]

Andrew, this is disappointing news.  When you talked about generating an
typedef list from the buildfarm, you were saying how great it would be
--- now a year later you post:

It'd be nice to get that dealt with before we run pg_indent, but it's
not like we'd be any worse off than before if we don't. In any case it's
surely no blocker for 8.4beta.

My apologies; the above are Tom's words, not Andrews.

Apology accepted.

What I promised was a list that was more comprehensive than what we were
using. I think I've already delivered on that, but I would like to do
better by including some other Operating Systems: particularly some BSD
flavors. Buildfarm owners with non-Linux non-Windows members please
take note. Email me if you need help with this.

Unless we come up with some tolerably correct and maintainable code
analysis tool for identifying typedefs, using the current heuristic
methods is apparently the best we can do. Nobody has suggested even an
outline for such a tool. I don't think using the buildfarm for this
heuristic method is great, and never suggested it would be. I do think
it's an improvement, which is what I promised. I'm sorry if you find the
result disappointing.

cheers

andrew

Andrew Dunstan wrote:

Andrew, this is disappointing news.  When you talked about generating an
typedef list from the buildfarm, you were saying how great it would be
--- now a year later you post:

It'd be nice to get that dealt with before we run pg_indent, but it's
not like we'd be any worse off than before if we don't. In any case it's
surely no blocker for 8.4beta.

My apologies; the above are Tom's words, not Andrews.

Apology accepted.

What I promised was a list that was more comprehensive than what we were
using. I think I've already delivered on that, but I would like to do
better by including some other Operating Systems: particularly some BSD
flavors. Buildfarm owners with non-Linux non-Windows members please
take note. Email me if you need help with this.

Unless we come up with some tolerably correct and maintainable code
analysis tool for identifying typedefs, using the current heuristic
methods is apparently the best we can do. Nobody has suggested even an
outline for such a tool. I don't think using the buildfarm for this
heuristic method is great, and never suggested it would be. I do think
it's an improvement, which is what I promised. I'm sorry if you find the
result disappointing.

Well, as you, I was hoping for a clear solution, and it seems we don't
have one. I think the false-positives problem is real and might make
the greater code coverage of the buildfarm worse than what we did for
8.3.

I think our only fallback is to find places that our BSD items miss,
perhaps Win32 cases, see what is different with those lists, and just
hard-code them in, because then we aren't importing a huge number of
additional typedefs that have uncertain consequences.

Frankly, I don't remember anyone complaining we didn't find any typedefs
in pgindent, though I think there might have been a few EXEC_BACKEND
cases, and maybe we can just hardcode those.

Frankly, pgindent has larger problems than an imcomplete typedef list. :-(

When I am ready to run pgindent I will ask for your typedef list and see
what the diff shows when I use your list and we can figure something out
then.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

Frankly, I don't remember anyone complaining we didn't find any typedefs
in pgindent,

There are lots and lots of places where it's obvious that pgindent
was misinformed on the subject, because it puts in a space where there
should not be one, eg "typename * ptr" instead of "typename *ptr".
Maybe I'm just being too anal in worrying about a space here or a space
there ... but then why do we run the thing at all?

This is the first time I've heard anyone suggest that false positives
could be a problem. What exactly would be the results of a false
match? Would it look worse than the false negatives do?

regards, tom lane

Bruce Momjian wrote:

Well, as you, I was hoping for a clear solution, and it seems we don't
have one. I think the false-positives problem is real and might make
the greater code coverage of the buildfarm worse than what we did for
8.3.

Huh? What false positive problem?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Frankly, I don't remember anyone complaining we didn't find any typedefs
in pgindent,

There are lots and lots of places where it's obvious that pgindent
was misinformed on the subject, because it puts in a space where there
should not be one, eg "typename * ptr" instead of "typename *ptr".
Maybe I'm just being too anal in worrying about a space here or a space
there ... but then why do we run the thing at all?

Sure, why not make it as good as we can.

This is the first time I've heard anyone suggest that false positives
could be a problem. What exactly would be the results of a false
match? Would it look worse than the false negatives do?

Well, I assume a false positive would do the opposite, meaning it would
not have a space where it should have one. I am also worried about a
typedef list that changes from release to release as buildfarm members
change; variability might be worse than correctness in this case.

Anyway, I think a diff of using my list and Andrew's list will show us
which one gets things clearest; the diff is going to highlight only
cases where the typedef lists change formatting.

Andrew, where exactly is the list I should try?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Alvaro Herrera wrote:

Bruce Momjian wrote:

Well, as you, I was hoping for a clear solution, and it seems we don't
have one. I think the false-positives problem is real and might make
the greater code coverage of the buildfarm worse than what we did for
8.3.

Huh? What false positive problem?

typedefs listed on platforms that match identifiers in our code that are
_not_ typedefs.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Well, as you, I was hoping for a clear solution, and it seems we don't
have one. I think the false-positives problem is real and might make
the greater code coverage of the buildfarm worse than what we did for
8.3.

Huh? What false positive problem?

typedefs listed on platforms that match identifiers in our code that are
_not_ typedefs.

Does this actually happen anywhere?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Well, as you, I was hoping for a clear solution, and it seems we don't
have one. I think the false-positives problem is real and might make
the greater code coverage of the buildfarm worse than what we did for
8.3.

Huh? What false positive problem?

typedefs listed on platforms that match identifiers in our code that are
_not_ typedefs.

Does this actually happen anywhere?

No idea; it was more a theoretical issue to say that having more
typedefs is not necessarily a good thing; they should ideally be the
typedefs we use, and Windows adds a lot of typedefs we don't use.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Anyway, I think a diff of using my list and Andrew's list will show us
which one gets things clearest; the diff is going to highlight only
cases where the typedef lists change formatting.

Andrew, where exactly is the list I should try?

fetch it from <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt;

cheers

andrew

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Alvaro Herrera wrote:

Huh? What false positive problem?

typedefs listed on platforms that match identifiers in our code that are
_not_ typedefs.

Does this actually happen anywhere?

No idea; it was more a theoretical issue to say that having more
typedefs is not necessarily a good thing; they should ideally be the
typedefs we use, and Windows adds a lot of typedefs we don't use.

Okay, so I went over the mingw list a bit (not exhaustively) and found
no typedef that's used as an identifier in our code.

Huh ... just found one. It's called "timezone", but it's used as an
identifier only in the function declaration (dt2local), not in the
function definition, which uses "tz" instead.

There's also ACL, but we only use it in macro definitions.

There are a bunch of other typedefs that the mingw port adds, but
several of them are actually used in our code (HANDLE, BOOL, etc).

I think this is minor enough that it should be ignored.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Andrew Dunstan wrote:

Bruce Momjian wrote:

Anyway, I think a diff of using my list and Andrew's list will show us
which one gets things clearest; the diff is going to highlight only
cases where the typedef lists change formatting.

Andrew, where exactly is the list I should try?

fetch it from <http://www.pgbuildfarm.org/cgi-bin/typedefs.pl&gt;

Thanks. I will run tests when we are ready for pg_indent and we can
then make a decision.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Thanks. I will run tests when we are ready for pg_indent and we can
then make a decision.

FWIW I was looking at this code for unrelated reasons and found a couple
of symbols that pgindent considers to be typedefs but it clearly are not
-- BufferHitCount and LocalBufferHitCount. It can be seen in
ShowBufferUsage().

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Bruce Momjian wrote:

Thanks. I will run tests when we are ready for pg_indent and we can
then make a decision.

FWIW I was looking at this code for unrelated reasons and found a couple
of symbols that pgindent considers to be typedefs but it clearly are not
-- BufferHitCount and LocalBufferHitCount. It can be seen in
ShowBufferUsage().

Are you saying you saw this in Andrew's typedef output, or from the 8.3
run of pg_indent?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Thanks. I will run tests when we are ready for pg_indent and we can
then make a decision.

FWIW I was looking at this code for unrelated reasons and found a couple
of symbols that pgindent considers to be typedefs but it clearly are not
-- BufferHitCount and LocalBufferHitCount. It can be seen in
ShowBufferUsage().

Are you saying you saw this in Andrew's typedef output, or from the 8.3
run of pg_indent?

This is on the 8.3 code. Notice how this is formatted:

hitrate = (float) BufferHitCount *100.0 / ReadBufferCount;

(line 1465)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Thanks. I will run tests when we are ready for pg_indent and we can
then make a decision.

FWIW I was looking at this code for unrelated reasons and found a couple
of symbols that pgindent considers to be typedefs but it clearly are not
-- BufferHitCount and LocalBufferHitCount. It can be seen in
ShowBufferUsage().

Are you saying you saw this in Andrew's typedef output, or from the 8.3
run of pg_indent?

This is on the 8.3 code. Notice how this is formatted:

hitrate = (float) BufferHitCount *100.0 / ReadBufferCount;

These symbols are not in the buildfarm's list of typedefs.

cheers

andrew

Alvaro Herrera wrote:

This is on the 8.3 code. Notice how this is formatted:

hitrate = (float) BufferHitCount *100.0 / ReadBufferCount;

Hmm, I just noticed that this is mentioned as a "known bug" in pgindent.
Nevermind ...

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Andrew Dunstan wrote:

FWIW I was looking at this code for unrelated reasons and found a couple
of symbols that pgindent considers to be typedefs but it clearly are not
-- BufferHitCount and LocalBufferHitCount. It can be seen in
ShowBufferUsage().

Are you saying you saw this in Andrew's typedef output, or from the 8.3
run of pg_indent?

This is on the 8.3 code. Notice how this is formatted:

hitrate = (float) BufferHitCount *100.0 / ReadBufferCount;

These symbols are not in the buildfarm's list of typedefs.

The good news is that LocalBufferHitCount isn't in my list of typedefs
from CVS HEAD, and probably not in 8.3 either. The bad news is that
pgindent pushes the '*' next to the 100.0 in my testing. :-(

I tested BSD indent alone with no arguments or typedef list and got the
same output, even after adding the space in the C file, so something
wrong must be happening in the binary.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

Andrew Dunstan wrote:

This is on the 8.3 code. Notice how this is formatted:

hitrate = (float) BufferHitCount *100.0 / ReadBufferCount;

The good news is that LocalBufferHitCount isn't in my list of typedefs
from CVS HEAD, and probably not in 8.3 either. The bad news is that
pgindent pushes the '*' next to the 100.0 in my testing. :-(

It's the (float), possibly in combination with the *, that does that.
There are many occurrences of this with other type names, eg (double).
I think it's too dumb to figure out that this is a cast and not a
variable declaration.

regards, tom lane