Re: Issue 929 in include-what-you-use: C / POSIX types headers: which headers are to be supported?

0 views
Skip to first unread message

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

unread,
Jun 27, 2021, 6:13:45 AMJun 27
to include-wh...@googlegroups.com
Comment #2 on issue 929 by kimgr: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

I'm not sure how the current set was chosen, but it seems to me there's some element of good taste - relying on sys/msg.h for `time_t` would feel slightly off.

I don't think minimum/maximum is the right tradeoff, rather just what 'makes sense' conceptually and practically. For example, `size_t` is enough like a builtin type that we have it mapped to a number of system headers. `time_t` is a little more special-purpose, so it's mapped more specifically.

Sorry I don't have a clear answer.

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

unread,
Jun 27, 2021, 7:02:21 AMJun 27
to include-wh...@googlegroups.com
Comment #3 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

> I'm not sure how the current set was chosen, but it seems to me there's some element of good taste - relying on sys/msg.h for `time_t` would feel slightly off.

Hmm. IMHO, it depends on 2 points of view. If you want a more strict (more iqwyu-like) approach, I'd only allow `<sys/types.h>` or `<time.h>` for `time_t`.

But then there's the other approach that POSIX mandates `<sys/msg.h>` to define `time_t`, and while I would never accept someone relying on that header just for `time_t`, maybe he's using features of that header, and they require using `time_t` too, so `<sys/msg.h>` should be enough, and IWYU shouldn't complain...

I think IWYU should allow for the second one, as it's standards compliant, and (eventhough a bit corrner casy) may be relevant for someone. And then the good taste is moved to the programmer instead.


>
> I don't think minimum/maximum is the right tradeoff, rather just what 'makes sense' conceptually and practically. For example, `size_t` is enough like a builtin type that we have it mapped to a number of system headers. `time_t` is a little more special-purpose, so it's mapped more specifically.
>
> Sorry I don't have a clear answer.

I'm preparing a big PR with the inclusion of `clockid_t` and all other types defined in `system_data_types(7)`. It has additions, and some removals. We'll be able to discuss it better with the code. Wait for a few minutes/hours for it :-)

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

unread,
Jun 27, 2021, 8:03:25 AMJun 27
to include-wh...@googlegroups.com
Comment #4 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

Fix this inconsistency in this PR: https://github.com/include-what-you-use/include-what-you-use/pull/930

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

unread,
Jun 27, 2021, 8:03:34 AMJun 27
to include-wh...@googlegroups.com
Comment #4 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

Fix for this inconsistency in this PR: https://github.com/include-what-you-use/include-what-you-use/pull/930

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

unread,
Jun 27, 2021, 8:07:11 AMJun 27
to include-wh...@googlegroups.com
Comment #4 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

> I'm not sure how the current set was chosen, but it seems to me there's some element of good taste - relying on sys/msg.h for `time_t` would feel slightly off.

Hmm. IMHO, it depends on 2 points of view. If you want a more strict (more iwyu-like) approach, I'd only allow `<sys/types.h>` or `<time.h>` for `time_t`.

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

unread,
Jun 27, 2021, 8:07:53 AMJun 27
to include-wh...@googlegroups.com
Comment #4 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

> I'm not sure how the current set was chosen, but it seems to me there's some element of good taste - relying on sys/msg.h for `time_t` would feel slightly off.

Hmm. IMHO, it depends on 2 points of view. If you want a more strict (more iwyu-like) approach, I'd only allow `<sys/types.h>` or `<time.h>` for `time_t`.

But then there's the other approach that POSIX mandates `<sys/msg.h>` to define `time_t`, and while I would never accept someone relying on that header just for `time_t`, maybe he's using features of that header, and they require using `time_t` too, so `<sys/msg.h>` should be enough, and IWYU shouldn't complain...

I think IWYU should allow for the second one, as it's standards compliant, and (eventhough a bit corner casy) may be relevant for someone. And then the good taste is moved to the programmer instead.

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

unread,
Jun 27, 2021, 1:36:11 PMJun 27
to include-wh...@googlegroups.com
Comment #5 on issue 929 by kimgr: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

Yep, good points! I think the behavior we want is that iwyu _allows_ these 'secondary' headers, but never _suggests_ them.

The tricky case is when header a.h uses eg time_t, but only includes b.h, which transitively includes time.h. Then iwyu will find time_t in some private glibc header, and the mappings should lead back to time.h, not e.g. sys/msg.h.

As I understand it, iwyu should pick the first mapping in that case, so as long as they're properly ordered, everything should work out.

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

unread,
Jun 27, 2021, 1:51:31 PMJun 27
to include-wh...@googlegroups.com
Comment #6 on issue 929 by alejandro-colomar: C / POSIX types headers: which headers are to be supported?
https://github.com/include-what-you-use/include-what-you-use/issues/929

> Yep, good points! I think the behavior we want is that iwyu _allows_ these 'secondary' headers, but never _suggests_ them.

Completely agree. That was also the decision when writing the manual page system_data_types(7).


>
> The tricky case is when header a.h uses eg time_t, but only includes b.h, which transitively includes time.h. Then iwyu will find time_t in some private glibc header, and the mappings should lead back to time.h, not e.g. sys/msg.h.
>
> As I understand it, iwyu should pick the first mapping in that case, so as long as they're properly ordered, everything should work out.

Okay.

Reply all
Reply to author
Forward
0 new messages