[clang] [clang][ExtractAPI][NFC] pass params by const reference (PR #94820)

2024-06-07 Thread Erick Velez via cfe-commits 

https://github.com/evelez7 created 
https://github.com/llvm/llvm-project/pull/94820

Change some parameters in DeclarationFragments.h to be passed by const 
reference. Caught by cppcheck.

Fixes #92756 but doesn't address return value `RT` for `getTopLevelRecords`. 
I'm not sure we'd want a const return of the top records, and returning `RT` by 
reference makes clang complain about returning a temporary object.

>From 98840a10f31705ab684375bf77dcab46ba9009ee Mon Sep 17 00:00:00 2001
From: Erick Velez 
Date: Fri, 7 Jun 2024 16:23:09 -0700
Subject: [PATCH] [clang][ExtractAPI][NFC] pass params by const reference

---
 clang/include/clang/ExtractAPI/DeclarationFragments.h | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/ExtractAPI/DeclarationFragments.h 
b/clang/include/clang/ExtractAPI/DeclarationFragments.h
index 535da90b98284..7dae4e2f8ac1d 100644
--- a/clang/include/clang/ExtractAPI/DeclarationFragments.h
+++ b/clang/include/clang/ExtractAPI/DeclarationFragments.h
@@ -199,7 +199,8 @@ class DeclarationFragments {
 return *this;
   }
 
-  DeclarationFragments &replace(std::string NewSpelling, unsigned Position) {
+  DeclarationFragments &replace(const std::string &NewSpelling,
+unsigned Position) {
 Fragments.at(Position).Spelling = NewSpelling;
 return *this;
   }
@@ -240,7 +241,7 @@ class DeclarationFragments {
 
 class AccessControl {
 public:
-  AccessControl(std::string Access) : Access(Access) {}
+  AccessControl(const std::string &Access) : Access(Access) {}
   AccessControl() : Access("public") {}
 
   const std::string &getAccess() const { return Access; }
@@ -262,7 +263,7 @@ class FunctionSignature {
 std::string Name;
 DeclarationFragments Fragments;
 
-Parameter(StringRef Name, DeclarationFragments Fragments)
+Parameter(StringRef Name, const DeclarationFragments &Fragments)
 : Name(Name), Fragments(Fragments) {}
   };
 
@@ -275,7 +276,7 @@ class FunctionSignature {
 return *this;
   }
 
-  void setReturnType(DeclarationFragments RT) { ReturnType = RT; }
+  void setReturnType(const DeclarationFragments &RT) { ReturnType = RT; }
 
   /// Determine if the FunctionSignature is empty.
   ///

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ExtractAPI][NFC] pass params by const reference (PR #94820)

2024-06-07 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Erick Velez (evelez7)


Changes

Change some parameters in DeclarationFragments.h to be passed by const 
reference. Caught by cppcheck.

Fixes #92756 but doesn't address return value `RT` for 
`getTopLevelRecords`. I'm not sure we'd want a const return of the top records, 
and returning `RT` by reference makes clang complain about returning a 
temporary object.

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


1 Files Affected:

- (modified) clang/include/clang/ExtractAPI/DeclarationFragments.h (+5-4) 


``diff
diff --git a/clang/include/clang/ExtractAPI/DeclarationFragments.h 
b/clang/include/clang/ExtractAPI/DeclarationFragments.h
index 535da90b98284..7dae4e2f8ac1d 100644
--- a/clang/include/clang/ExtractAPI/DeclarationFragments.h
+++ b/clang/include/clang/ExtractAPI/DeclarationFragments.h
@@ -199,7 +199,8 @@ class DeclarationFragments {
 return *this;
   }
 
-  DeclarationFragments &replace(std::string NewSpelling, unsigned Position) {
+  DeclarationFragments &replace(const std::string &NewSpelling,
+unsigned Position) {
 Fragments.at(Position).Spelling = NewSpelling;
 return *this;
   }
@@ -240,7 +241,7 @@ class DeclarationFragments {
 
 class AccessControl {
 public:
-  AccessControl(std::string Access) : Access(Access) {}
+  AccessControl(const std::string &Access) : Access(Access) {}
   AccessControl() : Access("public") {}
 
   const std::string &getAccess() const { return Access; }
@@ -262,7 +263,7 @@ class FunctionSignature {
 std::string Name;
 DeclarationFragments Fragments;
 
-Parameter(StringRef Name, DeclarationFragments Fragments)
+Parameter(StringRef Name, const DeclarationFragments &Fragments)
 : Name(Name), Fragments(Fragments) {}
   };
 
@@ -275,7 +276,7 @@ class FunctionSignature {
 return *this;
   }
 
-  void setReturnType(DeclarationFragments RT) { ReturnType = RT; }
+  void setReturnType(const DeclarationFragments &RT) { ReturnType = RT; }
 
   /// Determine if the FunctionSignature is empty.
   ///

``




https://github.com/llvm/llvm-project/pull/94820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ExtractAPI][NFC] pass params by const reference (PR #94820)

2024-06-10 Thread Daniel Grumberg via cfe-commits 


@@ -199,7 +199,8 @@ class DeclarationFragments {
 return *this;
   }
 
-  DeclarationFragments &replace(std::string NewSpelling, unsigned Position) {

daniel-grumberg wrote:

I would prefer to keep the value semantics version and instead move assign the 
value, that way callers can move the string in and avoid any copies.

https://github.com/llvm/llvm-project/pull/94820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ExtractAPI][NFC] pass params by const reference (PR #94820)

2024-06-10 Thread Daniel Grumberg via cfe-commits 


@@ -240,7 +241,7 @@ class DeclarationFragments {
 
 class AccessControl {
 public:
-  AccessControl(std::string Access) : Access(Access) {}

daniel-grumberg wrote:

Again I would prefer if we kept the value semantic ones and use `std::move`

https://github.com/llvm/llvm-project/pull/94820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ExtractAPI][NFC] pass params by const reference (PR #94820)

2024-06-10 Thread Daniel Grumberg via cfe-commits 

daniel-grumberg wrote:

Thanks for looking at these, I think it would be best to try and leverage move 
semantics where possible if we are going to change this code.

https://github.com/llvm/llvm-project/pull/94820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits