Обсуждение: gcc -ftabstop option
I suggest that we add the gcc (also clang) option -ftabstop=4. This has two effects: First, it produces more accurate column numbers in compiler errors and correctly indents the code excerpts that the compiler shows with those. Second, it enables the compiler's detection of confusingly indented code to work more correctly. (But the latter is only a potential problem for code that does not use tabs for indentation, so it would be mostly a help during development with sloppy editor setups.) Attached is a patch to discover the option in configure. One bit of trickery not addressed yet is that we might want to strip out the option and not expose it through PGXS, since we don't know what whitespacing rules external code uses. Thoughts?
Вложения
At Tue, 21 Jun 2022 12:49:24 +0200, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in > I suggest that we add the gcc (also clang) option -ftabstop=4. > > This has two effects: First, it produces more accurate column numbers > in compiler errors and correctly indents the code excerpts that the > compiler shows with those. Second, it enables the compiler's > detection of confusingly indented code to work more correctly. (But > the latter is only a potential problem for code that does not use tabs > for indentation, so it would be mostly a help during development with > sloppy editor setups.) > > Attached is a patch to discover the option in configure. > > One bit of trickery not addressed yet is that we might want to strip > out the option and not expose it through PGXS, since we don't know > what whitespacing rules external code uses. > > Thoughts? There's no strong reason for everyone to accept that change, but I myself don't mind whether the option is applied or not. There was a related discussion. https://www.postgresql.org/message-id/BDE54C55-438C-48E9-B2A3-08EB3A0CBB9F%40yesql.se > The -ftabstop option is (to a large extent, not entirely) to warn when tabs and > spaces are mixed creating misleading indentation that the author didn't even > notice due to tabwidth settings? ISTM we are better of getting these warnings > than suppressing them. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
> At Tue, 21 Jun 2022 12:49:24 +0200, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in >> One bit of trickery not addressed yet is that we might want to strip >> out the option and not expose it through PGXS, since we don't know >> what whitespacing rules external code uses. This part seems like a bigger problem than the option is worth. I agree that we can't assume third-party code prefers 4-space tabs. Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes: > There was a related discussion. > https://www.postgresql.org/message-id/BDE54C55-438C-48E9-B2A3-08EB3A0CBB9F%40yesql.se >> The -ftabstop option is (to a large extent, not entirely) to warn when tabs and >> spaces are mixed creating misleading indentation that the author didn't even >> notice due to tabwidth settings? ISTM we are better of getting these warnings >> than suppressing them. Isn't that kind of redundant given that (a) we have git whitespace warnings about this and (b) pgindent will take care of any such problems in the end? I'll grant the point about compiler warnings possibly not lining up precisely. But that's yet to bother me personally, so maybe I'm underestimating its value. regards, tom lane
> On 21 Jun 2022, at 12:49, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote: > Second, it enables the compiler's detection of confusingly indented code to work more correctly. Wouldn't we also need to add -Wmisleading-indentation for this to trigger any warnings? Adding -ftabstop only allows the compiler to be able to properly detect it. -- Daniel Gustafsson https://vmware.com/
On 22.06.22 21:48, Daniel Gustafsson wrote: >> On 21 Jun 2022, at 12:49, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote: > >> Second, it enables the compiler's detection of confusingly indented code to work more correctly. > > Wouldn't we also need to add -Wmisleading-indentation for this to trigger any > warnings? That is included in -Wall.