GCC header map lists some private headers as public

[GoogleCodeExporter]

tr1/functional_hash.h and tr1/hashtable.h are both listed as public. However, 
looking at the comments of those headers it's pretty clear they they are 
intended to be private headers, see:
http://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-html-USERS-4.3/a01894.html
http://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-html-USERS-4.3/a01876.html

Original issue reported on code.google.com by christop...@gmail.com on 28 Dec 2012 at 7:37

Attachments:

• header_map.patch

[GoogleCodeExporter]

Thanks!

I noticed you just marked the two headers as private -- do they map to a public 
header somewhere?

I'm not sure what IWYU's policy is if all defining headers are private... Could 
you test with a simple .cc file like this;

  #include <tr1/functional_hash.h>
  #include <tr1/hashtable.h>

  std::hash<int> h; // from functional_hash.h
  std::_Hashtable<...> ht; // from hashtable.h

I'm guessing the expected behavior here is to replace both #includes with 
<tr1/hashtable>.

Original comment by kim.gras...@gmail.com on 29 Dec 2012 at 1:12

[GoogleCodeExporter]

Thanks, Christopher. Different libstdc++ versions implement hashtable in 
different files, so I need to check how this patch plays with different 
libstdc++ versions.

Original comment by vsap...@gmail.com on 30 Dec 2012 at 8:49

[GoogleCodeExporter]

If that's the case we should probably start building .imp files for the 
respective versions we know about.

Original comment by kim.gras...@gmail.com on 31 Dec 2012 at 9:02

[GoogleCodeExporter]

I took some time to test this on my Ubuntu/GCC 4.6.3 setup, and it turns out 
IWYU asserts whenever <tr1/functional> is included.

The two headers both have @headername directives, which IWYU parses and 
processes, so that's how it knows which the correct public header is.

The failure is because the presence of @headername causes IWYU to mark the file 
as private, and the mapping file already has it marked as public, and IWYU 
doesn't like the conflict.

For the libstdc++ shipped with GCC 4.6.3, Christopher's patch is sane, and I'd 
like to apply it as-is.

Volodymyr; unless it messes up your environment on the Mac, I think we should 
deal with other libstdc++ versions separately. What we have now is clearly 
broken.

Original comment by kim.gras...@gmail.com on 2 Feb 2013 at 5:18

[GoogleCodeExporter]

What if instead of making <tr1/hashtable.h> and <tr1/functional_hash.h> 
private, we map them to <tr1/functional>, <tr1/unordered_map>, 
<tr1/unordered_set>? Corresponding patch is attached.

I have some doubts about mapping <tr1/functional_hash.h> to <tr1/unordered_map> 
and to <tr1/unordered_map>. I've done so that we don't require to #include 
<tr1/functional> when just use std::tr1::unordered_map. On the other hand when 
we have

#include <tr1/unordered_map>
void foo() {
  std::tr1::hash<int> h;
}

IWYU doesn't offer to remove <tr1/unordered_map> and add <tr1/functional>.

Original comment by vsap...@gmail.com on 4 Feb 2013 at 10:03

[GoogleCodeExporter]

I think you forgot to add the patch! :-)

If anything, we should probably do both, because they _are_ private.

We can't keep them public, because IWYU doesn't allow a mapping with 
conflicting visibilities -- @headername makes them private and then the mapping 
file attempts to make them public.

I see your point about std::tr1::hash; I think that mapping is fine.

Original comment by kim.gras...@gmail.com on 5 Feb 2013 at 5:08

[GoogleCodeExporter]

Here is the patch.

Original comment by vsap...@gmail.com on 5 Feb 2013 at 10:20

Attachments:

• gcc.stl.hash.diff

[GoogleCodeExporter]

Now that the GCC include mappings are back into hard-coded form, here's a 
version of your patch that should apply to HEAD.

I haven't had a chance to try this on my Ubuntu/GCC system, but I plan to 
commit it as soon as I get a chance to make sure it works there.

Do we need a test for this?

Original comment by kim.gras...@gmail.com on 17 Feb 2013 at 7:42

Attachments:

• issue-88.patch

[GoogleCodeExporter]

This runs fine on my 'g++ (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3' setup.

I've pushed it around a bit by adding a simple .cc file to #include 
<tr1/unordered_map> and <tr1/unordered_set>, but I'm a bit too tired to 
construct a full test case.

I'm going to commit this now, and we can see if we can hash out a testcase 
together at some point. Volodymyr, feel free to revert if you disagree!

Original comment by kim.gras...@gmail.com on 18 Feb 2013 at 9:03

[GoogleCodeExporter]

Patch is fine. I don't think that adding a test is necessary. Mappings are 
pretty straightforward and I doubt that tests will help to catch some tricky 
bugs. Actually mappings' tests will duplicate mappings themselves.

Original comment by vsap...@gmail.com on 19 Feb 2013 at 8:19

[GoogleCodeExporter]

OK, I'll close this. Fixed in r446.

Original comment by kim.gras...@gmail.com on 20 Feb 2013 at 8:20

• Changed state: Fixed