Redfinition error with -fcxx-modules

[torarnv]

I’m setting up Clang module maps for Qt, and hitting an error when trying to use the module from plain C++. Without -fcxx-modules things build fine:

❯ QT_DIR=/Users/torarne/build/qt/6.x && clang++ -c -std=c++17 -fmodules -F$QT_DIR/qtbase/lib -I$QT_DIR/qtbase/lib/QtCore.framework/Headers main.cpp

but with -fcxx-modules I get a redefinition error that points to the exact same file and line:

❯ QT_DIR=/Users/torarne/build/qt/6.x && clang++ -c -std=c++17 -fmodules -fcxx-modules -F$QT_DIR/qtbase/lib -I$QT_DIR/qtbase/lib/QtCore.framework/Headers main.cpp
In file included from main.cpp:3:
/Users/torarne/build/qt/6.x/qtbase/lib/QtCore.framework/Headers/qstring.h:64:30: error: redefinition of 'treat_as_integral_arg'
   64 | template <typename T> struct treat_as_integral_arg : std::false_type {};
      |                              ^
/Users/torarne/build/qt/6.x/qtbase/lib/QtCore.framework/Headers/qstring.h:64:30: note: previous definition is here
   64 | template <typename T> struct treat_as_integral_arg : std::false_type {};

What’s going on here? 🤔

[llvmbot]

@llvm/issue-subscribers-clang-modules

Author: Tor Arne Vestbø (torarnv)

I’m setting up Clang module maps for Qt, and hitting an error when trying to use the module from plain C++. Without `-fcxx-modules` things build fine:

❯ QT_DIR=/Users/torarne/build/qt/6.x && clang++ -c -std=c++17 -fmodules -F$QT_DIR/qtbase/lib -I$QT_DIR/qtbase/lib/QtCore.framework/Headers main.cpp

but with -fcxx-modules I get a redefinition error that points to the exact same file and line:

❯ QT_DIR=/Users/torarne/build/qt/6.x && clang++ -c -std=c++17 -fmodules -fcxx-modules -F$QT_DIR/qtbase/lib -I$QT_DIR/qtbase/lib/QtCore.framework/Headers main.cpp
In file included from main.cpp:3:
/Users/torarne/build/qt/6.x/qtbase/lib/QtCore.framework/Headers/qstring.h:64:30: error: redefinition of 'treat_as_integral_arg'
   64 | template <typename T> struct treat_as_integral_arg : std::false_type {};
      |                              ^
/Users/torarne/build/qt/6.x/qtbase/lib/QtCore.framework/Headers/qstring.h:64:30: note: previous definition is here
   64 | template <typename T> struct treat_as_integral_arg : std::false_type {};

What’s going on here? 🤔

[dwblaikie]

A reduced test case would likely be helpful before action can really be taken here - a handful of source files, a series of build commands that fails with modules and passes with modules disabled.

"try all of Qt" is probably out of scope for developers to do the reduction work here

