[PATCH] D155846: [NFC][clang] Fix static analyzer concerns

2023-07-24 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon abandoned this revision.
Fznamznon added a comment.

Oh shoot, I trusted the tool too much to not double check that the copy ctor is 
already deleted. Thank you for the catch and for the bug report.
I'll abandon this change then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155846

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


[PATCH] D155846: [NFC][clang] Fix static analyzer concerns

2023-07-21 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

> I agree. I'll try to isolate a reproducible test case and report it to the 
> vendor.

Done.

I'm ambivalent with regard to keeping or discarding the proposed change, but 
the static analysis issue should be triaged as a false positive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155846

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


[PATCH] D155846: [NFC][clang] Fix static analyzer concerns

2023-07-20 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

> there's no bug being fixed which makes me think this should go back to the 
> static analysis vendor to report this as a false positive.

I agree. I'll try to isolate a reproducible test case and report it to the 
vendor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155846

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


[PATCH] D155846: [NFC][clang] Fix static analyzer concerns

2023-07-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'm not opposed, but the copy ctor and copy assignment operators are already 
deleted by default in this case (e.g., the class has data members with deleted 
copy constructors). So I agree this is an NFC change, but there's no bug being 
fixed which makes me think this should go back to the static analysis vendor to 
report this as a false positive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155846

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


[PATCH] D155846: [NFC][clang] Fix static analyzer concerns

2023-07-20 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon created this revision.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Preprocessor class can own and free resources in its destructor, but
doesn't have user-written copy constructor or assignment operator, hence
any copy attempt will use compiler-generated copy constructor and may
cause use-after-free problems.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155846

Files:
  clang/include/clang/Lex/Preprocessor.h


Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -1166,6 +1166,9 @@
 
   ~Preprocessor();
 
+  Preprocessor(const Preprocessor &) = delete;
+  Preprocessor =(const Preprocessor &) = delete;
+
   /// Initialize the preprocessor using information about the target.
   ///
   /// \param Target is owned by the caller and must remain valid for the


Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -1166,6 +1166,9 @@
 
   ~Preprocessor();
 
+  Preprocessor(const Preprocessor &) = delete;
+  Preprocessor =(const Preprocessor &) = delete;
+
   /// Initialize the preprocessor using information about the target.
   ///
   /// \param Target is owned by the caller and must remain valid for the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits