Thread: pgindent vs try/catch
I'm fairly displeased with what pgindent has done to single-line PG_TRY
constructs, as in this example from pl_exec.c:
*************** exec_stmt_block(PLpgSQL_execstate * esta
*** 911,922 **** SPI_result_code_string(xrc)); PG_TRY();
! {
! rc = exec_stmts(estate, block->body);
! } PG_CATCH(); {
! ErrorData *edata; PLpgSQL_exceptions *exceptions;
int j;
--- 912,921 ---- SPI_result_code_string(xrc)); PG_TRY();
! rc = exec_stmts(estate, block->body); PG_CATCH(); {
! ErrorData *edata; PLpgSQL_exceptions *exceptions;
int j;
*************** exec_stmt_block(PLpgSQL_execstate * esta
On the whole I'd prefer that pgindent not suppress "unnecessary"
brace pairs at all.
regards, tom lane
Tom Lane said:
> I'm fairly displeased with what pgindent has done to single-line PG_TRY
> constructs, as in this example from pl_exec.c:
>
> *************** exec_stmt_block(PLpgSQL_execstate * esta
> *** 911,922 ****
> SPI_result_code_string(xrc));
>
> PG_TRY();
> ! {
> ! rc = exec_stmts(estate, block->body);
> ! }
> PG_CATCH();
> {
> ! ErrorData *edata;
> PLpgSQL_exceptions *exceptions;
> int j;
>
> --- 912,921 ----
> SPI_result_code_string(xrc));
>
> PG_TRY();
> ! rc = exec_stmts(estate, block->body);
> PG_CATCH();
> {
> ! ErrorData *edata;
> PLpgSQL_exceptions *exceptions;
> int j;
>
> *************** exec_stmt_block(PLpgSQL_execstate * esta
>
> On the whole I'd prefer that pgindent not suppress "unnecessary"
> brace pairs at all.
I had that argument a while ago with Bruce and lost :-) . It does horrible
things to if/else constructs too. The workaround is to put a comment in the
block. On the whole I agree with you, though. If I put braces in my program
it's for a reason, and the indenter shouldn't think it knows better than me.
cheers
andrew
Andrew Dunstan wrote:
> Tom Lane said:
>
>>I'm fairly displeased with what pgindent has done to single-line PG_TRY
>>constructs, as in this example from pl_exec.c:
>>
>>*************** exec_stmt_block(PLpgSQL_execstate * esta
>>*** 911,922 ****
>> SPI_result_code_string(xrc));
>>
>> PG_TRY();
>>! {
>>! rc = exec_stmts(estate, block->body);
>>! }
>> PG_CATCH();
>> {
>>! ErrorData *edata;
>> PLpgSQL_exceptions *exceptions;
>> int j;
>>
>>--- 912,921 ----
>> SPI_result_code_string(xrc));
>>
>> PG_TRY();
>>! rc = exec_stmts(estate, block->body);
>> PG_CATCH();
>> {
>>! ErrorData *edata;
>> PLpgSQL_exceptions *exceptions;
>> int j;
>>
>>*************** exec_stmt_block(PLpgSQL_execstate * esta
>>
>>On the whole I'd prefer that pgindent not suppress "unnecessary"
>>brace pairs at all.
>
>
>
> I had that argument a while ago with Bruce and lost :-) . It does horrible
> things to if/else constructs too. The workaround is to put a comment in the
> block. On the whole I agree with you, though. If I put braces in my program
> it's for a reason, and the indenter shouldn't think it knows better than me.
Well I'm not exactly a C coder, I'm a C++ one and it's quite common use the
extra scope in order to reduce the automatic variable life, I don't know how
much the extra scope are used in the C world, however remove an "extra scope"
like that is not only "horrible", is *wrong* and can be cause of future pain:
foo ( int a )
{ ... { int a; } // use the formal parameter
}
if the extra scope is removed the local variable "shadow" the formal
parameter. Some compilers do not warning you, IIRC the Digital had this funny omission,
( however you can miss the warning ) and hours of debugging are behind the corner.
I hope that Bruce change his mind...
Regards
Gaetano Mendola
Andrew Dunstan wrote: > I had that argument a while ago with Bruce and lost :-) . It does horrible > things to if/else constructs too. The workaround is to put a comment in the > block. On the whole I agree with you, though. If I put braces in my program > it's for a reason, and the indenter shouldn't think it knows better than me. I have removed the code from pgindent. Now how do we clean up the try/catch code that got messed up? The original purpose of that code was to clean up the code we inherited from Berkeley that had lots of excessive brace blocks and the pgindent code just remained. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I have removed the code from pgindent. Now how do we clean up the
> try/catch code that got messed up?
I've hand-restored the cases that are in the files I'm currently
editing. I'll look for more later.
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Bruce Momjian wrote:
| Gaetano Mendola wrote:
|
|>>I had that argument a while ago with Bruce and lost :-) . It does horrible
|>>things to if/else constructs too. The workaround is to put a comment in the
|>>block. On the whole I agree with you, though. If I put braces in my program
|>>it's for a reason, and the indenter shouldn't think it knows better than me.
|>
|>Well I'm not exactly a C coder, I'm a C++ one and it's quite common use the
|>extra scope in order to reduce the automatic variable life, I don't know how
|>much the extra scope are used in the C world, however remove an "extra scope"
|>like that is not only "horrible", is *wrong* and can be cause of future pain:
|>
|>
|>foo ( int a )
|>{
|> ...
|> {
|> int a;
|> }
|> // use the formal parameter
|>}
|>
|>if the extra scope is removed the local variable "shadow" the formal
|>parameter. Some compilers do not warning you, IIRC the Digital had this funny omission,
|>( however you can miss the warning ) and hours of debugging are behind the corner.
|>I hope that Bruce change his mind...
|
|
| I am a little confused by the above. It only removes braces that have
| one command in them.
This was not clear to me.
| What does "use the formal parameter" mean?
Emm, the variable argument I mean, is not "formal parameter" the right name ?
| FYI, C doesn't allow variables to be declared in for() like that, but I am
| still curious how C++ handles such cases.
the { ... } in c++ represent an extra scope this means that at the end of the
scope all variable declared inside are destroyed. A common way to use it is to
surround critical sections:
void foo( int a )
{
~ Mutex m;
~ ...
~ {
~ Lock myLock(m); // The lock is acquired inside the constructor
~ int a = 5;
~ //critical section code follow
~ ...
~ } // The unlock is performed in the destructor
};
at the end of the scope the destructor for the variable myLock is called.
In this way the lock is released ( with the appropriate code in the destructor)
without remember to unlock it and most important the lock is released also if
an exception is thrown; inside that extra scope the variable
"a" hide the function parameter, this code is perfectly legal in C++.
In the case of the for if you declare for ( A a = ... ) { } the lifespan
for the object a is the "for" body, and ansi C++ allow the reuse so you can have:
for ( A a = ... ) { }
for ( A a = ... ) { }
| I have no problem in removing this pgindent behavior.
I don't know all the implication in removing it or leave it, however I agree to
leave the extra scope in place.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFBRNi37UpzwH2SGd4RAgrlAKDo+xL+Vo8+2vyfpnhxmmPyEJOhXwCgpc4h
8cdAPGv/fqWE3UY2bRe4rlI=
=Wbra
-----END PGP SIGNATURE-----
OK, it never removed braces from things like:
int x;
{ int x; x=5;}
but anyway I think we all agree it was uglifying the code more than it
was clarifying.
---------------------------------------------------------------------------
Gaetano Mendola wrote:
[ PGP not available, raw data follows ]
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Bruce Momjian wrote:
>
> | Gaetano Mendola wrote:
> |
> |>>I had that argument a while ago with Bruce and lost :-) . It does horrible
> |>>things to if/else constructs too. The workaround is to put a comment in the
> |>>block. On the whole I agree with you, though. If I put braces in my program
> |>>it's for a reason, and the indenter shouldn't think it knows better than me.
> |>
> |>Well I'm not exactly a C coder, I'm a C++ one and it's quite common use the
> |>extra scope in order to reduce the automatic variable life, I don't know how
> |>much the extra scope are used in the C world, however remove an "extra scope"
> |>like that is not only "horrible", is *wrong* and can be cause of future pain:
> |>
> |>
> |>foo ( int a )
> |>{
> |> ...
> |> {
> |> int a;
> |> }
> |> // use the formal parameter
> |>}
> |>
> |>if the extra scope is removed the local variable "shadow" the formal
> |>parameter. Some compilers do not warning you, IIRC the Digital had this funny omission,
> |>( however you can miss the warning ) and hours of debugging are behind the corner.
> |>I hope that Bruce change his mind...
> |
> |
> | I am a little confused by the above. It only removes braces that have
> | one command in them.
>
> This was not clear to me.
>
>
> | What does "use the formal parameter" mean?
>
> Emm, the variable argument I mean, is not "formal parameter" the right name ?
>
>
> | FYI, C doesn't allow variables to be declared in for() like that, but I am
> | still curious how C++ handles such cases.
>
> the { ... } in c++ represent an extra scope this means that at the end of the
> scope all variable declared inside are destroyed. A common way to use it is to
> surround critical sections:
>
> void foo( int a )
> {
> ~ Mutex m;
> ~ ...
> ~ {
> ~ Lock myLock(m); // The lock is acquired inside the constructor
> ~ int a = 5;
> ~ //critical section code follow
> ~ ...
> ~ } // The unlock is performed in the destructor
> };
>
> at the end of the scope the destructor for the variable myLock is called.
> In this way the lock is released ( with the appropriate code in the destructor)
> without remember to unlock it and most important the lock is released also if
> an exception is thrown; inside that extra scope the variable
> "a" hide the function parameter, this code is perfectly legal in C++.
>
> In the case of the for if you declare for ( A a = ... ) { } the lifespan
> for the object a is the "for" body, and ansi C++ allow the reuse so you can have:
>
> for ( A a = ... ) { }
> for ( A a = ... ) { }
>
>
> | I have no problem in removing this pgindent behavior.
>
> I don't know all the implication in removing it or leave it, however I agree to
> leave the extra scope in place.
>
>
>
>
>
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.4 (MingW32)
> Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
> iD8DBQFBRNi37UpzwH2SGd4RAgrlAKDo+xL+Vo8+2vyfpnhxmmPyEJOhXwCgpc4h
> 8cdAPGv/fqWE3UY2bRe4rlI=
> =Wbra
> -----END PGP SIGNATURE-----
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>
[ End of raw data]
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073