Skip to content

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Aug 10, 2025

User description

Trying to improve coverage accuracy via a couple of simple additions.


PR Type

Enhancement


Description

  • Add -O0 flag to disable optimization for coverage

  • Configure Fypp preprocessor with extended line length

  • Enable no-continuation-lines mode for better coverage tracking


Diagram Walkthrough

flowchart LR
  A["CMakeLists.txt"] --> B["GNU Fortran Coverage"]
  A --> C["Fypp Preprocessor"]
  B --> D["-O0 flag added"]
  C --> E["--line-length=999"]
  C --> F["--line-numbering-mode=nocontlines"]
Loading

File Walkthrough

Relevant files
Configuration changes
CMakeLists.txt
Enhanced coverage configuration and preprocessor settings

CMakeLists.txt

  • Add -O0 compiler flag for GNU Fortran coverage builds
  • Configure Fypp preprocessor with extended line length (999)
  • Set no-continuation-lines mode for line numbering
+3/-0     

@sbryngelson sbryngelson marked this pull request as ready for review August 10, 2025 05:08
Copilot AI review requested due to automatic review settings August 10, 2025 05:08
@sbryngelson sbryngelson requested a review from a team as a code owner August 10, 2025 05:08
Copy link
Contributor

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 aims to improve coverage accuracy by making two simple adjustments: disabling compiler optimizations during coverage builds and modifying Fypp preprocessor settings to better preserve source code structure for coverage analysis.

  • Adds -O0 flag to disable optimizations when coverage is enabled with GNU Fortran compiler
  • Updates Fypp preprocessor to use extended line length and different line numbering mode

@qodo-merge-pro
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Build Impact

Adding -O0 for all GNU Fortran compilation in coverage builds can dramatically change performance and may expose timing-related issues; validate that it is gated only for coverage configurations and does not bleed into non-coverage builds.

        add_compile_options(
            $<$<COMPILE_LANGUAGE:Fortran>:-fprofile-arcs>
            $<$<COMPILE_LANGUAGE:Fortran>:-ftest-coverage>
            $<$<COMPILE_LANGUAGE:Fortran>:-O0>
	    )
Fypp Flags Robustness

The new Fypp options (--line-length=999 and --line-numbering-mode=nocontlines) may not be supported across all fypp versions; consider guarding with a version check or providing fallbacks to avoid CI failures on older environments.

                                 -D chemistry=False
                                 --line-numbering
                                 --no-folding
								 --line-length=999
		 						 --line-numbering-mode=nocontlines
                                 "${fpp}" "${f90}"
Formatting/Whitespace

Mixed tabs/spaces in the new fypp option lines can cause readability or style lint issues in CMake; align indentation consistently with surrounding code.

						 --line-length=999
 						 --line-numbering-mode=nocontlines

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 10, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Normalize whitespace in arguments

Avoid using tab-indented lines and mixed whitespace which can break command
parsing in some CMake/Fypp invocations and cause subtle errors. Normalize
indentation to spaces to ensure the arguments are passed exactly as intended.

CMakeLists.txt [362-366]

 --line-numbering
                                  --no-folding
-								 --line-length=999
-		 						 --line-numbering-mode=nocontlines
+                                 --line-length=999
+                                 --line-numbering-mode=nocontlines
                                  "${fpp}" "${f90}"
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies mixed tabs and spaces in the new code and proposes normalizing to spaces, which improves code style, readability, and robustness.

Low
  • Update

@sbryngelson sbryngelson merged commit 5e8f116 into MFlowCode:master Aug 11, 2025
28 of 29 checks passed
@sbryngelson sbryngelson deleted the coverage-improvement branch August 14, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant