[PATCH 1/2] Add gcc/make-unique.h

[PATCH 1/2] Add gcc/make-unique.h

[David Malcolm]

On Fri, 2022-07-08 at 22:16 +0100, Jonathan Wakely wrote:
> On Fri, 8 Jul 2022 at 21:47, David Malcolm via Gcc <gcc@gcc.gnu.org>
> wrote:
> > 
> > std::unique_ptr is C++11, and I'd like to use it in the
> > gcc/analyzer
> > subdirectory, at least.  The following patch eliminates a bunch of
> > "takes ownership" comments and manual "delete" invocations in favor
> > of simply using std::unique_ptr.
> > 
> > The problem is that the patch makes use of std::make_unique, but
> > that
> > was added in C++14.
> > 
> > I've heard that it's reasonably easy to reimplement
> > std::make_unique,
> > but I'm not sure that my C++11 skills are up to it.
> 
> You know we have an implementation of std::make_unique in GCC, with a
> GCC-compatible licence that you can look at, right? :-)
> 
> But it's not really necessary. There are only two reasons to prefer
> make_unique over just allocating an object with new and constructing
> a
> unique_ptr from it:
> 
> 1) avoid a "naked" new in your code (some coding styles like this,
> but
> it's not really important as long as the 'delete' is managed
> automatically by unique_ptr).
> 
> 2) exception-safety when allocating multiple objects as args to a
> function, see https://herbsutter.com/gotw/_102/ for details.
> Irrelevant for GCC, because we build without exceptions.

[moving from gcc to gcc-patches mailing list]

Also, I *think* it's a lot less typing, since I can write just:

  std::make_unique<name_of_type_which_could_be_long> (args)

rather than

  std::unique_ptr<name_of_type_which_could_be_long> (new name_of_type_which_could_be_long (args));

> 
> 
> 
> > Is there:
> > (a) an easy way to implement a std::make_unique replacement
> >     (e.g. in system.h? what to call it?), or
> 
> If you don't care about using it to create unique_ptr<T[]> arrays,
> it's trivial:
> 
>   template<typename T, typename... Args>
>     inline typename std::enable_if<!std::is_array<T>::value,
> std::unique_ptr<T>>::type
>     make_unique(Args&&... args)
>     { return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
> }
> 
> To add the overload that works for arrays is a little trickier.

Thanks!

I tried adding it to gcc/system.h, but anything that uses it needs to
have std::unique_ptr declared, which meant forcibly including <memory>
from gcc/system.h

So instead, here's a patch that adds a new gcc/make-unique.h header,
containing just the template decl above (in the root namespace, rather
than std::, which saves a bit more typing).

I've successfully bootstrapped&regression-tested a version of my earlier
analyzer patch that uses this patch (see patch 2 of the kit, which has
lots of usage examples).

OK for trunk?

Dave



This patch adds gcc/make-unique.h, containing a minimal C++11
implementation of make_unique (std::make_unique is C++14).

gcc/ChangeLog:
	* make-unique.h: New file.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/make-unique.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 gcc/make-unique.h

diff --git a/gcc/make-unique.h b/gcc/make-unique.h
new file mode 100644
index 00000000000..c99c5328545
--- /dev/null
+++ b/gcc/make-unique.h
@@ -0,0 +1,41 @@
+/* Minimal implementation of make_unique for C++11 compatibility.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_MAKE_UNIQUE
+#define GCC_MAKE_UNIQUE
+
+/* This header uses std::unique_ptr, but <memory> can't be directly
+   included due to issues with macros.  Hence it must be included from
+   system.h by defining INCLUDE_MEMORY in any source file using it.  */
+
+#ifndef INCLUDE_MEMORY
+# error "You must define INCLUDE_MEMORY before including system.h to use make-unique.h"
+#endif
+
+/* Minimal implementation of make_unique for C++11 compatibility
+   (std::make_unique is C++14).  */
+
+template<typename T, typename... Args>
+inline typename std::enable_if<!std::is_array<T>::value, std::unique_ptr<T>>::type
+make_unique(Args&&... args)
+{
+  return std::unique_ptr<T> (new T (std::forward<Args> (args)...));
+}
+
+#endif /* ! GCC_MAKE_UNIQUE */
-- 
2.26.3

[PATCH 2/2] analyzer: use std::unique_ptr for pending_diagnostic/note

[David Malcolm]

This patch uses the new gcc/make-unique.h header for C++11
compatibility.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

I can self-approve this, but it's dependent on the patch to add
gcc/make-unique.h.

gcc/analyzer/ChangeLog:
	* call-info.cc: Add define of INCLUDE_MEMORY.
	* checker-path.cc: Likewise.
	* constraint-manager.cc: Likewise.
	* diagnostic-manager.cc: Likewise.
	(saved_diagnostic::saved_diagnostic): Use std::unique_ptr for
	param d and field m_d.
	(saved_diagnostic::~saved_diagnostic): Remove explicit delete of m_d.
	(saved_diagnostic::add_note): Use std::unique_ptr for
	param pn.
	(saved_diagnostic::get_pending_diagnostic): Update for conversion
	of m_sd.m_d to unique_ptr.
	(diagnostic_manager::add_diagnostic): Use std::unique_ptr for
	param d.  Remove explicit deletion.
	(diagnostic_manager::add_note): Use std::unique_ptr for param pn.
	(diagnostic_manager::emit_saved_diagnostic): Update for conversion
	of m_sd.m_d to unique_ptr.
	(null_assignment_sm_context::warn): Use std::unique_ptr for
	param d.  Remove explicit deletion.
	* diagnostic-manager.h (saved_diagnostic::saved_diagnostic): Use
	std::unique_ptr for param d.
	(saved_diagnostic::add_note): Likewise for param pn.
	(saved_diagnostic::m_d): Likewise.
	(diagnostic_manager::add_diagnostic): Use std::unique_ptr for
	param d.
	(diagnostic_manager::add_note): Use std::unique_ptr for param pn.
	* engine.cc: Include "make-unique.h".
	(impl_region_model_context::warn): Update to use std::unique_ptr
	for param, removing explicit deletion.
	(impl_region_model_context::add_note): Likewise.
	(impl_sm_context::warn): Update to use std::unique_ptr
	for param.
	(impl_region_model_context::on_state_leak): Likewise for result of
	on_leak.
	(exploded_node::on_longjmp): Use make_unique when creating
	pending_diagnostic.
	* exploded-graph.h (impl_region_model_context::warn): Update to
	use std::unique_ptr for param.
	(impl_region_model_context::add_note): Likewise.
	* feasible-graph.cc: Add define of INCLUDE_MEMORY.
	* pending-diagnostic.cc: Likewise.
	* pending-diagnostic.h: Include analyzer.sm.h"
	* program-point.cc: Add define of INCLUDE_MEMORY.
	* program-state.cc: Likewise.
	* region-model-asm.cc: Likewise.
	* region-model-impl-calls.cc: Likewise.
	* region-model-manager.cc: Likewise.
	* region-model-reachability.cc: Likewise.
	* region-model.cc: Likewise.  Include "make-unique.h".
	(region_model::get_gassign_result): Use make_unique when creating
	pending_diagnostic.
	(region_model::check_for_poison): Likewise.
	(region_model::on_stmt_pre): Likewise.
	(annotating_ctxt: make_node): Use std::unique_ptr for result.
	(region_model::deref_rvalue): Use make_unique when creating
	pending_diagnostic.
	(region_model::check_for_writable_region): Likewise.
	(region_model::check_region_size): Likewise.
	(noop_region_model_context::add_note): Use std::unique_ptr for
	param.  Remove explicit deletion.
	* region-model.h: Include "analyzer/pending-diagnostic.h".
	(region_model_context::warn): Convert param to std::unique_ptr.
	(region_model_context::add_note): Likewise.
	(noop_region_model_context::warn): Likewise.
	(noop_region_model_context::add_note): Likewise.
	(region_model_context_decorator::warn): Likewise.
	(region_model_context_decorator::add_note): Likewise.
	(note_adding_context::warn): Likewise.
	(note_adding_context::make_note): Likewise for return type.
	(test_region_model_context::warn): Convert param to
	std::unique_ptr.
	* region.cc: Add define of INCLUDE_MEMORY.
	* sm-fd.cc: Likewise.  Include "make-unique.h".
	(fd_state_machine::on_open): Use make_unique when creating
	pending_diagnostic.
	(fd_state_machine::on_close): Likewise.
	(fd_state_machine::check_for_open_fd): Likewise.
	(fd_state_machine::on_leak): Likewise, converting return type to
	std::unique_ptr.
	* sm-file.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(fileptr_state_machine::on_stmt): Use make_unique when creating
	pending_diagnostic.
	(fileptr_state_machine::on_leak): Likewise, converting return type
	to std::unique_ptr.
	* sm-malloc.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(malloc_state_machine::on_stmt): Use make_unique when creating
	pending_diagnostic.
	(malloc_state_machine::handle_free_of_non_heap): Likewise.
	(malloc_state_machine::on_deallocator_call): Likewise.
	(malloc_state_machine::on_realloc_call): Likewise.
	(malloc_state_machine::on_leak): Likewise, converting return type
	to std::unique_ptr.
	* sm-pattern-test.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(pattern_test_state_machine::on_condition): Use make_unique when
	creating pending_diagnostic.
	* sm-sensitive.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(sensitive_state_machine::warn_for_any_exposure): Use make_unique
	when creating pending_diagnostic.
	* sm-signal.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(signal_state_machine::on_stmt): Use make_unique when creating
	pending_diagnostic.
	* sm-taint.cc: Add define of INCLUDE_MEMORY.  Include
	"make-unique.h".
	(taint_state_machine::check_for_tainted_size_arg): Use make_unique
	when creating pending_diagnostic.
	(taint_state_machine::check_for_tainted_divisor): Likewise.
	(region_model::check_region_for_taint): Likewise.
	(region_model::check_dynamic_size_for_taint): Likewise.
	* sm.cc: Add define of INCLUDE_MEMORY.  Include
	"analyzer/pending-diagnostic.h".
	(state_machine::on_leak): Move here from sm.h, changing return
	type to std::unique_ptr.
	* sm.h (state_machine::on_leak): Change return type to
	std::unique_ptr.  Move defn of base impl to sm.cc
	(sm_context::warn): Convert param d to std_unique_ptr.
	* state-purge.cc: Add define of INCLUDE_MEMORY.
	* store.cc: Likewise.
	* svalue.cc: Likewise.
	* trimmed-graph.cc: Likewise.
	* varargs.cc: Likewise.  Include "make-unique.h".
	(va_list_state_machine::check_for_ended_va_list): Use make_unique
	when creating pending_diagnostic.
	(va_list_state_machine::on_leak): Likewise, converting return type
	to std::unique_ptr.
	(region_model::impl_call_va_arg): Use make_unique when creating
	pending_diagnostic.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/analyzer_gil_plugin.c: Add define of
	INCLUDE_MEMORY.  Include "make-unique.h".
	(gil_state_machine::check_for_pyobject_in_call): Use make_unique
	when creating pending_diagnostic.
	(gil_state_machine::on_stmt): Likewise.
	(gil_state_machine::check_for_pyobject_usage_without_gil): Likewise.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/call-info.cc                     |  1 +
 gcc/analyzer/checker-path.cc                  |  1 +
 gcc/analyzer/constraint-manager.cc            |  1 +
 gcc/analyzer/diagnostic-manager.cc            | 40 +++++++-------
 gcc/analyzer/diagnostic-manager.h             | 12 ++---
 gcc/analyzer/engine.cc                        | 39 +++++++-------
 gcc/analyzer/exploded-graph.h                 |  4 +-
 gcc/analyzer/feasible-graph.cc                |  1 +
 gcc/analyzer/pending-diagnostic.cc            |  1 +
 gcc/analyzer/pending-diagnostic.h             |  1 +
 gcc/analyzer/program-point.cc                 |  1 +
 gcc/analyzer/program-state.cc                 |  1 +
 gcc/analyzer/region-model-asm.cc              |  1 +
 gcc/analyzer/region-model-impl-calls.cc       |  1 +
 gcc/analyzer/region-model-manager.cc          |  1 +
 gcc/analyzer/region-model-reachability.cc     |  1 +
 gcc/analyzer/region-model.cc                  | 53 +++++++++++--------
 gcc/analyzer/region-model.h                   | 31 +++++------
 gcc/analyzer/region.cc                        |  1 +
 gcc/analyzer/sm-fd.cc                         | 30 ++++++-----
 gcc/analyzer/sm-file.cc                       | 10 ++--
 gcc/analyzer/sm-malloc.cc                     | 53 ++++++++++---------
 gcc/analyzer/sm-pattern-test.cc               |  6 ++-
 gcc/analyzer/sm-sensitive.cc                  |  5 +-
 gcc/analyzer/sm-signal.cc                     |  6 ++-
 gcc/analyzer/sm-taint.cc                      | 22 ++++----
 gcc/analyzer/sm.cc                            | 10 ++++
 gcc/analyzer/sm.h                             | 12 ++---
 gcc/analyzer/state-purge.cc                   |  1 +
 gcc/analyzer/store.cc                         |  1 +
 gcc/analyzer/svalue.cc                        |  1 +
 gcc/analyzer/trimmed-graph.cc                 |  1 +
 gcc/analyzer/varargs.cc                       | 24 +++++----
 .../gcc.dg/plugin/analyzer_gil_plugin.c       | 12 +++--
 34 files changed, 223 insertions(+), 163 deletions(-)

