Re: Issue 945 in include-what-you-use: IWYU suggests forward declaration for unordered map's mapped type (with libstdc++)

113 views
Skip to first unread message

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

unread,
Oct 1, 2021, 12:16:34 PM10/1/21
to include-wh...@googlegroups.com
Comment #1 on issue 945 by cieplik: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

@kimgr, thanks for your answer! I've been wondering if IWUY should always keep the definition of `Foo`, no matter the library. According to https://stackoverflow.com/a/13089641, incomplete types are explicitly allowed for `forward_list`, `list`, and `vector` only.


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

unread,
Oct 1, 2021, 1:50:15 PM10/1/21
to include-wh...@googlegroups.com
Comment #2 on issue 945 by kimgr: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

Of course it should do the right thing. But IWYU doesn't have logic like "if libstdc++ do this, else do that". Rather, the libraries are so different in their implementation that they trigger different paths in IWYU, e.g. "a member of a type that is a typedef inside a template" vs "a member of a type that is a template argument in a base class made visible with a using decl".

Our rules around templates are trying to make sense, but they don't always succeed.


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

unread,
Oct 4, 2021, 10:35:43 AM10/4/21
to include-wh...@googlegroups.com
Comment #3 on issue 945 by cieplik: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

> IWYU doesn't have logic like "if libstdc++ do this, else do that".

That makes sense and I'm not suggesting it should be changed. What I'm trying to say is, in cases where the implementation is free to make choices, shouldn't we at least have a compatibility switch that would disable unsafe transformations (from Standard point of view)? So for the sample I posted, even though libc++ seems to compile fine with incomplete types, IWYU wouldn't suggest them as Standard in this case only makes guarantees about `forward_list`, `list`, and `vector`.

Otherwise, the only way to tackle this seems to be with lots of `// IWYU pragma: keep`?


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

unread,
Oct 4, 2021, 10:45:42 AM10/4/21
to include-wh...@googlegroups.com
Comment #4 on issue 945 by cieplik: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

> IWYU doesn't have logic like "if libstdc++ do this, else do that".

That makes sense and I'm not suggesting it should be changed. What I'm trying to say is, in cases where the implementation is free to make choices, shouldn't we at least have a compatibility switch that would disable unsafe transformations (from Standard point of view)? So for the sample I posted, even though libstdc++ seems to compile fine with incomplete types, IWYU wouldn't suggest them as Standard in this case only makes guarantees about `forward_list`, `list`, and `vector`.

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

unread,
Oct 4, 2021, 3:22:03 PM10/4/21
to include-wh...@googlegroups.com
Comment #4 on issue 945 by kimgr: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

I still think there's some disconnect between how IWYU works, and what you're describing.

IWYU does not permute the `#include` list until the program compiles with the minimal set. Rather, it looks at all uses in the main file of a compilation unit, and tries to attribute them to the right declaration location (whether inside itself, its associated header (`foo.h` for `foo.cpp`) or a header file in its transitive `#include` closure).

Note that it also pulls some tricks with templates to look at all uses in a template definition vs all uses in a template _instantiation_, and attribute them to the right place.

And it tries to understand what's a full use vs forward-declare use, something that's surprisingly non-obvious.

Once all uses and their corresponding declaration location are collected, they're run through a so-called mapping, to resolve things like `<bits/vector.tcc> => <vector>` or `std::string => <string>`.

Finally the mapped desired headers and forward-declarations are compared to the main file's actual headers/fwd-decls, and the difference is reported.

(note: I don't think it's exactly this clear-cut, but it really should be :))

So when `Foo` is mentioned as a forward-declaration with one standard library implementation and a full use with another, that indicates that IWYU's template instantiation logic or full vs fwd-decl analysis has made a wrong turn somewhere. Remember, from IWYU's point-of-view, `unordered_map` is just C++ code, it doesn't know it's part of the standard library until mapping-time, if ever.

That's why I'm curious about comparative logs from the two cases, to see how the ASTs are different, and how IWYU's analysis differs between the two.

And that's why I don't really see how to "disable unsafe transformations" -- IWYU doesn't really have a notion of safety, it assumes its analysis is 100% correct and lets it play out. And I'm not sure how to phrase any hints to it in this scenario...

As for short-term wins, `// IWYU pragma: keep` is probably your best bet.


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

unread,
Oct 4, 2021, 4:02:37 PM10/4/21
to include-wh...@googlegroups.com
Comment #5 on issue 945 by kimgr: IWYU suggests forward declaration for unordered_map's mapped_type (with libstdc++)
https://github.com/include-what-you-use/include-what-you-use/issues/945

I did an experiment and collected logs from your reduced testcase above.

There is some difference in implementation that causes a forward-decl use in one case, and full use in the other, so that explains why there is a difference.

But thinking about this and reading your suggestions again, I understand what you're requesting: the standard explicitly says only `forward_list`, `list` and `vector` are OK with an incomplete type. If we could detect that a use is NOT reported in the context of an instantiation of one of them, we could force IWYU to require a full use to achieve stable results independent of actual standard library being used.

I don't have a good idea for how to do that. Hopefully there's some way to nudge the analysis the right way for more consistency.


Reply all
Reply to author
Forward
0 new messages