Re: Issue 496 in include-what-you-use: Transforming <> include into ""

22 views
Skip to first unread message

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

unread,
Mar 29, 2021, 4:12:57 PM3/29/21
to include-wh...@googlegroups.com
Comment #23 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

I would also love to see it configurable. I am providing a header-only library so my files have to be included with `<...>`. Using `-isystem` is not an option for me because this silences some warnings in a compiler and I want to be aware of any issue with my code and fix it ASAP.

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

unread,
Mar 30, 2021, 2:46:21 PM3/30/21
to include-wh...@googlegroups.com
Comment #24 on issue 496 by johnmcfarlane: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

C++ Core Guideline on the subject in case it helps: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-incform

I'm considering trying out this util but not until it reflects how POSIX and MSVC work.

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

unread,
Mar 31, 2021, 2:23:19 AM3/31/21
to include-wh...@googlegroups.com
Comment #25 on issue 496 by kimgr: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

@johnmcfarlane Nice, this is the first mildly authoritative writing I see on the subject. It sounds to me like a quote-style could be passed on the command-line, e.g. `--quote_style=(google|cppcore|always_angled)` or something like that. I think those are the three major styles I've seen (and maybe one additional `whatever`, which is typically the most prevalent :-)). And `cppcore`, in that case, would make a good default.

The only open question is whether it's easy/possible to detect whether includer and includee are locally relative in all the strange cases we conjure up includes.

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

unread,
Mar 31, 2021, 4:42:34 AM3/31/21
to include-wh...@googlegroups.com
Comment #26 on issue 496 by mbrucher: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

The core guidelines basically states that if you can use <> without changing the "argument", then you should use it, and I think that's a good thing. I don't think `always_angled` can work though, without changing something and I'd rather have the user change paths if needed than IWYU. It's very easy to get the wrong include in for whatever reason (CLion does that sometimes, and at least in the codebase I have, it's always wrong).

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

unread,
Mar 31, 2021, 5:14:35 AM3/31/21
to include-wh...@googlegroups.com
Comment #27 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

> The only open question is whether it's easy/possible to detect whether includer and includee are locally relative in all the strange cases we conjure up includes.

I do not think that should be checked in this way. For example, in my library, I deliberately use `<units/....>` to refer to all headers as they will be exported in the interface and used by the customers. So you should not check if the source and header file are in the same project (relative to each other) or not.

I think that a better algorithm would be to:
1. Take the current directory of the implementation (`*.cpp`) file -> `CPP_dir`.
2. Take the header file path as declared by the user -> `H_path`.
3. If the `CPP_dir + / + H_path` header file exists than suggest quotes.
4. Otherwise, suggest angled braces.

I think that the bigger issue is not with validating what is there but with suggestions of what is missing. How to know if IWYU should suggest quotes or angled braces for missing headers? This is where a configuration option would be useful.

Additionally, if I decide one way in the code and header file is valid and can be found, IWYU should probably not force me to change it as it is doing now (asks me to change all angled braces to quotes with exactly the same content).

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

unread,
Mar 31, 2021, 5:16:07 AM3/31/21
to include-wh...@googlegroups.com
Comment #28 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

BTW, see this one for an example: https://github.com/mpusz/units/blob/master/example/avg_speed.cpp.

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

unread,
Mar 31, 2021, 5:17:23 AM3/31/21
to include-wh...@googlegroups.com

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

unread,
Mar 31, 2021, 6:26:08 AM3/31/21
to include-wh...@googlegroups.com
Comment #29 on issue 496 by johnmcfarlane: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

@mpusz


> For example, in my library, I deliberately use <units/....> to refer to all headers as they will be exported in the interface and used by the customers. So you should not check if the source and header file are in the same project (relative to each other) or not.

It sounds like your project would follow the `--quote_style=always_angled`. So the question does not apply to your project.

For `--quote_style=cppcore`, thinking out loud:

```python
if the included file is found under a header search path:
if the including file is a source file:
use <>
else if the including file is a header file:
if the including file is found under the same header search path as the included file
use ""
else
use <>
else
use ""
```

Not sure that's correct or complete though. It might be worth thinking about how one might write a Clang-Tidy rule enforcing rule SF.12 and working back from there.

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

unread,
Mar 31, 2021, 6:30:47 AM3/31/21
to include-wh...@googlegroups.com
Comment #30 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

> It sounds like your project would follow the --quote_style=always_angled. So the question does not apply to your project.

Well, not really. It applies only to the "library-part" of the project. It will not work for usage examples:

https://github.com/mpusz/units/blob/309da80c32c9c48c43f3daa6ec8db31d9075a4e5/example/glide_computer_example.cpp#L23-L27

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

unread,
Mar 31, 2021, 6:35:11 AM3/31/21
to include-wh...@googlegroups.com
Comment #31 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

I am not sure about the following:

> ```

> if the including file is found under the same header search path as the included file
> use ""
> else
> use <>

> ```

Does this mean that `<vector>` should include `"bits/stl_vector.h"` rather than `<bits/stl_vector.h>`?

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

unread,
Mar 31, 2021, 8:07:45 AM3/31/21
to include-wh...@googlegroups.com
Comment #32 on issue 496 by johnmcfarlane: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

I'm not sure that GCC follows the CCG. If it did, then yes, assuming it's not doing something special (e.g. providing some kind of preprocessor customisation point) then yes, `""` would make it slightly harder for other search paths containing a file called _bits/stl_vector.h_ to break things.

Assuming that

* the standard library was an ordinary library, and
* the header search paths were provided using `-isystem /bogus/library -isystem /usr/include/c++/10`, and
* there was a file, _/bogus/library/bits/stl_vector.h_

then you can see how using `<>` would break `std::vector`! That's one reason for the guideline. However, that's a rather far-fetched example. I think most implementers pull in their standard library files via a different method and the rules for that library are different to most other libraries. (For starters, `<vector>` doesn't even need to be a file at all.)

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

unread,
Mar 31, 2021, 11:47:01 AM3/31/21
to include-wh...@googlegroups.com
Comment #33 on issue 496 by mpusz: Transforming <> include into ""
https://github.com/include-what-you-use/include-what-you-use/issues/496

What I meant by the above is that:
1. External libraries should be included by dependents with `<>`
2. External libraries should refer to their implementation details with `<>`
3. Those external libraries have their own repositories/projects where they are implemented and tested
- in those projects, they should not be included with `-isystem` because this disables some compiler warnings which means that some bugs can leak to production
- their headers should also be included with `<>` at least in the usage examples but probably also in unit tests (for consistency)
- if there are any headers specific to unit tests or examples (not part of the exported library) they should be included with `""` when provided as a relative path and with `<>` when provided as an "absolute" path.

Reply all
Reply to author
Forward
0 new messages