Undesirable entries in typedefs list

Started by Tom Lanealmost 8 years ago11 messages
#1Tom Lane
tgl@sss.pgh.pa.us

I noticed that doing pgindent with the current typedefs list available
from the buildfarm caused a lot of havoc in what had been stable code.
Looking into the reasons, it seems that:

(1) "bool" is no longer listed as a typedef name (probably because
stdbool.h makes it a macro instead);

(2) "abs", "boolean", "iterator", "other", "pointer", "reference",
"string", and "type" all now are listed as typedef names.

It's probably okay to treat "boolean" as a typedef, but all those others
are complete disasters. Anyone know where they're coming from?

As for "bool", we could probably deal with that most reliably by
having pgindent add it as a special case. Maybe we could get it
back in there by having some trailing-edge buildfarm member
contribute typedefs, but that seems like a solution with a rather
limited half-life.

regards, tom lane

#2Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#1)
Re: Undesirable entries in typedefs list

Hi,

On 2018-03-24 13:25:34 -0400, Tom Lane wrote:

(2) "abs", "boolean", "iterator", "other", "pointer", "reference",
"string", and "type" all now are listed as typedef names.

It's probably okay to treat "boolean" as a typedef, but all those others
are complete disasters. Anyone know where they're coming from?

Semi informed theory: LLVM? I think I'd configured one of the LLVM
animals to collect typedefs, but that might have been a bad idea...

As for "bool", we could probably deal with that most reliably by
having pgindent add it as a special case. Maybe we could get it
back in there by having some trailing-edge buildfarm member
contribute typedefs, but that seems like a solution with a rather
limited half-life.

Could we combine the list of typedefs with one manually maintained
in-tree?

Greetings,

Andres Freund

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: Undesirable entries in typedefs list

Andres Freund <andres@anarazel.de> writes:

On 2018-03-24 13:25:34 -0400, Tom Lane wrote:

(2) "abs", "boolean", "iterator", "other", "pointer", "reference",
"string", and "type" all now are listed as typedef names.

It's probably okay to treat "boolean" as a typedef, but all those others
are complete disasters. Anyone know where they're coming from?

Semi informed theory: LLVM? I think I'd configured one of the LLVM
animals to collect typedefs, but that might have been a bad idea...

That was my first thought as well, since this seems to have changed
quite recently. I don't think it's a bad idea to be collecting typedefs
from something that compiles that code, because otherwise our own
typedefs in that area won't be known.

As for "bool", we could probably deal with that most reliably by
having pgindent add it as a special case. Maybe we could get it
back in there by having some trailing-edge buildfarm member
contribute typedefs, but that seems like a solution with a rather
limited half-life.

Could we combine the list of typedefs with one manually maintained
in-tree?

Perhaps. We might need a manually maintained blacklist, as well.

regards, tom lane

#4Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#3)
Re: Undesirable entries in typedefs list

Hi,

On 2018-03-24 13:34:45 -0400, Tom Lane wrote:

That was my first thought as well, since this seems to have changed
quite recently. I don't think it's a bad idea to be collecting typedefs
from something that compiles that code, because otherwise our own
typedefs in that area won't be known.

The number of typedefs we actually use is fairly small (~5-7), so we
could realistically maintain the them manually.

I'm about to head out, but afterwards I'm going to check what the
typedefs collected actually are when LLVM is enabled.

Greetings,

Andres Freund

#5Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#4)
1 attachment(s)
Re: Undesirable entries in typedefs list

On 2018-03-24 10:41:40 -0700, Andres Freund wrote:

I'm about to head out, but afterwards I'm going to check what the
typedefs collected actually are when LLVM is enabled.

It's indeed from llvm:

<3><5201>: Abbrev Number: 4 (DW_TAG_typedef)
<5202> DW_AT_name : (indirect string, offset: 0x8a0a4): string
<5206> DW_AT_decl_file : 13
<5207> DW_AT_decl_line : 74
<5208> DW_AT_type : <0x3896>

