https://github.com/legrosbuffle closed
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
legrosbuffle wrote:
Thanks
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -263,19 +264,26 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
void UnnecessaryCopyInitialization::check(
const MatchFinder::MatchResult ) {
- const auto *NewVar = Result.Nodes.getNodeAs("newVarDecl");
+ const auto =
@@ -289,74 +297,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH 1/5] [clang-tidy] performance-unnecessary-copy-init
@@ -289,74 +297,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
@@ -263,19 +264,26 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
void UnnecessaryCopyInitialization::check(
const MatchFinder::MatchResult ) {
- const auto *NewVar = Result.Nodes.getNodeAs("newVarDecl");
+ const auto =
https://github.com/PiotrZSL commented:
Last nits, and it will look fine.
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL edited
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -261,21 +262,27 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
this);
}
+UnnecessaryCopyInitialization::CheckContext::CheckContext(
legrosbuffle wrote:
I think it's a simple matter of preference, I
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH 1/4] [clang-tidy] performance-unnecessary-copy-init
@@ -261,21 +262,27 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
this);
}
+UnnecessaryCopyInitialization::CheckContext::CheckContext(
PiotrZSL wrote:
Problem is that code were moved from check method to
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH 1/3] [clang-tidy] performance-unnecessary-copy-init
https://github.com/legrosbuffle edited
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -32,14 +32,34 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck
{
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void storeOptions(ClangTidyOptions::OptionMap ) override;
+protected:
+ // A helper to manipulate the state common to
+
@@ -290,69 +296,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
@@ -261,21 +262,27 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
this);
}
+UnnecessaryCopyInitialization::CheckContext::CheckContext(
legrosbuffle wrote:
Given that we are deriving `IssueFix`,
@@ -32,14 +32,34 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck
{
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void storeOptions(ClangTidyOptions::OptionMap ) override;
+protected:
+ // A helper to manipulate the state common to
+
@@ -290,69 +296,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
https://github.com/PiotrZSL edited
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -261,21 +262,27 @@ void
UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
this);
}
+UnnecessaryCopyInitialization::CheckContext::CheckContext(
PiotrZSL wrote:
I'm not a fan of this constructor, it should exist.
https://github.com/fberger edited
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -290,69 +296,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
@@ -290,69 +296,72 @@ void UnnecessaryCopyInitialization::check(
// instantiations where the types differ and rely on implicit conversion
would
// no longer compile if we switched to a reference.
if (differentReplacedTemplateParams(
- NewVar->getType(),
https://github.com/fberger approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
legrosbuffle wrote:
Done. I've kept the code common to these two
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH 1/2] [clang-tidy] performance-unnecessary-copy-init
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH 1/2] [clang-tidy] performance-unnecessary-copy-init
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
PiotrZSL wrote:
Then create 2 "makeDiagnostic", one for one and one for
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init
Refactor
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
+const DeclStmt , ASTContext , bool IssueFix) {
+ const bool IsVarUnused =
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
+const DeclStmt , ASTContext , bool IssueFix) {
+ const bool IsVarUnused =
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
legrosbuffle wrote:
I can't really do that because each of the two uses
@@ -32,6 +32,12 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void storeOptions(ClangTidyOptions::OptionMap ) override;
+protected:
+ // This is virtual so that derived classes can
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
+const DeclStmt , ASTContext , bool IssueFix) {
+ const bool IsVarUnused =
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
PiotrZSL wrote:
Create Diagnostic fully in this method instead of
@@ -302,6 +303,20 @@ void UnnecessaryCopyInitialization::check(
}
}
+void UnnecessaryCopyInitialization::makeDiagnostic(
+DiagnosticBuilder Diagnostic, const VarDecl , const Stmt ,
+const DeclStmt , ASTContext , bool IssueFix) {
+ const bool IsVarUnused =
legrosbuffle wrote:
This is ready for another look. I took this as an opportunity to refactor the
duplicated parts of the code. Let me know what you think.
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 0bf4d8335063c1403dc91d8fccca42e3f03bad35 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init
Refactor
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From a072ae129b1ad928c96368a3208d35cc0705e260 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init
Refactor
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From d729f95270aff076ff11ccb1ef12cfb6dc99a82d Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init: Add a
legrosbuffle wrote:
> In theory changes that are required for "private" checks should be done by
> you on some own private fork instead of adding that "unused" code here. But
> only option I see is simply moving diagnostic to separate methods, and make
> those methods virtual.
I see, we
PiotrZSL wrote:
In theory changes that are required for "private" checks should be done by you
on some own private fork instead of adding that "unused" code here. But only
option I see is simply moving diagnostic to separate methods, and make those
methods virtual. That could be +-
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From a6bc3d7ef798f95fe6ae758e7a9502851e6d4b12 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init: Add a
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From 6050ba4784137bc20e58d553ea970e37237142ae Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init: Add a
https://github.com/legrosbuffle edited
https://github.com/llvm/llvm-project/pull/73921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
legrosbuffle wrote:
> First what's a purpose of this change. Is there any other check that will
> need it or not ?
We have a downstream check that wants to do additional processing after this
one. We can't submit that check because it depends on a large piece of
downstream infrastructure.
>
PiotrZSL wrote:
First what's a purpose of this change. Is there any other check that will need
it or not ?
Second is that actually onWarningEmitted should be more a emitWarning, and
those diag should be inside it, so that they could be changed if needed. Or
condition required to emit a
https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/73921
>From e0d1f9741d0dba3286fd8043cf6d5c89ecc2d378 Mon Sep 17 00:00:00 2001
From: Clement Courbet
Date: Thu, 30 Nov 2023 11:08:51 +0100
Subject: [PATCH] [clang-tidy] performance-unnecessary-copy-init: Add a
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff e620035a28d5d957623aa7b4aeda35ab5130e2c9
e251c440b42fe67dee4022d08d05e96d30d6aa02 --
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Clement Courbet (legrosbuffle)
Changes
... so that derived checks can implement custom behaviour. Does nothing by
default, which makes this an NFC.
---
Full diff: https://github.com/llvm/llvm-project/pull/73921.diff
2 Files
https://github.com/legrosbuffle created
https://github.com/llvm/llvm-project/pull/73921
... so that derived checks can implement custom behaviour. Does nothing by
default, which makes this an NFC.
>From e251c440b42fe67dee4022d08d05e96d30d6aa02 Mon Sep 17 00:00:00 2001
From: Clement Courbet
54 matches
Mail list logo