diff --git a/gcc/analyzer/call-info.cc b/gcc/analyzer/call-info.cc
index e1142d743a3..87ab35b9e87 100644
--- a/gcc/analyzer/call-info.cc
+++ b/gcc/analyzer/call-info.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 211cf3e0333..3e475865e04 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 4133a134778..b2344e79487 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 083f66bd739..3d6c1bf90bb 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
@@ -645,14 +646,14 @@ saved_diagnostic::saved_diagnostic (const state_machine *sm,
 				    tree var,
 				    const svalue *sval,
 				    state_machine::state_t state,
-				    pending_diagnostic *d,
+				    std::unique_ptr<pending_diagnostic> d,
 				    unsigned idx)
 : m_sm (sm), m_enode (enode), m_snode (snode), m_stmt (stmt),
  /* stmt_finder could be on-stack; we want our own copy that can
     outlive that.  */
   m_stmt_finder (stmt_finder ? stmt_finder->clone () : NULL),
   m_var (var), m_sval (sval), m_state (state),
-  m_d (d), m_trailing_eedge (NULL),
+  m_d (std::move (d)), m_trailing_eedge (NULL),
   m_idx (idx),
   m_best_epath (NULL), m_problem (NULL),
   m_notes ()
@@ -669,7 +670,6 @@ saved_diagnostic::saved_diagnostic (const state_machine *sm,
 saved_diagnostic::~saved_diagnostic ()
 {
   delete m_stmt_finder;
-  delete m_d;
   delete m_best_epath;
   delete m_problem;
 }
@@ -696,10 +696,10 @@ saved_diagnostic::operator== (const saved_diagnostic &other) const
 /* Add PN to this diagnostic, taking ownership of it.  */
 
 void
-saved_diagnostic::add_note (pending_note *pn)
+saved_diagnostic::add_note (std::unique_ptr<pending_note> pn)
 {
   gcc_assert (pn);
-  m_notes.safe_push (pn);
+  m_notes.safe_push (pn.release ());
 }
 
 /* Return a new json::object of the form
@@ -903,7 +903,7 @@ public:
 
   pending_diagnostic *get_pending_diagnostic () const
   {
-    return m_sd.m_d;
+    return m_sd.m_d.get ();
   }
 
   bool reachable_from_p (const exploded_node *src_enode) const
@@ -962,8 +962,7 @@ diagnostic_manager::diagnostic_manager (logger *logger, engine *eng,
 }
 
 /* Queue pending_diagnostic D at ENODE for later emission.
-   Return true/false signifying if the diagnostic was actually added.
-   Take ownership of D (or delete it).  */
+   Return true/false signifying if the diagnostic was actually added.  */
 
 bool
 diagnostic_manager::add_diagnostic (const state_machine *sm,
@@ -973,7 +972,7 @@ diagnostic_manager::add_diagnostic (const state_machine *sm,
 				    tree var,
 				    const svalue *sval,
 				    state_machine::state_t state,
-				    pending_diagnostic *d)
+				    std::unique_ptr<pending_diagnostic> d)
 {
   LOG_FUNC (get_logger ());
 
@@ -994,7 +993,6 @@ diagnostic_manager::add_diagnostic (const state_machine *sm,
 	  if (get_logger ())
 	    get_logger ()->log ("rejecting disabled warning %qs",
 				d->get_kind ());
-	  delete d;
 	  m_num_disabled_diagnostics++;
 	  return false;
 	}
@@ -1002,13 +1000,13 @@ diagnostic_manager::add_diagnostic (const state_machine *sm,
 
   saved_diagnostic *sd
     = new saved_diagnostic (sm, enode, snode, stmt, finder, var, sval,
-			    state, d, m_saved_diagnostics.length ());
+			    state, std::move (d), m_saved_diagnostics.length ());
   m_saved_diagnostics.safe_push (sd);
   enode->add_diagnostic (sd);
   if (get_logger ())
     log ("adding saved diagnostic %i at SN %i to EN %i: %qs",
 	 sd->get_index (),
-	 snode->m_index, enode->m_index, d->get_kind ());
+	 snode->m_index, enode->m_index, sd->m_d->get_kind ());
   return true;
 }
 
@@ -1020,17 +1018,17 @@ bool
 diagnostic_manager::add_diagnostic (exploded_node *enode,
 				    const supernode *snode, const gimple *stmt,
 				    stmt_finder *finder,
-				    pending_diagnostic *d)
+				    std::unique_ptr<pending_diagnostic> d)
 {
   gcc_assert (enode);
   return add_diagnostic (NULL, enode, snode, stmt, finder, NULL_TREE,
-			 NULL, 0, d);
+			 NULL, 0, std::move (d));
 }
 
 /* Add PN to the most recent saved_diagnostic.  */
 
 void
-diagnostic_manager::add_note (pending_note *pn)
+diagnostic_manager::add_note (std::unique_ptr<pending_note> pn)
 {
   LOG_FUNC (get_logger ());
   gcc_assert (pn);
@@ -1038,7 +1036,7 @@ diagnostic_manager::add_note (pending_note *pn)
   /* Get most recent saved_diagnostic.  */
   gcc_assert (m_saved_diagnostics.length () > 0);
   saved_diagnostic *sd = m_saved_diagnostics[m_saved_diagnostics.length () - 1];
-  sd->add_note (pn);
+  sd->add_note (std::move (pn));
 }
 
 /* Return a new json::object of the form
@@ -1393,13 +1391,13 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
 
   emission_path.inject_any_inlined_call_events (get_logger ());
 
-  emission_path.prepare_for_emission (sd.m_d);
+  emission_path.prepare_for_emission (sd.m_d.get ());
 
   location_t loc
     = get_emission_location (sd.m_stmt, sd.m_snode->m_fun, *sd.m_d);
 
   /* Allow the pending_diagnostic to fix up the locations of events.  */
-  emission_path.fixup_locations (sd.m_d);
+  emission_path.fixup_locations (sd.m_d.get ());
 
   gcc_rich_location rich_loc (loc);
   rich_loc.set_path (&emission_path);
@@ -1787,14 +1785,12 @@ struct null_assignment_sm_context : public sm_context
   }
 
   void warn (const supernode *, const gimple *,
-	     tree, pending_diagnostic *d) final override
+	     tree, std::unique_ptr<pending_diagnostic>) final override
   {
-    delete d;
   }
   void warn (const supernode *, const gimple *,
-	     const svalue *, pending_diagnostic *d) final override
+	     const svalue *, std::unique_ptr<pending_diagnostic>) final override
   {
-    delete d;
   }
 
   tree get_diagnostic_tree (tree expr) final override
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index 266eed8f9cb..fdab038d7a1 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -36,13 +36,13 @@ public:
 		    stmt_finder *stmt_finder,
 		    tree var, const svalue *sval,
 		    state_machine::state_t state,
-		    pending_diagnostic *d,
+		    std::unique_ptr<pending_diagnostic> d,
 		    unsigned idx);
   ~saved_diagnostic ();
 
   bool operator== (const saved_diagnostic &other) const;
 
-  void add_note (pending_note *pn);
+  void add_note (std::unique_ptr<pending_note> pn);
 
   json::object *to_json () const;
 
@@ -76,7 +76,7 @@ public:
   tree m_var;
   const svalue *m_sval;
   state_machine::state_t m_state;
-  pending_diagnostic *m_d; // owned
+  std::unique_ptr<pending_diagnostic> m_d;
   const exploded_edge *m_trailing_eedge;
 
 private:
@@ -117,14 +117,14 @@ public:
 		       tree var,
 		       const svalue *sval,
 		       state_machine::state_t state,
-		       pending_diagnostic *d);
+		       std::unique_ptr<pending_diagnostic> d);
 
   bool add_diagnostic (exploded_node *enode,
 		       const supernode *snode, const gimple *stmt,
 		       stmt_finder *finder,
-		       pending_diagnostic *d);
+		       std::unique_ptr<pending_diagnostic> d);
 
-  void add_note (pending_note *pn);
+  void add_note (std::unique_ptr<pending_note> pn);
 
   void emit_saved_diagnostics (const exploded_graph &eg);
 
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 888123f2b95..17ccc233d21 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "fold-const.h"
 #include "gcc-rich-location.h"
@@ -118,35 +119,29 @@ impl_region_model_context (program_state *state,
 }
 
 bool
-impl_region_model_context::warn (pending_diagnostic *d)
+impl_region_model_context::warn (std::unique_ptr<pending_diagnostic> d)
 {
   LOG_FUNC (get_logger ());
   if (m_stmt == NULL && m_stmt_finder == NULL)
     {
       if (get_logger ())
 	get_logger ()->log ("rejecting diagnostic: no stmt");
-      delete d;
       return false;
     }
   if (m_eg)
     return m_eg->get_diagnostic_manager ().add_diagnostic
       (m_enode_for_diag, m_enode_for_diag->get_supernode (),
-       m_stmt, m_stmt_finder, d);
+       m_stmt, m_stmt_finder, std::move (d));
   else
-    {
-      delete d;
-      return false;
-    }
+    return false;
 }
 
 void
-impl_region_model_context::add_note (pending_note *pn)
+impl_region_model_context::add_note (std::unique_ptr<pending_note> pn)
 {
   LOG_FUNC (get_logger ());
   if (m_eg)
-    m_eg->get_diagnostic_manager ().add_note (pn);
-  else
-    delete pn;
+    m_eg->get_diagnostic_manager ().add_note (std::move (pn));
 }
 
 void
@@ -417,10 +412,11 @@ public:
   }
 
   void warn (const supernode *snode, const gimple *stmt,
-	     tree var, pending_diagnostic *d) final override
+	     tree var,
+	     std::unique_ptr<pending_diagnostic> d) final override
   {
     LOG_FUNC (get_logger ());
-    gcc_assert (d); // take ownership
+    gcc_assert (d);
     impl_region_model_context old_ctxt
       (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
 
@@ -432,14 +428,15 @@ public:
 	 : m_old_smap->get_global_state ());
     m_eg.get_diagnostic_manager ().add_diagnostic
       (&m_sm, m_enode_for_diag, snode, stmt, m_stmt_finder,
-       var, var_old_sval, current, d);
+       var, var_old_sval, current, std::move (d));
   }
 
   void warn (const supernode *snode, const gimple *stmt,
-	     const svalue *sval, pending_diagnostic *d) final override
+	     const svalue *sval,
+	     std::unique_ptr<pending_diagnostic> d) final override
   {
     LOG_FUNC (get_logger ());
-    gcc_assert (d); // take ownership
+    gcc_assert (d);
     impl_region_model_context old_ctxt
       (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
 
@@ -449,7 +446,7 @@ public:
 	 : m_old_smap->get_global_state ());
     m_eg.get_diagnostic_manager ().add_diagnostic
       (&m_sm, m_enode_for_diag, snode, stmt, m_stmt_finder,
-       NULL_TREE, sval, current, d);
+       NULL_TREE, sval, current, std::move (d));
   }
 
   /* Hook for picking more readable trees for SSA names of temporaries,
@@ -880,12 +877,12 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
     }
 
   tree leaked_tree_for_diag = fixup_tree_for_diagnostic (leaked_tree);
-  pending_diagnostic *pd = sm.on_leak (leaked_tree_for_diag);
+  std::unique_ptr<pending_diagnostic> pd = sm.on_leak (leaked_tree_for_diag);
   if (pd)
     m_eg->get_diagnostic_manager ().add_diagnostic
       (&sm, m_enode_for_diag, m_enode_for_diag->get_supernode (),
        m_stmt, &stmt_finder,
-       leaked_tree_for_diag, sval, state, pd);
+       leaked_tree_for_diag, sval, state, std::move (pd));
 }
 
 /* Implementation of region_model_context::on_condition vfunc.
@@ -1682,7 +1679,9 @@ exploded_node::on_longjmp (exploded_graph &eg,
   /* Verify that the setjmp's call_stack hasn't been popped.  */
   if (!valid_longjmp_stack_p (longjmp_point, setjmp_point))
     {
-      ctxt->warn (new stale_jmp_buf (setjmp_call, longjmp_call, setjmp_point));
+      ctxt->warn (make_unique<stale_jmp_buf> (setjmp_call,
+					      longjmp_call,
+					      setjmp_point));
       return;
     }
 
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index 0613f558b8b..602f6a117a8 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -47,8 +47,8 @@ class impl_region_model_context : public region_model_context
 			     uncertainty_t *uncertainty,
 			     logger *logger = NULL);
 