which basically references std::string

<3><3896>: Abbrev Number: 24 (DW_TAG_class_type)
<3897> DW_AT_name : (indirect string, offset: 0x2db87): basic_string<char, std::char_traits<char>, std::allocator<char> >
<389b> DW_AT_byte_size : 32
<389c> DW_AT_decl_file : 12
<389d> DW_AT_decl_line : 77
<389e> DW_AT_sibling : <0x51fc>

Given that the PG code shouldn't refer to this, I think we might be able
to limit things by specifying --dwarf-depth=3 to the objdump output.

I've attached the difference between a objdump typedefs list roughly
equivalent to what the buildfarm uses. There's no difference when not
using llvm.

I'm a bit uncomfortable relying --dwarf-depth=3, with 3 being determined
purely experimentally though.

Greetings,

Andres Freund

Attachments:

typelist_diff_llvm.txttext/plain; charset=us-asciiDownload
--- /tmp/d0_llvm.txt	2018-03-24 13:34:12.700301122 -0700
+++ /tmp/d3_llvm.txt	2018-03-24 13:33:46.339261318 -0700
@@ -44,12 +44,8 @@
 A_Indirection
 AlenState
 Alias
-alias_iterator
-AliasListType
-AlignmentsTy
 AllocateDesc
 AllocateDescKind
-allocator_type
 AllocBlock
 AllocChunk
 allocfunc
@@ -131,7 +127,6 @@
 ArchiveMode
 ArchiverOutput
 ArchiverStage
-arg_iterator
 ArrayAnalyzeExtraData
 ArrayBuildState
 ArrayBuildStateAny
@@ -206,11 +201,7 @@
 Barrier
 BaseBackupCmd
 basebackup_options
-_Base_ptr
-BaseT
-__base_type
 base_yy_extra_type
-BasicBlockListType
 BeginDirectModify_function
 BeginForeignModify_function
 BeginForeignScan_function
@@ -251,7 +242,6 @@
 BlockedProcsData
 BlockId
 BlockIdData
-block_iterator
 BlockNumber
 BlockSampler
 BlockSamplerData
@@ -315,7 +305,6 @@
 __builtin_va_list
 BulkInsertState
 BumpPtrAllocator
-bundle_op_iterator
 Byte
 bytea
 Bytef
@@ -328,7 +317,6 @@
 CallStmt
 CancelRequestPacket
 CaseExpr
-CaseIt
 CaseTestExpr
 CaseWhen
 Cash
@@ -345,10 +333,8 @@
 CEOUC_WAIT_MODE
 CFuncHashTabEntry
 ChangeVarNodes_context
-_Char_alloc_type
 charbufferproc
 char_traits<char32_t>
-char_type
 check_agg_arguments_context
 check_function_callback
 check_network_data
@@ -412,7 +398,6 @@
 ComboCidEntryData
 ComboCidKey
 ComboCidKeyData
-ComdatSymTabType
 Command
 CommandDest
 CommandId
@@ -440,42 +425,13 @@
 ConnType
 ConsiderSplitContext
 Const
-const_alias_iterator
-const_arg_iterator
-const_arg_type_t
-_Const_Base_ptr
-const_block_iterator
-const_bundle_op_iterator
-ConstCaseIt
-const_global_iterator
-const_global_object_iterator
-const_global_value_iterator
 const_gvsummary_iterator
-const_handler_iterator
-const_handler_range
-const_ifunc_iterator
-__const_iterator
-const_iterator
-ConstIterator
-_Const_Link_type
-const_named_metadata_iterator
-const_op_iterator
-const_op_range
-const_pointer
-ConstPtrType
 Constraint
 ConstraintCategory
 ConstraintInfo
 ConstraintsSetStmt
 ConstrCheck
