Re: [Development] Using '#pragma once' instead of include guards?
> On 14 Oct 2022, at 15:47, Kyle Edwards via Development > wrote: > > On 10/14/22 03:15, Eike Ziller wrote: >>> However, there are ways to enforce the use of unique header guards. >>> clang-tidy has an extensible header guard check that can be customized >>> per-project, and plugin loading functionality. Qt could create a clang-tidy >>> plugin that sets up this header guard check and enforces a unique-enough >>> header guard in CI. >> That works to avoid clashes inside a project, but doesn't help if user >> applications mix Qt + their code + other libraries not under their control, >> which is similar to the issues of #pragma once. > > My point was that a naming scheme could be enforced that would make it very > unlikely for an external project's header guards to conflict with the ones > from Qt. I will leave it as an exercise to the reader to come up with such a > scheme. > > Kyle I suppose the original idea was that the ‘Q’ prefix is reserved for things in Qt (I vaguely remember that back in the early 2000s there was some sort of ‘official' list for those prefixes; I might be completely wrong about that though), just as C is the prefix for MFC classes, and T for classes in Turbo C or Symbian C++. But that could never have been more than a convention, and I don’t quite see how anything based on convention can be enforced. If in the future we add a class QLog or QJack or QDoubleRangeSlider to Qt, then folks that have made the unfortunate choice to use the prefix `Q` when they named their types will be unhappy. Application developers can use Qt in a namespace, but those that provide an SDK themselves won’t be able to solve the issue for users using both their headers and Qt headers. So if and when that time comes we’ll have to deal with it somehow, case by case. Whether we use `#pragma once` or conventional include guards is then perhaps just a minor problem - that's something either side can change without breaking source or binary compatibility. Would it help if we named our include guards QT_QFOO_H? It would reduce the probability of a clash, but the real problem of two QFoo classes in the global namespace won’t go away. Anyway, I’ve added the respective text to the coding convention wiki page. https://wiki.qt.io/Coding_Conventions Volker ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Using '#pragma once' instead of include guards?
On 10/14/22 03:15, Eike Ziller wrote: However, there are ways to enforce the use of unique header guards. clang-tidy has an extensible header guard check that can be customized per-project, and plugin loading functionality. Qt could create a clang-tidy plugin that sets up this header guard check and enforces a unique-enough header guard in CI. That works to avoid clashes inside a project, but doesn't help if user applications mix Qt + their code + other libraries not under their control, which is similar to the issues of #pragma once. My point was that a naming scheme could be enforced that would make it very unlikely for an external project's header guards to conflict with the ones from Qt. I will leave it as an exercise to the reader to come up with such a scheme. Kyle ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Duplicated test data tags
Milian Wolff (Friday, 14 October 2022 3:00 AM) wrote:
>> >> I have many times accidentally written bogus code that duplicated the
>> >> tags. Getting a warning is useful, so thanks for working on that!
>> >>
>> >> But we won't easily spot these in the thousands of lines of outputs a
>> >> large test suite is generating.
On Freitag, 14. Oktober 2022 10:55:54 CEST Edward Welbourne wrote:
>> Indeed, that strikes me as eminently plausible.
>> Especially as one of the things I've been picking up on (and I've fixed
>> various of them, at least in QtCore) as a result of chasing these down
>> is that we have quite a few tests with QWARN messages, that really
>> should be anticipated (so that the test fails if the warning isn't
>> produced; and so that the test log isn't cluttered with the warnings)
>> using QTest::ignoreWarning().
>>
>> I should also take this opportunity to encourage all developers to watch
>> out for QWARN messages in test output: if it's unexpected, it may be the
>> symptom of a lurking bug; otherwise, its presence should be tested for,
>> see preceding. If the code under test fails to produce a warning that
>> was expected, that's an issue the test should catch.
>> >> At the very least I would suggest something akin to QT_FATAL_WARNINGS
>> >> that can be set to more easily spot faulty client code.
>>
>> Perhaps, given the above, we should just encourage developers to use
>> QT_FATAL_WARNINGS more often ?
>>
>> I do note, however, that it seems not to have been documented; I shall
>> add a doc update to my branch.
Milian Wolff (Friday, October 14, 2022 12:29) replied:
> That is sadly not an option in many situations. The silencing of the warnings
> does not influence the fatal-abort to my knowledge? See
> `QMessageLogger::warning`:
>
> ```
> void QMessageLogger::warning(const char *msg, ...) const
> {
> va_list ap;
> va_start(ap, msg); // use variable arg list
> const QString message = qt_message(QtWarningMsg, context, msg, ap);
> va_end(ap);
>
> if (isFatal(QtWarningMsg))
> qt_message_fatal(QtWarningMsg, context, message);
> }
> ```
That sounds like something we should look into changing. It runs
counter to what QTest::ignoreWarnings() tells its user; they may
reasonably expect setting the environment variable to catch only
warnings they haven't suppressed. However, that's very much within
QMessageLogger code, not under QTest's control :-(
QTBUG-107659
> So while `qt_message` is intercepted, the `qt_message_fatal` is not which
> then
> would kill the app even for "expected" warnings.
:-(
> What's worse, there are many warnings where you don't know how many of them
> will occur. We e.g. run our unit test suite using the offscreen platform
> plugin and are drowning in warnings such as:
>
> ```
> This plugin does not support setParent!
> ```
>
> And so far I could not find a reliable way to prevent this.
QTBUG-107660
> Similarly, we need a custom OpenGL profile which always triggers this warning
> when Qt WebEngine is also used:
>
> ```
> An OpenGL Core Profile was requested, but it is not supported on the current
> platform. Falling back to a non-Core profile. Note that this might cause
> rendering issues.
> ```
>
> Meaning: there are sadly some situations where warnings are emitted that you
> do not have control over. Meaning, Qt_FATAL_WARNINGS is sadly not an option
> today :(
Sounds painful.
Thanks for the dose of real-world practicality. I'll need to give
further thought to how to make the new warnings "enforceable"
(without enforcing them all the time).
Eddy.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Duplicated test data tags
On Freitag, 14. Oktober 2022 10:55:54 CEST Edward Welbourne wrote:
> Milian Wolff (Friday, 14 October 2022 3:00 AM)
>
> >> I have many times accidentally written bogus code that duplicated the
> >> tags. Getting a warning is useful, so thanks for working on that!
> >>
> >> But we won't easily spot these in the thousands of lines of outputs a
> >> large test suite is generating.
>
> Indeed, that strikes me as eminently plausible.
> Especially as one of the things I've been picking up on (and I've fixed
> various of them, at least in QtCore) as a result of chasing these down
> is that we have quite a few tests with QWARN messages, that really
> should be anticipated (so that the test fails if the warning isn't
> produced; and so that the test log isn't cluttered with the warnings)
> using QTest::ignoreWarning().
>
> I should also take this opportunity to encourage all developers to watch
> out for QWARN messages in test output: if it's unexpected, it may be the
> symptom of a lurking bug; otherwise, its presence should be tested for,
> see preceding. If the code under test fails to produce a warning that
> was expected, that's an issue the test should catch.
>
> >> At the very least I would suggest something akin to QT_FATAL_WARNINGS
> >> that can be set to more easily spot faulty client code.
>
> Perhaps, given the above, we should just encourage developers to use
> QT_FATAL_WARNINGS more often ?
>
> I do note, however, that it seems not to have been documented; I shall
> add a doc update to my branch.
That is sadly not an option in many situations. The silencing of the warnings
does not influence the fatal-abort to my knowledge? See
`QMessageLogger::warning`:
```
void QMessageLogger::warning(const char *msg, ...) const
{
va_list ap;
va_start(ap, msg); // use variable arg list
const QString message = qt_message(QtWarningMsg, context, msg, ap);
va_end(ap);
if (isFatal(QtWarningMsg))
qt_message_fatal(QtWarningMsg, context, message);
}
```
So while `qt_message` is intercepted, the `qt_message_fatal` is not which then
would kill the app even for "expected" warnings.
What's worse, there are many warnings where you don't know how many of them
will occur. We e.g. run our unit test suite using the offscreen platform
plugin and are drowning in warnings such as:
```
This plugin does not support setParent!
```
And so far I could not find a reliable way to prevent this.
Similarly, we need a custom OpenGL profile which always triggers this warning
when Qt WebEngine is also used:
```
An OpenGL Core Profile was requested, but it is not supported on the current
platform. Falling back to a non-Core profile. Note that this might cause
rendering issues.
```
Meaning: there are sadly some situations where warnings are emitted that you
do not have control over. Meaning, Qt_FATAL_WARNINGS is sadly not an option
today :(
> Mitch Curtis (14 October 2022 03:40) replied:
> > QTest::failOnWarning (introduced in 6.3) could also be used by tests
> > to make that warning fail the test:
> >
> > https://doc.qt.io/qt-6/qtest.html#failOnWarning
>
> True enough; but you would need to add that to the start of your _data()
> function; I think a global setting is more in line with what's needed.
> Fortunately we do have QT_FATAL_WARNINGS; I had forgotten about that.
>
> So the question is: does it suffice to encourage developers to test with
> that enabled, or do we need something more specific to the particular
> issue of tag and column names in _data() functions ?
>
> Eddy.
--
Milian Wolff
m...@milianw.de
http://milianw.de
signature.asc
Description: This is a digitally signed message part.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Duplicated test data tags
Milian Wolff (Friday, 14 October 2022 3:00 AM) >> I have many times accidentally written bogus code that duplicated the >> tags. Getting a warning is useful, so thanks for working on that! >> >> But we won't easily spot these in the thousands of lines of outputs a >> large test suite is generating. Indeed, that strikes me as eminently plausible. Especially as one of the things I've been picking up on (and I've fixed various of them, at least in QtCore) as a result of chasing these down is that we have quite a few tests with QWARN messages, that really should be anticipated (so that the test fails if the warning isn't produced; and so that the test log isn't cluttered with the warnings) using QTest::ignoreWarning(). I should also take this opportunity to encourage all developers to watch out for QWARN messages in test output: if it's unexpected, it may be the symptom of a lurking bug; otherwise, its presence should be tested for, see preceding. If the code under test fails to produce a warning that was expected, that's an issue the test should catch. >> At the very least I would suggest something akin to QT_FATAL_WARNINGS >> that can be set to more easily spot faulty client code. Perhaps, given the above, we should just encourage developers to use QT_FATAL_WARNINGS more often ? I do note, however, that it seems not to have been documented; I shall add a doc update to my branch. Mitch Curtis (14 October 2022 03:40) replied: > QTest::failOnWarning (introduced in 6.3) could also be used by tests > to make that warning fail the test: > > https://doc.qt.io/qt-6/qtest.html#failOnWarning True enough; but you would need to add that to the start of your _data() function; I think a global setting is more in line with what's needed. Fortunately we do have QT_FATAL_WARNINGS; I had forgotten about that. So the question is: does it suffice to encourage developers to test with that enabled, or do we need something more specific to the particular issue of tag and column names in _data() functions ? Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Using '#pragma once' instead of include guards?
> On 13 Oct 2022, at 16:56, Kyle Edwards via Development > wrote: > > On 10/13/22 10:42, Jean-Michaël Celerier wrote: >> >The only way you’d have a strong case with this is if it has some other >> >significant benefit, like compilation speedup. >> >> The main benefit to me is that it entirely removes possibilities for >> conflict due to headers having the same name. At least Qt takes great care >> of avoiding this but still, notice that e.g. the authors of Qt3D's >> Qt3DCore::QTransform had to be careful to not just do #ifndef QTRANSFORM_H >> >> Now what happens when someone develops a different library but with a header >> guard similar to Qt's? >> >> If I grep into the various cloned projects on my hard drive, for instance I >> see >> >> #ifndef QRENDERER_H >> #ifndef QGLRENDERER_H >> #ifndef QSEARCHFIELD_H >> #ifndef QLOG_H >> #ifndef QJACK_H >> #ifndef QENC_H >> #ifndef QRANGESLIDER_H >> #ifndef QDOUBLERANGESLIDER_H >> >> etc... is the Qt project 100% confident that it will *never ever* use these >> names? With pragma once this is a 100% non-problem. > > I agree with previous points that while #pragma once can work well for a > standalone program, it has the potential to cause problems when used in > libraries that other developers use. Even CMake omitted #pragma once from the > one (admittedly deprecated) header that may be consumed externally > (cmCPluginAPI.h). > > However, there are ways to enforce the use of unique header guards. > clang-tidy has an extensible header guard check that can be customized > per-project, and plugin loading functionality. Qt could create a clang-tidy > plugin that sets up this header guard check and enforces a unique-enough > header guard in CI. That works to avoid clashes inside a project, but doesn't help if user applications mix Qt + their code + other libraries not under their control, which is similar to the issues of #pragma once. Header guards do have downsides too, but I suppose software developers got used to them and have the necessary workarounds in place. Which is a valid argument for not introducing #pragma once in Qt, which would require different workarounds, and create friction that would need a very compelling argument to inflict. -- Eike Ziller Principal Software Engineer The Qt Company GmbH Erich-Thilo-Straße 10 D-12489 Berlin eike.zil...@qt.io http://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@qt-project.org https://lists.qt-project.org/listinfo/development
