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

On Fri, 30 Sep 2022 at 19:32, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> On Fri, Sep 30, 2022 at 1:07 PM Drouvot, Bertrand
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> While resuming the work on [1] I noticed that:
>>
>> - there is an unused parameter in log_heap_visible()
>> - the comment associated to the function is not in "sync" with the
>> current implementation (referring a "block" that is not involved anymore)
>>
>> Attached a tiny patch as an attempt to address the above remarks.
>>
>> [1]: https://commitfest.postgresql.org/39/3740/
>
> It looks like that parameter was originally introduced and used in PG
> 9.4 where xl_heap_visible structure was having RelFileNode, which was
> later removed in PG 9.5, since then the RelFileNode rnode parameter is
> left out. This parameter got renamed to RelFileLocator rlocator by the
> commit b0a55e43299c4ea2a9a8c757f9c26352407d0ccc in HEAD.
>
> The attached patch LGTM.
>
> We recently committed another patch to remove an unused function
> parameter - 65b158ae4e892c2da7a5e31e2d2645e5e79a0bfd.
>
> It makes me think that why can't we remove the unused function
> parameters once and for all, say with a compiler flag such as
> -Wunused-parameter [1]? We might have to be careful in removing
> certain parameters which are not being used right now, but might be
> used in the near future though.
>
> [1] https://man7.org/linux/man-pages/man1/gcc.1.html
>
> -Wunused-parameter
> Warn whenever a function parameter is unused aside from its
> declaration.
>
> To suppress this warning use the "unused" attribute.

When I try to use -Wunused-parameter, I find there are many warnings :-( .

/home/japin/Codes/postgres/Debug/../src/backend/optimizer/geqo/geqo_pool.c: In function ‘free_chromo’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/geqo/geqo_pool.c:176:26: warning: unused parameter ‘root’ [-Wunused-parameter]
176 | free_chromo(PlannerInfo *root, Chromosome *chromo)
| ~~~~~~~~~~~~~^~~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/equivclass.c: In function ‘eclass_useful_for_merging’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/equivclass.c:3091:40: warning: unused parameter ‘root’ [-Wunused-parameter]
3091 | eclass_useful_for_merging(PlannerInfo *root,
| ~~~~~~~~~~~~~^~~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/indxpath.c: In function ‘ec_member_matches_indexcol’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/indxpath.c:3453:41: warning: unused parameter ‘root’ [-Wunused-parameter]
3453 | ec_member_matches_indexcol(PlannerInfo *root, RelOptInfo *rel,
| ~~~~~~~~~~~~~^~~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/indxpath.c:3453:59: warning: unused parameter ‘rel’ [-Wunused-parameter]
3453 | ec_member_matches_indexcol(PlannerInfo *root, RelOptInfo *rel,
| ~~~~~~~~~~~~^~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/indxpath.c: In function ‘relation_has_unique_index_for’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/indxpath.c:3511:44: warning: unused parameter ‘root’ [-Wunused-parameter]
3511 | relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
| ~~~~~~~~~~~~~^~~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/joinpath.c: In function ‘allow_star_schema_join’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/joinpath.c:356:37: warning: unused parameter ‘root’ [-Wunused-parameter]
356 | allow_star_schema_join(PlannerInfo *root,
| ~~~~~~~~~~~~~^~~~
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/joinpath.c: In function ‘paraminfo_get_equal_hashops’:
/home/japin/Codes/postgres/Debug/../src/backend/optimizer/path/joinpath.c:378:42: warning: unused parameter ‘root’ [-Wunused-parameter]
378 | paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
| ~~~~~~~~~~~~~^~~~

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.