Pull request 930 in include-what-you-use: Add (and fix some) mappings for C / POSIX types

0 views
Skip to first unread message

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 8:02:33 AMJun 27
to include-wh...@googlegroups.com
New pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

The source for the changes in this PR is the manual page for [`system_data_types(7)`](https://man7.org/linux/man-pages/man3/clockid_t.3.html), which I wrote. That manual page has been written comparing the source code of glibc and the C and POSIX standard documents.

As I add more types to that manual page, I'll send you more PRs of this kind.

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 8:12:40 AMJun 27
to include-wh...@googlegroups.com
Comment #1 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

About the commit messages:

I tried to follow some conventions in these commit messages so that it is easier to review:

"Add *more* mappings ..." is for commits that add mappings to an already supported type.
"Add mappings... " is for commits adding a type.
"Remove mappings ..." removes incorrect mappings".
"Fix mappings ..." is a mix of the above.

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 8:12:50 AMJun 27
to include-wh...@googlegroups.com
Comment #1 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

About the commit messages:

I tried to follow some conventions in these commit messages so that it is easier to review:

"Add **more** mappings ..." is for commits that add mappings to an already supported type.

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 8:13:52 AMJun 27
to include-wh...@googlegroups.com
Comment #1 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

About the commit messages:

I tried to follow some conventions in these commit messages so that it is easier to review:

"Add **more** mappings ..." is for commits that add mappings to an already supported type.
"Add mappings... " is for commits adding a type.

"Removeincorrect mappings ..." removes incorrect mappings (according to the relevant standards).

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 8:14:00 AMJun 27
to include-wh...@googlegroups.com
Comment #1 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

About the commit messages:

I tried to follow some conventions in these commit messages so that it is easier to review:

"Add **more** mappings ..." is for commits that add mappings to an already supported type.
"Add mappings... " is for commits adding a type.

"Remove incorrect mappings ..." removes incorrect mappings (according to the relevant standards).

notifi...@include-what-you-use.org

unread,
Jun 27, 2021, 1:37:56 PMJun 27
to include-wh...@googlegroups.com
Comment #1 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I'm unfortunately without computer for a while, so I'll circle back to this when I've managed to get a hold of a new machine. Thanks for the extensive work!

notifi...@include-what-you-use.org

unread,
Jul 6, 2021, 1:32:33 PMJul 6
to include-wh...@googlegroups.com
Comment #3 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Oh la la! Quite a bit of changes here... Could you rebase on latest master to fix the (unrelated) build error? That way we can see if the test suite is still happy with these mapping changes. Thanks!

notifi...@include-what-you-use.org

unread,
Jul 6, 2021, 2:28:22 PMJul 6
to include-wh...@googlegroups.com
Comment #4 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Okay, expect that for tonight :-)

I've got a few more mappings, this time about macros. Expect a new PR just after this one too ;)

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:07:47 PMJul 7
to include-wh...@googlegroups.com
Comment #5 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

IWYU seems to be unhappy with conflicting mappings for `sys/socket.h` (see test results)

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:36:18 PMJul 7
to include-wh...@googlegroups.com
Comment #6 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Hmmm, I these are the differences for `<sys/socket.h>`:

```
$ git diff iwyu/master..alx/master --color | grep sys/socket
+ { symbol: [ "sockaddr", private, "<sys/socket.h>", private ] },
+ { symbol: [ "socklen_t", private, "<sys/socket.h>", private ] },
+ { symbol: [ "ssize_t", private, "<sys/socket.h>", public ] },
+ { symbol: [ "size_t", private, "<sys/socket.h>", public ] },
+ { "sockaddr", kPrivate, "<sys/socket.h>", kPrivate },
+ { "socklen_t", kPrivate, "<sys/socket.h>", kPrivate },
+ { "ssize_t", kPrivate, "<sys/socket.h>", kPublic },
+ { "size_t", kPrivate, "<sys/socket.h>", kPublic },
```

I checked that these are correct, and I found a mistake in the manual page about `ssize_t` (the types are required by POSIX.1-2008 but not by POSIX.1-2001). But I doubt that is the reason... I'll check the source code of the test.

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:36:57 PMJul 7
to include-wh...@googlegroups.com
Comment #6 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Hmmm, I these are the differences for `<sys/socket.h>`:

```
$ git diff iwyu/master..alx/master --color | grep sys/socket
+ { symbol: [ "sockaddr", private, "<sys/socket.h>", private ] },
+ { symbol: [ "socklen_t", private, "<sys/socket.h>", private ] },
+ { symbol: [ "ssize_t", private, "<sys/socket.h>", public ] },
+ { symbol: [ "size_t", private, "<sys/socket.h>", public ] },
+ { "sockaddr", kPrivate, "<sys/socket.h>", kPrivate },
+ { "socklen_t", kPrivate, "<sys/socket.h>", kPrivate },
+ { "ssize_t", kPrivate, "<sys/socket.h>", kPublic },
+ { "size_t", kPrivate, "<sys/socket.h>", kPublic },
```

I checked that these are correct, and I found a mistake in the manual page about `ssize_t` (the types are required in `<aio.h>` and `<sys/socket.h>` by POSIX.1-2008 but not by POSIX.1-2001). But I doubt that is the reason... I'll check the source code of the test.

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:38:25 PMJul 7
to include-wh...@googlegroups.com
Comment #6 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Hmmm, I these are the differences for `<sys/socket.h>`:

```
$ git diff iwyu/master..alx/master --color | grep sys/socket
+ { symbol: [ "sockaddr", private, "<sys/socket.h>", private ] },
+ { symbol: [ "socklen_t", private, "<sys/socket.h>", private ] },
+ { symbol: [ "ssize_t", private, "<sys/socket.h>", public ] },
+ { symbol: [ "size_t", private, "<sys/socket.h>", public ] },
+ { "sockaddr", kPrivate, "<sys/socket.h>", kPrivate },
+ { "socklen_t", kPrivate, "<sys/socket.h>", kPrivate },
+ { "ssize_t", kPrivate, "<sys/socket.h>", kPublic },
+ { "size_t", kPrivate, "<sys/socket.h>", kPublic },
```

I checked that these are correct, and I found a mistake in the manual page about `ssize_t` (the types are required in `<aio.h>`, `<mqueue.h>`, and `<sys/socket.h>` by POSIX.1-2008 but not by POSIX.1-2001). But I doubt that is the reason... I'll check the source code of the test.

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:45:44 PMJul 7
to include-wh...@googlegroups.com
Comment #7 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

IWYU has a few mechanisms for auto-mapping as well (based on comments and pragmas in libc headers, for example). I wonder if the new mappings somehow conflict with those.

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:47:17 PMJul 7
to include-wh...@googlegroups.com
Comment #7 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Hmm, that's probably the case. Do you know if/how we could fix that? Maybe disable it for identifiers that are explicitly mapped?

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:57:57 PMJul 7
to include-wh...@googlegroups.com
Comment #9 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I grepped `size_t` and other already mapped types to see if there was some exception for them, but I couldn't find anything. Why did it start failing now?

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 3:58:20 PMJul 7
to include-wh...@googlegroups.com
Comment #10 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Oops. Clicked the wrong button

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 4:03:53 PMJul 7
to include-wh...@googlegroups.com
Comment #11 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Maybe the assertion is misguided, not sure. I think you can run with higher verbose level to see some of the mappings unfold, e.g. `IWYU_VERBOSE=6 ./run_iwyu_tests.py sometestname`

notifi...@include-what-you-use.org

unread,
Jul 7, 2021, 5:10:24 PMJul 7
to include-wh...@googlegroups.com
Comment #12 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I couldn't manage to build this project. I get a bunch of errors (due to dependencies). Do you have a full list of dependencies for debian (preferably Sid) or ubuntu (preferably Focal)?
Or if you can test it, could you show the logs here?

Thanks.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 1:45:23 AMJul 8
to include-wh...@googlegroups.com
Comment #13 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I installed a fresh ubuntu focal just now, and I _think_ the only packages I added were inspired by https://github.com/include-what-you-use/include-what-you-use/blob/master/.github/workflows/ci.yml#L23.

That is, you need to add https://apt.llvm.org as a source and then install `llvm-13-dev`, `libclang-13-dev` and `clang-13` (might work without `-13` marker as well). That should be enough for the build instructions in https://github.com/include-what-you-use/include-what-you-use/blob/master/README.md#how-to-build to work.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 2:22:35 AMJul 8
to include-wh...@googlegroups.com
Comment #14 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Ahh, yes, I was missing `libclang-dev`

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 2:45:33 AMJul 8
to include-wh...@googlegroups.com
Comment #15 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

A few news:

- I tried removing the 8 lines involving `<sys/socket.h>`, and then only 1 test fails.
- The verbose level doesn't change the output at all (eventhough it is correctly recognized).

I'll add a commit to remove the sys/socket lines and we can see the new test results.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 2:59:00 AMJul 8
to include-wh...@googlegroups.com
Comment #15 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

A few news:

- I tried removing the 8 lines involving `<sys/socket.h>`, and then only 1 test fails.

- The verbose level doesn't change the output at all (even though it is correctly recognized).

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 2:59:13 AMJul 8
to include-wh...@googlegroups.com
Comment #15 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

A few news:

- I tried removing the 8 lines involving `<sys/socket.h>`, and then only 1 test fails.
- The verbose level doesn't change the output at all (even though it is correctly recognized).

I'll add a commit to remove the `<sys/socket.h>` lines and we can see the new test results.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 5:08:01 PMJul 8
to include-wh...@googlegroups.com
Comment #15 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Now this is failing:

```
105/139 Test #14: cxx.test_badinc ...................................***Failed 6.03 sec
INFO:root:Testing iwyu on tests/cxx/badinc.cc
>>> Running /home/runner/work/include-what-you-use/include-what-you-use/build/bin/include-what-you-use -Xiwyu --verbose=3 -Xiwyu --mapping_file=tests/cxx/badinc.imp -I . tests/cxx/badinc.cc

(tests/cxx/badinc-inl.h has correct #includes/fwd-decls)
tests/cxx/badinc.h:64:7: warning: I2_Class is defined in "tests/cxx/badinc-i2.h", which isn't directly #included.
...
tests/cxx/badinc.h:374:27: warning: I2_TemplateClass::~I2_TemplateClass<FOO> is defined in "tests/cxx/badinc-i2-inl.h", which isn't directly #included.

tests/cxx/badinc.h should add these lines:
#include <stdio.h> // for printf, NULL
#include <set> // for set
#include <vector> // for vector
#include "tests/cxx/badinc-i2-inl.h" // for I2_Class::~I2_Class, I2_TemplateClass::~I2_TemplateClass<FOO>, I2_Class::I2_Class, I2_Class::InlFileFn, I2_Class::InlFileStaticFn, I2_Class::InlFileTemplateFn, I2_TemplateClass::I2_TemplateClass<FOO>, I2_TemplateClass::InlFileTemplateClassFn
#include "tests/cxx/badinc-i2.h" // for I2_Enum, I2_Class, I2_Struct, I21, I2_EnumForTypedefs, I22, I2_TemplateClass, I2_Typedef, TemplateForHClassTplFn (ptr only), I2_MACRO, I2_TypedefOnly_Class (ptr only)

tests/cxx/badinc.h should remove these lines:
- #include <ctype.h> // lines 14-14
- #include <math.h> // lines 16-16
- #include "tests/cxx/badinc-d2.h" // lines 19-19
- class H_ForwardDeclareClass; // lines 22-22
- template <typename T> class I2_TypedefOnly_Class; // lines 28-28

The full include-list for tests/cxx/badinc.h:
#include <errno.h> // for errno
#include <stdio.h> // for printf, NULL
#include <queue> // for queue
#include <set> // for set
#include <string> // for string
#include <vector> // for vector
#include "tests/cxx/badinc-d3.h" // for D3_Enum, D31
#include "tests/cxx/badinc-i2-inl.h" // for I2_Class::~I2_Class, I2_TemplateClass::~I2_TemplateClass<FOO>, I2_Class::I2_Class, I2_Class::InlFileFn, I2_Class::InlFileStaticFn, I2_Class::InlFileTemplateFn, I2_TemplateClass::I2_TemplateClass<FOO>, I2_TemplateClass::InlFileTemplateClassFn
#include "tests/cxx/badinc-i2.h" // for I2_Enum, I2_Class, I2_Struct, I21, I2_EnumForTypedefs, I22, I2_TemplateClass, I2_Typedef, TemplateForHClassTplFn (ptr only), I2_MACRO, I2_TypedefOnly_Class (ptr only)
class Cc_Class; // lines 25-25
class Cc_Struct; // lines 24-24
class H_Class::H_Class_DefinedInI1; // lines 107-107
class H_Class::H_Class_Subdecl; // lines 105-105
class H_Class::H_Class_UnusedSubdecl; // lines 106-106
template <typename T> class H_ScopedPtr; // lines 34-34
---
tests/cxx/badinc.cc:81:35: warning: MACRO_CALLING_I6_FUNCTION is defined in "tests/cxx/badinc-i1.h", which isn't directly #included.
...
tests/cxx/badinc.cc:1845:22: warning: kI1ConstInt is defined in "tests/cxx/badinc-i1.h", which isn't directly #included.

tests/cxx/badinc.cc should add these lines:
#include <ctype.h> // for isascii
#include <stddef.h> // for offsetof
#include <list> // for list
#include "tests/cxx/badinc-i1.h" // for I1_Class, I1_TemplateClass, I1_Enum, I1_ClassPtr, I1_TemplateMethodOnlyClass, I1_TemplateFunction, I1_const_ptr, I1_Struct, kI1ConstInt, I11, I1_Function, i1_GlobalFunction, operator==, I12, OperateOn, I1_MACRO_SYMBOL_WITHOUT_VALUE, I1_NamespaceClass, I1_MACRO_SYMBOL_WITH_VALUE, I1_MACRO_SYMBOL_WITH_VALUE2, I1_MemberPtr, I1_Union, I1_UnnamedStruct, I2_OperatorDefinedInI1Class::operator<<, EmptyDestructorClass, I13, I1_And_I2_OverloadedFunction, I1_Base, I1_Class::NestedStruct, I1_FunctionPtr, I1_NamespaceTemplateFn, I1_Typedef, I1_TypedefOnly_Class, MACRO_CALLING_I6_FUNCTION, H_Class::H_Class_DefinedInI1, I1_I2_Class_Typedef, I1_MACRO_LOGGING_CLASS, I1_MACRO_SYMBOL_WITH_VALUE0, I1_ManyPtrStruct (ptr only), I1_NamespaceStruct, I1_OverloadedFunction, I1_PtrAndUseOnSameLine, I1_PtrDereferenceClass, I1_PtrDereferenceStatic, I1_PtrDereferenceStruct, I1_SiblingClass, I1_StaticMethod, I1_Subclass, I1_SubclassesI2Class, I1_TemplateClass<>::I1_TemplateClass_int, I1_TemplateClassFwdDeclaredInD2 (ptr only), I1_TemplateSubclass, I1_TypedefOnly_Class<>::i, I1_UnusedNamespaceStruct (ptr only), i1_int, i1_int_global, i1_int_global2, i1_int_global2sub, i1_int_global3, i1_int_global3sub, i1_int_global4, i1_int_global4sub, i1_int_globalsub, i1_ns2, i1_ns4, i1_ns5
class D2_Class;
class D2_ForwardDeclareClass;
class D2_Subclass;
class I1_ForwardDeclareClass;
namespace d3_namespace { struct D3_Struct; }
namespace i3_ns1 { namespace i3_ns2 { namespace i3_ns3 { template <typename A, int B> struct I3_ForwardDeclareNamespaceTemplateStruct; } } }
namespace i3_ns1 { namespace { struct I3_UnnamedNamespaceStruct; } }
struct I3_ForwardDeclareStruct;
template <typename A, int B, char C> struct I3_ForwardDeclareTemplateStruct;

tests/cxx/badinc.cc should remove these lines:
- #include <math.h> // lines 59-59
- #include <algorithm> // lines 75-75
- #include <algorithm> // lines 76-76
- #include <clocale> // lines 73-73
- #include <locale> // lines 70-70
- #include "tests/cxx/badinc-d2.h" // lines 64-64
- class Cc_ForwardDeclare_Function::I2_Class; // lines 1005-1005
- class I3_UnusedClass; // lines 162-162
- template <class T = I1_Class, I1_Enum E = I11> class Cc_DeclareOnlyTemplateClass; // lines 309-309

The full include-list for tests/cxx/badinc.cc:
#include "tests/cxx/badinc.h"
#include "tests/cxx/badinc-inl.h"
#include <ctype.h> // for isascii
#include <setjmp.h>
#include <stddef.h> // for offsetof
#include <algorithm> // for find
#include <fstream> // for fstream
#include <list> // for list
#include <string> // for basic_string, string, operator+, basic_string<>::iterator
#include <typeinfo> // for type_info
#include "tests/cxx/badinc-d1.h" // for D1_I1_Typedef, D1_Enum, D1CopyClassFn, D1Function, D1_TemplateClass, D1_CopyClass, D1_Subclass, D11, D1_Class, D1_StructPtr, D1_TemplateStructWithDefaultParam, MACRO_CALLING_I4_FUNCTION
#include "tests/cxx/badinc-d4.h" // for operator<<, D4_ClassForOperator
#include "tests/cxx/badinc-i1.h" // for I1_Class, I1_TemplateClass, I1_Enum, I1_ClassPtr, I1_TemplateMethodOnlyClass, I1_TemplateFunction, I1_const_ptr, I1_Struct, kI1ConstInt, I11, I1_Function, i1_GlobalFunction, operator==, I12, OperateOn, I1_MACRO_SYMBOL_WITHOUT_VALUE, I1_NamespaceClass, I1_MACRO_SYMBOL_WITH_VALUE, I1_MACRO_SYMBOL_WITH_VALUE2, I1_MemberPtr, I1_Union, I1_UnnamedStruct, I2_OperatorDefinedInI1Class::operator<<, EmptyDestructorClass, I13, I1_And_I2_OverloadedFunction, I1_Base, I1_Class::NestedStruct, I1_FunctionPtr, I1_NamespaceTemplateFn, I1_Typedef, I1_TypedefOnly_Class, MACRO_CALLING_I6_FUNCTION, H_Class::H_Class_DefinedInI1, I1_I2_Class_Typedef, I1_MACRO_LOGGING_CLASS, I1_MACRO_SYMBOL_WITH_VALUE0, I1_ManyPtrStruct (ptr only), I1_NamespaceStruct, I1_OverloadedFunction, I1_PtrAndUseOnSameLine, I1_PtrDereferenceClass, I1_PtrDereferenceStatic, I1_PtrDereferenceStruct, I1_SiblingClass, I1_StaticMethod, I1_Subclass, I1_SubclassesI2Class, I1_TemplateClass<>::I1_TemplateClass_int, I1_TemplateClassFwdDeclaredInD2 (ptr only), I1_TemplateSubclass, I1_TypedefOnly_Class<>::i, I1_UnusedNamespaceStruct (ptr only), i1_int, i1_int_global, i1_int_global2, i1_int_global2sub, i1_int_global3, i1_int_global3sub, i1_int_global4, i1_int_global4sub, i1_int_globalsub, i1_ns2, i1_ns4, i1_ns5
#include "tests/cxx/badinc2.c"
class D2_Class;
class D2_ForwardDeclareClass;
class D2_Subclass;
class ForwardDeclareOnlyClass; // lines 167-167
class ForwardDeclareOnlyForTypedefClass; // lines 172-172
class I1_ForwardDeclareClass;
class I3_ForwardDeclareClass; // lines 159-159
class MacroClass; // lines 170-170
namespace d3_namespace { struct D3_Struct; }
namespace i3_ns1 { namespace i3_ns2 { namespace i3_ns3 { struct I3_ForwardDeclareNamespaceStruct; } } } // lines 164-164
namespace i3_ns1 { namespace i3_ns2 { namespace i3_ns3 { template <typename A, int B> struct I3_ForwardDeclareNamespaceTemplateStruct; } } }
namespace i3_ns1 { namespace { struct I3_UnnamedNamespaceStruct; } }
struct Cc_C_Struct; // lines 182-182
struct I3_ForwardDeclareStruct;
template <class T> struct Cc_OnlySpecializedStruct; // lines 177-177
template <typename A, int B, char C> struct I3_ForwardDeclareTemplateStruct;
template <typename T> struct I3_SimpleForwardDeclareTemplateStruct; // lines 160-161
---

Traceback (most recent call last):
File "run_iwyu_tests.py", line 127, in <module>
exit(TestIwyuOnRelevantFiles(runner_args.run_test_file))
File "run_iwyu_tests.py", line 64, in TestIwyuOnRelevantFiles
iwyu_test_util.TestIwyuOnRelativeFile(filename, files_to_check, verbose=True)
File "/home/runner/work/include-what-you-use/include-what-you-use/iwyu_test_util.py", line 476, in TestIwyuOnRelativeFile
raise AssertionError(''.join(failures))
AssertionError:
tests/cxx/badinc.cc:1098: Unexpected diagnostic:
va_list is defined in <stdio.h>, which isn't directly #included.

tests/cxx/badinc.cc:1098: Unmatched regex:
va_list is...*<stdarg.h>

Unexpected summary diffs for tests/cxx/badinc.cc:
+++
@@ -1,6 +1,5 @@
tests/cxx/badinc.cc should add these lines:
#include <ctype.h>
-#include <stdarg.h>
#include <stddef.h>
#include <list>
#include "tests/cxx/badinc-i1.h"
@@ -30,7 +29,6 @@
#include "tests/cxx/badinc-inl.h"
#include <ctype.h> // for isascii
#include <setjmp.h>
-#include <stdarg.h> // for va_list
#include <stddef.h> // for offsetof
#include <algorithm> // for find
#include <fstream> // for fstream
---
```

This test is outdated. Now that we added the information that `va_list` is also defined by `<stdio.h>`, the test should be updated.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 5:23:44 PMJul 8
to include-wh...@googlegroups.com
Comment #17 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Locally it is passing the tests for me.

We have two problems now:

- Investigate why is `<sys/socket.h>` problematic.
- Fix the test about `va_list`.

But that's not urgent.

notifi...@include-what-you-use.org

unread,
Jul 8, 2021, 5:24:32 PMJul 8
to include-wh...@googlegroups.com
Comment #17 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Locally it is passing the tests for me.

I removed the lines about `va_list` from the test.


We have two problems now:

- Investigate why is `<sys/socket.h>` problematic.

- Fix the test about `va_list` to be able to reintroduce it

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 1:39:55 AMJul 9
to include-wh...@googlegroups.com
Comment #18 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

@alejandro-colomar Cool!

That test is the original test, so it covers basically everything in a single place. Later developments have added one test per mechanism to avoid cross-pollution.

It feels wrong to go to `<stdio.h>` as a primary candidate for `va_list`, but if `<stdio.h>` is already available in the translation unit and `<stdarg.h>` isn't, it makes more sense. I'll see if I can carve out some time to prove that one way or the other. Unless you beat me to it :-)

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 4:19:19 AMJul 9
to include-wh...@googlegroups.com
Comment #18 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I removed the lines about `va_list` from the test.

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 4:19:37 AMJul 9
to include-wh...@googlegroups.com
Comment #18 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

