[run-clang-tidy] new replacement overlaps with an existing replacement

Hi,

I'm running clang-tidy 7.0 (also tried 5.0) to modernise some aspects
of Boost.GIL (https://github.com/boostorg/gil) source code.

I've noticed, clang-tidy 7.0 (also 5.0) does not apply fixes for some of
modernize-use-* checks, especially modernize-use-using.

I run it this way:

cd ${BOOST_ROOT}/libs/gil
cmake -S . -B _build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
run-clang-tidy.py -p=_build -header-filter='boost\/gil\/.*'
-checks='-*,modernize-use-using' -fix

Then, I see huge number of "The new replacement overlaps with an
existing replacement." diagnostics.
Below, I copied an extract that hopefully is useful to figure out what
is happening and going wrong.

Basically, there are two class templates and a bunch of tag types:

1. file_stream_device
https://github.com/boostorg/gil/blob/e0288ece9ec50534e7d02166863d6799a5932e25/include/boost/gil/io/device.hpp#L49
2. get_write_device with partial specialisations, `enable_if`-ed
https://github.com/boostorg/gil/blob/e0288ece9ec50534e7d02166863d6799a5932e25/include/boost/gil/io/get_write_device.hpp#L19-L59

The file_stream_device is specialised for a tag and final
get_write_device specialisation is matched.

And, clang-tidy tries to substitute this alias in get_write_device

typedef detail::file_stream_device< FormatTag > type;

not with

using type = detail::file_stream_device< FormatTag > ;

but with `using type` for each FormatTag-based specialisation

New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<bmp_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<bmp_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<jpeg_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<pnm_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<targa_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<targa_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<tiff_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<tiff_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.

Why clang-tidy tries to re-fix the typedef with new replacement
instead of keeping the existing one, the generic one?

i.e. using type = detail::file_stream_device<FormatTag>

I've tried to prepare a minimal example, but I couldn't reproduce this issue.

I observed, that if I manually prepare compile_database.json with
single .cpp file that just `#include <boost/gil/io/device.hpp>`,
that is the header with definition of the base templates
and no definitions with higher level specialisations for format tags
are included,
then clang-tidy applies the expected fixes without any warnings.

Could anyone share any insights about this issue?

Best regards,

Hi Mateusz,

comments inline.

Hi,

I'm running clang-tidy 7.0 (also tried 5.0) to modernise some aspects
of Boost.GIL (https://github.com/boostorg/gil) source code.

I've noticed, clang-tidy 7.0 (also 5.0) does not apply fixes for some of
modernize-use-* checks, especially modernize-use-using.

I run it this way:

cd ${BOOST_ROOT}/libs/gil
cmake -S . -B _build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
run-clang-tidy.py -p=_build -header-filter='boost\/gil\/.*'
-checks='-*,modernize-use-using' -fix

Then, I see huge number of "The new replacement overlaps with an
existing replacement." diagnostics.
Below, I copied an extract that hopefully is useful to figure out what
is happening and going wrong.

Basically, there are two class templates and a bunch of tag types:

1. file_stream_device
https://github.com/boostorg/gil/blob/e0288ece9ec50534e7d02166863d6799a5932e25/include/boost/gil/io/device.hpp#L49
2. get_write_device with partial specialisations, `enable_if`-ed
https://github.com/boostorg/gil/blob/e0288ece9ec50534e7d02166863d6799a5932e25/include/boost/gil/io/get_write_device.hpp#L19-L59

The file_stream_device is specialised for a tag and final
get_write_device specialisation is matched.

And, clang-tidy tries to substitute this alias in get_write_device

typedef detail::file_stream_device< FormatTag > type;

not with

using type = detail::file_stream_device< FormatTag > ;

but with `using type` for each FormatTag-based specialisation

New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<bmp_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<bmp_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<jpeg_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<png_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<pnm_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<targa_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<targa_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<tiff_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.
New replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<tiff_tag>"
Existing replacement:
/mnt/d/boost.wsl/libs/gil/include/boost/gil/io/get_write_device.hpp:
1885:+52:"using type = detail::file_stream_device<FormatTag>"
The new replacement overlaps with an existing replacement.

Why clang-tidy tries to re-fix the typedef with new replacement
instead of keeping the existing one, the generic one?

Maybe a bug that does not handle some possible cases well. I know, that
`use-using` has its problems sometimes.

i.e. using type = detail::file_stream_device<FormatTag>

I've tried to prepare a minimal example, but I couldn't reproduce this issue.

Do you know which `TU` triggered the behaviour? If you find it out you
can fully preprocess the file and check if the behaviour is still
present. Then you can `creduce` the issue or create a bug-report and we
reduce it

I observed, that if I manually prepare compile_database.json with
single .cpp file that just `#include <boost/gil/io/device.hpp>`,
that is the header with definition of the base templates
and no definitions with higher level specialisations for format tags
are included,
then clang-tidy applies the expected fixes without any warnings.

Could anyone share any insights about this issue?

What can happen (especially template-heavy code) that multiple
inclusions of a file (as always happening) results in different fixes
for the same code-location.

`run-clang-tidy` does the replacements at the end. That means if
multiple TUs will result in different fixes there will be collisions and
it is not resolveable which one is correct.

Best, Jonas

>
> Why clang-tidy tries to re-fix the typedef with new replacement
> instead of keeping the existing one, the generic one?

Maybe a bug that does not handle some possible cases well. I know, that
`use-using` has its problems sometimes.

Hi Jonas,

I think I may be hitting some of the cases or my configuration of TU-s
is not optimal for clang-tidy refactoring.

For instance, I observed that overlapping replacements issue
while running run clang-tidy against generated .cpp files
aiming to test Boost.GIL headers are self-contained.

That is, for each X header in Boost.GIL headers, there is .cpp file

#include <boost/gil/X.hpp>
int main() {}

Plus, there are also .cpp files with regular (unit) tests, with
specific specialisations of templates, etc.

> i.e. using type = detail::file_stream_device<FormatTag>
>
> I've tried to prepare a minimal example, but I couldn't reproduce this issue.

Do you know which `TU` triggered the behaviour? If you find it out you
can fully preprocess the file and check if the behaviour is still present.
Then you can `creduce` the issue or create a bug-report and we reduce it

I tried to identify such TU-s, but all my attempts did not reproduce
the problem.
I chose candidates from .cpp files that occur in clang-tidy log
immediately before
overlapping replacements reported in the original error.

For example, these two replacements from
- bmp_read_test.cpp file listed before using type =
detail::file_stream_device<bmp_tag>
- jpeg_read_test.cpp file listed before using type =
detail::file_stream_device<jpeg_tag>

I preprocessed those files with g++ -E into
bmp_read_test.i.cpp
jpeg_read_test.i.cpp
Then, modified compile_commands.json to run clang-tidy against those
*.i.cpp files.

This seems to work fine,I see typedef replaced as expected

using type = detail::file_stream_device<FormatTag>;

That seems to confirm no bug in clang-tidy, but the issue is specific
to my setup.

> I observed, that if I manually prepare compile_database.json with
> single .cpp file that just `#include <boost/gil/io/device.hpp>`,
> that is the header with definition of the base templates
> and no definitions with higher level specialisations for format tags
> are included,
> then clang-tidy applies the expected fixes without any warnings.
>
> Could anyone share any insights about this issue?

What can happen (especially template-heavy code) that multiple
inclusions of a file (as always happening) results in different fixes
for the same code-location.

`run-clang-tidy` does the replacements at the end. That means if
multiple TUs will result in different fixes there will be collisions and
it is not resolveable which one is correct.

I suspect that is the case indeed.

I have created new test in GIL which generates single .cpp file
that includes all headers (https://github.com/boostorg/gil/pull/184):
I generated compile_commands.json and manually removed all commands
but the one with .cpp including all headers.
Finally, I run clang-tidy against it, I get no warning about overlapping
replacements and all typedef-s are replaced with correct using declaration.

I think think that is the optimal workflow for the header-only Boost.GIL libary.

Thanks for help!

Best regards,

Great to hear!