-  bool warn (pending_diagnostic *d) final override;
-  void add_note (pending_note *pn) final override;
+  bool warn (std::unique_ptr<pending_diagnostic> d) final override;
+  void add_note (std::unique_ptr<pending_note> pn) final override;
   void on_svalue_leak (const svalue *) override;
   void on_liveness_change (const svalue_set &live_svalues,
 			   const region_model *model) final override;
diff --git a/gcc/analyzer/feasible-graph.cc b/gcc/analyzer/feasible-graph.cc
index fe7e79fe902..01806ad9fab 100644
--- a/gcc/analyzer/feasible-graph.cc
+++ b/gcc/analyzer/feasible-graph.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc
index eff050f6757..bfe2630e19a 100644
--- a/gcc/analyzer/pending-diagnostic.cc
+++ b/gcc/analyzer/pending-diagnostic.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 4ea469e1879..6ca8ab9f4aa 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #define GCC_ANALYZER_PENDING_DIAGNOSTIC_H
 
 #include "diagnostic-path.h"
+#include "analyzer/sm.h"
 
 namespace ana {
 
diff --git a/gcc/analyzer/program-point.cc b/gcc/analyzer/program-point.cc
index 6c296d5ddc8..bf09cf233bf 100644
--- a/gcc/analyzer/program-point.cc
+++ b/gcc/analyzer/program-point.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 90a56e3fba4..581559c2c0f 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/region-model-asm.cc b/gcc/analyzer/region-model-asm.cc
index bb73e6eed60..d5a6cd43206 100644
--- a/gcc/analyzer/region-model-asm.cc
+++ b/gcc/analyzer/region-model-asm.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 55d6fa7f76b..801709a5b05 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 17713b07c30..bda74b3dd35 100644
--- a/gcc/analyzer/region-model-manager.cc
+++ b/gcc/analyzer/region-model-manager.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/region-model-reachability.cc b/gcc/analyzer/region-model-reachability.cc
index 12d09c3e500..86f98315ed6 100644
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 8b7b4e1f697..6d54b450820 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -19,8 +19,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -814,14 +816,17 @@ region_model::get_gassign_result (const gassign *assign,
 	      if (TREE_CODE (rhs2_cst) == INTEGER_CST)
 		{
 		  if (tree_int_cst_sgn (rhs2_cst) < 0)
-		    ctxt->warn (new shift_count_negative_diagnostic
-				  (assign, rhs2_cst));
+		    ctxt->warn
+		      (make_unique<shift_count_negative_diagnostic>
+			 (assign, rhs2_cst));
 		  else if (compare_tree_int (rhs2_cst,
 					     TYPE_PRECISION (TREE_TYPE (rhs1)))
 			   >= 0)
-		    ctxt->warn (new shift_count_overflow_diagnostic
-				  (assign, TYPE_PRECISION (TREE_TYPE (rhs1)),
-				   rhs2_cst));
+		    ctxt->warn
+		      (make_unique<shift_count_overflow_diagnostic>
+			 (assign,
+			  int (TYPE_PRECISION (TREE_TYPE (rhs1))),
+			  rhs2_cst));
 		}
 	  }
 
@@ -1039,8 +1044,9 @@ region_model::check_for_poison (const svalue *sval,
       const region *src_region = NULL;
       if (pkind == POISON_KIND_UNINIT)
 	src_region = get_region_for_poisoned_expr (expr);
-      if (ctxt->warn (new poisoned_value_diagnostic (diag_arg, pkind,
-						     src_region)))
+      if (ctxt->warn (make_unique<poisoned_value_diagnostic> (diag_arg,
+							      pkind,
+							      src_region)))
 	{
 	  /* We only want to report use of a poisoned value at the first
 	     place it gets used; return an unknown value to avoid generating
@@ -1229,7 +1235,7 @@ region_model::on_stmt_pre (const gimple *stmt,
 	  {
 	    /* Handle the builtin "__analyzer_dump_path" by queuing a
 	       diagnostic at this exploded_node.  */
-	    ctxt->warn (new dump_path_diagnostic ());
+	    ctxt->warn (make_unique<dump_path_diagnostic> ());
 	  }
 	else if (is_special_named_call_p (call, "__analyzer_dump_region_model",
 					  0))
@@ -1763,9 +1769,10 @@ check_external_function_for_access_attr (const gcall *call,
 	      m_access (access)
 	    {
 	    }
-	    pending_note *make_note () final override
+	    std::unique_ptr<pending_note> make_note () final override
 	    {
-	      return new reason_attr_access (m_callee_fndecl, m_access);
+	      return make_unique<reason_attr_access>
+		(m_callee_fndecl, m_access);
 	    }
 	  private:
 	    tree m_callee_fndecl;
@@ -2573,7 +2580,8 @@ region_model::deref_rvalue (const svalue *ptr_sval, tree ptr_tree,
 		const poisoned_svalue *poisoned_sval
 		  = as_a <const poisoned_svalue *> (ptr_sval);
 		enum poison_kind pkind = poisoned_sval->get_poison_kind ();
-		ctxt->warn (new poisoned_value_diagnostic (ptr, pkind, NULL));
+		ctxt->warn (make_unique<poisoned_value_diagnostic>
+			      (ptr, pkind, NULL));
 	      }
 	  }
       }
@@ -2730,14 +2738,16 @@ region_model::check_for_writable_region (const region* dest_reg,
       {
 	const function_region *func_reg = as_a <const function_region *> (base_reg);
 	tree fndecl = func_reg->get_fndecl ();
-	ctxt->warn (new write_to_const_diagnostic (func_reg, fndecl));
+	ctxt->warn (make_unique<write_to_const_diagnostic>
+		      (func_reg, fndecl));
       }
       break;
     case RK_LABEL:
       {
 	const label_region *label_reg = as_a <const label_region *> (base_reg);
 	tree label = label_reg->get_label ();
-	ctxt->warn (new write_to_const_diagnostic (label_reg, label));
+	ctxt->warn (make_unique<write_to_const_diagnostic>
+		      (label_reg, label));
       }
       break;
     case RK_DECL:
@@ -2750,11 +2760,11 @@ region_model::check_for_writable_region (const region* dest_reg,
 	   "this" param is "T* const").  */
 	if (TREE_READONLY (decl)
 	    && is_global_var (decl))
-	  ctxt->warn (new write_to_const_diagnostic (dest_reg, decl));
+	  ctxt->warn (make_unique<write_to_const_diagnostic> (dest_reg, decl));
       }
       break;
     case RK_STRING:
-      ctxt->warn (new write_to_string_literal_diagnostic (dest_reg));
+      ctxt->warn (make_unique<write_to_string_literal_diagnostic> (dest_reg));
       break;
     }
 }
@@ -3189,8 +3199,9 @@ region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
 	tree cst_cap = cst_cap_sval->get_constant ();
 	if (!capacity_compatible_with_type (cst_cap, pointee_size_tree,
 					    is_struct))
-	  ctxt->warn (new dubious_allocation_size (lhs_reg, rhs_reg,
-						   cst_cap));
+	  ctxt->warn (make_unique <dubious_allocation_size> (lhs_reg,
+							     rhs_reg,
+							     cst_cap));
       }
       break;
     default:
@@ -3201,8 +3212,9 @@ region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
 	    if (!v.get_result ())
 	      {
 		tree expr = get_representative_tree (capacity);
-		ctxt->warn (new dubious_allocation_size (lhs_reg, rhs_reg,
-			    expr));
+		ctxt->warn (make_unique <dubious_allocation_size> (lhs_reg,
+								   rhs_reg,
+								   expr));
 	      }
 	  }
       break;
@@ -4683,9 +4695,8 @@ region_model::unset_dynamic_extents (const region *reg)
 /* class noop_region_model_context : public region_model_context.  */
 
 void
-noop_region_model_context::add_note (pending_note *pn)
+noop_region_model_context::add_note (std::unique_ptr<pending_note>)
 {
-  delete pn;
 }
 
 void
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 6dda43f5658..59bf1277f60 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "analyzer/svalue.h"
 #include "analyzer/region.h"
+#include "analyzer/pending-diagnostic.h"
 
 using namespace ana;
 
@@ -902,11 +903,11 @@ class region_model_context
  public:
   /* Hook for clients to store pending diagnostics.
      Return true if the diagnostic was stored, or false if it was deleted.  */
-  virtual bool warn (pending_diagnostic *d) = 0;
+  virtual bool warn (std::unique_ptr<pending_diagnostic> d) = 0;
 
-  /* Hook for clients to add a note to the last previously stored pending diagnostic.
-     Takes ownership of the pending_node (or deletes it).  */
-  virtual void add_note (pending_note *pn) = 0;
+  /* Hook for clients to add a note to the last previously stored
+     pending diagnostic.  */
+  virtual void add_note (std::unique_ptr<pending_note> pn) = 0;
 
   /* Hook for clients to be notified when an SVAL that was reachable
      in a previous state is no longer live, so that clients can emit warnings
@@ -980,8 +981,8 @@ class region_model_context
 class noop_region_model_context : public region_model_context
 {
 public:
-  bool warn (pending_diagnostic *) override { return false; }
-  void add_note (pending_note *pn) override;
+  bool warn (std::unique_ptr<pending_diagnostic>) override { return false; }
+  void add_note (std::unique_ptr<pending_note>) override;
   void on_svalue_leak (const svalue *) override {}
   void on_liveness_change (const svalue_set &,
 			   const region_model *) override {}
@@ -1054,14 +1055,14 @@ private:
 class region_model_context_decorator : public region_model_context
 {
  public:
-  bool warn (pending_diagnostic *d) override
+  bool warn (std::unique_ptr<pending_diagnostic> d) override
   {
-    return m_inner->warn (d);
+    return m_inner->warn (std::move (d));
   }
 
-  void add_note (pending_note *pn) override
+  void add_note (std::unique_ptr<pending_note> pn) override
   {
-    m_inner->add_note (pn);
+    m_inner->add_note (std::move (pn));
   }
 
   void on_svalue_leak (const svalue *sval) override
@@ -1168,9 +1169,9 @@ protected:
 class note_adding_context : public region_model_context_decorator
 {
 public:
-  bool warn (pending_diagnostic *d) override
+  bool warn (std::unique_ptr<pending_diagnostic> d) override
   {
-    if (m_inner->warn (d))
+    if (m_inner->warn (std::move (d)))
       {
 	add_note (make_note ());
 	return true;
@@ -1180,7 +1181,7 @@ public:
   }
 
   /* Hook to make the new note.  */
-  virtual pending_note *make_note () = 0;
+  virtual std::unique_ptr<pending_note> make_note () = 0;
 
 protected:
   note_adding_context (region_model_context *inner)
@@ -1315,9 +1316,9 @@ using namespace ::selftest;
 class test_region_model_context : public noop_region_model_context
 {
 public:
-  bool warn (pending_diagnostic *d) final override
+  bool warn (std::unique_ptr<pending_diagnostic> d) final override
   {
-    m_diagnostics.safe_push (d);
+    m_diagnostics.safe_push (d.release ());
     return true;
   }
 
diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 5b00e6a5f46..d64e3fb7097 100644
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 8e4300b06e2..5a1c6b78ec8 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -19,8 +19,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -95,7 +97,7 @@ public:
                      const svalue *rhs) const final override;
 
   bool can_purge_p (state_t s) const final override;
