Errors in libc++. Diagnostic mismatch

[GoogleCodeExporter]

Steps to reproduce:
Prerequisites: system where libc++ is default stdlib, for example, Mac OS X 
10.9+.
1. Run IWYU test suite.

Actual result:
Some tests fail because of warnings like
    tests/cxx/badinc.h:295: Unexpected diagnostic:
    std::__1::vector is defined in <vector>, which isn't directly #included.

    tests/cxx/badinc.h:295: Unmatched regex:
    std::vector is...*<vector>

Expected result:
All tests should pass.

Original issue reported on code.google.com by vsap...@gmail.com on 18 May 2014 at 4:50

[GoogleCodeExporter]

I've considered the following approaches to fix this issue.

Omit "__1::" in IWYU diagnostics.  Discarded this approach because Clang 
mentions "__1::" in its own diagnostics.

Update in tests all expected IWYU diagnostics.  "// IWYU: std::vector 
is...*<vector>" becomes "// IWYU: std::(__1::)?vector is...*<vector>".  
Discarded this approach because it incurs maintenance cost - we would have to 
keep "std::" diagnostics up to date in all tests (what if at some moment __2 is 
added to __1?).

Update test script, so that diagnostic message
    std::__1::vector is defined in <vector>, which isn't directly #included.
matches to "// IWYU: std::vector is...*<vector>".  I've chosen this approach, 
corresponding patch is attached.

Thoughts?

Original comment by vsap...@gmail.com on 18 May 2014 at 5:10

Attachments:

• issue133.patch

[GoogleCodeExporter]

This feels like a hack to me.

Can't we just make the regex more lenient so we don't incur maintenance cost?

Something like:

"// IWYU: std::([^:]+::)?vector is...*<vector>"

?

That way we accept std::vector in any sub-namespace of std. It's messy, 
though... Wouldn't fixing issue 132 also solve this?

Original comment by kim.gras...@gmail.com on 20 May 2014 at 6:56

[GoogleCodeExporter]

Fixing issue #132 won't help, diagnostic mismatch isn't limited to template 
arguments.

Updating regular expressions in tests seems to be more honest, but it feels 
like copy-paste solution with all copy-pasting flaws.  Modifying regexp in 
iwyu_test_util.py looks like a hack and masks actual complexity.  I prefer a 
change in iwyu_test_util.py because the change is more localised, complex 
regular expressions in tests aren't very readable (personally I read tests more 
often than iwyu_test_util.py) and hiding complexity isn't always bad.

Original comment by vsap...@gmail.com on 21 May 2014 at 7:41

[GoogleCodeExporter]

What if we fixed reporting in IWYU (iwyu_output.cc:GetQualifiedNameAsString) 
the same way we do in issue #132?

We could move a GetQualifiedNameAsString helper out into iwyu_ast_util.h and 
then use it from both iwyu_cache and iwyu_output.

I don't see a need to be 100% consistent with Clang here, since users will 
never write std::__1::vector. Clang needs to point to the exact right type, we 
only want to point to the type as written.

Original comment by kim.gras...@gmail.com on 21 May 2014 at 8:02

[GoogleCodeExporter]

OK, I'll investigate this approach.  This is a good point, "__1" part in 
"std::__1::vector" isn't very useful.  It is definitely useful at linking 
stage, but not for IWYU.

Original comment by vsap...@gmail.com on 21 May 2014 at 8:32

[GoogleCodeExporter]

Used GetWrittenQualifiedNameAsString to determine symbol names.  New patch is 
attached.

By the way, it also helps with no_char_traits test failure.  Earlier 
"std::char_traits" in libstdcpp_symbol_map was ignored because actually it was 
"std::__1::char_traits".

Original comment by vsap...@gmail.com on 31 May 2014 at 2:38

Attachments:

• issue133r2.patch

[GoogleCodeExporter]

Looks good and all tests I expect to pass do so on Windows.

Please commit!

Original comment by kim.gras...@gmail.com on 31 May 2014 at 10:08

[GoogleCodeExporter]

I just noticed some superfluous whitespace on line 26 of the last patch. It'd 
be nice if you could trim that before committing, thanks!

Original comment by kim.gras...@gmail.com on 31 May 2014 at 10:14

[GoogleCodeExporter]

This issue was closed by revision r550.

Original comment by vsap...@gmail.com on 1 Jun 2014 at 10:20

• Changed state: Fixed

[GoogleCodeExporter]

Thanks, Kim.  Committed with cleaned up trailing spaces.

Original comment by vsap...@gmail.com on 1 Jun 2014 at 10:24