Re: [HACKERS] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] typedefs for indent
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] typedefs for indent
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 -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] typedefs for indent
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + AES_KEY AFFIX ARRAY_TYPE ASN1_BIT_STRING ASN1_BMPSTRING ASN1_BOOLEAN ASN1_CTX ASN1_ENCODING ASN1_ENCODING_st ASN1_ENUMERATED ASN1_GENERALIZEDTIME ASN1_GENERALSTRING ASN1_HEADER ASN1_IA5STRING ASN1_INTEGER ASN1_ITEM ASN1_ITEM_EXP ASN1_METHOD ASN1_NULL ASN1_OBJECT ASN1_OCTET_STRING ASN1_PRINTABLESTRING ASN1_STRING ASN1_STRING_TABLE ASN1_T61STRING ASN1_TEMPLATE ASN1_TIME ASN1_TLC ASN1_TYPE ASN1_UNIVERSALSTRING ASN1_UTCTIME ASN1_UTF8STRING ASN1_VALUE ASN1_VISIBLESTRING A_ArrayExpr A_Const A_Expr A_Expr_Kind A_Indices A_Indirection A_Star AbsoluteTime AccessPriv Acl AclItem AclMaskHow AclMode AclObjectKind AclResult ActiveSnapshotElt AffixNode AffixNodeData AfterTriggerEvent AfterTriggerEventChunk AfterTriggerEventData AfterTriggerEventDataOneCtid AfterTriggerEventList AfterTriggerShared AfterTriggerSharedData AfterTriggers AfterTriggersData Agg AggClauseCounts AggHashEntry AggHashEntryData AggInfo AggState AggStatePerAgg AggStatePerAggData AggStatePerGroup AggStatePerGroupData AggStrategy Aggref AggrefExprState Alias AllocBlock AllocBlockData AllocChunk AllocChunkData AllocPointer AllocSet AllocSetContext AllocateDesc AllocateDescKind AlterDatabaseSetStmt AlterDatabaseStmt AlterDomainStmt AlterFdwStmt AlterForeignServerStmt AlterFunctionStmt AlterObjectSchemaStmt AlterOpFamilyStmt AlterOptionOp AlterOwnerStmt AlterRoleSetStmt AlterRoleStmt AlterSeqStmt AlterTSConfigurationStmt AlterTSDictionaryStmt AlterTableCmd AlterTableStmt AlterTableType AlterUserMappingStmt AlteredTableInfo AlternativeSubPlan AlternativeSubPlanState AnalyzeAttrFetchFunc AnlIndexData Append AppendPath AppendRelInfo AppendState Archive ArchiveEntryPtr ArchiveFormat ArchiveHandle ArchiveMode ArchiverStage ArrayBuildState ArrayCoerceExpr ArrayCoerceExprState ArrayConstIterState ArrayExpr ArrayExprIterState ArrayExprState ArrayMapState ArrayMetaState ArrayParseState ArrayRef ArrayRefExprState ArrayTuple ArrayType AttInMetadata AttrDefInfo AttrDefault AttrNumber AuthRequest AutoVacOpts AutoVacuumShmemStruct AutoVacuumSignal AuxProcType BF_KEY BIGNUM BIO BIO_F_BUFFER_CTX BIO_METHOD BIO_dummy BITVEC BITVECP BIT_STRING_BITNAME BIT_STRING_BITNAME_st BMS_Membership BN_BLINDING BN_CTX BN_MONT_CTX BN_RECP_CTX BOX BTBuildState BTCycleId BTMetaPageData BTOneVacInfo BTPageOpaque BTPageOpaqueData BTPageState BTScanOpaque BTScanOpaqueData BTScanPos BTScanPosData BTScanPosItem BTSpool BTStack BTStackData BTVacInfo BTVacState BTWriteState BUF_MEM Backend BackendId BackslashQuoteType BgWriterRequest BgWriterShmemStruct BitmapAnd BitmapAndPath BitmapAndState BitmapHeapPath BitmapHeapScan BitmapHeapScanState BitmapIndexScan BitmapIndexScanState BitmapOr BitmapOrPath BitmapOrState Bitmapset BkpBlock Block BlockId BlockIdData BlockNumber BlockSampler BlockSamplerData BoolExpr BoolExprState BoolExprType BoolPtr BoolTestType BooleanTest BpChar Bucket BufFile BufFlags Buffer BufferAccessStrategy BufferAccessStrategyData BufferAccessStrategyType BufferDesc BufferLookupEnt BufferStrategyControl BufferTag BuildAccumulator BulkInsertState BulkInsertStateData Byte Bytef CACHESIGN CAC_state CAST_KEY CFuncHashTabEntry CHKVAL CIRCLE CMPDAffix COMPAT_MODE COMP_CTX COMP_METHOD CONF CONF_IMODULE CONF_METHOD CONF_MODULE CONF_VALUE CPFunction CPPFunction CRYPTO_EX_DATA CRYPTO_EX_DATA_FUNCS CRYPTO_EX_DATA_IMPL CRYPTO_EX_dup CRYPTO_EX_free CRYPTO_EX_new CRYPTO_MEM_LEAK_CB CRYPTO_dynlock CachedPlan CachedPlanSource CancelRequestPacket CaseExpr CaseExprState CaseTestExpr CaseWhen CaseWhenState Cash CastInfo CatCList CatCTup CatCache CatCacheHeader CatalogId CatalogIndexState ChangeVarNodes_context CheckPoint CheckPointStmt CheckpointStatsData Chromosome City ClonePtr ClosePortalStmt ClosePtr ClusterStmt CmdType CoalesceExpr CoalesceExprState CoerceToDomain CoerceToDomainState CoerceToDomainValue CoerceViaIO CoerceViaIOState CoercionCodes
Re: [HACKERS] typedefs for indent
Bruce Momjian br...@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 -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] typedefs for indent
Tom Lane wrote: Bruce Momjian br...@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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] typedefs for indent
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 cheers andrew -- 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] typedefs for indent
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 Thanks. I will run tests when we are ready for pg_indent and we can then make a decision. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] typedefs for indent
Andrew Dunstan wrote: This URL http://www.pgbuildfarm.org/cgi-bin/typedefs.pl 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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
bruce wrote: Andrew Dunstan wrote: This URL http://www.pgbuildfarm.org/cgi-bin/typedefs.pl 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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
Bruce Momjian wrote: bruce wrote: Andrew Dunstan wrote: This URL http://www.pgbuildfarm.org/cgi-bin/typedefs.pl 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 -- 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] typedefs for indent
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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
Bruce Momjian br...@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 -- 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] typedefs for indent
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 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 -- 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] typedefs for indent
Andrew Dunstan and...@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 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 -- 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] typedefs for indent
Tom Lane wrote: Andrew Dunstan and...@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 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=dungbeetledt=2009-03-22%20064401stg=typedefs Cygwin: http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=brown_batdt=2009-03-21%20130058stg=typedefs MinGW: http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dawn_batdt=2009-03-21%20142340stg=typedefs cheers andrew -- 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] typedefs for indent
Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan and...@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 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=dungbeetledt=2009-03-22%20064401stg=typedefs Cygwin: http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=brown_batdt=2009-03-21%20130058stg=typedefs MinGW: http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dawn_batdt=2009-03-21%20142340stg=typedefs Or from now on, use this for the individual URL list: http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list=1 cheers andrew -- 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] typedefs for indent
Andrew Dunstan and...@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 -- 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] typedefs for indent
Tom Lane wrote:
Andrew Dunstan and...@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
--
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] typedefs for indent
Tom Lane wrote: Andrew Dunstan and...@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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] typedefs for indent
Alvaro Herrera alvhe...@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 -- 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] typedefs for indent
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 -- 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] typedefs for indent
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 -- 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] typedefs for indent
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 [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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=dungbeetledt=2008-07-21%20204856stg=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 -- 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] typedefs for indent
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 [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] typedefs for indent
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 -- 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] 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 [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] typedefs for indent
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] typedefs for indent
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: while read line ; do grep -q -w -r --exclude=*.data --exclude=*.out --exclude=*.sql --exclude=*.po --exclude='*akefile' $line pgsql.head/src/backend pgsql.head/src/include pgsql.head/src/bin pgsql.head/src/interfaces pgsql.head/src/pl pgsql.head/src/port pgsql.head/src/timezone/*.[ch] pgsql.head/src/test/regress/*.[ch] pgsql.head/contrib echo $line; done (This filter runs a lot faster than the one Alvaro posted.) If someone can point me where to get objdump for OSX I'll look at generating a list there too. The results can be seen at: http://developer.postgresql.org/~adunstan/linux-found http://developer.postgresql.org/~adunstan/mingw-found http://developer.postgresql.org/~adunstan/cygwin-found http://developer.postgresql.org/~adunstan/bruce-bsdos-found counts: 2010 bruce-bsdos-found 2036 cygwin-found 1979 linux-found 2125 mingw-found It seems clear (as we expected) that this process is sensitive to both the build system and build options used. It's not just additive, though. Bruce has some symbols that my linux build doesn't have because he didn't build with openssh. 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? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