-  pending_diagnostic *on_leak (tree var) const final override;
+  std::unique_ptr<pending_diagnostic> on_leak (tree var) const final override;
 
   bool is_unchecked_fd_p (state_t s) const;
   bool is_valid_fd_p (state_t s) const;
@@ -683,7 +685,8 @@ fd_state_machine::on_open (sm_context *sm_ctxt, const supernode *node,
     }
   else
     {
-      sm_ctxt->warn (node, stmt, NULL_TREE, new fd_leak (*this, NULL_TREE));
+      sm_ctxt->warn (node, stmt, NULL_TREE,
+		     make_unique <fd_leak> (*this, NULL_TREE));
     }
 }
 
@@ -706,7 +709,8 @@ fd_state_machine::on_close (sm_context *sm_ctxt, const supernode *node,
 
   if (is_closed_fd_p (state))
     {
-      sm_ctxt->warn (node, stmt, arg, new double_close (*this, diag_arg));
+      sm_ctxt->warn (node, stmt, arg,
+		     make_unique <double_close> (*this, diag_arg));
       sm_ctxt->set_next_state (stmt, arg, m_stop);
     }
 }
@@ -738,7 +742,8 @@ fd_state_machine::check_for_open_fd (
   if (is_closed_fd_p (state))
     {
       sm_ctxt->warn (node, stmt, arg,
-                     new fd_use_after_close (*this, diag_arg, callee_fndecl));
+                     make_unique<fd_use_after_close> (*this, diag_arg,
+						      callee_fndecl));
     }
 
   else
@@ -748,7 +753,8 @@ fd_state_machine::check_for_open_fd (
           if (!is_constant_fd_p (state))
             sm_ctxt->warn (
                 node, stmt, arg,
-                new unchecked_use_of_fd (*this, diag_arg, callee_fndecl));
+                make_unique<unchecked_use_of_fd> (*this, diag_arg,
+						  callee_fndecl));
         }
       switch (callee_fndecl_dir)
         {
@@ -757,8 +763,8 @@ fd_state_machine::check_for_open_fd (
             {
               tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
               sm_ctxt->warn (node, stmt, arg,
-                             new fd_access_mode_mismatch (
-                                 *this, diag_arg, DIR_WRITE, callee_fndecl));
+                             make_unique<fd_access_mode_mismatch>
+			       (*this, diag_arg, DIR_WRITE, callee_fndecl));
             }
 
           break;
@@ -768,8 +774,8 @@ fd_state_machine::check_for_open_fd (
             {
               tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
               sm_ctxt->warn (node, stmt, arg,
-                             new fd_access_mode_mismatch (
-                                 *this, diag_arg, DIR_READ, callee_fndecl));
+                             make_unique<fd_access_mode_mismatch>
+			       (*this, diag_arg, DIR_READ, callee_fndecl));
             }
           break;
         }
@@ -840,10 +846,10 @@ fd_state_machine::can_purge_p (state_t s) const
     return true;
 }
 
-pending_diagnostic *
+std::unique_ptr<pending_diagnostic>
 fd_state_machine::on_leak (tree var) const
 {
-  return new fd_leak (*this, var);
+  return make_unique<fd_leak> (*this, var);
 }
 } // namespace
 
@@ -854,4 +860,4 @@ make_fd_state_machine (logger *logger)
 }
 } // namespace ana
 
-#endif // ENABLE_ANALYZER
\ No newline at end of file
+#endif // ENABLE_ANALYZER
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index f6cb29c7806..7a128162d1d 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -19,8 +19,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -82,7 +84,7 @@ public:
 		     const svalue *rhs) const final override;
 
   bool can_purge_p (state_t s) const final override;
-  pending_diagnostic *on_leak (tree var) const final override;
+  std::unique_ptr<pending_diagnostic> on_leak (tree var) const final override;
 
   /* State for a FILE * returned from fopen that hasn't been checked for
      NULL.
@@ -407,7 +409,7 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
 	      {
 		tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 		sm_ctxt->warn (node, stmt, arg,
-			       new double_fclose (*this, diag_arg));
+			       make_unique<double_fclose> (*this, diag_arg));
 		sm_ctxt->set_next_state (stmt, arg, m_stop);
 	      }
 	    return true;
@@ -474,10 +476,10 @@ fileptr_state_machine::can_purge_p (state_t s) const
    fileptr_state_machine, for complaining about leaks of FILE * in
    state 'unchecked' and 'nonnull'.  */
 
-pending_diagnostic *
+std::unique_ptr<pending_diagnostic>
 fileptr_state_machine::on_leak (tree var) const
 {
-  return new file_leak (*this, var);
+  return make_unique<file_leak> (*this, var);
 }
 
 } // anonymous namespace
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 553fcd80085..557c7a63ac0 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -19,8 +19,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -387,7 +389,7 @@ public:
 		     const svalue *rhs) const final override;
 
   bool can_purge_p (state_t s) const final override;
-  pending_diagnostic *on_leak (tree var) const final override;
+  std::unique_ptr<pending_diagnostic> on_leak (tree var) const final override;
 
   bool reset_when_passed_to_unknown_fn_p (state_t s,
 					  bool is_mutable) const final override;
@@ -1729,9 +1731,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 			{
 			  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 			  sm_ctxt->warn (node, stmt, arg,
-					 new possible_null_arg (*this, diag_arg,
-								callee_fndecl,
-								i));
+					 make_unique<possible_null_arg>
+					   (*this, diag_arg, callee_fndecl, i));
 			  const allocation_state *astate
 			    = as_a_allocation_state (state);
 			  sm_ctxt->set_next_state (stmt, arg,
@@ -1741,8 +1742,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 			{
 			  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 			  sm_ctxt->warn (node, stmt, arg,
-					 new null_arg (*this, diag_arg,
-						       callee_fndecl, i));
+					 make_unique<null_arg>
+					   (*this, diag_arg, callee_fndecl, i));
 			  sm_ctxt->set_next_state (stmt, arg, m_stop);
 			}
 		    }
@@ -1784,7 +1785,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    {
 	      tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 	      sm_ctxt->warn (node, stmt, arg,
-			     new possible_null_deref (*this, diag_arg));
+			     make_unique<possible_null_deref> (*this,
+							       diag_arg));
 	      const allocation_state *astate = as_a_allocation_state (state);
 	      sm_ctxt->set_next_state (stmt, arg, astate->get_nonnull ());
 	    }
@@ -1792,7 +1794,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	    {
 	      tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 	      sm_ctxt->warn (node, stmt, arg,
-			     new null_deref (*this, diag_arg));
+			     make_unique<null_deref> (*this, diag_arg));
 	      sm_ctxt->set_next_state (stmt, arg, m_stop);
 	    }
 	  else if (freed_p (state))
@@ -1800,8 +1802,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
 	      tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
 	      const allocation_state *astate = as_a_allocation_state (state);
 	      sm_ctxt->warn (node, stmt, arg,
-			     new use_after_free (*this, diag_arg,
-						 astate->m_deallocator));
+			     make_unique<use_after_free>
+			       (*this, diag_arg, astate->m_deallocator));
 	      sm_ctxt->set_next_state (stmt, arg, m_stop);
 	    }
 	}
@@ -1853,8 +1855,8 @@ malloc_state_machine::handle_free_of_non_heap (sm_context *sm_ctxt,
       freed_reg = old_model->deref_rvalue (ptr_sval, arg, NULL);
     }
   sm_ctxt->warn (node, call, arg,
-		 new free_of_non_heap (*this, diag_arg, freed_reg,
-				       d->m_name));
+		 make_unique<free_of_non_heap>
+		   (*this, diag_arg, freed_reg, d->m_name));
   sm_ctxt->set_next_state (call, arg, m_stop);
 }
 
@@ -1882,11 +1884,11 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt,
 	{
 	  /* Wrong allocator.  */
 	  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
-	  pending_diagnostic *pd
-	    = new mismatching_deallocation (*this, diag_arg,
-					    astate->m_deallocators,
-					    d);
-	  sm_ctxt->warn (node, call, arg, pd);
+	  sm_ctxt->warn (node, call, arg,
+			 make_unique<mismatching_deallocation>
+			   (*this, diag_arg,
+			    astate->m_deallocators,
+			    d));
 	}
       sm_ctxt->set_next_state (call, arg, d->m_freed);
     }
@@ -1898,7 +1900,7 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt,
       /* freed -> stop, with warning.  */
       tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
       sm_ctxt->warn (node, call, arg,
-		     new double_free (*this, diag_arg, d->m_name));
+		     make_unique<double_free> (*this, diag_arg, d->m_name));
       sm_ctxt->set_next_state (call, arg, m_stop);
     }
   else if (state == m_non_heap)
