[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D149718#4342320 , @Manna wrote:

> In D149718#4341384 , @uabelho wrote:
>
>> In D149718#4341052 , @uabelho 
>> wrote:
>>
>>> Compiling with clang 15.0.5 I get the following warning/error with this 
>>> patch:
>>>
>>>   ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly 
>>> defaulted move assignment operator is implicitly deleted 
>>> [-Werror,-Wdefaulted-function-deleted]
>>> AttributePool =(AttributePool &) = default;
>>>^
>>>   ../../clang/include/clang/Sema/ParsedAttr.h:674:21: note: move assignment 
>>> operator of 'AttributePool' is implicitly deleted because field 'Factory' 
>>> is of reference type 'clang::AttributeFactory &'
>>> AttributeFactory 
>>>   ^
>>>   1 error generated.
>>
>> Also seen in this buildbot:
>>  https://lab.llvm.org/buildbot/#/builders/214/builds/7476
>
> Thank you @uabelho for reporting. Build error is fixed by 
> https://github.com/llvm/llvm-project/commit/292a6c1c2395f990bbde8d968825243e4fe9b954

Great, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

In D149718#4341384 , @uabelho wrote:

> In D149718#4341052 , @uabelho wrote:
>
>> Compiling with clang 15.0.5 I get the following warning/error with this 
>> patch:
>>
>>   ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly 
>> defaulted move assignment operator is implicitly deleted 
>> [-Werror,-Wdefaulted-function-deleted]
>> AttributePool =(AttributePool &) = default;
>>^
>>   ../../clang/include/clang/Sema/ParsedAttr.h:674:21: note: move assignment 
>> operator of 'AttributePool' is implicitly deleted because field 'Factory' is 
>> of reference type 'clang::AttributeFactory &'
>> AttributeFactory 
>>   ^
>>   1 error generated.
>
> Also seen in this buildbot:
>  https://lab.llvm.org/buildbot/#/builders/214/builds/7476

Thank you @uabelho for reporting. Build error is fixed by 
https://github.com/llvm/llvm-project/commit/292a6c1c2395f990bbde8d968825243e4fe9b954


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D149718#4341052 , @uabelho wrote:

> Compiling with clang 15.0.5 I get the following warning/error with this patch:
>
>   ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly 
> defaulted move assignment operator is implicitly deleted 
> [-Werror,-Wdefaulted-function-deleted]
> AttributePool =(AttributePool &) = default;
>^
>   ../../clang/include/clang/Sema/ParsedAttr.h:674:21: note: move assignment 
> operator of 'AttributePool' is implicitly deleted because field 'Factory' is 
> of reference type 'clang::AttributeFactory &'
> AttributeFactory 
>   ^
>   1 error generated.

Also seen in this buildbot:
 https://lab.llvm.org/buildbot/#/builders/214/builds/7476


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Compiling with clang 15.0.5 I get the following warning/error with this patch:

  ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly 
defaulted move assignment operator is implicitly deleted 
[-Werror,-Wdefaulted-function-deleted]
AttributePool =(AttributePool &) = default;
   ^
  ../../clang/include/clang/Sema/ParsedAttr.h:674:21: note: move assignment 
operator of 'AttributePool' is implicitly deleted because field 'Factory' is of 
reference type 'clang::AttributeFactory &'
AttributeFactory 
  ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ebff1ac1b98: [NFC][Clang] Fix Coverity issues of copy 
without assign (authored by Manna).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,11 +696,13 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
+  AttributePool =(AttributePool &) = default;
 
   AttributeFactory () const { return Factory; }
 
@@ -912,6 +914,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,11 +696,13 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
+  AttributePool =(AttributePool &) = default;
 
   AttributeFactory () const { return Factory; }
 
@@ -912,6 +914,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = 

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

In D149718#4333563 , @tahonermann 
wrote:

> Looks good. Thanks @Manna!

Thank you @tahonermann for reviews!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.

Looks good. Thanks @Manna!




Comment at: clang/include/clang/Sema/Sema.h:1786
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;

Manna wrote:
> tahonermann wrote:
> > Manna wrote:
> > > tahonermann wrote:
> > > > Since this class declares a move constructor, the declaration of the 
> > > > implicit move assignment operator is inhibited and the implicitly 
> > > > declared assignment operator is defined as deleted. This change 
> > > > declares a move assignment operator but doesn't define it (perhaps `= 
> > > > delete` was intended?). If support for assignment is not desired, then 
> > > > I think a declaration of a deleted copy assignment operator is all that 
> > > > is needed (matching the change made for `Strategy` below). Otherwise, I 
> > > > think a defaulted copy assignment operator should be declared and a 
> > > > move assignment operator should be defined that implements the same 
> > > > behavior as the move constructor.
> > > Thanks @tahonermann for the comments. 
> > > 
> > > >> think a defaulted copy assignment operator should be declared and a 
> > > >> move assignment operator should be defined that implements the same 
> > > >> behavior as the move constructor.
> > > 
> > > I have updated patch based on further analysis and my understanding. This 
> > > seems reasonable to me.
> > This change still declares a move assignment operator, but doesn't provide 
> > a definition. The move constructor is implemented in 
> > `clang/lib/Sema/Sema.cpp`, so I would expect to see the move assignment 
> > operator definition provided there as well.
> Thanks @tahonermann for the comments. I have removed `Sema.h`  change from 
> the current patch. I will address it in a separate review pull request. Need 
> to look into this more.  
Ok, sounds good.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:1786
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;

tahonermann wrote:
> Manna wrote:
> > tahonermann wrote:
> > > Since this class declares a move constructor, the declaration of the 
> > > implicit move assignment operator is inhibited and the implicitly 
> > > declared assignment operator is defined as deleted. This change declares 
> > > a move assignment operator but doesn't define it (perhaps `= delete` was 
> > > intended?). If support for assignment is not desired, then I think a 
> > > declaration of a deleted copy assignment operator is all that is needed 
> > > (matching the change made for `Strategy` below). Otherwise, I think a 
> > > defaulted copy assignment operator should be declared and a move 
> > > assignment operator should be defined that implements the same behavior 
> > > as the move constructor.
> > Thanks @tahonermann for the comments. 
> > 
> > >> think a defaulted copy assignment operator should be declared and a move 
> > >> assignment operator should be defined that implements the same behavior 
> > >> as the move constructor.
> > 
> > I have updated patch based on further analysis and my understanding. This 
> > seems reasonable to me.
> This change still declares a move assignment operator, but doesn't provide a 
> definition. The move constructor is implemented in `clang/lib/Sema/Sema.cpp`, 
> so I would expect to see the move assignment operator definition provided 
> there as well.
Thanks @tahonermann for the comments. I have removed `Sema.h`  change from the 
current patch. I will address it in a separate review pull request. Need to 
look into this more.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 521073.
Manna marked 2 inline comments as done.
Manna added a comment.

I have updated patch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,11 +696,13 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
+  AttributePool =(AttributePool &) = default;
 
   AttributeFactory () const { return Factory; }
 
@@ -912,6 +914,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,11 +696,13 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
+  AttributePool =(AttributePool &) = default;
 
   AttributeFactory () const { return Factory; }
 
@@ -912,6 +914,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Tom Honermann via Phabricator via cfe-commits]

tahonermann requested changes to this revision.
tahonermann added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Sema/Sema.h:1786
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;

Manna wrote:
> tahonermann wrote:
> > Since this class declares a move constructor, the declaration of the 
> > implicit move assignment operator is inhibited and the implicitly declared 
> > assignment operator is defined as deleted. This change declares a move 
> > assignment operator but doesn't define it (perhaps `= delete` was 
> > intended?). If support for assignment is not desired, then I think a 
> > declaration of a deleted copy assignment operator is all that is needed 
> > (matching the change made for `Strategy` below). Otherwise, I think a 
> > defaulted copy assignment operator should be declared and a move assignment 
> > operator should be defined that implements the same behavior as the move 
> > constructor.
> Thanks @tahonermann for the comments. 
> 
> >> think a defaulted copy assignment operator should be declared and a move 
> >> assignment operator should be defined that implements the same behavior as 
> >> the move constructor.
> 
> I have updated patch based on further analysis and my understanding. This 
> seems reasonable to me.
This change still declares a move assignment operator, but doesn't provide a 
definition. The move constructor is implemented in `clang/lib/Sema/Sema.cpp`, 
so I would expect to see the move assignment operator definition provided there 
as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna marked 6 inline comments as done.
Manna added inline comments.



Comment at: clang/include/clang/Sema/ParsedAttr.h:704
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
 

tahonermann wrote:
> This class has a move constructor, but the implicit declaration of a move 
> assignment operator will be inhibited due to the presence of this and other 
> special member functions. That means that an attempted move assignment will 
> find the deleted copy assignment. That is ok; if support for move assignment 
> is desired some day, it can be added then.
I have added defaulted move assignment operator which seems needed based on 
analysis with other bugs.  



Comment at: clang/include/clang/Sema/Sema.h:1786
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;

tahonermann wrote:
> Since this class declares a move constructor, the declaration of the implicit 
> move assignment operator is inhibited and the implicitly declared assignment 
> operator is defined as deleted. This change declares a move assignment 
> operator but doesn't define it (perhaps `= delete` was intended?). If support 
> for assignment is not desired, then I think a declaration of a deleted copy 
> assignment operator is all that is needed (matching the change made for 
> `Strategy` below). Otherwise, I think a defaulted copy assignment operator 
> should be declared and a move assignment operator should be defined that 
> implements the same behavior as the move constructor.
Thanks @tahonermann for the comments. 

>> think a defaulted copy assignment operator should be declared and a move 
>> assignment operator should be defined that implements the same behavior as 
>> the move constructor.

I have updated patch based on further analysis and my understanding. This seems 
reasonable to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 520384.
Manna added a comment.

Thanks @tahonermann for reviews. I have addressed review comments and updated 
patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,7 +1783,9 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
+SemaDiagnosticBuilder =(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
 bool isImmediate() const { return ImmediateDiag.has_value(); }
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,11 +696,13 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
+  AttributePool =(AttributePool &) = default;
 
   AttributeFactory () const { return Factory; }
 
@@ -912,6 +914,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,7 +762,9 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
+  Strategy =(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,7 +1783,9 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
+SemaDiagnosticBuilder =(const 

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Tom Honermann via Phabricator via cfe-commits]

tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.

This looks good with one exception; I think the changes for class 
`SemaDiagnosticBuilder` are not what was intended.




Comment at: clang/include/clang/Analysis/BodyFarm.h:41-44
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;

This looks fine; move constructor and assignment declarations are inhibited.



Comment at: clang/include/clang/Sema/ParsedAttr.h:704
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &) = default;
 

This class has a move constructor, but the implicit declaration of a move 
assignment operator will be inhibited due to the presence of this and other 
special member functions. That means that an attempted move assignment will 
find the deleted copy assignment. That is ok; if support for move assignment is 
desired some day, it can be added then.



Comment at: clang/include/clang/Sema/ParsedAttr.h:915-916
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 

This looks fine; move constructor and assignment declarations are inhibited.



Comment at: clang/include/clang/Sema/Sema.h:1786
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;

Since this class declares a move constructor, the declaration of the implicit 
move assignment operator is inhibited and the implicitly declared assignment 
operator is defined as deleted. This change declares a move assignment operator 
but doesn't define it (perhaps `= delete` was intended?). If support for 
assignment is not desired, then I think a declaration of a deleted copy 
assignment operator is all that is needed (matching the change made for 
`Strategy` below). Otherwise, I think a defaulted copy assignment operator 
should be declared and a move assignment operator should be defined that 
implements the same behavior as the move constructor.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:764-766
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;

This is ok. The implicit move assignment declaration is inhibited and since 
copy assignment is deleted, no assignment is supported. If move assignment is 
desired someday, it can be added.



Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:44-45
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 

This looks fine; implicit move construction and assignment declarations are 
inhibited.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

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

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 519331.
Manna added a comment.

Fix clang-format errors and typo in comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149718/new/

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
@@ -912,6 +913,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
@@ -912,6 +913,7 @@
 public:
   

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Manna added a reviewer: tahonermann.
Herald added a reviewer: NoQ.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

This patch adds missing assignment operator to the class which has user-defined 
copy/move constructor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter&) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
@@ -912,6 +913,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Copy assignment opearator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter&) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) =