Esri / arcgis-maps-sdk-toolkit-qt

ArcGIS Maps SDK for Qt Toolkit
Apache License 2.0
132 stars 61 forks source link

Q_OBJECT in comment breaks AutoMoc in CMake #492

Closed Zhumos closed 2 years ago

Zhumos commented 2 years ago

While this toolkit doesn't include CMake support yet, this might save someone a lot of frustration in the future.

The Problem

In the following code snippet, Q_OBJECT is embedded in the comment block (line 46). This unfortunately confuses AutoMoc into thinking it's a real class and a valid macro reference (I'm guessing the AutoMoc parser doesn't keep track of comment blocks).

Minor aside: typo on line 49, should be CONSTANT

https://github.com/Esri/arcgis-runtime-toolkit-qt/blob/5c6f5abc3ad2f95e32dc9239782a4c2f7e23eecd/uitools/cpp/Esri/ArcGISRuntime/Toolkit/Internal/GenericListModel.cpp#L26-L52

Output error from build:

FAILED: Display/Resources/ArcGISToolkitPlugin_autogen/timestamp Display/Resources/ArcGISToolkitPlugin_autogen/mocs_compilation.cpp C:/Workspace/C++/Project/_build/debug/Display/Resources/ArcGISToolkitPlugin_autogen/timestamp C:/Workspace/C++/Project/_build/debug/Display/Resources/ArcGISToolkitPlugin_autogen/mocs_compilation.cpp 
cmd.exe /C "cd /D C:\Workspace\C++\Project\_build\debug\Display\Resources && "C:\Program Files\JetBrains\CLion 2021.1.1\bin\cmake\win\bin\cmake.exe" -E cmake_autogen C:/Workspace/C++/Project/_build/debug/Display/Resources/CMakeFiles/ArcGISToolkitPlugin_autogen.dir/AutogenInfo.json Debug && "C:\Program Files\JetBrains\CLion 2021.1.1\bin\cmake\win\bin\cmake.exe" -E touch C:/Workspace/C++/Project/_build/debug/Display/Resources/ArcGISToolkitPlugin_autogen/timestamp && "C:\Program Files\JetBrains\CLion 2021.1.1\bin\cmake\win\bin\cmake.exe" -E cmake_transform_depfile Ninja gccdepfile C:/Workspace/C++/Project C:/Workspace/C++/Project/Display/Resources C:/Workspace/C++/Project/_build/debug C:/Workspace/C++/Project/_build/debug/Display/Resources C:/Workspace/C++/Project/_build/debug/Display/Resources/ArcGISToolkitPlugin_autogen/deps C:/Workspace/C++/Project/_build/debug/CMakeFiles/d/52ce3b2ef2644acdd9502b46303cbead86a354f588258b8d45cfd10d8f3cf3ed.d"

AutoMoc error
 -------------
 "SRC:/Display/Resources/arcgis-runtime-toolkit-qt/uitools/cpp/Esri/ArcGISRuntime/Toolkit/Internal/GenericListModel.cpp"
 contains a "Q_OBJECT" macro, but does not include "GenericListModel.moc"!
 Consider to
   - add #include "GenericListModel.moc"
   - enable SKIP_AUTOMOC for this file

Workaround

Removing Q_OBJECT, or otherwise invalidating the line inside the comment allows AutoMoc to complete successfully.

Environment

OS: Windows 11

Versions:

Below is the CMakeLists.txt file I'm using. I've added this repo as a submodule to my project and the CMake sits one directory up from it. My project uses add_subdirectory to pull it in. To find the runtime itself, I use the cmake file provided in the ideintegration directory.

set(ModuleName ArcGISToolkitPlugin)

set(UIToolsPath     ${CMAKE_CURRENT_SOURCE_DIR}/arcgis-runtime-toolkit-qt/uitools)
set(RegisterPath    ${UIToolsPath}/register/Esri/ArcGISRuntime/Toolkit)
set(ToolkitSrcPath  ${UIToolsPath}/cpp/Esri/ArcGISRuntime/Toolkit)

set(RegistrationSourceFiles
    ${RegisterPath}/register.h
    ${RegisterPath}/register.cpp
    ${RegisterPath}/internal/register_cpp.h
    ${RegisterPath}/internal/register_cpp.cpp
)

set(QmlSourceFiles
    ${UIToolsPath}/images/esri_arcgisruntime_toolkit_images.qrc
    ${UIToolsPath}/import/Esri/ArcGISRuntime/Toolkit/esri_arcgisruntime_toolkit_view.qrc
)

