Use clang-tidy -fix to replace 0 by nullptr

[lrineau]

Summary of Changes

As a followup to #3979, and because I hear a lot about clang-tidy, I wanted to try and use clang-tidy, to see if one can automatically replace 0 or NULL by nullptr. I have created a tiny .clang-tidy file.

The only check applied for now is modernize-use-nullptr.

To use it:

cmake -DCMAKE_CXX_CLANG_TIDY="clang-tidy;-fix" build

and then I have used:

make && ctest -L Mesh_2

to compile (with clang-tidy enabled) the whole testsuite of Mesh_2. The option -fix of clang-tidy tells it to modify the source code directly. And then I have checked the results because committing: all was good. But I have modified the strange modifications made by #3979 in Hash_map/include/CGAL/Tools/chained_map.h (NULLKEY had been modified to nullptrKEY by error).

Release Management

• Affected package(s): Mesh_2 and dependencies
• License and copyright ownership: maintenance by GeometryFactory

I am not 100% sure that we want to apply that sort of pull-request to master for CGAL-5.0, because, like PRs #3979, #3978, and #3975, that increases the chance of conflicts with bug-fix pull-requests based on CGAL-4.13 or CGAL-4.14.

[MaelRL]

What is missing to this PR?

[lrineau]

There is a lot that is missing in this PR. I have applied clang-tidy only to Mesh_2 and its dependencies. I created that issue to start a discussion: do we want that tool clang-tidy applied to CGAL as a whole? And if yes, which rules of clang-tidy? So far I have experimented only with modernize-use-nullptr.

[MaelRL]

A possible path could be to extend this PR to all packages but just for nullptr and not include any .clang_tidy / NOLINTNEXTLINE so that it can be integrated. Afterwards, we could open an issue and look at all clang tidy options.

[lrineau]

I agree. We should coordinate with other massive PR, like #3895.

[maxGimeno]

@lrineau is this PR still needed ? If yes, then it needs an update.

[lrineau]

@lrineau is this PR still needed ? If yes, then it needs an update.

It is not "needed". That is just an improvement on CGAL code.

But it is useful.

@MaelRL #4041 (comment):

A possible path could be to extend this PR to all packages but just for nullptr and not include any .clang_tidy / NOLINTNEXTLINE so that it can be integrated. Afterwards, we could open an issue and look at all clang tidy options.

Why do you want to remove the /.clang-tidy file? clang-tidy could be a new useful tool in our testsuite.

[maxGimeno]

ping @MaelRL

[MaelRL]

I just meant: it could be ignored temporarily so that there is no extra pollution in the results for warnings that we know we can first fix "off-testsuite". But sure, it would be useful to have.

[maxGimeno]

Conflicts with master

[maxGimeno]

Successfully tested in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.3-Ic-106.shtml