-const_reference
-const_reverse_iterator
-const_reverse_self_iterator
 ConstrType
-const_self_iterator
-const_use_iterator
-const_user_iterator
-const_void_pointer
 contain_aggs_of_level_context
 ControlData
 ControlFileData
@@ -575,7 +531,6 @@
 DefElem
 DefElemAction
 DefineStmt
-deleter_type
 DeleteStmt
 deparse_columns
 deparse_context
@@ -591,7 +546,6 @@
 destructor
 __dev_t
 dev_t
-DiagnosticHandlerTy
 DICompositeTypeArray
 DictISpell
 DictSimple
@@ -599,7 +553,6 @@
 DictSubState
 DictSyn
 DictThesaurus
-difference_type
 DIGlobalVariableExpressionArray
 DIImportedEntityArray
 DILocalVariableArray
@@ -688,7 +641,6 @@
 EDGE
 EDIPARTYNAME
 EditableObjectType
-element_iterator
 ElementsState
 eLogType
 emit_log_hook_type
@@ -716,7 +668,6 @@
 EquivalenceMember
 ErrorContextCallback
 ErrorData
-error_type
 EState
 EstimateDSMForeignScan_function
 eval_const_expressions_context
@@ -786,7 +737,6 @@
 ExtensionInfo
 ExtensionMemberId
 ExtensionVersionInfo
-ExtraData
 FakeRelCacheEntry
 FakeRelCacheEntryData
 false_type
@@ -812,7 +762,6 @@
 finalize_primnode_context
 find_expr_references_context
 FindSplitData
-first_type
 FixedParallelExecutorState
 FixedParallelState
 FixedParamState
@@ -986,7 +935,6 @@
 FunctionCallInfoData
 FunctionInlineState
 FunctionInlineStates
-FunctionListType
 FunctionParameter
 FunctionParameterMode
 FunctionScan
@@ -1097,11 +1045,7 @@
 gistxlogPage
 gistxlogPageSplit
 gistxlogPageUpdate
-global_iterator
-GlobalListType
-global_object_iterator
 GlobalTransaction
-global_value_iterator
 GlobalValueSummaryList
 GlobalValueSummaryMapTy
 globhook_t
@@ -1137,12 +1081,9 @@
 GucStackState
 GucStringAssignHook
 GucStringCheckHook
-GUID
 gvsummary_iterator
 GVSummaryMapTy
 gzFile
-handler_iterator
-handler_range
 Hash
 HASHACTION
 HashAllocFunc
@@ -1203,8 +1144,6 @@
 IdentLine
 IfStackElem
 ifState
-ifunc_iterator
-IFuncListType
 ilist
 imaxdiv_t
 import_error_callback_arg
@@ -1254,13 +1193,11 @@
 InferenceElem
 INFIX
 InfoItem
-_Inherited
 InhInfo
 InitializeDSMForeignScan_function
 InitializeWorkerForeignScan_function
 initproc
 InitSampleScan_function
-InlineAsmDiagHandlerTy
 InlineCodeBlock
 inline_error_callback_arg
 InlineSearchPath
@@ -1270,10 +1207,8 @@
 __ino_t
 ino_t
 in_port_t
-input_iterator
 inquiry
 InsertStmt
-InstListType
 instr_time
 Instrumentation
 int128
@@ -1311,10 +1246,8 @@
 intmax_t
 IntoClause
 intptr_t
-int_type
 InvalidationChunk
 InvalidationListHeader
-_Invoker_type
 IOFuncSelector
 _IO_lock_t
 IpcMemoryId
@@ -1328,14 +1261,11 @@
 ItemIdData
 itemIdSort
 itemIdSortData
-ItemParentClass
 ItemPointer
 ItemPointerData
 IterateDirectModify_function
 IterateForeignScan_function
 IterateJsonStringValuesState
-iterator
-iterator_type
 iternextfunc
 JEntry
 JHashState
@@ -1384,7 +1314,6 @@
 JsValue
 JunkFilter
 KeyArray
-key_compare
 keyEntryData
 Keymap
 KEYMAP_ENTRY
@@ -1392,7 +1321,6 @@
 KeySuffix
 __key_t
 key_t
-key_type
 KeyWord
 LabelProvider
 LargeObjectDesc
@@ -1417,7 +1345,6 @@
 LimitStateCond
 LINE
 line_t
-_Link_type
 List
 ListCell
 ListDictionary
@@ -1426,7 +1353,6 @@
 ListenStmt
 ListParsedLex
 list_qsort_comparator
-ListTy
 lldiv_t
 LLVMAttributeRef
 LLVMBasicBlockRef
@@ -1525,10 +1451,6 @@
 macaddr_sortsupport_state
 MAGIC
 __make_not_void
-_Manager_type
-MapEntryTy
-mapped_type
-MapT
 map_variable_attnos_context
 Material
 MaterialPath
@@ -1542,9 +1464,7 @@
 __mbstate_t
 mbstate_t
 mbverifier
-MD5_u32plus
 MdfdVec
-MDMapT
 MemoryContext
 MemoryContextCallback
 MemoryContextCallbackFunction
@@ -1577,7 +1497,6 @@
 ModifyTablePath
 ModifyTableState
 ModuleHash
-ModuleInfo
 ModulePathStringTableTy
 MorphOpaque
 movedb_failure_params
@@ -1591,8 +1510,6 @@
 MultiXactOffset
 MultiXactStateData
 MultiXactStatus
-mutable_op_range
-mutex_type
 MVDependencies
 MVDependency
 MVNDistinct
@@ -1605,8 +1522,6 @@
 NameData
 NamedLWLockTranche
 NamedLWLockTrancheRequest
-NamedMDListType
-named_metadata_iterator
 NamedTuplestoreScan
 NamedTuplestoreScanState
 NameHashEntry
@@ -1627,12 +1542,7 @@
 __nlink_t
 Node
 NODE
-_Node_allocator
-node_base_type
-node_pointer
-node_reference
 NodeTag
-node_type
 NonEmptyRange
 Notification
 NotifyStmt
@@ -1699,8 +1609,6 @@
 OpfamilyInfo
 OpFamilyMember
 OpFamilyOpFuncGroup
-op_iterator
-op_range
 OprCacheEntry
 OprCacheKey
 OprInfo
@@ -1709,7 +1617,6 @@
 OSAPerGroupState
 OSAPerQueryState
 OSInfo
-other
 OTHERNAME
 OutputContext
 OutputPluginCallbacks
@@ -1718,7 +1625,6 @@
 OverrideSearchPath
 OverrideStackEntry
 OverridingKind
-OwnerTy
 PADLIST
 PADNAME
 PADNAMELIST
@@ -1757,7 +1663,6 @@
 ParamExecData
 ParamExternData
 ParamFetchHook
-param_iterator
 ParamKind
 ParamListInfo
 ParamPathInfo
@@ -2085,10 +1990,7 @@
 PMSignalReason
 PMState
 Point
-pointer
 Pointer
-PointersTy
-__pointer_type
 PolicyInfo
 POLYGON
 PolyNumAggState
@@ -2281,7 +2183,6 @@
 ReadExtraTocPtrType
 ReadFunc
 ReassignOwnedStmt
-rebind_alloc
 RecheckForeignScan_function
 RecordCacheEntry
 RecordCompareData
@@ -2295,7 +2196,6 @@
 RecursiveUnionState
 reduce_outer_joins_state
 REENTR
-reference
 RefetchForeignRow_function
 RefreshMatViewStmt
 regex_arc_t
@@ -2392,7 +2292,6 @@
 ReplicationStateOnDisk
 RepOriginId
 reprfunc
