Issue 892 in include-what-you-use: Qt scoped includes
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
Consider a file that uses `QString` and therefore its header could look like
```cxx
#include <QString>
```
Using a mapping file for Qt generated via https://github.com/include-what-you-use/include-what-you-use/blob/master/mapgen/iwyu-mapgen-qt.py this works as expected.
There is however another way of including `QString` that is equally feasible:
```cxx
#include <QtCore/QString>
```
In this case iwyu suggests to remove this include statement and instead propose to include `<QString>`. This is obviously not what one would expect.
I suspect that in order to fix this, the mappings script has to be updated to somehow tell iwyu that `QtCore/QString` and `QString` are one and the same thing.
Or more broadly: iwyu should take into account that different include statements can actually be the same if the same directory is accessible through multiple entries in the search-path (which is what I assume is the case for Qt that seems to put the `qt5` directory as well as its subdirectories (e.g. `qt5/QtCore`) in the search path.
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
> I suspect that in order to fix this, the mappings script has to be updated to somehow tell iwyu that QtCore/QString and QString
> are one and the same thing.
Yes.
> Or more broadly: iwyu should take into account that different include statements can actually be the same if the same
> directory is accessible through multiple entries in the search-path (which is what I assume is the case for Qt that seems
> to put the qt5 directory as well as its subdirectories (e.g. qt5/QtCore) in the search path.
That logic exists, and iwyu prefers the shortest spelling.
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
> Yes
Any hints on how that'd work? :)
> That logic exists, and iwyu prefers the shortest spelling.
Ah okay. I guess that's fine for suggesting missing includes but maybe there is a way to check whether the include suggestion made is actually already in there just in a longer variant?
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
@Krzmbrzl I think if you wanted them to be considered equivalent you could set up a manual override in the mapping script that generates a public-to-public mapping from QString -> QtCore/QString, something like:
```diff
diff --git a/mapgen/iwyu-mapgen-qt.py b/mapgen/iwyu-mapgen-qt.py
index 0fca4ad..2d8935f 100755
--- a/mapgen/iwyu-mapgen-qt.py
+++ b/mapgen/iwyu-mapgen-qt.py
@@ -69,6 +69,12 @@ QOBJECT_SYMBOLS = [
]
+QTCORE_EQUIV = [
+ ("QString", "QtCore/QString"),
+ # ... etc...
+]
+
+
class QtHeader(object):
""" Carry data associated with a Qt header """
def __init__(self, headername):
@@ -86,7 +92,7 @@ class QtHeader(object):
return self._private_headers
-def generate_imp_lines(symbols_map, includes_map):
+def generate_imp_lines(symbols_map, includes_map, equiv_map):
""" Generate json-formatted strings in .imp format.
This should ideally return a jsonable structure instead, and use json.dump
@@ -113,6 +119,12 @@ def generate_imp_lines(symbols_map, includes_map):
yield jsonline({"include": [map_from, "private", map_to, "public"]},
indent=2)
+ for base, equiv in equiv_map:
+ map_from = r'@["<]%s[">]' % (base)
+ map_to = "<" + equiv + ">"
+ yield jsonline({"include": [map_from, "public", map_to, "public"]},
+ indent=2)
+
def add_mapping_rules(header, symbols_map, includes_map):
""" Add symbol and include mappings for a Qt module. """
@@ -126,6 +138,7 @@ def main(qtroot):
symbols_map = []
includes_map = []
deferred_headers = []
+ qtcore_equiv_map = []
# Add manual overrides.
symbols_map += [("qDebug", "QtGlobal")]
@@ -153,7 +166,7 @@ def main(qtroot):
# Print mappings
print(OUTFILEHDR)
print("[")
- print(",\n".join(generate_imp_lines(symbols_map, includes_map)))
+ print(",\n".join(generate_imp_lines(symbols_map, includes_map, QTCORE_EQUIV)))
print("]")
return 0
```
If there's a systematic way to detect those (e.g. for every module header, check if there's a corresponding header under QtCore), that might even make sense for mainline.
> Ah okay. I guess that's fine for suggesting missing includes but maybe there is a way to check whether the include suggestion
> made is actually already in there just in a longer variant?
That might be possible, but I'm not entirely sure in which pass what happens there. I think sometimes lookups are made before we know the full set of included headers.
Generally IWYU wants there to be _one_ canonical header for each symbol, and uses mappings as a last resort to make sure it's the right one. Mappings are there as a crutch to help soften that policy.
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
Thanks for the hint :+1:
> If there's a systematic way to detect those (e.g. for every module header, check if there's a corresponding header under QtCore), that might even make sense for mainline.
Well (at least on my system) it seems like the equivalence stuff is directly reflected by the directory structure of the Qt include directory. So taking that into account could be enough to figure that out.
I'll see if I find the time to look into that...
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
Note that after having done some more research, I think I may have to revoke my initial statement:
> There is however another way of including QString that is equally feasible:
It seems like this is not actually an officially supported way of including Qt headers. It's definitely possible to do so and it seems Qt does that internally, but external code seems to be instructed to prefer the non-scoped includes instead :thinking: :eyes:
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
@Krzmbrzl Do you have a link to that recommendation? Might be nice to put in as a reference in the mapgen/iwyu-mapgen-qt.py script.
notifi...@include-what-you-use.org
https://github.com/include-what-you-use/include-what-you-use/issues/892
Unfortunately I did not find an explicit recommendation. I only found https://stackoverflow.com/questions/43276777/cant-include-as-qtcore-qstring and all of the docs for the Qt classes suggest including the headers directly. See e.g. https://doc.qt.io/qt-5/qstring.html (at the top)