Обсуждение: Small changes to facilitate Win32 port
Hi there. I'm yet another developer working full-time on a native windows port. I'm also working closely with Jan Wieck (next office). I know there is a reluctance to modify the code base to support native win32, and I realize that no decision has yet been made. However, ... A few of the identifier names used in postgres collide with WIN32 or MFC names.To keep my working copy of the code as closeto the released source as possible, I do have some superficial changes that I would like to put in the code base early: 1. Rename to avoid structures/functions with same name:a. PROC => PGPROCb. GetUserName() => GetUserNameFromId()c. GetCurrentTime()=> GetCurrentDateTime() 2. Add _P to the following lex/yacc tokens to avoid collisionsCONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT 3. Rename two local macrosa. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.cb. IGNORE => IGNORE_TOK in include/utils/datetime.h& backend/utils/adt/datetime.c Thanks, Katie Ward kward6@yahoo.com __________________________________________________ Do You Yahoo!? Yahoo! - Official partner of 2002 FIFA World Cup http://fifaworldcup.yahoo.com
It's more likely that your changes will go through if you just submit a patch! cvs diff -c Chris ----- Original Message ----- From: "Katherine Ward" <kward6@yahoo.com> To: <pgsql-hackers@postgresql.org> Sent: Thursday, May 30, 2002 2:33 PM Subject: [HACKERS] Small changes to facilitate Win32 port > Hi there. I'm yet another developer working full-time on a native windows > port. I'm also working closely with Jan Wieck (next office). I know there is > a reluctance to modify the code base to support native win32, and I realize > that no decision has yet been made. However, ... > > A few of the identifier names used in postgres collide with WIN32 or MFC names. > To keep my working copy of the code as close to the released source as > possible, I do have some superficial changes that I would like to put in the > code base early: > > 1. Rename to avoid structures/functions with same name: > a. PROC => PGPROC > b. GetUserName() => GetUserNameFromId() > c. GetCurrentTime() => GetCurrentDateTime() > > 2. Add _P to the following lex/yacc tokens to avoid collisions > CONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT > > 3. Rename two local macros > a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c > b. IGNORE => IGNORE_TOK in include/utils/datetime.h & > backend/utils/adt/datetime.c > > Thanks, > Katie Ward > kward6@yahoo.com > > > __________________________________________________ > Do You Yahoo!? > Yahoo! - Official partner of 2002 FIFA World Cup > http://fifaworldcup.yahoo.com > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html >
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> It's more likely that your changes will go through if you just submit a
> patch!
I think the question was more directed at "do we like these names?",
which should certainly be asked before going to the trouble of making a
patch.
>> 2. Add _P to the following lex/yacc tokens to avoid collisions
>> CONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT
I'm tempted to suggest that we should stick _P on *all* the lexer token
symbols, rather than having an inconsistent set of names where some of
them have _P and some do not. Or perhaps _T (for token) would be a more
sensible convention; I'm not sure why _P was used in the first place.
>> 3. Rename two local macros
>> a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c
>> b. IGNORE => IGNORE_TOK in include/utils/datetime.h &
>> backend/utils/adt/datetime.c
It's fairly amazing that IGNORE is the only one of the datetime.h field
names that's bitten anyone (so far). Macros named TZ, YEAR, MONTH, DAY,
HOUR, MINUTE, SECOND, UNITS all look like trouble waiting to happen
(and UNKNOWN_FIELD looks like someone already had to beat a retreat from
calling it UNKNOWN ;-)). I'm inclined to suggest that these names
should be uniformly changed to DTF_FOO (DTF for "datetime field").
The macro names appearing before the field name list look like trouble
as well --- anyone have an interest in changing them? Thomas, this is
pretty much your turf; what do you think?
regards, tom lane
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: Thursday, May 30, 2002 3:25 PM > To: Christopher Kings-Lynne > Cc: Katherine Ward; Thomas Lockhart; pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] Small changes to facilitate Win32 port > > > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > It's more likely that your changes will go through if you > just submit a > > patch! > > I think the question was more directed at "do we like these names?", > which should certainly be asked before going to the trouble > of making a > patch. > > >> 2. Add _P to the following lex/yacc tokens to avoid collisions > >> CONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT > > I'm tempted to suggest that we should stick _P on *all* the > lexer token > symbols, rather than having an inconsistent set of names where some of > them have _P and some do not. Or perhaps _T (for token) > would be a more > sensible convention; I'm not sure why _P was used in the first place. Before you do that, realize that you are violating the implementation's namespace, and your code is therefore not legal ANSI/ISO C. From ISO/IEC 9899:1999 (E) ©ISO/IEC: 7.1.3 Reserved identifiers 1 Each header declares or defines all identifiers listed in its associated subclause, and optionally declares or defines identifiers listed in its associated future library directions subclause and identifiers which are always reserved either for any use or for use as file scope identifiers. - All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. - All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces. - Each macro name in any of the following subclauses (including the future library directions) is reserved for use as specified if any of its associated headers is included; unless explicitly stated otherwise (see 7.1.4). - All identifiers with external linkage in any of the following subclauses (including the future library directions) are always reserved for use as identifiers with external linkage.154) - Each identifier with file scope listed in any of the following subclauses (including the future library directions) is reserved for use as a macro name and as an identifier with file scope in the same name space if any of its associated headers is included. > >> 3. Rename two local macros > >> a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c > >> b. IGNORE => IGNORE_TOK in include/utils/datetime.h & > >> backend/utils/adt/datetime.c > > It's fairly amazing that IGNORE is the only one of the > datetime.h field > names that's bitten anyone (so far). Macros named TZ, YEAR, > MONTH, DAY, > HOUR, MINUTE, SECOND, UNITS all look like trouble waiting to happen > (and UNKNOWN_FIELD looks like someone already had to beat a > retreat from > calling it UNKNOWN ;-)). I'm inclined to suggest that these names > should be uniformly changed to DTF_FOO (DTF for "datetime field"). > The macro names appearing before the field name list look like trouble > as well --- anyone have an interest in changing them? Thomas, this is > pretty much your turf; what do you think? > > regards, tom lane > > ---------------------------(end of > broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly >
"Dann Corbit" <DCorbit@connx.com> writes:
>> I'm tempted to suggest that we should stick _P on *all* the
>> lexer token
>> symbols, rather than having an inconsistent set of names where some of
>> them have _P and some do not. Or perhaps _T (for token)
>> would be a more
>> sensible convention; I'm not sure why _P was used in the first place.
> Before you do that, realize that you are violating the implementation's
> namespace, and your code is therefore not legal ANSI/ISO C.
What?
The _P or _T is a suffix, not a prefix...
regards, tom lane
Christopher Kings-Lynne wrote: > It's more likely that your changes will go through if you just submit a > patch! I suggested to discuss it first, since it's IMHO more likely that the changes go through if they are commonly accepted in the first place. Jan > cvs diff -c > > Chris > > ----- Original Message ----- > From: "Katherine Ward" <kward6@yahoo.com> > To: <pgsql-hackers@postgresql.org> > Sent: Thursday, May 30, 2002 2:33 PM > Subject: [HACKERS] Small changes to facilitate Win32 port > > > > Hi there. I'm yet another developer working full-time on a native windows > > port. I'm also working closely with Jan Wieck (next office). I know > there is > > a reluctance to modify the code base to support native win32, and I > realize > > that no decision has yet been made. However, ... > > > > A few of the identifier names used in postgres collide with WIN32 or MFC > names. > > To keep my working copy of the code as close to the released source as > > possible, I do have some superficial changes that I would like to put in > the > > code base early: > > > > 1. Rename to avoid structures/functions with same name: > > a. PROC => PGPROC > > b. GetUserName() => GetUserNameFromId() > > c. GetCurrentTime() => GetCurrentDateTime() > > > > 2. Add _P to the following lex/yacc tokens to avoid collisions > > CONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT > > > > 3. Rename two local macros > > a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c > > b. IGNORE => IGNORE_TOK in include/utils/datetime.h & > > backend/utils/adt/datetime.c > > > > Thanks, > > Katie Ward > > kward6@yahoo.com > > > > > > __________________________________________________ > > Do You Yahoo!? > > Yahoo! - Official partner of 2002 FIFA World Cup > > http://fifaworldcup.yahoo.com > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 5: Have you checked our extensive FAQ? > > > > http://www.postgresql.org/users-lounge/docs/faq.html > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
> >> 2. Add _P to the following lex/yacc tokens to avoid collisions
> >> CONST, CHAR, DELETE, FLOAT, GROUP, IN, OUT
> I'm tempted to suggest that we should stick _P on *all* the lexer token
> symbols, rather than having an inconsistent set of names where some of
> them have _P and some do not. Or perhaps _T (for token) would be a more
> sensible convention; I'm not sure why _P was used in the first place.
"P" for "Parser". The symbols are used past the lexer, but are isolated
to other places in the parser, and are (or should be) stripped out
beyond there.
> >> 3. Rename two local macros
> >> a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c
> >> b. IGNORE => IGNORE_TOK in include/utils/datetime.h &
> >> backend/utils/adt/datetime.c
> It's fairly amazing that IGNORE is the only one of the datetime.h field
> names that's bitten anyone (so far). Macros named TZ, YEAR, MONTH, DAY,
> HOUR, MINUTE, SECOND, UNITS all look like trouble waiting to happen
> (and UNKNOWN_FIELD looks like someone already had to beat a retreat from
> calling it UNKNOWN ;-)). I'm inclined to suggest that these names
> should be uniformly changed to DTF_FOO (DTF for "datetime field").
> The macro names appearing before the field name list look like trouble
> as well --- anyone have an interest in changing them? Thomas, this is
> pretty much your turf; what do you think?
If the lexer/parser should have postfix qualifiers, let's use postfix
for other naming conventions too (or switch everything to prefix, but be
consistant in the conventions).
No problem with qualifying the names, though you have likely overstated
the case for it; "fairly amazing" after 6 years of use on over a dozen
platforms probably qualifies as a good test of reality and we aren't
quite to the point of having to invoke miracles and magic to explain why
it works ;)
In any case, we would certainly be open to accepting patches for the
limited number of cases Katherine has identified, and would welcome
patches which are more comprehensive if they were available.
- Thomas
Thomas Lockhart <lockhart@fourpalms.org> writes:
>> I'm tempted to suggest that we should stick _P on *all* the lexer token
>> symbols, rather than having an inconsistent set of names where some of
>> them have _P and some do not. Or perhaps _T (for token) would be a more
>> sensible convention; I'm not sure why _P was used in the first place.
> "P" for "Parser".
Oh, okay. I'm not intent on changing it, just was wondering what the
motivation was. What do you think of changing all the token symbols to
be FOO_P? (Or P_FOO, per your comment, but I'd just as soon leave alone
the ones that already have a suffix.)
> The symbols are used past the lexer, but are isolated
> to other places in the parser, and are (or should be) stripped out
> beyond there.
Right at the moment we have half a dozen cases where they leak past the
parser, e.g. TransactionStmt. I've been intending to clean that up.
I concur that we don't want anything past parse analysis to depend on
token values, since they change anytime the keyword set changes.
> If the lexer/parser should have postfix qualifiers, let's use postfix
> for other naming conventions too (or switch everything to prefix, but be
> consistant in the conventions).
I'd settle for local consistency: if we need prefixes/suffixes on some
of the datetime field names, let's make all of them have one. But I
don't feel compelled to cause a flag day over the whole source tree ;-).
At least not all at once.
regards, tom lane
> > "P" for "Parser".
> Oh, okay. I'm not intent on changing it, just was wondering what the
> motivation was. What do you think of changing all the token symbols to
> be FOO_P? (Or P_FOO, per your comment, but I'd just as soon leave alone
> the ones that already have a suffix.)
No problem here. I have a mild preference for suffix notation, and the
"P_FOO" was *your* idea (or at least the "DTF_FOO" one was). Anyway,
suffixes are my preference, but not I'm not enthused enough about it to
argue hard one way or the other.
> > The symbols are used past the lexer, but are isolated
> > to other places in the parser, and are (or should be) stripped out
> > beyond there.
> Right at the moment we have half a dozen cases where they leak past the
> parser, e.g. TransactionStmt. I've been intending to clean that up.
> I concur that we don't want anything past parse analysis to depend on
> token values, since they change anytime the keyword set changes.
Right.
> > If the lexer/parser should have postfix qualifiers, let's use postfix
> > for other naming conventions too (or switch everything to prefix, but be
> > consistant in the conventions).
> I'd settle for local consistency: if we need prefixes/suffixes on some
> of the datetime field names, let's make all of them have one. But I
> don't feel compelled to cause a flag day over the whole source tree ;-).
> At least not all at once.
Well, this doesn't need to be an issue or argument. We have little or no
precedent for prefix notation that I can recall, we have postfix
notation in the parser, and we don't have a "namespace convention" for
other areas afaik. So if we make changes, let's do it with a convention,
and at least extend one local convention to another local area.
Question to all: Any objection to postfix? If so, why?
And to answer Katherine's original questions:
1) OK. (function renaming)
2) OK. ("_P" suffix on a few more parser tokens)
3) MEM_FREE_IT - OK, but is it a macro specific to something in dynhash?
If so, how about using something more specific than "it"?
IGNORE_TOK - How about "IGNORE_DTF" or "IGNORE_D"? Let's make it a bit
specific to date/time stuff.
- Thomas
> Christopher Kings-Lynne wrote: > > It's more likely that your changes will go through if you just submit a > > patch! > > I suggested to discuss it first, since it's IMHO more likely > that the changes go through if they are commonly accepted in > the first place. Yep - sorry, didn't pick up on that... Chris
Thomas Lockhart <lockhart@fourpalms.org> writes:
> Question to all: Any objection to postfix? If so, why?
Well, I suggested DTF_FOO by analogy to the DTK_FOO name set that appears
elsewhere in that same header. If you want to rename those to FOO_DTK
in parallel, I have no objection.
> IGNORE_TOK - How about "IGNORE_DTF" or "IGNORE_D"? Let's make it a bit
> specific to date/time stuff.
Agreed. That thought was what motivated me to gripe in the first place.
regards, tom lane
Katherine Ward writes: > A few of the identifier names used in postgres collide with WIN32 or MFC names. Does Windows and/or MFC make any kind of statement about what kinds of identifiers it reserves for its own use? > b. GetUserName() => GetUserNameFromId() > c. GetCurrentTime() => GetCurrentDateTime() > a. MEM_FREE => MEM_FREE_IT in backend/utils/hash/dynahash.c > b. IGNORE => IGNORE_TOK in include/utils/datetime.h & backend/utils/adt/datetime.c It might be better to add PG prefixes consistently if there are clashes. -- Peter Eisentraut peter_e@gmx.net