Re: [Proposal] Level4 Warnings show many shadow vars

Поиск
Список
Период
Сортировка
От Stephen Frost
Тема Re: [Proposal] Level4 Warnings show many shadow vars
Дата
Msg-id 20191211214455.GZ6962@tamriel.snowman.net
обсуждение исходный текст
Ответ на RE: [Proposal] Level4 Warnings show many shadow vars  (Ranier Vilela <ranier_gyn@hotmail.com>)
Список pgsql-hackers
Greetings,

* Ranier Vilela (ranier_gyn@hotmail.com) wrote:
> 1.So I would like to ask you if at least what has consensus could be used.
> Or is it better to leave everything as it is?

As I tried to say before- I'm all for working to eliminate the shadow
variables, but it should be on a case-by-case basis where the change
proposed is a well considered change by someone who has taken the time
to understand what the code is doing, looked at the call sites, made a
well reasoned argument for why the change is an improvement and reduces
confusion (which can't just be "because the compiler said we should make
this change"- compilers aren't nearly intelligent enough to give us the
right answer about what the code should look like- they can only point
out potential issues, and they're often bad at even doing that), and
then proposed a patch for each particular case where the patch is
addressing a specific set of shadow variable cases that somehow go
together.

A patch to address the numTables issues in pg_dump would be great, for
example.  A single patch that renames numTables in pg_dump and then
makes a bunch of completely unrelated changes to things in the backend
isn't what I'd consider a reasonable grouping of changes.

> 2.About local shadow variables, would you find it safe to do redundant declaration removals of the type: int i? Is it
worthit to work on that?
 

I really don't think you're going to find much in the PG code where
there would be general consensus of a broad renaming or modifying of
variables without having to put serious thought into the specific
change.

At least, I hope people would push back on that kind of rote change.

In other words, without looking at the specific cases you're talking
about, I don't know if I'd agree with them or not, but please don't just
submit patches that just rename things without having looked at the
code, gained some understanding of what the code does, and considered if
the change you want to make is a well reasoned improvement and makes the
code easier to read and understand.

Thanks,

Stephen

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Ranier Vilela
Дата:
Сообщение: RE: [Proposal] Level4 Warnings show many shadow vars
Следующее
От: Robert Haas
Дата:
Сообщение: non-exclusive backup cleanup is mildly broken