Re: log_heap_visible(): remove unused parameter and update comment

On Fri, Sep 30, 2022 at 7:48 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>
> On Fri, 30 Sep 2022 at 22:09, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > On Fri, Sep 30, 2022 at 7:30 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
> >>
> >> When I try to use -Wunused-parameter, I find there are many warnings :-( .
> >
> > Great!
> >
> > I think we can't just remove every unused parameter, for instance, it
> > makes sense to retain PlannerInfo *root parameter even though it's not
> > used now, in future it may be. But if the parameter is of type
> > unrelated to the context of the function, like the one committed
> > 65b158ae4e892c2da7a5e31e2d2645e5e79a0bfd and like the proposed patch
> > to some extent, it could be removed.
> >
> > Others may have different thoughts here.
>
> Maybe we can define a macro like UNUSED(x) for those parameters, and
> call this macro on the parameter that we might be useful, then
> we can use -Wunused-parameter when compiling. Any thoughts?

We have the pg_attribute_unused() macro already. I'm not sure if
adding -Wunused-parameter for compilation plus using
pg_attribute_unused() for unused-yet-contextually-required variables
is a great idea. But it has some merits as it avoids unused variables
lying around in the code. However, we can even discuss this in a
separate thread IMO to hear more from other hackers.

While on this, I noticed that pg_attribute_unused() is being used for
npages in AdvanceXLInsertBuffer(), but the npages variable is actually
being used there. I think we can remove it.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com