@@ -1936,11 +1938,10 @@ malloc_state_machine::on_realloc_call (sm_context *sm_ctxt,
 	{
 	  /* Wrong allocator.  */
 	  tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
-	  pending_diagnostic *pd
-	    = new mismatching_deallocation (*this, diag_arg,
-					    astate->m_deallocators,
-					    d);
-	  sm_ctxt->warn (node, call, arg, pd);
+	  sm_ctxt->warn (node, call, arg,
+			 make_unique<mismatching_deallocation>
+			   (*this, diag_arg,
+			    astate->m_deallocators, d));
 	  sm_ctxt->set_next_state (call, arg, m_stop);
 	  if (path_context *path_ctxt = sm_ctxt->get_path_context ())
 	    path_ctxt->terminate_path ();
@@ -1951,7 +1952,7 @@ malloc_state_machine::on_realloc_call (sm_context *sm_ctxt,
       /* freed -> stop, with warning.  */
       tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
       sm_ctxt->warn (node, call, arg,
-		     new double_free (*this, diag_arg, "free"));
+		     make_unique<double_free> (*this, diag_arg, "free"));
       sm_ctxt->set_next_state (call, arg, m_stop);
       if (path_context *path_ctxt = sm_ctxt->get_path_context ())
 	path_ctxt->terminate_path ();
@@ -2033,10 +2034,10 @@ malloc_state_machine::can_purge_p (state_t s) const
    (for complaining about leaks of pointers in state 'unchecked' and
    'nonnull').  */
 
-pending_diagnostic *
+std::unique_ptr<pending_diagnostic>
 malloc_state_machine::on_leak (tree var) const
 {
-  return new malloc_leak (*this, var);
+  return make_unique<malloc_leak> (*this, var);
 }
 
 /* Implementation of state_machine::reset_when_passed_to_unknown_fn_p vfunc
diff --git a/gcc/analyzer/sm-pattern-test.cc b/gcc/analyzer/sm-pattern-test.cc
index 9b2ad68e26a..8ad2affa773 100644
--- a/gcc/analyzer/sm-pattern-test.cc
+++ b/gcc/analyzer/sm-pattern-test.cc
@@ -21,8 +21,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -143,8 +145,8 @@ pattern_test_state_machine::on_condition (sm_context *sm_ctxt,
 
   if (tree lhs_expr = sm_ctxt->get_diagnostic_tree (lhs))
     {
-      pending_diagnostic *diag = new pattern_match (lhs_expr, op, rhs_cst);
-      sm_ctxt->warn (node, stmt, lhs_expr, diag);
+      sm_ctxt->warn (node, stmt, lhs_expr,
+		     make_unique<pattern_match> (lhs_expr, op, rhs_cst));
     }
 }
 
diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc
index 83c19068be6..1e28b82e69f 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -20,8 +20,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "function.h"
@@ -184,7 +186,8 @@ sensitive_state_machine::warn_for_any_exposure (sm_context *sm_ctxt,
     {
       tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
       sm_ctxt->warn (node, stmt, arg,
-		     new exposure_through_output_file (*this, diag_arg));
+		     make_unique<exposure_through_output_file> (*this,
+								diag_arg));
     }
 }
 
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index b601f450513..4011ff1a873 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -21,8 +21,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -357,8 +359,8 @@ signal_state_machine::on_stmt (sm_context *sm_ctxt,
 	  if (signal_unsafe_p (callee_fndecl))
 	    if (sm_ctxt->get_global_state () == m_in_signal_handler)
 	      sm_ctxt->warn (node, stmt, NULL_TREE,
-			     new signal_unsafe_call (*this, call,
-						     callee_fndecl));
+			     make_unique<signal_unsafe_call>
+			       (*this, call, callee_fndecl));
     }
 
   return false;
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index 4075cf6d868..5a3217c3853 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -21,8 +21,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -973,10 +975,11 @@ taint_state_machine::check_for_tainted_size_arg (sm_context *sm_ctxt,
 	    TREE_STRING_POINTER (access->to_external_string ());
 	  tree diag_size = sm_ctxt->get_diagnostic_tree (size_arg);
 	  sm_ctxt->warn (node, call, size_arg,
-			 new tainted_access_attrib_size (*this, diag_size, b,
-							 callee_fndecl,
-							 access->sizarg,
-							 access_str));
+			 make_unique<tainted_access_attrib_size>
+			   (*this, diag_size, b,
+			    callee_fndecl,
+			    access->sizarg,
+			    access_str));
 	}
     }
 }
@@ -1011,7 +1014,7 @@ taint_state_machine::check_for_tainted_divisor (sm_context *sm_ctxt,
 
       tree diag_divisor = sm_ctxt->get_diagnostic_tree (divisor_expr);
       sm_ctxt->warn (node, assign, divisor_expr,
-		     new tainted_divisor (*this, diag_divisor, b));
+		     make_unique <tainted_divisor> (*this, diag_divisor, b));
       sm_ctxt->set_next_state (assign, divisor_sval, m_stop);
     }
 }
@@ -1074,7 +1077,7 @@ region_model::check_region_for_taint (const region *reg,
 	    if (taint_sm.get_taint (state, index->get_type (), &b))
 	    {
 	      tree arg = get_representative_tree (index);
-	      ctxt->warn (new tainted_array_index (taint_sm, arg, b));
+	      ctxt->warn (make_unique<tainted_array_index> (taint_sm, arg, b));
 	    }
 	  }
 	  break;
@@ -1096,7 +1099,7 @@ region_model::check_region_for_taint (const region *reg,
 	    if (taint_sm.get_taint (state, effective_type, &b))
 	      {
 		tree arg = get_representative_tree (offset);
-		ctxt->warn (new tainted_offset (taint_sm, arg, b));
+		ctxt->warn (make_unique<tainted_offset> (taint_sm, arg, b));
 	      }
 	  }
 	  break;
@@ -1121,7 +1124,7 @@ region_model::check_region_for_taint (const region *reg,
 	    if (taint_sm.get_taint (state, size_sval->get_type (), &b))
 	      {
 		tree arg = get_representative_tree (size_sval);
-		ctxt->warn (new tainted_size (taint_sm, arg, b));
+		ctxt->warn (make_unique<tainted_size> (taint_sm, arg, b));
 	      }
 	  }
 	  break;
@@ -1167,7 +1170,8 @@ region_model::check_dynamic_size_for_taint (enum memory_space mem_space,
   if (taint_sm.get_taint (state, size_in_bytes->get_type (), &b))
     {
       tree arg = get_representative_tree (size_in_bytes);
-      ctxt->warn (new tainted_allocation_size (taint_sm, arg, b, mem_space));
+      ctxt->warn (make_unique<tainted_allocation_size>
+		    (taint_sm, arg, b, mem_space));
     }
 }
 
diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
index d17d5c765b4..434e4a53b31 100644
--- a/gcc/analyzer/sm.cc
+++ b/gcc/analyzer/sm.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
@@ -41,6 +42,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/store.h"
 #include "analyzer/svalue.h"
 #include "analyzer/program-state.h"
+#include "analyzer/pending-diagnostic.h"
 
 #if ENABLE_ANALYZER
 
@@ -123,6 +125,14 @@ state_machine::get_state_by_name (const char *name) const
   gcc_unreachable ();
 }
 
+/* Base implementation of state_machine::on_leak.  */
+
+std::unique_ptr<pending_diagnostic>
+state_machine::on_leak (tree var ATTRIBUTE_UNUSED) const
+{
+  return NULL;
+}
+
 /* Dump a multiline representation of this state machine to PP.  */
 
 void
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index 353a6db53b0..b857d85d022 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -114,10 +114,8 @@ public:
   virtual bool can_purge_p (state_t s) const = 0;
 
   /* Called when VAR leaks (and !can_purge_p).  */
-  virtual pending_diagnostic *on_leak (tree var ATTRIBUTE_UNUSED) const
-  {
-    return NULL;
-  }
+  virtual std::unique_ptr<pending_diagnostic>
+  on_leak (tree var ATTRIBUTE_UNUSED) const;
 
   /* Return true if S should be reset to "start" for values passed (or reachable
      from) calls to unknown functions.  IS_MUTABLE is true for pointers as
@@ -241,9 +239,11 @@ public:
   /* Called by state_machine in response to pattern matches:
      issue a diagnostic D using NODE and STMT for location information.  */
   virtual void warn (const supernode *node, const gimple *stmt,
-		     tree var, pending_diagnostic *d) = 0;
+		     tree var,
+		     std::unique_ptr<pending_diagnostic> d) = 0;
   virtual void warn (const supernode *node, const gimple *stmt,
-		     const svalue *var, pending_diagnostic *d) = 0;
+		     const svalue *var,
+		     std::unique_ptr<pending_diagnostic> d) = 0;
 
   /* For use when generating trees when creating pending_diagnostics, so that
      rather than e.g.
diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc
index 7a061a19480..3467ce3e35a 100644
--- a/gcc/analyzer/state-purge.cc
+++ b/gcc/analyzer/state-purge.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index d558d477115..904e28baa5c 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 78a6eeff05f..007d808b832 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/trimmed-graph.cc b/gcc/analyzer/trimmed-graph.cc
index 6c85910c5f2..beb9ecb27f1 100644
--- a/gcc/analyzer/trimmed-graph.cc
+++ b/gcc/analyzer/trimmed-graph.cc
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
diff --git a/gcc/analyzer/varargs.cc b/gcc/analyzer/varargs.cc
index c92a56dd2f9..f781604c871 100644
--- a/gcc/analyzer/varargs.cc
+++ b/gcc/analyzer/varargs.cc
@@ -19,8 +19,10 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_MEMORY
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "tree.h"
 #include "function.h"
 #include "basic-block.h"
@@ -217,7 +219,7 @@ public:
   {
     return s != m_started;
   }
-  pending_diagnostic *on_leak (tree var) const final override;
+  std::unique_ptr<pending_diagnostic> on_leak (tree var) const final override;
 
   /* State for a va_list that the result of a va_start or va_copy.  */
   state_t m_started;
@@ -558,8 +560,8 @@ va_list_state_machine::check_for_ended_va_list (sm_context *sm_ctxt,
 {
   if (sm_ctxt->get_state (call, arg) == m_ended)
     sm_ctxt->warn (node, call, arg,
-		   new va_list_use_after_va_end (*this, arg, NULL_TREE,
-						 usage_fnname));
+		   make_unique<va_list_use_after_va_end>
+		     (*this, arg, NULL_TREE, usage_fnname));
 }
 
 /* Get the svalue with associated va_list_state_machine state for a
@@ -634,10 +636,10 @@ va_list_state_machine::on_va_end (sm_context *sm_ctxt,
 /* Implementation of state_machine::on_leak vfunc for va_list_state_machine
    (for complaining about leaks of values in state 'started').  */
 
-pending_diagnostic *
+std::unique_ptr<pending_diagnostic>
 va_list_state_machine::on_leak (tree var) const
 {
-  return new va_list_leak (*this, NULL, var);
+  return make_unique<va_list_leak> (*this, NULL, var);
 }
 
 } // anonymous namespace
@@ -989,17 +991,19 @@ region_model::impl_call_va_arg (const call_details &cd)
 		  else
 		    {
 		      if (ctxt)
-			ctxt->warn (new va_arg_type_mismatch (va_list_tree,
-							      arg_reg,
-							      lhs_type,
-							      arg_type));
+			ctxt->warn (make_unique <va_arg_type_mismatch>
+				      (va_list_tree,
+				       arg_reg,
+				       lhs_type,
+				       arg_type));
 		      saw_problem = true;
 		    }
 		}
 	      else
 		{
 		  if (ctxt)
-		    ctxt->warn (new va_list_exhausted (va_list_tree, arg_reg));
+		    ctxt->warn (make_unique <va_list_exhausted> (va_list_tree,
+								 arg_reg));
 		  saw_problem = true;
 		}
 	    }
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
index 2a8bf11b3e9..cf0baa5670d 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
@@ -4,10 +4,12 @@
 */
 /* { dg-options "-g" } */
 
+#define INCLUDE_MEMORY
 #include "gcc-plugin.h"
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "make-unique.h"
 #include "diagnostic.h"
 #include "tree.h"
 #include "gimple.h"
@@ -309,9 +311,9 @@ gil_state_machine::check_for_pyobject_in_call (sm_context *sm_ctxt,
       if (type_based_on_pyobject_p (type))
 	{
 	  sm_ctxt->warn (node, call, NULL_TREE,
-			 new fncall_without_gil (*this, call,
-						 callee_fndecl,
-						 i));
+			 make_unique<fncall_without_gil> (*this, call,
+							  callee_fndecl,
+							  i));
 	  sm_ctxt->set_global_state (m_stop);
 	}
     }
@@ -337,7 +339,7 @@ gil_state_machine::on_stmt (sm_context *sm_ctxt,
 	      if (global_state == m_released_gil)
 		{
 		  sm_ctxt->warn (node, stmt, NULL_TREE,
-				 new double_save_thread (*this, call));
+				 make_unique<double_save_thread> (*this, call));
 		  sm_ctxt->set_global_state (m_stop);
 		}
 	      else
@@ -393,7 +395,7 @@ gil_state_machine::check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
   if (type_based_on_pyobject_p (type))
     {
       sm_ctxt->warn (node, stmt, NULL_TREE,
-		     new pyobject_usage_without_gil (*this, op));
+		     make_unique<pyobject_usage_without_gil> (*this, op));
       sm_ctxt->set_global_state (m_stop);
     }
 }
-- 
2.26.3

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022, 01:25 David Malcolm, <dmalcolm@redhat.com> wrote:

> On Fri, 2022-07-08 at 22:16 +0100, Jonathan Wakely wrote:
> > On Fri, 8 Jul 2022 at 21:47, David Malcolm via Gcc <gcc@gcc.gnu.org>
> > wrote:
> > >
> > > std::unique_ptr is C++11, and I'd like to use it in the
> > > gcc/analyzer
> > > subdirectory, at least.  The following patch eliminates a bunch of
> > > "takes ownership" comments and manual "delete" invocations in favor
> > > of simply using std::unique_ptr.
> > >
> > > The problem is that the patch makes use of std::make_unique, but
> > > that
> > > was added in C++14.
> > >
> > > I've heard that it's reasonably easy to reimplement
> > > std::make_unique,
> > > but I'm not sure that my C++11 skills are up to it.
> >
> > You know we have an implementation of std::make_unique in GCC, with a
> > GCC-compatible licence that you can look at, right? :-)
> >
> > But it's not really necessary. There are only two reasons to prefer
> > make_unique over just allocating an object with new and constructing
> > a
> > unique_ptr from it:
> >
> > 1) avoid a "naked" new in your code (some coding styles like this,
> > but
> > it's not really important as long as the 'delete' is managed
> > automatically by unique_ptr).
> >
> > 2) exception-safety when allocating multiple objects as args to a
> > function, see https://herbsutter.com/gotw/_102/ for details.
> > Irrelevant for GCC, because we build without exceptions.
>
> [moving from gcc to gcc-patches mailing list]
>
> Also, I *think* it's a lot less typing, since I can write just:
>
>   std::make_unique<name_of_type_which_could_be_long> (args)
>
> rather than
>
>   std::unique_ptr<name_of_type_which_could_be_long> (new
> name_of_type_which_could_be_long (args));
>
> >
> >
> >
> > > Is there:
> > > (a) an easy way to implement a std::make_unique replacement
> > >     (e.g. in system.h? what to call it?), or
> >
> > If you don't care about using it to create unique_ptr<T[]> arrays,
> > it's trivial:
> >
> >   template<typename T, typename... Args>
> >     inline typename std::enable_if<!std::is_array<T>::value,
> > std::unique_ptr<T>>::type
> >     make_unique(Args&&... args)
> >     { return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
> > }
> >
> > To add the overload that works for arrays is a little trickier.
>
> Thanks!
>
> I tried adding it to gcc/system.h, but anything that uses it needs to
> have std::unique_ptr declared, which meant forcibly including <memory>
> from gcc/system.h
>
> So instead, here's a patch that adds a new gcc/make-unique.h header,
> containing just the template decl above (in the root namespace, rather
> than std::, which saves a bit more typing).
>

Adding things to std isn't allowed anyway, so that's correct.