I removed the lines about `va_list` from the test.
Locally it is passing the tests for me.

We have two problems now:

- Investigate why `<sys/socket.h>` is problematic.

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 11:13:50 AMJul 9
to include-wh...@googlegroups.com
Comment #19 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

`badinc.cc` includes `<stdio.h>` indirectly, so it's correct that it's suggested for `va_list` after your mapping changes.

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 11:28:14 AMJul 9
to include-wh...@googlegroups.com
Comment #20 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

And for some reason you've made the `sys/socket.h` symbol mappings private-to-private, where all others are private-to-public: https://github.com/include-what-you-use/include-what-you-use/pull/930/commits/888a91286e18c75ffe4d97b99d1659daaa8e42b8#diff-2ab58a9a3375bbbdb027d23af0a61ae0c5880be9d44ecaa4157b7693124e4f90R135

Changing those to public instead makes the test suite pass without assertions.

Once that's functionally fixed, I can start trying to review the individual commits. Thanks!

notifi...@include-what-you-use.org

unread,
Jul 9, 2021, 12:31:14 PMJul 9
to include-wh...@googlegroups.com
Comment #21 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

>
>
> And for some reason you've made the `sys/socket.h` symbol mappings private-to-private, where all others are private-to-public: [888a912#diff-2ab58a9a3375bbbdb027d23af0a61ae0c5880be9d44ecaa4157b7693124e4f90R135](https://github.com/include-what-you-use/include-what-you-use/commit/888a91286e18c75ffe4d97b99d1659daaa8e42b8#diff-2ab58a9a3375bbbdb027d23af0a61ae0c5880be9d44ecaa4157b7693124e4f90R135)

