Hi,
I noticed that there are many header files being
included which need not be included.
I have tried this in a few files and found the
compilation and regression to be working.
I have attached the patch for the files that
I tried.
I tried this in CentOS, I did not find the header
files to be pla
Hi,
I noticed that there are many header files being
included which need not be included.
I have tried this in a few files and found the
compilation and regression to be working.
I have attached the patch for the files that
I tried.
I tried this in CentOS, I did not find the header
files to be pla
On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> I noticed that there are many header files being included which need
> not be included. I have tried this in a few files and found the
> compilation and regression to be working. I have attached the patch
> for the files that I tried.
On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier wrote:
>
> On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > I noticed that there are many header files being included which need
> > not be included. I have tried this in a few files and found the
> > compilation and regression to be
On Wed, Jul 31, 2019 at 11:31 AM vignesh C wrote:
>
> On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier wrote:
> >
> > On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > > I noticed that there are many header files being included which need
> > > not be included. I have tried this in
On Wed, Jul 31, 2019 at 11:55 AM Amit Kapila wrote:
>
> On Wed, Jul 31, 2019 at 11:31 AM vignesh C wrote:
> >
> > On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier
> > wrote:
> > >
> > > On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > > > I noticed that there are many header files
On Wed, Jul 31, 2019 at 11:55:37AM +0530, Amit Kapila wrote:
> If we can come up with some such tool, we might be able to integrate
> it with Thomas's patch tester [1] wherein it can apply the patch,
> verify if there are unnecessary includes in the patch and report the
> same.
>
> [1] - http://co
Hi,
On 2019-07-31 11:19:08 +0530, vignesh C wrote:
> I noticed that there are many header files being
> included which need not be included.
> I have tried this in a few files and found the
> compilation and regression to be working.
> I have attached the patch for the files that
> I tried.
> I tr
Michael Paquier writes:
> On Wed, Jul 31, 2019 at 11:55:37AM +0530, Amit Kapila wrote:
>> If we can come up with some such tool, we might be able to integrate
>> it with Thomas's patch tester [1] wherein it can apply the patch,
>> verify if there are unnecessary includes in the patch and report th
On 2019-Jul-31, vignesh C wrote:
> I noticed that there are many header files being
> included which need not be included.
Yeah, we have tooling for this already in src/tools/pginclude. It's
been used before, and it has wreaked considerable havoc; see "git log
--grep pgrminclude".
I think doing
Hi,
On 2019-07-31 11:23:22 -0400, Alvaro Herrera wrote:
> I think removing unnecessary include lines from header files is much
> more useful than from .c files. However, nowadays even I am not very
> convinced that that is a very fruitful use of time, since many/most
> developers use ccache which
On 2019-Jul-31, Andres Freund wrote:
> IDK, I find the compilation times annoying. And it's gotten quite
> noticably worse with all the speculative execution mitigations. Although
> to some degree that's not really the fault of individual compilations,
> but our buildsystem being pretty slow.
We'
Alvaro Herrera writes:
> On 2019-Jul-31, Andres Freund wrote:
>> * I think a lot of the interlinking stems from the bad idea to use
>> typedef's everywhere. In contrast to structs they cannot be forward
>> declared portably in our version of C. We should use a lot more struct
>> forward declaratio
Hi,
On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
> Alvaro Herrera writes:
> > On 2019-Jul-31, Andres Freund wrote:
> >> * I think a lot of the interlinking stems from the bad idea to use
> >> typedef's everywhere. In contrast to structs they cannot be forward
> >> declared portably in our versio
Andres Freund writes:
> On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
>> Yeah. I seem to recall a proposal that nodes.h should contain
>>
>> typedef struct Foo Foo;
>>
>> for every node type Foo, and then the other headers would just
>> fill in the structs, and we could get rid of a lot of ad-h
Hi,
On 2019-07-31 19:25:01 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
> >> Yeah. I seem to recall a proposal that nodes.h should contain
> >>
> >> typedef struct Foo Foo;
> >>
> >> for every node type Foo, and then the other headers would j
On Wed, Jul 31, 2019 at 12:26 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-07-31 11:19:08 +0530, vignesh C wrote:
> > I noticed that there are many header files being
> > included which need not be included.
> > I have tried this in a few files and found the
> > compilation and regression to be wor
On 2019-Aug-04, vignesh C wrote:
> Made the fixes based on your comments, updated patch has the changes
> for the same.
Well, you fixed the two things that seem to me quoted as examples of
problems, but you didn't fix other occurrences of the same issues
elsewhere. For example, you remove lwlock
Alvaro Herrera writes:
> Then there's the removal, which is in tuplesort.c because of
> INT_MAX as added by commit d26559dbf356 and still present ...
One has to be especially wary of removing system-header inclusions;
the fact that they don't seem to be needed on your own machine doesn't
prove t
On 2019-Aug-05, Tom Lane wrote:
> Alvaro Herrera writes:
> > Then there's the removal, which is in tuplesort.c because of
> > INT_MAX as added by commit d26559dbf356 and still present ...
>
> One has to be especially wary of removing system-header inclusions;
> the fact that they don't seem to
On Thu, Aug 8, 2019 at 9:00 AM Alvaro Herrera wrote:
> On 2019-Aug-05, Tom Lane wrote:
> > Alvaro Herrera writes:
> > > Then there's the removal, which is in tuplesort.c because of
> > > INT_MAX as added by commit d26559dbf356 and still present ...
> >
> > One has to be especially wary of removi
On Thu, Aug 8, 2019 at 9:47 AM Thomas Munro wrote:
> transaction unit
*translation unit
--
Thomas Munro
https://enterprisedb.com
Hi,
On 2019-08-03 12:37:33 -0700, Andres Freund wrote:
> Think the first three are pretty clearly a good idea, I'm a bit less
> sanguine about the fourth:
> Headers like utils/timestamp.h are often included just because we need a
> TimestampTz type somewhere, or call GetCurrentTimestamp(). Approxi
Andres Freund writes:
> I've pushed the other ones.
Checking whether header files compile standalone shows you were overly
aggressive about removing fmgr.h includes:
In file included from /tmp/headerscheck.Ss8bVx/test.c:3:
./src/include/utils/selfuncs.h:143: error: expected declaration specifier
I wrote:
> (My headerscheck script is missing that header; I need to update it to
> match the latest version of cpluspluscheck.)
I did that, and ended up with the attached. I'm rather tempted to stick
this into src/tools/ alongside cpluspluscheck, because it seems to find
rather different trouble
On 2019-Aug-18, Tom Lane wrote:
> I wrote:
> > (My headerscheck script is missing that header; I need to update it to
> > match the latest version of cpluspluscheck.)
>
> I did that, and ended up with the attached. I'm rather tempted to stick
> this into src/tools/ alongside cpluspluscheck, beca
Alvaro Herrera writes:
> On 2019-Aug-18, Tom Lane wrote:
>> I did that, and ended up with the attached. I'm rather tempted to stick
>> this into src/tools/ alongside cpluspluscheck, because it seems to find
>> rather different trouble spots than cpluspluscheck does. Thoughts?
> Yeah, let's incl
Hi,
On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
> Andres Freund writes:
> > I've pushed the other ones.
>
> Checking whether header files compile standalone shows you were overly
> aggressive about removing fmgr.h includes:
>
> In file included from /tmp/headerscheck.Ss8bVx/test.c:3:
> ./src/
Hi,
On 2019-08-19 13:07:50 -0700, Andres Freund wrote:
> On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
> > That's with a script I use that's like cpluspluscheck except it tests
> > with plain gcc not g++. I attached it for the archives' sake.
> >
> > Oddly, cpluspluscheck does not complain about
On 2019-Aug-19, Andres Freund wrote:
> > I wish we could move the whole logic of those scripts into makefiles, so
> > we could employ parallelism.
>
> Hm. Perhaps the way to do that would be to use gcc's -include to include
> postgres.h, and use -Wc++-compat to detect c++ issues, rather than usin
Andres Freund writes:
> On 2019-08-19 13:07:50 -0700, Andres Freund wrote:
>> On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
>>> Oddly, cpluspluscheck does not complain about those cases, but it
>>> does complain about
>> Hm. I don't understand why it's not complaining. Wonder if it's a
>> questio
On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> I noticed that there are many header files being included which need
> not be included. I have tried this in a few files and found the
> compilation and regression to be working. I have attached the patch
> for the files that I tried.
On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier wrote:
>
> On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > I noticed that there are many header files being included which need
> > not be included. I have tried this in a few files and found the
> > compilation and regression to be
On Wed, Jul 31, 2019 at 11:31 AM vignesh C wrote:
>
> On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier wrote:
> >
> > On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > > I noticed that there are many header files being included which need
> > > not be included. I have tried this in
On Wed, Jul 31, 2019 at 11:55 AM Amit Kapila wrote:
>
> On Wed, Jul 31, 2019 at 11:31 AM vignesh C wrote:
> >
> > On Wed, Jul 31, 2019 at 11:26 AM Michael Paquier
> > wrote:
> > >
> > > On Wed, Jul 31, 2019 at 11:19:08AM +0530, vignesh C wrote:
> > > > I noticed that there are many header files
On Wed, Jul 31, 2019 at 11:55:37AM +0530, Amit Kapila wrote:
> If we can come up with some such tool, we might be able to integrate
> it with Thomas's patch tester [1] wherein it can apply the patch,
> verify if there are unnecessary includes in the patch and report the
> same.
>
> [1] - http://co
Hi,
On 2019-07-31 11:19:08 +0530, vignesh C wrote:
> I noticed that there are many header files being
> included which need not be included.
> I have tried this in a few files and found the
> compilation and regression to be working.
> I have attached the patch for the files that
> I tried.
> I tr
Michael Paquier writes:
> On Wed, Jul 31, 2019 at 11:55:37AM +0530, Amit Kapila wrote:
>> If we can come up with some such tool, we might be able to integrate
>> it with Thomas's patch tester [1] wherein it can apply the patch,
>> verify if there are unnecessary includes in the patch and report th
On 2019-Jul-31, vignesh C wrote:
> I noticed that there are many header files being
> included which need not be included.
Yeah, we have tooling for this already in src/tools/pginclude. It's
been used before, and it has wreaked considerable havoc; see "git log
--grep pgrminclude".
I think doing
Hi,
On 2019-07-31 11:23:22 -0400, Alvaro Herrera wrote:
> I think removing unnecessary include lines from header files is much
> more useful than from .c files. However, nowadays even I am not very
> convinced that that is a very fruitful use of time, since many/most
> developers use ccache which
On 2019-Jul-31, Andres Freund wrote:
> IDK, I find the compilation times annoying. And it's gotten quite
> noticably worse with all the speculative execution mitigations. Although
> to some degree that's not really the fault of individual compilations,
> but our buildsystem being pretty slow.
We'
Alvaro Herrera writes:
> On 2019-Jul-31, Andres Freund wrote:
>> * I think a lot of the interlinking stems from the bad idea to use
>> typedef's everywhere. In contrast to structs they cannot be forward
>> declared portably in our version of C. We should use a lot more struct
>> forward declaratio
Hi,
On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
> Alvaro Herrera writes:
> > On 2019-Jul-31, Andres Freund wrote:
> >> * I think a lot of the interlinking stems from the bad idea to use
> >> typedef's everywhere. In contrast to structs they cannot be forward
> >> declared portably in our versio
Andres Freund writes:
> On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
>> Yeah. I seem to recall a proposal that nodes.h should contain
>>
>> typedef struct Foo Foo;
>>
>> for every node type Foo, and then the other headers would just
>> fill in the structs, and we could get rid of a lot of ad-h
Hi,
On 2019-07-31 19:25:01 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-07-31 16:55:31 -0400, Tom Lane wrote:
> >> Yeah. I seem to recall a proposal that nodes.h should contain
> >>
> >> typedef struct Foo Foo;
> >>
> >> for every node type Foo, and then the other headers would j
On Wed, Jul 31, 2019 at 12:26 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-07-31 11:19:08 +0530, vignesh C wrote:
> > I noticed that there are many header files being
> > included which need not be included.
> > I have tried this in a few files and found the
> > compilation and regression to be wor
On 2019-Aug-04, vignesh C wrote:
> Made the fixes based on your comments, updated patch has the changes
> for the same.
Well, you fixed the two things that seem to me quoted as examples of
problems, but you didn't fix other occurrences of the same issues
elsewhere. For example, you remove lwlock
Alvaro Herrera writes:
> Then there's the removal, which is in tuplesort.c because of
> INT_MAX as added by commit d26559dbf356 and still present ...
One has to be especially wary of removing system-header inclusions;
the fact that they don't seem to be needed on your own machine doesn't
prove t
On 2019-Aug-05, Tom Lane wrote:
> Alvaro Herrera writes:
> > Then there's the removal, which is in tuplesort.c because of
> > INT_MAX as added by commit d26559dbf356 and still present ...
>
> One has to be especially wary of removing system-header inclusions;
> the fact that they don't seem to
On Thu, Aug 8, 2019 at 9:00 AM Alvaro Herrera wrote:
> On 2019-Aug-05, Tom Lane wrote:
> > Alvaro Herrera writes:
> > > Then there's the removal, which is in tuplesort.c because of
> > > INT_MAX as added by commit d26559dbf356 and still present ...
> >
> > One has to be especially wary of removi
On Thu, Aug 8, 2019 at 9:47 AM Thomas Munro wrote:
> transaction unit
*translation unit
--
Thomas Munro
https://enterprisedb.com
Hi,
On 2019-08-03 12:37:33 -0700, Andres Freund wrote:
> Think the first three are pretty clearly a good idea, I'm a bit less
> sanguine about the fourth:
> Headers like utils/timestamp.h are often included just because we need a
> TimestampTz type somewhere, or call GetCurrentTimestamp(). Approxi
Andres Freund writes:
> I've pushed the other ones.
Checking whether header files compile standalone shows you were overly
aggressive about removing fmgr.h includes:
In file included from /tmp/headerscheck.Ss8bVx/test.c:3:
./src/include/utils/selfuncs.h:143: error: expected declaration specifier
I wrote:
> (My headerscheck script is missing that header; I need to update it to
> match the latest version of cpluspluscheck.)
I did that, and ended up with the attached. I'm rather tempted to stick
this into src/tools/ alongside cpluspluscheck, because it seems to find
rather different trouble
On 2019-Aug-18, Tom Lane wrote:
> I wrote:
> > (My headerscheck script is missing that header; I need to update it to
> > match the latest version of cpluspluscheck.)
>
> I did that, and ended up with the attached. I'm rather tempted to stick
> this into src/tools/ alongside cpluspluscheck, beca
Alvaro Herrera writes:
> On 2019-Aug-18, Tom Lane wrote:
>> I did that, and ended up with the attached. I'm rather tempted to stick
>> this into src/tools/ alongside cpluspluscheck, because it seems to find
>> rather different trouble spots than cpluspluscheck does. Thoughts?
> Yeah, let's incl
Hi,
On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
> Andres Freund writes:
> > I've pushed the other ones.
>
> Checking whether header files compile standalone shows you were overly
> aggressive about removing fmgr.h includes:
>
> In file included from /tmp/headerscheck.Ss8bVx/test.c:3:
> ./src/
Hi,
On 2019-08-19 13:07:50 -0700, Andres Freund wrote:
> On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
> > That's with a script I use that's like cpluspluscheck except it tests
> > with plain gcc not g++. I attached it for the archives' sake.
> >
> > Oddly, cpluspluscheck does not complain about
On 2019-Aug-19, Andres Freund wrote:
> > I wish we could move the whole logic of those scripts into makefiles, so
> > we could employ parallelism.
>
> Hm. Perhaps the way to do that would be to use gcc's -include to include
> postgres.h, and use -Wc++-compat to detect c++ issues, rather than usin
Andres Freund writes:
> On 2019-08-19 13:07:50 -0700, Andres Freund wrote:
>> On 2019-08-18 14:37:34 -0400, Tom Lane wrote:
>>> Oddly, cpluspluscheck does not complain about those cases, but it
>>> does complain about
>> Hm. I don't understand why it's not complaining. Wonder if it's a
>> questio
60 matches
Mail list logo
