Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ament_clang_tidy - Fix Reporting when WarningsAsErrors is specified in config (backport #397) #488

Merged
merged 1 commit into from
May 10, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 1, 2024

Issue Summary

When WarningsAsErrors is specified in .clang-tidy, this can lead to some failures in result reporting. Specifically, the following test program (src/factorial.cpp):

int Factorial(int n) {
    int result = 1;
    int fake = 1;
    for (int i = 1; i <= n; i++) {
        result *= i;
    }
    return result;
}

fails with this output:

The invocation of "clang-tidy" failed with error code 1: Command '['/usr/bin/clang-tidy', '-p', 'path/test_examples_cmake', '--header-filter', 'include/test_examples_cmake/.*', 'path']' returned non-zero exit status 1.

when running ament_clang_tidy.

After applying this fix, I get the following output, which is expected

src/factorial.cpp:36:9: error: unused variable 'fake' [clang-diagnostic-unused-variable,-warnings-as-errors]
    int fake = 1;
        ^

Relevant section of .clang-tidy, for reference:

Checks:
    'bugprone*,
     clang-analyzer*,
     cert*,
     performance*,
     portability*,
     readability*,
     hicpp*,
     -hicpp-signed-bitwise,
     google-readability-todo,
    '
FormatStyle:     file
WarningsAsErrors: '*'

Fix

By trying to access the output attribute of the raised exception, we can populate the output in cases where exceptions were raised and there was valid output.


This is an automatic backport of pull request #397 done by [Mergify](https://mergify.com).

…n config (#397)

* feature

Signed-off-by: Matt Condino <mwcondino@gmail.com>
(cherry picked from commit f985e67)
@mhidalgo-bdai
Copy link

mhidalgo-bdai commented May 2, 2024

Seems OK. Same for the rest of backport PRs.

@sloretz
Copy link
Contributor

sloretz commented May 9, 2024

Assigned @clalancette to run CI and merge 🧇

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit c5e554a into humble May 10, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/humble/pr-397 branch May 10, 2024 11:35
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.

None yet

5 participants