include-what-you-use/include-what-you-use

False possive with GCC lib with <chrono>

Closed this issue · 6 comments

I am using g++ 13 and iwyu 14

g++ (Ubuntu 13.2.0-4ubuntu3) 13.2.0

include-what-you-use 0.18 based on Ubuntu clang version 14.0.6

And my code that uses std::chrono::high_resolution_clock makes IWYU to ask for

/home/user/boost-multi/include/boost/multi/adaptors/fftw/test/transpose_square.cpp should add these lines:
#include <bits/chrono.h>                  // for operator-, duration, system...

/home/user/boost-multi/include/boost/multi/adaptors/fftw/test/transpose_square.cpp should remove these lines:
- #include <chrono>  // lines 10-10

I think this is a false positive that might suggest a needed modification in /usr/share/include-what-you-use/gcc.stl.headers.imp consisting of adding:

    { "include": ["@<bits/chrono.h>", "private", "<chrono>", "public"] },

Is this a reasonable request?

Thanks!

PS: this seems a near duplicate of #1277 , close if necessary

Yeah, I think that would make sense. Don't forget the built-in mapping table at https://github.com/include-what-you-use/include-what-you-use/blob/master/iwyu_include_picker.cc#L655 if you want to make this change.

do you mean that it is better to add this as a built-in mapping? modifying the .cc file only?

not sure what is the criterion for one or the other.

The built-in mapping in .cc will always be used, the external mapping in *.imp will only be used if explicitly provided on command-line with -Xiwyu --mapping_file=....

Yes, I understand that. Are you trying to say that it makes sense to have mappings for the standard library in the .cc file?

Seems reasonable, but I am trying to understand the logic since there is also a stl.imp in the iwyu distribution.

Yeah, it's a little confusing:

  • At first there were only built-in mappings in the .cc file
  • I contributed external mappings, with the intent of removing the built-in mappings (as they are not strictly portable)
  • I think we failed to figure out how to select external mappings, or something like that (~10 years ago, so not sure), so the built-in mappings stayed on
  • We kept the external mappings to be able to do e.g. -Xiwyu --no_default_mappings -Xiwyu --mapping_file=gcc.libc.imp for more selective mappings...

.... and now we have redundant definitions. But messing about with code generation for this didn't seem like it would be much clearer, so we've just left it like this for now. Maybe once mappings are a little better understood we can reconcile or kill off one of them.

I was just trying to remind you to change both if you change either.

Fixed on master as of 07e2f47