Skip to content

Feat/compile time wrap check#7074

Open
shivansh023023 wants to merge 2 commits intoTheHPXProject:masterfrom
shivansh023023:feat/compile-time-wrap-check
Open

Feat/compile time wrap check#7074
shivansh023023 wants to merge 2 commits intoTheHPXProject:masterfrom
shivansh023023:feat/compile-time-wrap-check

Conversation

@shivansh023023
Copy link
Copy Markdown
Contributor

Fixes #

Proposed Changes

  • Add INTERFACE compile definition HPX_HAVE_WRAP_MAIN_CONFIGURED to both hpx_wrap and hpx_auto_wrap targets in wrap/CMakeLists.txt (inside the existing HPX_WITH_DYNAMIC_HPX_MAIN + Linux guard). This injects the define automatically into any downstream CMake consumer, ensuring zero noise for CMake users.
  • Add a #warning in wrap/include/hpx/hpx_main.hpp that fires when all of these are true: __linux__, HPX_HAVE_DYNAMIC_HPX_MAIN, HPX_HAVE_STATIC_LINKING, and HPX_HAVE_WRAP_MAIN_CONFIGURED is not defined. This catches the missing -Wl,-wrap=main linker flag at compile time instead of letting it become a runtime crash.
  • The warning message is actionable: it tells the user exactly which flag to add and how to suppress the warning if they've already handled it.

Any background context you want to provide?

This is the compile-time component of a safety net for the UX gap where -Wl,-wrap=main does not propagate to non-CMake consumers (Godbolt / Compiler Explorer, Makefiles, raw g++ invocations). It complements:

Design trade-off: Non-CMake users who have already added -Wl,-wrap=main manually will still see the #warning (since they lack the define). They can silence it with -DHPX_HAVE_WRAP_MAIN_CONFIGURED. A suppressible compile-time warning is far better than a silent compilation followed by a cryptic runtime crash.

This work is part of the GSoC 2026 effort to integrate HPX into Compiler Explorer.

Checklist

  • I have added a new feature and have added tests to go along with it.
    • Verified with 6 preprocessor logic tests covering all define combinations (Linux/macOS, static/shared, dynamic-main on/off, suppress define present/absent). Warning fires in exactly one case: Linux + static + dynamic-main + no suppress define.
  • I have fixed a bug and have added a regression test. (N/A — this is a new diagnostic, not a bugfix)
  • I have added a test using random numbers (N/A)

Signed-off-by: shivansh023023 <singhshivansh023@gmail.com>
@shivansh023023 shivansh023023 requested a review from hkaiser as a code owner March 20, 2026 22:01
@shivansh023023 shivansh023023 force-pushed the feat/compile-time-wrap-check branch from e1557d8 to 399b4ed Compare March 20, 2026 22:03
@StellarBot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Mar 20, 2026

Have you seen the existing HPX_HAVE_DYNAMIC_HPX_MAIN? Would that be sufficient for what you're trying to achieve?

@shivansh023023
Copy link
Copy Markdown
Contributor Author

Have you seen the existing HPX_HAVE_DYNAMIC_HPX_MAIN? Would that be sufficient for what you're trying to achieve?

That's a good point, @hkaiser . I did consider HPX_HAVE_DYNAMIC_HPX_MAIN, but that macro indicates that the feature is enabled in the HPX build configuration.

The reason for the new HPX_HAVE_WRAP_MAIN_CONFIGURED (injected via INTERFACE compile definitions) is to distinguish between:

CMake users: Who get the linker flag automatically and should not see the warning.

Non-CMake users: Who have the feature enabled in their HPX installation but haven't necessarily passed the -Wl,-wrap=main flag to their manual link command.

Without the new definition, hpx_main.hpp wouldn't be able to tell if the user linking manually has actually applied the flag or not. Does that make sense, or is there a way to leverage the existing macro to detect the linker-state that I might have missed?

Signed-off-by: shivansh023023 <singhshivansh023@gmail.com>
@shivansh023023 shivansh023023 force-pushed the feat/compile-time-wrap-check branch from 399b4ed to 17f832f Compare March 21, 2026 07:52
@codacy-production
Copy link
Copy Markdown

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 2bfc2691
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (2bfc269) Report Missing Report Missing Report Missing
Head commit (17f832f) 49031 0 0.00%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#7074) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Mar 21, 2026

@shivansh023023 Could you please have a look at the compilation failure generated?

shivansh023023 added a commit to shivansh023023/hpx that referenced this pull request Mar 22, 2026
Add a compile-time diagnostic that fires when dynamic main() wrapping is
enabled (HPX_HAVE_DYNAMIC_HPX_MAIN) but the -Wl,-wrap=main linker flag
was not configured (HPX_HAVE_WRAP_MAIN_CONFIGURED). Uses #pragma message
instead of #warning to avoid -Werror failures.
CMake targets hpx_wrap and hpx_auto_wrap now define
HPX_HAVE_WRAP_MAIN_CONFIGURED, suppressing the diagnostic for properly
linked consumers.
Refs: TheHPXProject#7074

Signed-off-by: shivansh023023 <singhshivansh023@gmail.com>
@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Mar 24, 2026

The CIs still report that one of the translation units can't find the header <hpx/config/static_linker_check.hpp>. Did you miss adding the file to this PR?

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Mar 28, 2026

@shivansh023023 ping?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants