Re: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it - Mailing list pgsql-hackers
| From | Jelte Fennema |
|---|---|
| Subject | Re: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it |
| Date | |
| Msg-id | AM5PR83MB01781F7E9B445243EACA57CFF7C19@AM5PR83MB0178.EURPRD83.prod.outlook.com Whole thread Raw |
| In response to | Re: Allow declaration after statement and reformat code to use it (Bruce Momjian <bruce@momjian.us>) |
| Responses |
Re: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it
(Jelte Fennema <Jelte.Fennema@microsoft.com>)
Re: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it (Ranier Vilela <ranier.vf@gmail.com>) |
| List | pgsql-hackers |
However, even if such an idea were to get the green light, I think I wouldtake the obligatory regex jokes seriously, and instead use something likesrcML [0] and do the analysis and modification on proper parse trees.
@Chapman I think that's a reasonable ask. This is the first time I heard of srcML. It indeed looks like it would be a much better tool than a regex to do this refactor. The refactor logic would still be roughly the same though because the regex essentially extracts and modifies a parse tree too. Just one that only has the bare minimum requirements for this specific refactor. I would like to state, that we've used this regex to refactor both Citus and pg_auto_failover, and no bugs turned up because of it so far. This is probably, because a regex is such a horrible way of writing a parser in the first place. So, any bugs in the regex will very likely create code that does not compile at all, instead of code that can be compiled but has some obscure bug .
I think effectively it would need to be run on all supported versions,@Bruce @Tom @Chapman I agree that doing this refactor only on the current master branch would cause way more pain due to backpatching, than the amount of happiness it provides by having easier to reason about code. I like the idea of avoiding this backpatching problem by running it . Luckily, running the automatic refactor on all branches should be pretty easy. However, it seems like PG11 and below only require a C89 compatible compiler. Am I correct in assuming increasing that requirement to C99 is not really an option? If so, I think it might be best if I revive this thread in ~27 months when PG11 is not supported anymore.which means the risk of introducing errors would have to be very low.
The rule against declaration-after-statement was kept
after significant discussion, which you can find in the archives if youlook.
@Tom I guess my search skills in the archives are not very good, because I cannot find any big discussion on about declaration-after-statement. The only discussion I can find is this: https://www.postgresql.org/message-id/877efaol77.fsf@news-spur.riddles.org.uk If you're able to find that thread, could you share it?
C needs readability, not fewer lines.Aside from horrible code, it doesn't improve 0.1% on anything.
@Ranier I tried to explain in my initial email that it not only reduces lines, but more importantly improves readability quite a bit IMHO:
2. Declarations are closer to the actual usage. This is advised by the "Code Complete" book [2] and has the following advantages:a. This limits variable scope to what is necessary. Which in turn makes the mental model you have to keep of a function when reading the code simpler.b. In most cases it allows you to see the the type of a variable without needing to go to the top of the function.3. You can do input checking and assertions at the top of the function, instead of having to put declarations in front of it. This makes it clear which invariants hold for the function. (as seen in the example above and the changes for pg_file_rename[3])
If you disagree with those statements, could you explain why?
From: Bruce Momjian <bruce@momjian.us>
Sent: Thursday, August 19, 2021 20:57
To: Chapman Flack <chap@anastigmatix.net>
Cc: Ranier Vilela <ranier.vf@gmail.com>; Jelte Fennema <Jelte.Fennema@microsoft.com>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it
Sent: Thursday, August 19, 2021 20:57
To: Chapman Flack <chap@anastigmatix.net>
Cc: Ranier Vilela <ranier.vf@gmail.com>; Jelte Fennema <Jelte.Fennema@microsoft.com>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: [EXTERNAL] Re: Allow declaration after statement and reformat code to use it
[You don't often get email from bruce@momjian.us. Learn why this is important at http://aka.ms/LearnAboutSenderIdentification.]
On Thu, Aug 19, 2021 at 11:40:44AM -0400, Chapman Flack wrote:
> I'm in sympathy with all of those points. I've never believed that the
> arbitrary separation of declaration from use that was forced by C < 99
> made anything more readable. If the project were started now from scratch,
> I would be all in favor of declaring at first use.
>
> But I think Tom's concern about backpatching hazards may carry a lot
> of weight in practice.
I think effectively it would need to be run on all supported versions,
which means the risk of introducing errors would have to be very low.
--
Bruce Momjian <bruce@momjian.us> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmomjian.us%2F&data=04%7C01%7CJelte.Fennema%40microsoft.com%7C9dc5a18c6b2947842ea408d963432ccf%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637649963502570152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZVU2bVto4aJkmh5c6Yz1Ey3pJpGHrBhgFzTDBI1Z2jg%3D&reserved=0
EDB https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fenterprisedb.com%2F&data=04%7C01%7CJelte.Fennema%40microsoft.com%7C9dc5a18c6b2947842ea408d963432ccf%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637649963502570152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=N25dABUPg3FMRdRMihMcZb4m1oB2rvko%2FXd2rfuCDFY%3D&reserved=0
If only the physical world exists, free will is an illusion.
On Thu, Aug 19, 2021 at 11:40:44AM -0400, Chapman Flack wrote:
> I'm in sympathy with all of those points. I've never believed that the
> arbitrary separation of declaration from use that was forced by C < 99
> made anything more readable. If the project were started now from scratch,
> I would be all in favor of declaring at first use.
>
> But I think Tom's concern about backpatching hazards may carry a lot
> of weight in practice.
I think effectively it would need to be run on all supported versions,
which means the risk of introducing errors would have to be very low.
--
Bruce Momjian <bruce@momjian.us> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmomjian.us%2F&data=04%7C01%7CJelte.Fennema%40microsoft.com%7C9dc5a18c6b2947842ea408d963432ccf%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637649963502570152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZVU2bVto4aJkmh5c6Yz1Ey3pJpGHrBhgFzTDBI1Z2jg%3D&reserved=0
EDB https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fenterprisedb.com%2F&data=04%7C01%7CJelte.Fennema%40microsoft.com%7C9dc5a18c6b2947842ea408d963432ccf%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637649963502570152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=N25dABUPg3FMRdRMihMcZb4m1oB2rvko%2FXd2rfuCDFY%3D&reserved=0
If only the physical world exists, free will is an illusion.
pgsql-hackers by date: