[Development] On the use of the inline keyword

[Volker Hilsheimer]

Thanks for the background info, Cristian!


It’s fine to agree that, for new code, the inline goes only on the declaration, and must not be on the definition.

For existing code: throwing the somewhat but not totally (I think) naive regexp

^inline\s+([A-Za-z\*&<>,:\s]+\s+)?[A-Za-z\*&<>,\s:]+::.*\(.*\)[\s\na-z]*\{

at the headers in my local worktree (which doesn’t include qtwebengine) gives me > 1500 hits in >200 files.

Before we now go on a campaign to clean all this up (as is already being reviewed for qstring.h), I’d rather evaluate other options for the existing code base.

Can we disable this warning some in some other way on MinGW? Or, knowing the likely reason for it, live with it and fix things when and if we run into them (which seems to be reasonably rare)?


Volker


On 23 Aug 2023, at 17:30, Cristian Adam via Development <development at qt-project.org> wrote:

Hi,

The issue here is that MinGW GCC is issuing a warning and that we compile with -Werror to treat warnings as errors.

c++ - Importing inline functions in MinGW - Stack Overflow<https://stackoverflow.com/questions/11546403/importing-inline-functions-in-mingw> has a similar case.

LLVM-MinGW using clang is also issuing warnings, but these warnings can be suppressed via -Wno-ignored-attributes.

No idea why GCC has the warning not part of the -Wignored-attributes like clang does.

Cheers,
Cristian
________________________________
From: Development <development-bounces at qt-project.org> on behalf of Marc Mutz via Development <development at qt-project.org>
Sent: Wednesday, August 23, 2023 16:27
To: Qt development mailing list <development at qt-project.org>
Subject: [Development] On the use of the inline keyword

Hi,

Every now and then we get a sporadic MinGW error because someone writes
the moral equivalent of the following:

    class Q_FOO_EXPORT QMeep {
        ~~~~
        QBar bar() const;
        ~~~~
    };
    inline QBar QMeep::bar() const;

Resulting in something like this:

     qmeep.h: error: 'QBar QMeep::bar() const' redeclared without
dllimport attribute after being referenced with dll linkage [-Werror]

The last time I remember this came up was in 5.8
(https://bugreports.qt.io/browse/QTBUG-56459). It's so sporadic that
Ivan didn't manage to repro when I pointed this out as a potential problem in
API review, until it suddenly hit today in code that was unchanged since Qt
5.0: https://testresults.qt.io/coin/integration/qt/qtbase/tasks/1698267962

I don't claim to know what's the cause (it's probably use of the
function in inline implementation), but I do know the fix, and that's to
put `inline` on the _declaration_, but _not_ the definition. While it
doesn't hurt to put it on the definition, it's exactly this practice
that lets other platforms compile this code that MinGW then out of a
sudden starts to complain about. By _not_ putting `inline` on the
definition, only on the declaration, we cause all platforms to complain
if we get it wrong ("multiple definition errors at link time" or "inline
function not defined").

So, if we want this as a minimal-complexity rule:

- the `inline` keyword goes _only_ on declarations, never on definitions
- the `inline` keyword should be omitted on the following declarations:
   - of constexpr functions (they're implicitly inline)
   - of consteval functions (ditto)
   - of in-class-body member function definitions (they, too, are
implicitly inline)

Please add this to your things to look out for in reviews.

Thanks,
Marc

--
Marc Mutz <marc.mutz at qt.io>
Principal Software Engineer

The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io

Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B
--
Development mailing list
Development at qt-project.org
https://lists.qt-project.org/listinfo/development
--
Development mailing list
Development at qt-project.org<mailto:Development at qt-project.org>
https://lists.qt-project.org/listinfo/development

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230824/db510711/attachment-0001.htm>