> I've successfully bootstrapped&regression-tested a version of my earlier
> analyzer patch that uses this patch (see patch 2 of the kit, which has
> lots of usage examples).
>
> OK for trunk?
>
> Dave
>
>
>
> This patch adds gcc/make-unique.h, containing a minimal C++11
> implementation of make_unique (std::make_unique is C++14).
>
> gcc/ChangeLog:
>         * make-unique.h: New file.
>
> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> ---
>  gcc/make-unique.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 gcc/make-unique.h
>
> diff --git a/gcc/make-unique.h b/gcc/make-unique.h
> new file mode 100644
> index 00000000000..c99c5328545
> --- /dev/null
> +++ b/gcc/make-unique.h
> @@ -0,0 +1,41 @@
> +/* Minimal implementation of make_unique for C++11 compatibility.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_MAKE_UNIQUE
> +#define GCC_MAKE_UNIQUE
> +
> +/* This header uses std::unique_ptr, but <memory> can't be directly
> +   included due to issues with macros.  Hence it must be included from
> +   system.h by defining INCLUDE_MEMORY in any source file using it.  */
> +
> +#ifndef INCLUDE_MEMORY
> +# error "You must define INCLUDE_MEMORY before including system.h to use
> make-unique.h"
> +#endif
>

You also need <type_traits> for the enable_if and is_array traits. With
libstdc++ that gets included by <memory> but that's guaranteed for other
library implementations.

I don't know if that had the same kind of issues as other system headers or
if it can just be included here.


+
> +/* Minimal implementation of make_unique for C++11 compatibility
> +   (std::make_unique is C++14).  */
> +
> +template<typename T, typename... Args>
> +inline typename std::enable_if<!std::is_array<T>::value,
> std::unique_ptr<T>>::type
> +make_unique(Args&&... args)
> +{
> +  return std::unique_ptr<T> (new T (std::forward<Args> (args)...));
> +}
> +
> +#endif /* ! GCC_MAKE_UNIQUE */
> --
> 2.26.3
>
>

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022 at 07:48, Jonathan Wakely wrote:
> You also need <type_traits> for the enable_if and is_array traits. With libstdc++ that gets included by <memory> but that's guaranteed for other library implementations.

Sorry, I was replying from my phone and missed a word. That's **not**
guaranteed for other library implementations.

(It's not formally guaranteed for libstdc++ either, but in practice
nearly every libstdc++ header includes <type_traits> because nearly
every header needs part of it, and that's unlikely to change).

[PATCH v3] Add gcc/make-unique.h

[David Malcolm]

Changed in v3: added include of <type_traits>
v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604137.html
v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598189.html

On Tue, 2022-07-12 at 07:48 +0100, Jonathan Wakely wrote:
> On Tue, 12 Jul 2022, 01:25 David Malcolm, <dmalcolm@redhat.com>
> wrote:
> 
> > On Fri, 2022-07-08 at 22:16 +0100, Jonathan Wakely wrote:
> > > On Fri, 8 Jul 2022 at 21:47, David Malcolm via Gcc
> > > <gcc@gcc.gnu.org>
> > > wrote:
> > > > 
> > > > std::unique_ptr is C++11, and I'd like to use it in the
> > > > gcc/analyzer
> > > > subdirectory, at least.  The following patch eliminates a bunch
> > > > of
> > > > "takes ownership" comments and manual "delete" invocations in
> > > > favor
> > > > of simply using std::unique_ptr.
> > > > 
> > > > The problem is that the patch makes use of std::make_unique,
> > > > but
> > > > that
> > > > was added in C++14.
> > > > 
> > > > I've heard that it's reasonably easy to reimplement
> > > > std::make_unique,
> > > > but I'm not sure that my C++11 skills are up to it.
> > > 
> > > You know we have an implementation of std::make_unique in GCC,
> > > with a
> > > GCC-compatible licence that you can look at, right? :-)
> > > 
> > > But it's not really necessary. There are only two reasons to
> > > prefer
> > > make_unique over just allocating an object with new and
> > > constructing
> > > a
> > > unique_ptr from it:
> > > 
> > > 1) avoid a "naked" new in your code (some coding styles like
> > > this,
> > > but
> > > it's not really important as long as the 'delete' is managed
> > > automatically by unique_ptr).
> > > 
> > > 2) exception-safety when allocating multiple objects as args to a
> > > function, see https://herbsutter.com/gotw/_102/ for details.
> > > Irrelevant for GCC, because we build without exceptions.
> > 
> > [moving from gcc to gcc-patches mailing list]
> > 
> > Also, I *think* it's a lot less typing, since I can write just:
> > 
> >   std::make_unique<name_of_type_which_could_be_long> (args)
> > 
> > rather than
> > 
> >   std::unique_ptr<name_of_type_which_could_be_long> (new
> > name_of_type_which_could_be_long (args));
> > 
> > > 
> > > 
> > > 
> > > > Is there:
> > > > (a) an easy way to implement a std::make_unique replacement
> > > >     (e.g. in system.h? what to call it?), or
> > > 
> > > If you don't care about using it to create unique_ptr<T[]>
> > > arrays,
> > > it's trivial:
> > > 
> > >   template<typename T, typename... Args>
> > >     inline typename std::enable_if<!std::is_array<T>::value,
> > > std::unique_ptr<T>>::type
> > >     make_unique(Args&&... args)
> > >     { return std::unique_ptr<T>(new
> > > T(std::forward<Args>(args)...));
> > > }
> > > 
> > > To add the overload that works for arrays is a little trickier.
> > 
> > Thanks!
> > 
> > I tried adding it to gcc/system.h, but anything that uses it needs
> > to
> > have std::unique_ptr declared, which meant forcibly including
> > <memory>
> > from gcc/system.h
> > 
> > So instead, here's a patch that adds a new gcc/make-unique.h
> > header,
> > containing just the template decl above (in the root namespace,
> > rather
> > than std::, which saves a bit more typing).
> > 
> 
> Adding things to std isn't allowed anyway, so that's correct.
> 
> 
> > I've successfully bootstrapped&regression-tested a version of my
> > earlier
> > analyzer patch that uses this patch (see patch 2 of the kit, which
> > has
> > lots of usage examples).
> > 
> > OK for trunk?
> > 
> > Dave

[...snip...]

> > +#ifndef INCLUDE_MEMORY
> > +# error "You must define INCLUDE_MEMORY before including system.h
> > to use
> > make-unique.h"
> > +#endif
> > 
> 
> You also need <type_traits> for the enable_if and is_array traits.
> With
> libstdc++ that gets included by <memory> but that's guaranteed for
> other
> library implementations.
> 
> I don't know if that had the same kind of issues as other system
> headers or
> if it can just be included here.

I've added an include of <type_traits> in this version of the patch.

> 
> 
> +
> > +/* Minimal implementation of make_unique for C++11 compatibility
> > +   (std::make_unique is C++14).  */
> > +
> > +template<typename T, typename... Args>
> > +inline typename std::enable_if<!std::is_array<T>::value,
> > std::unique_ptr<T>>::type
> > +make_unique(Args&&... args)
> > +{
> > +  return std::unique_ptr<T> (new T (std::forward<Args>
> > (args)...));
> > +}
> > +
> > +#endif /* ! GCC_MAKE_UNIQUE */
> > --
> > 2.26.3
> > 
> > 

This patch adds gcc/make-unique.h, containing a minimal C++11
implementation of make_unique (std::make_unique is C++14).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
in conjunction with a followup series of patches which use this
in dozens of places in the analyzer.

OK for trunk?

gcc/ChangeLog:
	* make-unique.h: New file.
---
 gcc/make-unique.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 gcc/make-unique.h

diff --git a/gcc/make-unique.h b/gcc/make-unique.h
new file mode 100644
index 00000000000..c9a7d6ef6ce
--- /dev/null
+++ b/gcc/make-unique.h
@@ -0,0 +1,44 @@
+/* Minimal implementation of make_unique for C++11 compatibility.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_MAKE_UNIQUE
+#define GCC_MAKE_UNIQUE
+
+/* This header uses std::unique_ptr, but <memory> can't be directly
+   included due to issues with macros.  Hence <memory> must be included
+   from system.h by defining INCLUDE_MEMORY in any source file using
+   make-unique.h.  */
+
+#ifndef INCLUDE_MEMORY
+# error "You must define INCLUDE_MEMORY before including system.h to use make-unique.h"
+#endif
+
+#include <type_traits>
+
+/* Minimal implementation of make_unique for C++11 compatibility
+   (std::make_unique is C++14).  */
+
+template<typename T, typename... Args>
+inline typename std::enable_if<!std::is_array<T>::value, std::unique_ptr<T>>::type
+make_unique(Args&&... args)
+{
+  return std::unique_ptr<T> (new T (std::forward<Args> (args)...));
+}
+
+#endif /* ! GCC_MAKE_UNIQUE */
-- 
2.26.3

Re: [PATCH v3] Add gcc/make-unique.h

[Jason Merrill]

On 10/26/22 16:40, David Malcolm via Gcc-patches wrote:
> Changed in v3: added include of <type_traits>
> v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604137.html
> v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598189.html
> 
> On Tue, 2022-07-12 at 07:48 +0100, Jonathan Wakely wrote:
>> On Tue, 12 Jul 2022, 01:25 David Malcolm, <dmalcolm@redhat.com>
>> wrote:
>>
>>> On Fri, 2022-07-08 at 22:16 +0100, Jonathan Wakely wrote:
>>>> On Fri, 8 Jul 2022 at 21:47, David Malcolm via Gcc
>>>> <gcc@gcc.gnu.org>
>>>> wrote:
>>>>>
>>>>> std::unique_ptr is C++11, and I'd like to use it in the
>>>>> gcc/analyzer
>>>>> subdirectory, at least.  The following patch eliminates a bunch
>>>>> of
>>>>> "takes ownership" comments and manual "delete" invocations in
>>>>> favor
>>>>> of simply using std::unique_ptr.
>>>>>
>>>>> The problem is that the patch makes use of std::make_unique,
>>>>> but
>>>>> that
>>>>> was added in C++14.
>>>>>
>>>>> I've heard that it's reasonably easy to reimplement
>>>>> std::make_unique,
>>>>> but I'm not sure that my C++11 skills are up to it.
>>>>
>>>> You know we have an implementation of std::make_unique in GCC,
>>>> with a
>>>> GCC-compatible licence that you can look at, right? :-)
>>>>
>>>> But it's not really necessary. There are only two reasons to
>>>> prefer
>>>> make_unique over just allocating an object with new and
>>>> constructing
>>>> a
>>>> unique_ptr from it:
>>>>
>>>> 1) avoid a "naked" new in your code (some coding styles like
>>>> this,
>>>> but
>>>> it's not really important as long as the 'delete' is managed
>>>> automatically by unique_ptr).
>>>>
>>>> 2) exception-safety when allocating multiple objects as args to a
>>>> function, see https://herbsutter.com/gotw/_102/ for details.
>>>> Irrelevant for GCC, because we build without exceptions.
>>>
>>> [moving from gcc to gcc-patches mailing list]
>>>
>>> Also, I *think* it's a lot less typing, since I can write just:
>>>
>>>    std::make_unique<name_of_type_which_could_be_long> (args)
>>>
>>> rather than
>>>
>>>    std::unique_ptr<name_of_type_which_could_be_long> (new
>>> name_of_type_which_could_be_long (args));
>>>
>>>>
>>>>
>>>>
>>>>> Is there:
>>>>> (a) an easy way to implement a std::make_unique replacement
>>>>>      (e.g. in system.h? what to call it?), or
>>>>
>>>> If you don't care about using it to create unique_ptr<T[]>
>>>> arrays,
>>>> it's trivial:
>>>>
>>>>    template<typename T, typename... Args>
>>>>      inline typename std::enable_if<!std::is_array<T>::value,
>>>> std::unique_ptr<T>>::type
>>>>      make_unique(Args&&... args)
>>>>      { return std::unique_ptr<T>(new
>>>> T(std::forward<Args>(args)...));
>>>> }
>>>>
>>>> To add the overload that works for arrays is a little trickier.
>>>
>>> Thanks!
>>>
>>> I tried adding it to gcc/system.h, but anything that uses it needs
>>> to
>>> have std::unique_ptr declared, which meant forcibly including
>>> <memory>
>>> from gcc/system.h
>>>
>>> So instead, here's a patch that adds a new gcc/make-unique.h
>>> header,
>>> containing just the template decl above (in the root namespace,
>>> rather
>>> than std::, which saves a bit more typing).
>>>
>>
>> Adding things to std isn't allowed anyway, so that's correct.
>>
>>
>>> I've successfully bootstrapped&regression-tested a version of my
>>> earlier
>>> analyzer patch that uses this patch (see patch 2 of the kit, which
>>> has
>>> lots of usage examples).
>>>
>>> OK for trunk?
>>>
>>> Dave
> 
> [...snip...]
> 
>>> +#ifndef INCLUDE_MEMORY
>>> +# error "You must define INCLUDE_MEMORY before including system.h
>>> to use
>>> make-unique.h"
>>> +#endif
>>>
>>
>> You also need <type_traits> for the enable_if and is_array traits.
>> With
>> libstdc++ that gets included by <memory> but that's guaranteed for
>> other
>> library implementations.
>>
>> I don't know if that had the same kind of issues as other system
>> headers or
>> if it can just be included here.
> 
> I've added an include of <type_traits> in this version of the patch.
> 
>>
>>
>> +
>>> +/* Minimal implementation of make_unique for C++11 compatibility
>>> +   (std::make_unique is C++14).  */
>>> +
>>> +template<typename T, typename... Args>
>>> +inline typename std::enable_if<!std::is_array<T>::value,
>>> std::unique_ptr<T>>::type
>>> +make_unique(Args&&... args)
>>> +{
>>> +  return std::unique_ptr<T> (new T (std::forward<Args>
>>> (args)...));
>>> +}
>>> +
>>> +#endif /* ! GCC_MAKE_UNIQUE */
>>> --
>>> 2.26.3
>>>
>>>
> 
> This patch adds gcc/make-unique.h, containing a minimal C++11
> implementation of make_unique (std::make_unique is C++14).
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> in conjunction with a followup series of patches which use this
> in dozens of places in the analyzer.
> 
> OK for trunk?