-_Rep_type
 ReScanForeignScan_function
 re_scream_pos_data
 ReservoirState
@@ -2410,11 +2309,7 @@
 ResultPath
 ResultRelInfo
 ResultState
-ret_type
-RetType
 ReturnSetInfo
-reverse_iterator
-reverse_self_iterator
 RevmapContents
 rewrite_event
 RewriteMappingDataEntry
@@ -2498,9 +2393,6 @@
 segcountproc
 Selectivity
 SelectStmt
-Self
-_Self
-self_iterator
 SemiAntiJoinFactors
 sem_t
 SeqScan
@@ -2535,7 +2427,6 @@
 set_rel_pathlist_hook_type
 setter
 SetToDefault
-set_type
 SetupWorkerPtrType
 SHA256_CTX
 SHA512_CTX
@@ -2597,12 +2488,10 @@
 SimpleStats
 SimpleStringList
 SimpleStringListCell
-SimpleType
 SingleBoundSortItem
 SISeg
 Size
 size_t
-size_type
 SlabBlock
 SlabChunk
 SlabContext
@@ -2750,12 +2639,10 @@
 stemmer_module
 stmtCacheEntry
 StopList
-storage_type
 storeRes_func
 StrategyNumber
 StreamCtl
 stream_stop_callback
-string
 StringInfo
 StringInfoData
 StripnullState
@@ -2774,20 +2661,12 @@
 substitute_actual_srf_parameters_context
 substitute_multiple_relids_context
 SubTransactionId
-subtype_iterator
-subtype_reverse_iterator
 SubXactCallback
 SubXactCallbackItem
 SubXactEvent
-succ_const_iterator
-succ_const_range
-succ_iterator
-succ_range
 __suseconds_t
 SVCOMPARE_t
 svtype
-SwitchInstT
-SwitchInstType
 symbol
 SyncRepConfigData
 SyscacheCallbackFunction
@@ -2976,10 +2855,6 @@
 Tcl_WideInt
 Tcl_WideUInt
 Tcl_ZlibStream
-td_const_iterator
-td_const_range
-td_iterator
-td_range
 TempDICompileUnit
 TempDICompositeType
 TempDIDerivedType
@@ -3038,7 +2913,6 @@
 TocEntry
 TokenAuxData
 TokenizedLine
-_Tp_alloc_type
 TParser
 TParserCharTest
 TParserPosition
@@ -3134,9 +3008,7 @@
 TxidEpoch
 TxidSnapshot
 TYPCATEGORY
-type
 Type
-_Type
 TypeCacheEntry
 TypeCacheEnumData
 TypeCast
@@ -3198,9 +3070,7 @@
 UpdateStmt
 UpperRelationKind
 UpperUniquePath
-use_iterator
 UserAuth
-user_iterator
 UserMapping
 UserOpts
 utf_local_conversion_func
@@ -3213,17 +3083,11 @@
 VacuumStmt
 validate_string_relopt
 va_list
-ValTy
 Value
-ValueAdder
-value_compare
-ValueMapCVH
-ValueMapT
 ValueName
 ValuesScan
 ValuesScanState
 ValueToValueMapTy
-value_type
 varatt_expanded
 varattrib_1b
 varattrib_1b_e
@@ -3243,7 +3107,6 @@
 VarParamState
 VarString
 VarStringSortSupport
-vector_type
 VersionedQuery
 ViewCheckOption
 ViewOptions
@@ -3318,8 +3181,6 @@
 WordEntryPos
 WordEntryPosVector
 WordEntryPosVector1
-_WordT
-WordType
 WorkerInfo
 WorkerInfoData
 WorkerInstrumentation
@@ -3328,7 +3189,6 @@
 Working_State
 WorkTableScan
 WorkTableScanState
-wrap
 WritebackContext
 writebufferproc
 WriteBufPtrType
@@ -3463,7 +3323,6 @@
 XPVIO
 XPVIV
 XPVNV
