ecm_qt_declare_logging_category(): more unique include guard for header
ClosedPublic

Authored by kossebau on May 15 2017, 2:52 PM.

Details

Summary

The old guard was created just from the identifier + _H, which runs the
chance to clash in projects which use an identifier matching the project
name and which also have a class or central file header which is named
by the project and then has an include guard matching the filename.
Example:
project ABC -> abc.h with ABC_H guard
identifier ABC, header debug.h -> debug.h with ABC_H guard
any.cpp including both abc.h and debug.h will see only one content

Using both the header file name and identifier for the guard name
and prefixing it additionally with a macro specific term should make
the guard both follow the usual pattern for guards matching the
file name and also add some namespacing to allow for similar named
header files in bigger projects (e.g. "debug.h") which could be
included in the same include tree.

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.May 15 2017, 2:52 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptMay 15 2017, 2:52 PM

What about #pragma once? Is there some real-world compiler that doesn't support it?

Yes, #pragma once might be the nicer solution here.

I stayed away from proposing it though, as for one it is not a real standard by specifications and also by KDE coding traditions.
And I would not like to be the one adding (and thus being responsible) the guinea pig to find out if that pragma is good enough for the real world ;)

Given the generated file is build-specific, one could write a configure test to check if the compiler supports it and then use that.
Left for the next person though, the patch as-is solves the issues I hit already ;)

A different reasoning for why not yet using #pragma once: this macro targets users of projects with at least cmake and Qt. Unless Qt itself does not use that pragma, let's not risk to screw over people who try to reuse ECM for some non-mainstream setup, unless we have no other choice.

If noone objects or has further comments, will commit this Wednesday, 24th.

This revision was automatically updated to reflect the committed changes.