Skip to content

Make JsonProcessor.h self-contained - #239

Open
bob sira (bobsira) wants to merge 1 commit into
mainfrom
fix/jsonprocessor-self-contained-header
Open

bob sira (bobsira) wants to merge 1 commit into
mainfrom
fix/jsonprocessor-self-contained-header

Conversation

@bobsira

Copy link
Copy Markdown
Contributor

Summary

Fixes #223.

JsonProcessor.h depended on the precompiled header to supply JSON, Windows, STL, and project-type declarations. Including it first in a translation unit therefore failed to compile.

  • Include the required Windows/STL headers, the library-provided nlohmann/json_fwd.hpp, and Utility.h for AttributesMap.
  • Forward-declare LogSource and the existing _LoggerSettings type instead of pulling in the settings implementation.
  • Add the Windows, math, and wide-character headers required by Utility.h itself.
  • Include JsonProcessor.h first in the existing parser tests and disable PCH for that source in the Visual Studio project. The CMake test target already compiles without PCH.
  • Document this compile-time regression check.

No runtime logic or API signatures change.

Validation

  • Before the fix, compiling the existing parser test with JsonProcessor.h first failed with missing declarations; it succeeds after the fix.
  • A translation unit containing only #include "JsonProcessor.h" compiles on x64 and x86 with MSVC /Y- /W4 /WX.
  • CMake x64 Release builds of LogMonitor and LogMonitorTests pass.
  • All 7 selected parser and utility tests pass.
  • Native Visual Studio test-source compilation passes for x64 and Win32. Evaluated project metadata and fresh compiler records confirm PCH is disabled for JsonProcessorTests.cpp.

Validation was scoped to the changed headers and their consumers; the full EventMonitor suite was not rerun. Its pre-existing historical-replay crash is tracked separately in #238.

Declare header dependencies explicitly, supply the dependencies required by Utility.h, and use forward declarations for JSON and settings types. Compile the existing parser test without PCH and include JsonProcessor.h first to guard against regressions.

Addresses #223.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JsonProcessor.h is not self-contained (missing includes / forward declarations)

1 participant