(& I'm sorry - these things are hard to debug and we don't have good tools for reducing modules failures (unlike we do for non-modular failures with tools like cvise, etc - which can be applied to modules, but requires more work/is missing some tools to make them more convenient)

[torarnv]

@dwblaikie Fully understand the need for a reduced test case 😄

I've attached a copy of a pre-built QtCore.framework.

❯ export FW_DIR=/path/to/fws # Extraction location 

# <QtCore/qstring.h> syntax works, as expected
❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$FW_DIR <(echo '#include <QtCore/qstring.h>')

# <qstring.h> syntax, commonly used in Qt user projects does not, due to missing include path
❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$FW_DIR <(echo '#include <qstring.h>')
/dev/fd/16:1:10: fatal error: 'qstring.h' file not found
    1 | #include <qstring.h>
      |          ^~~~~~~~~~~
1 error generated.

# Which is why builds often pass an explicit `-I` to the framework headers, resulting in:
❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$FW_DIR -I$FW_DIR/QtCore.framework/Headers <(echo '#include <qstring.h>')
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:65:30: error: redefinition of 'treat_as_integral_arg'
   65 | template <typename T> struct treat_as_integral_arg : std::false_type {};
      |                              ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:65:30: note: previous definition is here
   65 | template <typename T> struct treat_as_integral_arg : std::false_type {};
      |                              ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:66:20: error: redefinition of 'treat_as_integral_arg<unsigned short>'
   66 | template <> struct treat_as_integral_arg<unsigned short> : std::true_type {};
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:66:20: note: previous definition is here
   66 | template <> struct treat_as_integral_arg<unsigned short> : std::true_type {};
      |                    ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:67:20: error: redefinition of 'treat_as_integral_arg<short>'
   67 | template <> struct treat_as_integral_arg<  signed short> : std::true_type {};
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:67:20: note: previous definition is here
   67 | template <> struct treat_as_integral_arg<  signed short> : std::true_type {};
      |                    ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:68:20: error: redefinition of 'treat_as_integral_arg<unsigned char>'
   68 | template <> struct treat_as_integral_arg<unsigned  char> : std::true_type {};
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:68:20: note: previous definition is here
   68 | template <> struct treat_as_integral_arg<unsigned  char> : std::true_type {};
      |                    ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:69:20: error: redefinition of 'treat_as_integral_arg<signed char>'
   69 | template <> struct treat_as_integral_arg<  signed  char> : std::true_type {};
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:69:20: note: previous definition is here
   69 | template <> struct treat_as_integral_arg<  signed  char> : std::true_type {};
      |                    ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:77:27: error: redefinition of 'isLatin1'
   77 | constexpr bool QtPrivate::isLatin1(QLatin1StringView) noexcept
      |                           ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:77:27: note: previous definition is here
   77 | constexpr bool QtPrivate::isLatin1(QLatin1StringView) noexcept
      |                           ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:83:18: error: redefinition of 'compare'
   83 | int QStringView::compare(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                  ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:83:18: note: previous definition is here
   83 | int QStringView::compare(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                  ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:91:19: error: redefinition of 'startsWith'
   91 | bool QStringView::startsWith(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:91:19: note: previous definition is here
   91 | bool QStringView::startsWith(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:99:19: error: redefinition of 'endsWith'
   99 | bool QStringView::endsWith(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:99:19: note: previous definition is here
   99 | bool QStringView::endsWith(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:107:24: error: redefinition of 'indexOf'
  107 | qsizetype QStringView::indexOf(QLatin1StringView s, qsizetype from, Qt::CaseSensitivity cs) const noexcept
      |                        ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:107:24: note: previous definition is here
  107 | qsizetype QStringView::indexOf(QLatin1StringView s, qsizetype from, Qt::CaseSensitivity cs) const noexcept
      |                        ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:115:19: error: redefinition of 'contains'
  115 | bool QStringView::contains(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:115:19: note: previous definition is here
  115 | bool QStringView::contains(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                   ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:117:24: error: redefinition of 'lastIndexOf'
  117 | qsizetype QStringView::lastIndexOf(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                        ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:117:24: note: previous definition is here
  117 | qsizetype QStringView::lastIndexOf(QLatin1StringView s, Qt::CaseSensitivity cs) const noexcept
      |                        ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:119:24: error: redefinition of 'lastIndexOf'
  119 | qsizetype QStringView::lastIndexOf(QLatin1StringView s, qsizetype from, Qt::CaseSensitivity cs) const noexcept
      |                        ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:119:24: note: previous definition is here
  119 | qsizetype QStringView::lastIndexOf(QLatin1StringView s, qsizetype from, Qt::CaseSensitivity cs) const noexcept
      |                        ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:127:24: error: redefinition of 'count'
  127 | qsizetype QStringView::count(QLatin1StringView s, Qt::CaseSensitivity cs) const
      |                        ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:127:24: note: previous definition is here
  127 | qsizetype QStringView::count(QLatin1StringView s, Qt::CaseSensitivity cs) const
      |                        ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:140:27: error: redefinition of 'QAnyStringView'
  140 | constexpr QAnyStringView::QAnyStringView(QLatin1StringView str) noexcept
      |                           ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:140:27: note: previous definition is here
  140 | constexpr QAnyStringView::QAnyStringView(QLatin1StringView str) noexcept
      |                           ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:143:45: error: redefinition of 'asLatin1StringView'
  143 | constexpr QLatin1StringView QAnyStringView::asLatin1StringView() const
      |                                             ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:143:45: note: previous definition is here
  143 | constexpr QLatin1StringView QAnyStringView::asLatin1StringView() const
      |                                             ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:151:42: error: redefinition of 'visit'
  151 | constexpr decltype(auto) QAnyStringView::visit(Visitor &&v) const
      |                                          ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:151:42: note: previous definition is here
  151 | constexpr decltype(auto) QAnyStringView::visit(Visitor &&v) const
      |                                          ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:165:33: error: redefinition of 'front'
  165 | constexpr QChar QAnyStringView::front() const
      |                                 ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:165:33: note: previous definition is here
  165 | constexpr QChar QAnyStringView::front() const
      |                                 ^
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:167:12: error: no matching member function for call to 'visit'
  167 |     return visit([] (auto that) { return QAnyStringView::toQChar(that.front()); });
      |            ^~~~~
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:151:42: note: candidate template ignored: substitution failure [with Visitor = (lambda at /Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qstring.h:167:18)]
  151 | constexpr decltype(auto) QAnyStringView::visit(Visitor &&v) const
      |                                          ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.

QtCore.framework.zip

[dwblaikie]

yeah - I'm guessing (though it's possible someone'll pick up & run with what's here - but C++ clang header modules aren't terribly actively maintained/being worked on at the moment, unfortunately) if you want to spend time to move this forward, it'll be necessary to go further - try to isolate an example that's only a few files and a few lines long in each file, if you've got the time/inclination.

[Bigcheese]

What I believe is happening here is that you can't have a -I into a framework directory with modules. Specifically:

• #include <qstring.h> is processed textually because there's no module map next to it.
• The #include <QtCore/qchar.h> inside qstring.h is found in the -F which does have a module map, triggering module import.
• After the module import, textual processing of qstring.h continues, which eventually hits the duplicate definition.

You can verify this by passing -E, you should see the start of the qstring.h header and then #pragma clang module import QtCore.

At some point I plan to make it an error for a header to ever change between being processed textually and as part of a module.

Additionally, your module map should be something like:

framework module QtCore {
    requires cplusplus17
    umbrella "Headers"
    module * { export * }
    private textual header "header_impl.h"
    ...
    exclude header "6.11.0/header"
    ...
}

The changes here are using an umbrella directory instead of naming all the headers specifically, using inferred submodules, and making the impl headers private so they can only be included by other parts of QtCore.

Inferred submodules allow imports of specific headers to only see what they should from those headers, instead of seeing the entire module if they only include one header.

[torarnv]

Thanks @Bigcheese 🙌🏻 I had a suspicion something like that was going on.

Qt generally (historically) passes both -I$INSTALL_PATH/include and -I$INSTALL_PATH/include/QtFoo (for each Qt module the project depends on, e.g. QtCore, QtGui, etc), to enable both the #include <QtCore/qfoo.h> and #include <qfoo.h> forms. And both forms are widely used by user project, as well as Qt internally.

We wanted to maintain compatibility when using framework builds on Apple platforms, which is why we add -I$INSTALL_PATH/lib/QtFoo.framework/Headers.

The changes here are using an umbrella directory instead of naming all the headers specifically, using inferred submodules, and making the impl headers private so they can only be included by other parts of QtCore.

Thanks for the tips on structuring the module.modulemap! I tried structuring it like this, to see if it helped the include issue, but, that gave me new errors for the case that was already working 🤔

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$FW_DIR <(echo '#include <QtCore/qstring.h>')
While building module 'QtCore' imported from /dev/fd/16:1:
In file included from <module-includes>:100:
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h:38:12: error: redefinition of 'Launch'
   38 | enum class Launch { Sync, Async, Inherit };
      |            ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture.h:14:10: note: '/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h' included multiple times, additional include site in header from module 'QtCore.qfuture'
   14 | #include <QtCore/qfuture_impl.h>
      |          ^
<module-includes>:100:10: note: '/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h' included multiple times, additional include site here
  100 | #include "Headers/qfuture_impl.h"
      |          ^
/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h:38:12: note: unguarded header; consider using #ifdef guards or #pragma once
   38 | enum class Launch { Sync, Async, Inherit };
      |            ^
While building module 'QtCore' imported from /dev/fd/16:1:
In file included from <module-includes>:100:

[snip]

Maybe I've misunderstood the textual header part. Note that these are implementation headers without explicit headers guards. The only header that includes qfuture_impl.h is qfuture.h, and the former has this at the top:

#ifndef QFUTURE_H
#error Do not include qfuture_impl.h directly
#endif

Regarding making the _impl headers private, that's the approach I went for first, but there's at least one inter-module dependency in Qt that would prevent this, and trying to fix that concluded (for now) that _impl headers are not necessarily module-private in Qt.

The good news is that when building Qt with frameworks disabled I can support both include styles:

❯ clang++ -c -x c++ -std=c++17 -fmodules -fcxx-modules -I$NO_FW_DIR/include -I$NO_FW_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>') -fmodules-cache-path=/tmp/modulecache -Wnon-modular-include-in-module -Wnon-modular-include-in-framework-module -Wincomplete-umbrella -Wquoted-include-in-framework-header -Watimport-in-framework-header -Wframework-include-private-from-public -Wincomplete-framework-module-declaration -fdiagnostics-absolute-paths -fdiagnostics-show-note-include-stack -Wno-error -Wno-deprecated-declarations

❯ clang++ -c -x c++ -std=c++17 -fmodules -fcxx-modules -I$NO_FW_DIR/include -I$NO_FW_DIR/include/QtCore <(echo '#include <qstring.h>') -fmodules-cache-path=/tmp/modulecache -Wnon-modular-include-in-module -Wnon-modular-include-in-framework-module -Wincomplete-umbrella -Wquoted-include-in-framework-header -Watimport-in-framework-header -Wframework-include-private-from-public -Wincomplete-framework-module-declaration -fdiagnostics-absolute-paths -fdiagnostics-show-note-include-stack -Wno-error -Wno-deprecated-declarations

Without any build issue, and a QtCore module map is generated in /tmp/modulecache.

Is that intentional (that this works) for non-framework module-maps, or a possible sharp corner down the line?

If it's intentional it could be a solution for us for providing Clang module maps for Swift on non-Apple platforms. On Apple platforms we probably still want to build Qt as frameworks, as that's required for other reasons such as privacy manifests. But perhaps we could ship headers both inside the frameworks, and under $INSTALL_PATH/include. That seems to work at least:

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <qstring.h>')

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>')

[Bigcheese]

<module-includes>:100:10: note: '/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h' included multiple times, additional include site here
  100 | #include "Headers/qfuture_impl.h"
      |          ^

This is a bug in the handling of framework + umbrella directories, since generally frameworks have umbrella headers instead. It should not include qfuture_impl.h in the <module-includes> buffer.

Maybe I've misunderstood the textual header part. Note that these are implementation headers without explicit headers guards.

Yeah, these are one of the few use cases of textual headers.

Regarding making the _impl headers private, that's the approach I went for first, but there's at least one inter-module dependency in Qt that would prevent this, and trying to fix that concluded (for now) that _impl headers are not necessarily module-private in Qt.

Ah, I assumed they were all just cases of splitting up a header into multiple files. We recommend private textual so people get the error, but if you already have an error message for the ones that need it then that's fine.

The good news is that when building Qt with frameworks disabled I can support both include styles:
[...]
Without any build issue, and a QtCore module map is generated in /tmp/modulecache.

Is that intentional (that this works) for non-framework module-maps, or a possible sharp corner down the line?

I'm not sure how this would work with the module map in /tmp/modulecache. If the module map is in $NO_FW_DIR/include/QtCore then this is fine. The general rule is that the module map must be discoverable relative to the headers it applies to. Currently Clang is not strict about this and you can end up with include order dependent behavior, but I plan to make such cases errors soon.

However if the module map is in $NO_FW_DIR/include and there is a -I into $NO_FW_DIR/include/QtCore, then that's a problem because #include <qstring.h> wouldn't see the module map but #include <QtCore/qstring.h> would. This can incidentally work if Clang happened to parse the module map in $NO_FW_DIR/include first though.

If it's intentional it could be a solution for us for providing Clang module maps for Swift on non-Apple platforms. On Apple platforms we probably still want to build Qt as frameworks, as that's required for other reasons such as privacy manifests. But perhaps we could ship headers both inside the frameworks, and under $INSTALL_PATH/include. That seems to work at least:

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <qstring.h>')

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>')

This should have the same issue as the first case assuming -I$MIXED_DIR/include/QtCore doesn't have a module map. If it does then you should be getting a duplicate module definition error. I don't see a good way to make this work correctly for frameworks other than using forwarding headers in -I$MIXED_DIR/include/QtCore that just contain #include <QtCore/HEADER.h> for each header.

[torarnv]

<module-includes>:100:10: note: '/Users/torarne/build/qt/6.x-modules/install2/QtCore.framework/Headers/qfuture_impl.h' included multiple times, additional include site here
  100 | #include "Headers/qfuture_impl.h"
      |          ^

This is a bug in the handling of framework + umbrella directories, since generally frameworks have umbrella headers instead. It should not include qfuture_impl.h in the <module-includes> buffer.

Ah! Should I file a separate issue for this?

Ah, I assumed they were all just cases of splitting up a header into multiple files. We recommend private textual so people get the error, but if you already have an error message for the ones that need it then that's fine.

Ack.

The good news is that when building Qt with frameworks disabled I can support both include styles:
[...]
Without any build issue, and a QtCore module map is generated in /tmp/modulecache.
Is that intentional (that this works) for non-framework module-maps, or a possible sharp corner down the line?

I'm not sure how this would work with the module map in /tmp/modulecache.

Sorry, I was being unclear 😢

What I meant was that when the module.modulemap is in $NO_FW_DIR/include/QtCore, and I pass both $NO_FW_DIR/include and $NO_FW_DIR/include/QtCore as include dirs, I get a compiled module in the module cache dir specified by -fmodules-cache-path=/tmp/modulecache. So it's building the module in both cases, without any warnings/errors.

But perhaps we could ship headers both inside the frameworks, and under $INSTALL_PATH/include. That seems to work at least
This should have the same issue as the first case assuming -I$MIXED_DIR/include/QtCore doesn't have a module map. If it does then you should be getting a duplicate module definition error.

In my test of this there's a module map in both the framework, and the non-framework QtCore include dir:

❯ find $MIXED_DIR | grep modulemap
/Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/include/QtCore/module.modulemap
/Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/lib/QtCore.framework/Modules/module.modulemap

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>')

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <qstring.h>')

And both compilations work. But this perhaps produces two different modules named QtCore?

If I remove the one outside the framework header dir, I do get the same issue as in the original report:

❯ mv /Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/include/QtCore/module.modulemap /Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/include/QtCore/module.modulemap.nope

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>')

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <qstring.h>')
In file included from /dev/fd/16:1:
/Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/include/QtCore/qstring.h:65:30: error: redefinition of 'treat_as_integral_arg'
   65 | template <typename T> struct treat_as_integral_arg : std::false_type {};

I don't see a good way to make this work correctly for frameworks other than using forwarding headers in -I$MIXED_DIR/include/QtCore that just contain #include <QtCore/HEADER.h> for each header.

That could be a workable solution, good idea! We already generate forwarding headers to enable #include <QString> style includes, so the machinery to do this is already there, and working:

❯ cat $MIXED_DIR/include/QtCore/QString
#include <QtCore/qstring.h> // IWYU pragma: export

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QString>')

[Bigcheese]

This is a bug in the handling of framework + umbrella directories, since generally frameworks have umbrella headers instead. It should not include qfuture_impl.h in the <module-includes> buffer.

Ah! Should I file a separate issue for this?

I'll file one, after this I did some testing and have a small reproducer.

In my test of this there's a module map in both the framework, and the non-framework QtCore include dir:

❯ find $MIXED_DIR | grep modulemap
/Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/include/QtCore/module.modulemap
/Users/torarne/build/qt/6.x-modules/mixed-fw-and-include/lib/QtCore.framework/Modules/module.modulemap

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <QtCore/qstring.h>')

❯ clang++ -x c++ -fsyntax-only -std=c++17 -fmodules -fcxx-modules -F$MIXED_DIR/lib -I$MIXED_DIR/include/QtCore <(echo '#include <qstring.h>')

And both compilations work. But this perhaps produces two different modules named QtCore?

Oh, I think this works because each case is only importing one of them. I believe it should fail if you import both into one TU. For #include <qstring.h> I think it's able to build the module because of -fmodule-name=QtCore, which suppresses include translation for that module, so #include <QtCore/qchar.h> is processed textually. Clang should also reject this when parsing the module map, but in this case I think module name shadowing kicks in and allows the duplicate module names since the primary module map is on the internal command line for building the module.

This is not the intended behavior, and I'd like to remove module name shadowing at some point because it's broken in other ways too.

I don't see a good way to make this work correctly for frameworks other than using forwarding headers in -I$MIXED_DIR/include/QtCore that just contain #include <QtCore/HEADER.h> for each header.

That could be a workable solution, good idea! We already generate forwarding headers to enable #include <QString> style includes, so the machinery to do this is already there, and working:

Sounds good, that's going to be much more modules friendly.