OK.

> gcc/ChangeLog:
> 	* make-unique.h: New file.
> ---
>   gcc/make-unique.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>   create mode 100644 gcc/make-unique.h
> 
> diff --git a/gcc/make-unique.h b/gcc/make-unique.h
> new file mode 100644
> index 00000000000..c9a7d6ef6ce
> --- /dev/null
> +++ b/gcc/make-unique.h
> @@ -0,0 +1,44 @@
> +/* Minimal implementation of make_unique for C++11 compatibility.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_MAKE_UNIQUE
> +#define GCC_MAKE_UNIQUE
> +
> +/* This header uses std::unique_ptr, but <memory> can't be directly
> +   included due to issues with macros.  Hence <memory> must be included
> +   from system.h by defining INCLUDE_MEMORY in any source file using
> +   make-unique.h.  */
> +
> +#ifndef INCLUDE_MEMORY
> +# error "You must define INCLUDE_MEMORY before including system.h to use make-unique.h"
> +#endif
> +
> +#include <type_traits>
> +
> +/* Minimal implementation of make_unique for C++11 compatibility
> +   (std::make_unique is C++14).  */
> +
> +template<typename T, typename... Args>
> +inline typename std::enable_if<!std::is_array<T>::value, std::unique_ptr<T>>::type
> +make_unique(Args&&... args)
> +{
> +  return std::unique_ptr<T> (new T (std::forward<Args> (args)...));
> +}
> +
> +#endif /* ! GCC_MAKE_UNIQUE */

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote:

> I tried adding it to gcc/system.h, but anything that uses it needs to
> have std::unique_ptr declared, which meant forcibly including <memory>
> from gcc/system.h

Did you consider making gcc/system.h include gcc/make-unique.h itself 
if INCLUDE_MEMORY is defined?  Something like:

 #ifdef INCLUDE_MEMORY
 # include <memory>
+ #include "make-unique.h"
 #endif

This is because std::make_unique is defined in <memory> in C++14.  This would
mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated.

> (in the root namespace, rather than std::, which saves a bit more typing).

It's less typing now, but it will be more churn once GCC requires C++14 (or later), at
which point you'll naturally want to get rid of the custom make_unique.  More churn
since make_unique -> std::make_unique may require re-indentation of arguments, etc.
For that reason, I would suggest instead to put the function (and any other straight
standard library backport) in a 3-letter namespace already, like, gcc::make_unique
or gnu::make_unique.  That way, when the time comes that GCC requires C++14,
the patch to replace gcc::make_unique won't have to worry about reindenting code,
it'll just replace gcc -> std.

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote:
>
> On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote:
>
> > I tried adding it to gcc/system.h, but anything that uses it needs to
> > have std::unique_ptr declared, which meant forcibly including <memory>
> > from gcc/system.h
>
> Did you consider making gcc/system.h include gcc/make-unique.h itself
> if INCLUDE_MEMORY is defined?  Something like:
>
>  #ifdef INCLUDE_MEMORY
>  # include <memory>
> + #include "make-unique.h"
>  #endif
>
> This is because std::make_unique is defined in <memory> in C++14.  This would
> mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated.

That's a good idea.

> > (in the root namespace, rather than std::, which saves a bit more typing).
>
> It's less typing now, but it will be more churn once GCC requires C++14 (or later), at
> which point you'll naturally want to get rid of the custom make_unique.  More churn
> since make_unique -> std::make_unique may require re-indentation of arguments, etc.
> For that reason, I would suggest instead to put the function (and any other straight
> standard library backport) in a 3-letter namespace already, like, gcc::make_unique
> or gnu::make_unique.  That way, when the time comes that GCC requires C++14,
> the patch to replace gcc::make_unique won't have to worry about reindenting code,
> it'll just replace gcc -> std.

Or (when the time comes) don't change gcc->std and do:

namespace gcc {
  using std::make_unique;
}

or just leave it in the global namespace as in your current patch, and
at a later date add a using-declaration to the global namespace:

using std::make_unique;

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 2:45 p.m., Jonathan Wakely wrote:
> On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote:
>>
>> On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote:
>>
>>> I tried adding it to gcc/system.h, but anything that uses it needs to
>>> have std::unique_ptr declared, which meant forcibly including <memory>
>>> from gcc/system.h
>>
>> Did you consider making gcc/system.h include gcc/make-unique.h itself
>> if INCLUDE_MEMORY is defined?  Something like:
>>
>>  #ifdef INCLUDE_MEMORY
>>  # include <memory>
>> + #include "make-unique.h"
>>  #endif
>>
>> This is because std::make_unique is defined in <memory> in C++14.  This would
>> mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated.
> 
> That's a good idea.
> 
>>> (in the root namespace, rather than std::, which saves a bit more typing).
>>
>> It's less typing now, but it will be more churn once GCC requires C++14 (or later), at
>> which point you'll naturally want to get rid of the custom make_unique.  More churn
>> since make_unique -> std::make_unique may require re-indentation of arguments, etc.
>> For that reason, I would suggest instead to put the function (and any other straight
>> standard library backport) in a 3-letter namespace already, like, gcc::make_unique
>> or gnu::make_unique.  That way, when the time comes that GCC requires C++14,
>> the patch to replace gcc::make_unique won't have to worry about reindenting code,
>> it'll just replace gcc -> std.
> 
> Or (when the time comes) don't change gcc->std and do:
> 
> namespace gcc {
>   using std::make_unique;
> }

It will seem like a pointless indirection then, IMO.

> 
> or just leave it in the global namespace as in your current patch, and
> at a later date add a using-declaration to the global namespace:
> 
> using std::make_unique;
> 

That's not very idiomatic, though.  Let me turn this into a reverse question:

If GCC required C++14 today, would you be doing the above, either importing make_unique
to the global namespace, or into namespace gcc?   I think it's safe to say that, no,
nobody would be doing that.  So once GCC requires C++14, why would you want to preserve
once-backported symbols in a namespace other than std, when you no longer have a reason to?
It will just be another unnecessary thing that newcomers at that future time will have
to learn.

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022 at 15:06, Pedro Alves <pedro@palves.net> wrote:
>
> On 2022-07-12 2:45 p.m., Jonathan Wakely wrote:
> > On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote:
> >>
> >> On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote:
> >>
> >>> I tried adding it to gcc/system.h, but anything that uses it needs to
> >>> have std::unique_ptr declared, which meant forcibly including <memory>
> >>> from gcc/system.h
> >>
> >> Did you consider making gcc/system.h include gcc/make-unique.h itself
> >> if INCLUDE_MEMORY is defined?  Something like:
> >>
> >>  #ifdef INCLUDE_MEMORY
> >>  # include <memory>
> >> + #include "make-unique.h"
> >>  #endif
> >>
> >> This is because std::make_unique is defined in <memory> in C++14.  This would
> >> mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated.
> >
> > That's a good idea.
> >
> >>> (in the root namespace, rather than std::, which saves a bit more typing).
> >>
> >> It's less typing now, but it will be more churn once GCC requires C++14 (or later), at
> >> which point you'll naturally want to get rid of the custom make_unique.  More churn
> >> since make_unique -> std::make_unique may require re-indentation of arguments, etc.
> >> For that reason, I would suggest instead to put the function (and any other straight
> >> standard library backport) in a 3-letter namespace already, like, gcc::make_unique
> >> or gnu::make_unique.  That way, when the time comes that GCC requires C++14,
> >> the patch to replace gcc::make_unique won't have to worry about reindenting code,
> >> it'll just replace gcc -> std.
> >
> > Or (when the time comes) don't change gcc->std and do:
> >
> > namespace gcc {
> >   using std::make_unique;
> > }
>
> It will seem like a pointless indirection then, IMO.
>
> >
> > or just leave it in the global namespace as in your current patch, and
> > at a later date add a using-declaration to the global namespace:
> >
> > using std::make_unique;
> >
>
> That's not very idiomatic, though.  Let me turn this into a reverse question:
>
> If GCC required C++14 today, would you be doing the above, either importing make_unique
> to the global namespace, or into namespace gcc?   I think it's safe to say that, no,
> nobody would be doing that.

Erm, I might well do exactly that, for either case.

I don't see a problem with 'using std::make_unique;' into the global
namespace in GCC code. It's not a library header being included by
arbitrary code, it's a single application that isn't going to have
conflicts for some other ::make_unique defined in GCC (because the
::make_unique that is being proposed today would be removed once
C++14's std::make_unique can be used).


>  So once GCC requires C++14, why would you want to preserve
> once-backported symbols in a namespace other than std, when you no longer have a reason to?
> It will just be another unnecessary thing that newcomers at that future time will have
> to learn.

I also don't see a problem with importing std::make_unique into
namespace gcc for local use alongside other things in namespace gcc. I
do consider that idiomatic. It says "the make_unique for gcc code is
std::make_unique". It means you only need a 'using namespace gcc;' at
the top of a source file and you get access to everything in namespace
gcc, even if it is something like std::make_unique that was originally
defined in a different namespace.

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:

>>  So once GCC requires C++14, why would you want to preserve
>> once-backported symbols in a namespace other than std, when you no longer have a reason to?
>> It will just be another unnecessary thing that newcomers at that future time will have
>> to learn.
> 
> I also don't see a problem with importing std::make_unique into
> namespace gcc for local use alongside other things in namespace gcc. I
> do consider that idiomatic. It says "the make_unique for gcc code is
> std::make_unique". It means you only need a 'using namespace gcc;' at
> the top of a source file and you get access to everything in namespace
> gcc, even if it is something like std::make_unique that was originally
> defined in a different namespace.
> 

If that's the approach, then GCC should import std::unique_ptr, std::move,
std::foo, std::bar into the gcc namespace too, no?  Are you really going
to propose that?

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net> wrote:

> On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
>
> >>  So once GCC requires C++14, why would you want to preserve
> >> once-backported symbols in a namespace other than std, when you no
> longer have a reason to?
> >> It will just be another unnecessary thing that newcomers at that future
> time will have
> >> to learn.
> >
> > I also don't see a problem with importing std::make_unique into
> > namespace gcc for local use alongside other things in namespace gcc. I
> > do consider that idiomatic. It says "the make_unique for gcc code is
> > std::make_unique". It means you only need a 'using namespace gcc;' at
> > the top of a source file and you get access to everything in namespace
> > gcc, even if it is something like std::make_unique that was originally
> > defined in a different namespace.
> >
>
> If that's the approach, then GCC should import std::unique_ptr, std::move,
> std::foo, std::bar into the gcc namespace too, no?  Are you really going
> to propose that?
>

No, I don't follow the logic of "if you do it for one thing you must do it
for everything". That's a straw man. But I don't really mind how this gets
done. Your suggestion is fine.

>

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 7:22 p.m., Jonathan Wakely wrote:
> 
> 
> On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote:
> 
>     On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
> 
>     >>  So once GCC requires C++14, why would you want to preserve
>     >> once-backported symbols in a namespace other than std, when you no longer have a reason to?
>     >> It will just be another unnecessary thing that newcomers at that future time will have
>     >> to learn.
>     >
>     > I also don't see a problem with importing std::make_unique into
>     > namespace gcc for local use alongside other things in namespace gcc. I
>     > do consider that idiomatic. It says "the make_unique for gcc code is
>     > std::make_unique". It means you only need a 'using namespace gcc;' at
>     > the top of a source file and you get access to everything in namespace
>     > gcc, even if it is something like std::make_unique that was originally
>     > defined in a different namespace.
>     >
> 
>     If that's the approach, then GCC should import std::unique_ptr, std::move,
>     std::foo, std::bar into the gcc namespace too, no?  Are you really going
>     to propose that?
> 
> 
> No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine.
> 

It isn't a strawman, Jon.  Maybe there's some miscommunication.  The conversion started (and part of it is
still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize
for that.  When we get to the point when we require C++14, make_unique is no longer different from any other
symbol in the std namespace, and there will be no reason to treat it differently anymore.  Like, if someone at
that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with
std::make_unique, there will be no real ground to object to that, why wouldn't you want it?  This is very
much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced
it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history.

Cheers,
Pedro Alves

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 7:36 p.m., Pedro Alves wrote:
> On 2022-07-12 7:22 p.m., Jonathan Wakely wrote:
>>
>>
>> On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote:
>>
>>     On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
>>
>>     >>  So once GCC requires C++14, why would you want to preserve
>>     >> once-backported symbols in a namespace other than std, when you no longer have a reason to?
>>     >> It will just be another unnecessary thing that newcomers at that future time will have
>>     >> to learn.
>>     >
>>     > I also don't see a problem with importing std::make_unique into
>>     > namespace gcc for local use alongside other things in namespace gcc. I
>>     > do consider that idiomatic. It says "the make_unique for gcc code is
>>     > std::make_unique". It means you only need a 'using namespace gcc;' at
>>     > the top of a source file and you get access to everything in namespace
>>     > gcc, even if it is something like std::make_unique that was originally
>>     > defined in a different namespace.
>>     >
>>
>>     If that's the approach, then GCC should import std::unique_ptr, std::move,
>>     std::foo, std::bar into the gcc namespace too, no?  Are you really going
>>     to propose that?
>>
>>
>> No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine.
>>
> 
> It isn't a strawman, Jon.  Maybe there's some miscommunication.  The conversion started (and part of it is
> still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize
> for that.  When we get to the point when we require C++14, make_unique is no longer different from any other
> symbol in the std namespace, and there will be no reason to treat it differently anymore.  Like, if someone at
> that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with
> std::make_unique, there will be no real ground to object to that, why wouldn't you want it?  This is very
> much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced
> it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history.

Sorry to reply to myself -- but I'm not sure it is clear what I meant above in the last sentence, so let
me try again: 'the "gnu::unique_ptr" wasn't rewritten as an import of std::unique_ptr into the gnu namespace
just because of history.'

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022 at 19:41, Pedro Alves <pedro@palves.net> wrote:
>
> On 2022-07-12 7:36 p.m., Pedro Alves wrote:
> > On 2022-07-12 7:22 p.m., Jonathan Wakely wrote:
> >>
> >>
> >> On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote:
> >>
> >>     On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
> >>
> >>     >>  So once GCC requires C++14, why would you want to preserve
> >>     >> once-backported symbols in a namespace other than std, when you no longer have a reason to?
> >>     >> It will just be another unnecessary thing that newcomers at that future time will have
> >>     >> to learn.
> >>     >
> >>     > I also don't see a problem with importing std::make_unique into
> >>     > namespace gcc for local use alongside other things in namespace gcc. I
> >>     > do consider that idiomatic. It says "the make_unique for gcc code is
> >>     > std::make_unique". It means you only need a 'using namespace gcc;' at
> >>     > the top of a source file and you get access to everything in namespace
> >>     > gcc, even if it is something like std::make_unique that was originally
> >>     > defined in a different namespace.
> >>     >
> >>
> >>     If that's the approach, then GCC should import std::unique_ptr, std::move,
> >>     std::foo, std::bar into the gcc namespace too, no?  Are you really going
> >>     to propose that?
> >>
> >>
> >> No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine.
> >>
> >
> > It isn't a strawman, Jon.  Maybe there's some miscommunication.  The conversion started (and part of it is
> > still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize
> > for that.  When we get to the point when we require C++14, make_unique is no longer different from any other
> > symbol in the std namespace, and there will be no reason to treat it differently anymore.  Like, if someone at
> > that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with
> > std::make_unique, there will be no real ground to object to that, why wouldn't you want it?  This is very
> > much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced
> > it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history.
>
> Sorry to reply to myself -- but I'm not sure it is clear what I meant above in the last sentence, so let
> me try again: 'the "gnu::unique_ptr" wasn't rewritten as an import of std::unique_ptr into the gnu namespace
> just because of history.'

[OFFLIST]

I considered doing exactly that. But because namespace gnu wasn't used
anywhere else in GCC it didn't make sense. If it had been put in
namespace gcc, which is still used elsewhere in the GCC codebase, I
might have decided differently. But keeping namespace 'gnu' with no
content except 'using std::unique_ptr;' would have been pointless. It
wouldn't have made it easier for other things in namespace gnu to
refer to it, because there were no other things. It wouldn't have
allowed 'using namespace gnu;' to make various useful utilities
available with a single using-directive, because that would only make
one thing available, and 'using std::unique_ptr;' does that just as
effectively.

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022 at 19:58, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> [OFFLIST]

Oops, sorry, reply-all fail. I meant to spare everybody any further
musing on this topic.

Really leaving the thread now!

Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely]

On Tue, 12 Jul 2022, 19:36 Pedro Alves, <pedro@palves.net> wrote:

> On 2022-07-12 7:22 p.m., Jonathan Wakely wrote:
> >
> >
> > On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:
> pedro@palves.net>> wrote:
> >
> >     On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
> >
> >     >>  So once GCC requires C++14, why would you want to preserve
> >     >> once-backported symbols in a namespace other than std, when you
> no longer have a reason to?
> >     >> It will just be another unnecessary thing that newcomers at that
> future time will have
> >     >> to learn.
> >     >
> >     > I also don't see a problem with importing std::make_unique into
> >     > namespace gcc for local use alongside other things in namespace
> gcc. I
> >     > do consider that idiomatic. It says "the make_unique for gcc code
> is
> >     > std::make_unique". It means you only need a 'using namespace gcc;'
> at
> >     > the top of a source file and you get access to everything in
> namespace
> >     > gcc, even if it is something like std::make_unique that was
> originally
> >     > defined in a different namespace.
> >     >
> >
> >     If that's the approach, then GCC should import std::unique_ptr,
> std::move,
> >     std::foo, std::bar into the gcc namespace too, no?  Are you really
> going
> >     to propose that?
> >
> >
> > No, I don't follow the logic of "if you do it for one thing you must do
> it for everything". That's a straw man. But I don't really mind how this
> gets done. Your suggestion is fine.
> >
>
> It isn't a strawman, Jon.  Maybe there's some miscommunication.  The
> conversion started (and part of it is
> still quoted above), by thinking about what we'd do once we get to C++14,
> and my suggestion to optimize
> for that.



Yeah, and I don't think optimizing for indentation is the right trade off.
Putting something in a namespace with a three-letter name just so you don't
have to re-indent some statements in a few years seems odd.

I see no technical difficulty replacing a custom make_unique (in any
namespace) with std::make_unique at a later date.

If a namespace made sense to avoid name clashes, or to enable finding
functions by ADL, or other technical reasons, I'd be all for it. But no
such rationale was given, and using a namespace for indentation just seems
odd to me.

But I really don't care. Putting it in the global namespace or a 'gcc'
namespace or anything else appropriate to GCC is fine. I'll leave this
thread now. I don't think this is very constructive and I'm sorry for
objecting to the suggestion.


  When we get to the point when we require C++14, make_unique is no longer
> different from any other
> symbol in the std namespace, and there will be no reason to treat it
> differently anymore.  Like, if someone at
> that point proposes to remove the global make_unique or gcc::make_unique,
> and replace all references with
> std::make_unique, there will be no real ground to object to that, why
> wouldn't you want it?  This is very
> much like when you removed "gnu::unique_ptr" (not going to miss it) a few
> months back -- you replaced
> it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of
> history.
>
> Cheers,
> Pedro Alves
>

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 7:50 p.m., Jonathan Wakely wrote:

> Yeah, and I don't think optimizing for indentation is the right trade off. Putting something in a namespace with a three-letter name just so you don't have to re-indent some statements in a few years seems odd.
> 
> I see no technical difficulty replacing a custom make_unique (in any namespace) with std::make_unique at a later date.
> 
> If a namespace made sense to avoid name clashes, or to enable finding functions by ADL, or other technical reasons, I'd be all for it. But no such rationale was given, and using a namespace for indentation just seems odd to me.
> 
> But I really don't care. Putting it in the global namespace or a 'gcc' namespace or anything else appropriate to GCC is fine. I'll leave this thread now. I don't think this is very constructive and I'm sorry for objecting to the suggestion.

That's fair.  Cheers!  Hope we'll be able to laugh about it in Prague!

Re: [PATCH 1/2] Add gcc/make-unique.h

[David Malcolm]

On Tue, 2022-07-12 at 17:40 +0100, Pedro Alves wrote:
> On 2022-07-12 4:14 p.m., Jonathan Wakely wrote:
> 
> > >  So once GCC requires C++14, why would you want to preserve

I look forward to the happy day when we can use C++14 in GCC's
implementation, but I don't see it happening anytime soon.

GCC's needs may differ from those of GDB's.  I'm not very familiar with
GDB's insides, but, for example, GCC has its own garbage-collector
which complicates everything to do with memory managements.

Right now I have comments expressing ownership of some pointers, and
e.g. "takes ownership of ...".  It would be wonderful to take some baby
steps into using C++11 to express the ownership directly in code.

> > > once-backported symbols in a namespace other than std, when you
> > > no longer have a reason to?
> > > It will just be another unnecessary thing that newcomers at that
> > > future time will have
> > > to learn.
> > 
> > I also don't see a problem with importing std::make_unique into
> > namespace gcc for local use alongside other things in namespace
> > gcc. I
> > do consider that idiomatic. It says "the make_unique for gcc code
> > is
> > std::make_unique". It means you only need a 'using namespace gcc;'
> > at
> > the top of a source file and you get access to everything in
> > namespace
> > gcc, even if it is something like std::make_unique that was
> > originally
> > defined in a different namespace.

Jonathan's idea sounds good to me.

> > 
> 
> If that's the approach, then GCC should import std::unique_ptr,
> std::move,
> std::foo, std::bar into the gcc namespace too, no?  Are you really
> going
> to propose that?

Pedro, it feels to me like you're constructing a strawman here. 
Neither me nor Jonathan are proposing that.

I just want to be able to comfortably use std::unique_ptr in GCC in the
places for which it makes sense, and being able to use "make_unique" is
a part of that.

Hope this is constructive
Dave

Re: [PATCH 1/2] Add gcc/make-unique.h

[Pedro Alves]

On 2022-07-12 7:36 p.m., David Malcolm wrote:
> On Tue, 2022-07-12 at 17:40 +0100, Pedro Alves wrote:

>>>
>>
>> If that's the approach, then GCC should import std::unique_ptr,
>> std::move,
>> std::foo, std::bar into the gcc namespace too, no?  Are you really
>> going
>> to propose that?
> 
> Pedro, it feels to me like you're constructing a strawman here. 
> Neither me nor Jonathan are proposing that.
> 

See my other reply.

I am actually appalled at how the conversion seems to have derailed.

> I just want to be able to comfortably use std::unique_ptr in GCC in the
> places for which it makes sense, and being able to use "make_unique" is
> a part of that.

My suggestion was simple:

wrap your make_unique in namespace gcc, so that later on when we get to
C++14, yanking it out causes as little churn as possible, with just a 3 letters
for 3 letters replacement.  I never thought this would be objectionable.

If you don't want to do that, that's fine, the churn will happen in the future,
but it's no big deal.  You'll get to live with shorter make_unique for a few years
(my bet is not forever).

> 
> Hope this is constructive
> Dave
> 
>