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

[clang-tidy] fix misc-const-correctness to work with function-try-blocks #99925

Merged

Conversation

schenker
Copy link
Contributor

Make the clang-tidy check misc-const-correctness work with function-try-blocks.

Fixes #99860.

@llvmbot
Copy link
Member

llvmbot commented Jul 22, 2024

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Thomas Schenker (schenker)

Changes

Make the clang-tidy check misc-const-correctness work with function-try-blocks.

Fixes #99860.


Full diff: https://github.com/llvm/llvm-project/pull/99925.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp (+8-9)
  • (modified) clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h (+2-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp (+9)
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
index 8b500de0c028c..e20cf6fbcb55a 100644
--- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
@@ -93,13 +93,12 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
   // shall be run.
   const auto FunctionScope =
       functionDecl(
-          hasBody(
-              compoundStmt(forEachDescendant(
-                               declStmt(containsAnyDeclaration(
-                                            LocalValDecl.bind("local-value")),
-                                        unless(has(decompositionDecl())))
-                                   .bind("decl-stmt")))
-                  .bind("scope")))
+          hasBody(stmt(forEachDescendant(
+                           declStmt(containsAnyDeclaration(
+                                        LocalValDecl.bind("local-value")),
+                                    unless(has(decompositionDecl())))
+                               .bind("decl-stmt")))
+                      .bind("scope")))
           .bind("function-decl");
 
   Finder->addMatcher(FunctionScope, this);
@@ -109,7 +108,7 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
 enum class VariableCategory { Value, Reference, Pointer };
 
 void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *LocalScope = Result.Nodes.getNodeAs<CompoundStmt>("scope");
+  const auto *LocalScope = Result.Nodes.getNodeAs<Stmt>("scope");
   const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("local-value");
   const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl");
 
@@ -198,7 +197,7 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
   }
 }
 
-void ConstCorrectnessCheck::registerScope(const CompoundStmt *LocalScope,
+void ConstCorrectnessCheck::registerScope(const Stmt *LocalScope,
                                           ASTContext *Context) {
   auto &Analyzer = ScopesCache[LocalScope];
   if (!Analyzer)
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
index 08ffde524522a..bba060e555d00 100644
--- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
@@ -32,10 +32,10 @@ class ConstCorrectnessCheck : public ClangTidyCheck {
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  void registerScope(const CompoundStmt *LocalScope, ASTContext *Context);
+  void registerScope(const Stmt *LocalScope, ASTContext *Context);
 
   using MutationAnalyzer = std::unique_ptr<ExprMutationAnalyzer>;
-  llvm::DenseMap<const CompoundStmt *, MutationAnalyzer> ScopesCache;
+  llvm::DenseMap<const Stmt *, MutationAnalyzer> ScopesCache;
   llvm::DenseSet<SourceLocation> TemplateDiagnosticsCache;
 
   const bool AnalyzeValues;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a23483e6df6d2..ed005d8fa19ea 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,9 @@ Improvements to clang-tidy
 - Improved :program:`check_clang_tidy.py` script. Added argument `-export-fixes`
   to aid in clang-tidy and test development.
 
+- Improved :doc:`misc-const-correctness
+  <clang-tidy/checks/misc/const-correctness>` to work with function-try-blocks.
+
 - Fixed bug where big values for unsigned check options overflowed into negative values
   when being printed with `--dump-config`.
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
index cb6bfcc1dccba..2af4bfb0bd449 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -56,6 +56,15 @@ void some_function(double np_arg0, wchar_t np_arg1) {
   np_local6--;
 }
 
+int function_try_block() try {
+  int p_local0 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  // CHECK-FIXES: int const p_local0
+  return p_local0;
+} catch (...) {
+  return 0;
+}
+
 void nested_scopes() {
   int p_local0 = 2;
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

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

Release notes need small fix, except that looks fine

@schenker schenker requested a review from PiotrZSL July 22, 2024 19:59
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

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

LGTM

@PiotrZSL PiotrZSL merged commit cd9e42c into llvm:main Jul 23, 2024
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 23, 2024

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-ubuntu-fast running on sie-linux-worker while building clang-tools-extra at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/2968

Here is the relevant piece of the build log for the reference:

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang Tools :: clang-tidy/checkers/misc/const-correctness-values.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Running ['clang-tidy', '/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp', '-fix', '--checks=-*,misc-const-correctness', '-config={CheckOptions: {     misc-const-correctness.TransformValues: true,      misc-const-correctness.WarnPointersAsValues: false,      misc-const-correctness.TransformPointersAsValues: false    }}', '--', '-fno-delayed-template-parsing', '-std=c++11', '-nostdinc++']...
clang-tidy /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp -fix --checks=-*,misc-const-correctness -config={CheckOptions: {     misc-const-correctness.TransformValues: true,      misc-const-correctness.WarnPointersAsValues: false,      misc-const-correctness.TransformPointersAsValues: false    }} -- -fno-delayed-template-parsing -std=c++11 -nostdinc++ failed:
78 warnings and 1 error generated.
Error while processing /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp.
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:38:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
   38 |   int p_local0 = 2;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:59:26: error: cannot use 'try' with exceptions disabled [clang-diagnostic-error]
   59 | int function_try_block() try {
      |                          ^
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:60:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
   60 |   int p_local0 = 0;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:69:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
   69 |   int p_local0 = 2;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:75:5: warning: variable 'p_local1' of type 'int' can be declared 'const' [misc-const-correctness]
   75 |     int p_local1 = 42;
      |     ^
      |         const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:89:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
   89 |   int p_local0 = 1;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:99:3: warning: variable 'p_local1' of type 'int' can be declared 'const' [misc-const-correctness]
   99 |   int p_local1 = 0;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:106:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
  106 |   int p_local0 = 1;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:139:3: warning: variable 'p_local1' of type 'int' can be declared 'const' [misc-const-correctness]
  139 |   int p_local1 = 42;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:144:3: warning: variable 'p_local2' of type 'int' can be declared 'const' [misc-const-correctness]
  144 |   int p_local2 = 42;
      |   ^
      |       const 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/test/clang-tidy/checkers/misc/Output/const-correctness-values.cpp.tmp.cpp:163:3: warning: variable 'p_local0' of type 'int' can be declared 'const' [misc-const-correctness]
  163 |   int p_local0 = 42;
...

@PiotrZSL
Copy link
Member

Looks like missing -fexceptions

PiotrZSL pushed a commit that referenced this pull request Jul 23, 2024
…cks (#99925)

Make the clang-tidy check misc-const-correctness work with
function-try-blocks.

Fixes #99860.
@schenker
Copy link
Contributor Author

schenker commented Jul 23, 2024

Looks like missing -fexceptions

@PiotrZSL thanks for fixing it

sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
…cks (llvm#99925)

Make the clang-tidy check misc-const-correctness work with
function-try-blocks.

Fixes llvm#99860.
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
…cks (llvm#99925)

Make the clang-tidy check misc-const-correctness work with
function-try-blocks.

Fixes llvm#99860.
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…cks (#99925)

Make the clang-tidy check misc-const-correctness work with
function-try-blocks.

Fixes #99860.
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…-try-blocks" (#100069)

Summary: Reverts #99925

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251283
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…cks (#99925)

Summary:
Make the clang-tidy check misc-const-correctness work with
function-try-blocks.

Fixes #99860.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251131
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Related to #99925.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251161
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.

[clang-tidy] false negative for misc-const-correctness in function-try-block
5 participants