enabled and mitigated `readability-const-return-type` clang-tidy warnings #5644

firewave · 2023-11-09T10:08:58Z

No description provided.

firewave · 2023-11-09T11:15:35Z

Still need to file upstream tickets about the false positive and lacking context:

/home/runner/work/cppcheck/cppcheck/lib/utils.h:39:5: error: return type 'typename pair<const int, const Variable *>::first_type' (aka 'const int') is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type,-warnings-as-errors]
   39 |     typename Pair::first_type operator()(const Pair& p) const {
      |     ^

chrchr-github · 2023-11-09T11:21:29Z

gui/projectfiledialog.cpp

@@ -772,6 +772,7 @@ void ProjectFileDialog::addSingleSuppression(const Suppressions::Suppression &su
 void ProjectFileDialog::setSuppressions(const QList<Suppressions::Suppression> &suppressions)
 {
    mUI->mListSuppressions->clear();
+    // NOLINTNEXTLINE(readability-const-return-type) - neeeds to be a copy since it might be a reference to mSuppressions


Is this really related?

What do you mean?

There is no return type on that line, so what does clang-tidy warn about?

Good point. Maybe yet another false positive in this check. Although it is a valid warning and possibly even a false negative on our side.

Sounds like the NOLINT should be for performance-unnecessary-copy-initialization?

If it would report that warning and we would not suppress it correctly the CI would fail.

Yeah, but unmatched NOLINTs don't seem to cause a warning: llvm/llvm-project#69674

Yeah - this is unrelated. Will revert.

…ings

firewave · 2023-11-24T00:27:28Z

I filed llvm/llvm-project#73270 about the missing context.

firewave force-pushed the tidy-const-return branch from 0956a56 to 6586d00 Compare November 9, 2023 10:39

firewave marked this pull request as draft November 9, 2023 10:39

firewave force-pushed the tidy-const-return branch from 6586d00 to 9b5a688 Compare November 9, 2023 11:14

firewave marked this pull request as ready for review November 9, 2023 11:15

chrchr-github reviewed Nov 9, 2023

View reviewed changes

firewave marked this pull request as draft November 9, 2023 12:14

enabled and mitigated readability-const-return-type clang-tidy warn…

6bfcc81

…ings

firewave force-pushed the tidy-const-return branch from 9b5a688 to 6bfcc81 Compare November 24, 2023 00:27

firewave marked this pull request as ready for review November 24, 2023 00:27

chrchr-github merged commit 86bb7c9 into danmar:main Nov 24, 2023
68 checks passed

firewave deleted the tidy-const-return branch November 24, 2023 17:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enabled and mitigated `readability-const-return-type` clang-tidy warnings #5644

enabled and mitigated `readability-const-return-type` clang-tidy warnings #5644

firewave commented Nov 9, 2023

firewave commented Nov 9, 2023

chrchr-github Nov 9, 2023

firewave Nov 9, 2023

chrchr-github Nov 9, 2023

firewave Nov 9, 2023

chrchr-github Nov 9, 2023

firewave Nov 9, 2023

chrchr-github Nov 9, 2023

firewave Nov 18, 2023

firewave Nov 24, 2023

firewave commented Nov 24, 2023

enabled and mitigated readability-const-return-type clang-tidy warnings #5644

enabled and mitigated readability-const-return-type clang-tidy warnings #5644

Conversation

firewave commented Nov 9, 2023

firewave commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

firewave commented Nov 24, 2023

enabled and mitigated `readability-const-return-type` clang-tidy warnings #5644

enabled and mitigated `readability-const-return-type` clang-tidy warnings #5644