Skip to content

Conversation

@islas
Copy link
Collaborator

@islas islas commented Aug 6, 2024

TYPE: enhancement

KEYWORDS: cmake, configuration

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
The current solution for implicitly matching symbols between C and Fortran relies on user-modification of a stanza for their system. While stanzas can be configured to a specific architecture and options selection, this approach is limited and overly specific to user configuration when it could be programmatically determined. Likewise, if a new stanza is introduced it will likely only be tested by one user on their particular system. Thus, if this stanza could be used elsewhere but needs underscores on C symbols for Fortran usage it will not be evident why the stanza does not work for this new user.

Solution:
Programmatically determine if underscores are needed in C compilation in the CMake build. Sanitize out any definition of underscore in the stanza so that CMake configuration checks take precedence.

@islas islas requested review from a team as code owners August 6, 2024 03:32
@islas
Copy link
Collaborator Author

islas commented Aug 6, 2024

Requires #2056, #2053, #2086, #2087, #2088, and #2090

@islas islas mentioned this pull request Aug 6, 2024
@amstokely amstokely self-requested a review September 23, 2024 20:56
amstokely
amstokely previously approved these changes Sep 23, 2024
@islas islas force-pushed the cmake-confcheck-underscore branch from b263d5f to 2f2b7df Compare October 16, 2025 23:10
@islas islas requested review from a team as code owners October 16, 2025 23:10
@islas islas changed the base branch from release-v4.6.1 to develop October 16, 2025 23:20
@islas
Copy link
Collaborator Author

islas commented Oct 16, 2025

@amstokely @mgduda This is ready to be reviewed again. The plan is to get this and its dependent PRs into the upcoming release. Thanks!

@islas islas removed request for a team October 16, 2025 23:23
@amstokely amstokely requested a review from Copilot October 21, 2025 20:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a CMake-based system to programmatically determine whether underscores are needed when linking C and Fortran code, replacing the previous manual stanza-based configuration approach. This eliminates platform-specific configuration errors and makes the build system more portable.

Key Changes:

  • Adds compile/link tests to automatically detect C-Fortran symbol naming conventions
  • Removes hardcoded underscore definitions from architecture configuration files
  • Applies the detected underscore requirements as compile definitions across the project

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
confcheck/check_underscore.f90 Test program that calls a C function from Fortran to verify symbol compatibility
confcheck/check_underscore.c C implementation that conditionally adds underscores based on preprocessor definitions
confcheck/CMakeLists.txt Configuration checks that test both NOUNDERSCORE and UNDERSCORE symbol conventions
cmake/confcheck.cmake Minor improvements to check result handling and error messaging
arch/configure_reader.py Sanitization logic to strip underscore definitions from legacy stanzas
CMakeLists.txt Applies the detected underscore convention as a compile definition

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

endif()


# Check if underscores are needing for implicit c-Fortran intrefacing functions that
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'needing' to 'needed' and 'intrefacing' to 'interfacing'.

Suggested change
# Check if underscores are needing for implicit c-Fortran intrefacing functions that
# Check if underscores are needed for implicit c-Fortran interfacing functions that

Copilot uses AI. Check for mistakes.
# don't use ISO C binding to map symbols
wrf_conf_check(
# try_run() with more than one source was introduced in CMake 3.25+
# so we won't use it here until we see reasonable need to incease the
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'incease' to 'increase'.

Suggested change
# so we won't use it here until we see reasonable need to incease the
# so we won't use it here until we see reasonable need to increase the

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,8 @@
#include <stdio.h>
#ifdef UNDERSCORE
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check uses #ifdef UNDERSCORE but the CMake configuration defines -DUNDERSCORE which sets it to a value. Consider using #if defined(UNDERSCORE) or checking if the define should be set without a value (e.g., just -DUNDERSCORE without =1).

Suggested change
#ifdef UNDERSCORE
#if defined(UNDERSCORE)

Copilot uses AI. Check for mistakes.
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.

2 participants