-YieldCallbackTy
 YY_BUFFER_STATE
 YY_CHAR
 YYLTYPE
In reply to: Andres Freund (#5)
Re: Undesirable entries in typedefs list

On Sat, Mar 24, 2018 at 1:36 PM, Andres Freund <andres@anarazel.de> wrote:

I've attached the difference between a objdump typedefs list roughly
equivalent to what the buildfarm uses. There's no difference when not
using llvm.

I'm a bit uncomfortable relying --dwarf-depth=3, with 3 being determined
purely experimentally though.

A quick look at the DWARF4 standard [1]http://dwarfstd.org/doc/DWARF4.pdf -- Peter Geoghegan suggests that this refers to
lexical depth. So, I agree that that doesn't seem like a great idea.

[1]: http://dwarfstd.org/doc/DWARF4.pdf -- Peter Geoghegan
--
Peter Geoghegan

#7Andres Freund
andres@anarazel.de
In reply to: Peter Geoghegan (#6)
Re: Undesirable entries in typedefs list

On 2018-03-24 14:51:32 -0700, Peter Geoghegan wrote:

On Sat, Mar 24, 2018 at 1:36 PM, Andres Freund <andres@anarazel.de> wrote:

I've attached the difference between a objdump typedefs list roughly
equivalent to what the buildfarm uses. There's no difference when not
using llvm.

I'm a bit uncomfortable relying --dwarf-depth=3, with 3 being determined
purely experimentally though.

A quick look at the DWARF4 standard [1] suggests that this refers to
lexical depth. So, I agree that that doesn't seem like a great idea.

[1] http://dwarfstd.org/doc/DWARF4.pdf

Are you referring to Section 3.4? That's something different afaict. Or
which bit are you thinking of?

See e.g. https://sourceware.org/bugzilla/show_bug.cgi?id=22250

"
Another addition "depth" starts counting from zero. Zero usually is the
first DIE (compilation_unit or partial_unit). Specifying --dwarf-depth=0
will only print the Compilation Unit headers, followed by a blank line
and no ... markers. Unless --dwarf-depth is given with a non-zero
argument, then only a blank line is printed.
"

My understanding is that it controls through how many levels of nesting
in type definitions objdump recurses through. The type of typedefs we
really care about are going to be at the global level. Which should make
them available at dwarf-depth=2. Then we have a few typedefs which are
solely done inside functions, which is why we need dwarf-depth=3.

Greetings,

Andres Freund

#8Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Tom Lane (#1)
Re: Undesirable entries in typedefs list

On Sun, Mar 25, 2018 at 3:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I noticed that doing pgindent with the current typedefs list available
from the buildfarm caused a lot of havoc in what had been stable code.
Looking into the reasons, it seems that:

(1) "bool" is no longer listed as a typedef name (probably because
stdbool.h makes it a macro instead);

(2) "abs", "boolean", "iterator", "other", "pointer", "reference",
"string", and "type" all now are listed as typedef names.

It's probably okay to treat "boolean" as a typedef, but all those others
are complete disasters. Anyone know where they're coming from?

As for "bool", we could probably deal with that most reliably by
having pgindent add it as a special case. Maybe we could get it
back in there by having some trailing-edge buildfarm member
contribute typedefs, but that seems like a solution with a rather
limited half-life.

pgindent already has a list of blacklisted typedefs (see lines 121 to 123)

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#9Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Andrew Dunstan (#8)
1 attachment(s)
Re: Undesirable entries in typedefs list

On Sun, Mar 25, 2018 at 11:32 AM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

On Sun, Mar 25, 2018 at 3:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I noticed that doing pgindent with the current typedefs list available
from the buildfarm caused a lot of havoc in what had been stable code.
Looking into the reasons, it seems that:

(1) "bool" is no longer listed as a typedef name (probably because
stdbool.h makes it a macro instead);

(2) "abs", "boolean", "iterator", "other", "pointer", "reference",
"string", and "type" all now are listed as typedef names.

It's probably okay to treat "boolean" as a typedef, but all those others
are complete disasters. Anyone know where they're coming from?

As for "bool", we could probably deal with that most reliably by
having pgindent add it as a special case. Maybe we could get it
back in there by having some trailing-edge buildfarm member
contribute typedefs, but that seems like a solution with a rather
limited half-life.

pgindent already has a list of blacklisted typedefs (see lines 121 to 123)

Here's a patch to pgindent which I think will do what you want fairly
simply, assuming you have identified all the offending tokens.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

pgindent-whiteblacklist.patchapplication/octet-stream; name=pgindent-whiteblacklist.patchDownload
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index a32aaa6..99fd99d 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -55,6 +55,13 @@ my @files;
 my $filtered_typedefs_fh;
 
 
+# make sure these entries are there
+my @whitelist = ("bool\n");
+# make sure these aren't there
+my %blacklist = map { "$_\n" => 1}
+  qw ( FD_SET date interval timestamp ANY
+       abs boolean iterator other pointer reference string type);
+
 sub check_indent
 {
 	system("$indent -? < $devnull > $devnull 2>&1");
@@ -118,9 +125,12 @@ sub load_typedefs
 		}
 	}
 
-	# remove certain entries
+	# add whitelisted entries
+	push(@typedefs, @whitelist);
+
+	# remove blacklisted entries
 	@typedefs =
-	  grep { !m/^(FD_SET|date|interval|timestamp|ANY)\n?$/ } @typedefs;
+	  grep { ! $blacklist{$_} } @typedefs;
 
 	# write filtered typedefs
 	my $filter_typedefs_fh = new File::Temp(TEMPLATE => "pgtypedefXXXXX");
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#9)
Re: Undesirable entries in typedefs list

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

pgindent already has a list of blacklisted typedefs (see lines 121 to 123)

Oh, so it does.

Here's a patch to pgindent which I think will do what you want fairly
simply, assuming you have identified all the offending tokens.

Hm, this does not work for me (with perl 5.10.1), I get syntax errors like

Not enough arguments for map at src/tools/pgindent/pgindent line 64, near "abs allocfunc iterator other pointer printfunc reference string type )"
(Might be a runaway multi-line () string starting on line 63)
syntax error at src/tools/pgindent/pgindent line 64, near "abs allocfunc iterator other pointer printfunc reference string type )"
Execution of src/tools/pgindent/pgindent aborted due to compilation errors.

After reading the perlfunc man page I changed

-my %blacklist = map { "$_\n" => 1 }
+my %blacklist = map { +"$_\n" => 1 }

and then it seems to work, but man this is an ugly language :-(

regards, tom lane

#11Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Tom Lane (#10)
Re: Undesirable entries in typedefs list

On Wed, Mar 28, 2018 at 8:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

pgindent already has a list of blacklisted typedefs (see lines 121 to 123)

Oh, so it does.

Here's a patch to pgindent which I think will do what you want fairly
simply, assuming you have identified all the offending tokens.

Hm, this does not work for me (with perl 5.10.1), I get syntax errors like

Not enough arguments for map at src/tools/pgindent/pgindent line 64, near "abs allocfunc iterator other pointer printfunc reference string type )"
(Might be a runaway multi-line () string starting on line 63)
syntax error at src/tools/pgindent/pgindent line 64, near "abs allocfunc iterator other pointer printfunc reference string type )"
Execution of src/tools/pgindent/pgindent aborted due to compilation errors.

After reading the perlfunc man page I changed

-my %blacklist = map { "$_\n" => 1 }
+my %blacklist = map { +"$_\n" => 1 }

and then it seems to work, but man this is an ugly language :-(

Yes, there are lots of quirks that can make it fun to deal with.

Thanks for fixing and committing.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services