Good catch!
I guess I replaced `bits/socket` in this line, and then copy&pasted:

```
{ symbol: [ "socklen_t", private, "<bits/socket.h>", private ] },
```


>
> Changing those to public instead makes the test suite pass without assertions.
>
> Once that's functionally fixed, I can start trying to review the individual commits. Thanks!

Great! Let me amend those commits.

notifi...@include-what-you-use.org

unread,
Jul 10, 2021, 3:14:12 PMJul 10
to include-wh...@googlegroups.com
Comment #22 on pull request 930 by alejandro-colomar: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

Could you please provide a code suggestion for `va_list`? I'm not sure I know what should be done.

notifi...@include-what-you-use.org

unread,
Jul 10, 2021, 3:20:50 PMJul 10
to include-wh...@googlegroups.com
Comment #23 on pull request 930 by kimgr: Add (and fix some) mappings for C / POSIX types
https://github.com/include-what-you-use/include-what-you-use/pull/930

> Could you please provide a code suggestion for `va_list`? I'm not sure I know what should be done.

I had something like this in mind:
```diff
diff --git a/tests/cxx/badinc.cc b/tests/cxx/badinc.cc
index e58a98f..24b4a2d 100644
--- a/tests/cxx/badinc.cc
+++ b/tests/cxx/badinc.cc
@@ -1094,8 +1094,10 @@ int main() {
// IWYU: I1_PtrDereferenceClass needs a declaration
I1_PtrDereferenceClass* local_i1_ptrdereference_class = 0;
int x;
- // IWYU: va_list is...*<stdarg.h>
- va_list vl; // in gcc, va_list is an internal type, so this tests <built-in>
+ // va_list is normally in stdarg.h, but we already have stdio.h available,
+ // so mappings will source it from there.
+ // IWYU: va_list is...*<stdio.h>
+ va_list vl;
D1_I1_Typedef d1_i1_typedef;
// IWYU: i1_int is...*badinc-i1.h
int vararray[i1_int];
@@ -1859,7 +1861,6 @@ int main() {


tests/cxx/badinc.cc should add these lines:
#include <ctype.h>
-#include <stdarg.h>
#include <stddef.h>
#include <list>
#include "tests/cxx/badinc-i1.h"

@@ -1889,7 +1890,6 @@ The full include-list for tests/cxx/badinc.cc:

#include "tests/cxx/badinc-inl.h"
#include <ctype.h> // for isascii
#include <setjmp.h>
-#include <stdarg.h> // for va_list
#include <stddef.h> // for offsetof
#include <algorithm> // for find
#include <fstream> // for fstream

```

It fails on master and succeeds on your branch, as far as I can tell.

Reply all
Reply to author
Forward
0 new messages