set(ToolkitSourceFiles
    ${ToolkitSrcPath}/Internal/GenericListModel.h
    ${ToolkitSrcPath}/Internal/GenericListModel.cpp
    ${ToolkitSrcPath}/Internal/BasemapGalleryImageProvider.h
    ${ToolkitSrcPath}/Internal/BasemapGalleryImageProvider.cpp
    ${ToolkitSrcPath}/Internal/GenericTableProxyModel.h
    ${ToolkitSrcPath}/Internal/GenericTableProxyModel.cpp
    ${ToolkitSrcPath}/Internal/GeoViews.h
    ${ToolkitSrcPath}/Internal/MetaElement.h
    ${ToolkitSrcPath}/Internal/MetaElement.cpp

    ${ToolkitSrcPath}/AuthenticationController.h
    ${ToolkitSrcPath}/AuthenticationController.cpp
    ${ToolkitSrcPath}/BasemapGalleryController.h
    ${ToolkitSrcPath}/BasemapGalleryController.cpp
    ${ToolkitSrcPath}/BasemapGalleryItem.h
    ${ToolkitSrcPath}/BasemapGalleryItem.cpp
    ${ToolkitSrcPath}/CoordinateConversionConstants.h
    ${ToolkitSrcPath}/CoordinateConversionConstants.cpp
    ${ToolkitSrcPath}/CoordinateConversionController.h
    ${ToolkitSrcPath}/CoordinateConversionController.cpp
    ${ToolkitSrcPath}/CoordinateConversionOption.h
    ${ToolkitSrcPath}/CoordinateConversionOption.cpp
    ${ToolkitSrcPath}/CoordinateConversionResult.h
    ${ToolkitSrcPath}/CoordinateConversionResult.cpp
    ${ToolkitSrcPath}/CoordinateOptionDefaults.h
    ${ToolkitSrcPath}/CoordinateOptionDefaults.cpp
    ${ToolkitSrcPath}/LocatorSearchSource.h
    ${ToolkitSrcPath}/LocatorSearchSource.cpp
    ${ToolkitSrcPath}/NorthArrowController.h
    ${ToolkitSrcPath}/NorthArrowController.cpp
    ${ToolkitSrcPath}/OverviewMapController.h
    ${ToolkitSrcPath}/OverviewMapController.cpp
    ${ToolkitSrcPath}/PopupViewController.h
    ${ToolkitSrcPath}/PopupViewController.cpp
    ${ToolkitSrcPath}/ScalebarController.h
    ${ToolkitSrcPath}/ScalebarController.cpp
    ${ToolkitSrcPath}/SearchResult.h
    ${ToolkitSrcPath}/SearchResult.cpp
    ${ToolkitSrcPath}/SearchSourceInterface.h
    ${ToolkitSrcPath}/SearchSourceInterface.cpp
    ${ToolkitSrcPath}/SearchSuggestion.h
    ${ToolkitSrcPath}/SearchSuggestion.cpp
    ${ToolkitSrcPath}/SearchViewController.h
    ${ToolkitSrcPath}/SearchViewController.cpp
    ${ToolkitSrcPath}/SmartLocatorSearchSource.h
    ${ToolkitSrcPath}/SmartLocatorSearchSource.cpp
    ${ToolkitSrcPath}/TimeSliderController.h
    ${ToolkitSrcPath}/TimeSliderController.cpp
)

add_library(${ModuleName}
    ${ToolkitSourceFiles}
    ${RegistrationSourceFiles}
    ${QmlSourceFiles}
)

target_link_libraries(${ModuleName}
    PUBLIC
        Qt5::Core
        Qt5::Quick
        ArcGISRuntime::Cpp
)

target_include_directories(${ModuleName}
    PUBLIC
        ${UIToolsPath}/register
    PRIVATE
        ${UIToolsPath}/cpp
        ${UIToolsPath}/cpp/Esri/ArcGISRuntime/Toolkit
)

target_compile_definitions(${ModuleName}
    PRIVATE 
        CPP_ARCGISRUNTIME_TOOLKIT
)
Zhumos commented 2 years ago

I have found that the cleanest (and possibly final?) workaround is to set SKIP_AUTOMOC on the GenericListModel.cpp file as recommended by the AutoMoc error. While it's also possible to omit the source files from add_library, some IDE/generator combos will then not list them as project source files (which may or may not be desirable).

Adding the code snippet below to the CMake file above causes the build to complete successfully.

set_property(
    SOURCE ${ToolkitSrcPath}/Internal/GenericListModel.cpp
    PROPERTY SKIP_AUTOMOC ON
)

I have also logged an issue against AutoMoc which can be found here

anmacdonald commented 2 years ago

@Zhumos I appreciate all the work you've put into this investigation!

Hopefully the behaviour of CMake + moc will be fixed in the future. Since we have no timescale for when this will happen, I'll look into the options we can do on our end for anyone else that hits this problem.

Options are:

  1. Put #ifndef Q_MOC_RUN in a block around the GenericListModel comment.
  2. Remove Q_OBJECT
  3. Try to format Q_OBJECT in such a way that the moc bug does not trigger.

Overall, I favour the second option, removing Q_OBJECT, which does not detract from the example code itself.

As you currently have a good workaround, I will put in a fix into our dev branch for 100.14.

Zhumos commented 2 years ago

@anmacdonald,

Thank you for both the other workaround options and the fix, didn't know about the Q_MOC_RUN macro!

The ball is definitely in CMake's court for properly fixing it but it would (I assume) require implementation of handling comments in their parser to fix an edge case so I doubt it will move too quickly...

Out of curiosity (and if you don't mind answering), is there a particular reason why the qdoc comments are in the implementation file rather than the header? I've only ever seen documentation comments like these in header files and I'm quite interested to know what benefits there are doing it the other way around.

anmacdonald commented 2 years ago

@Zhumos It's a good question, but not one I have a good answer for on the design level. Most C++ projects will be written with Doxygen in mind, which favors documenting in the header. We use Qt's QDoc to build our API docs, which favors source-file documentation.

Per the QDoc docs:

Next, QDoc uses the values of the headerdirs variable and/or the headers variable to find and parse all the header files for your project. QDoc does not scan header files for QDoc comments. It parses the header files to build a master tree of all the items that should be documented, in other words, the items that QDoc should find QDoc comments for.

After parsing all the header files and building the master tree of items to be documented, QDoc uses the value of the sourcedirs variable and/or the value of the sources variable to find and parse all the .cpp and .qdoc files for your project. These are the files QDoc scans for QDoc comments.