public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] Using std::unique_ptr and std::make_unique in our code
@ 2022-07-08 20:46 David Malcolm
  2022-07-08 21:15 ` Gabriel Ravier
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: David Malcolm @ 2022-07-08 20:46 UTC (permalink / raw)
  To: gcc

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.

Is there:
(a) an easy way to implement a std::make_unique replacement
    (e.g. in system.h? what to call it?), or
(b) some C++11-compatible way to do the same thing?
without accidentally bringing in C++14 features.

Thoughts?
Dave

---
 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                    | 37 ++++++++--------
 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              | 44 +++++++++++--------
 gcc/analyzer/region-model.h               | 31 +++++++-------
 gcc/analyzer/region.cc                    |  1 +
 gcc/analyzer/sm-file.cc                   | 11 +++--
 gcc/analyzer/sm-malloc.cc                 | 51 +++++++++++++----------
 gcc/analyzer/sm-pattern-test.cc           |  5 ++-
 gcc/analyzer/sm-sensitive.cc              |  5 ++-
 gcc/analyzer/sm-signal.cc                 |  5 ++-
 gcc/analyzer/sm-taint.cc                  | 24 +++++++----
 gcc/analyzer/sm.cc                        |  9 ++++
 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                   | 25 ++++++-----
 32 files changed, 194 insertions(+), 138 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..41d82f6e5dc 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, d->get_kind ()); // FIXME
   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..1e437de029f 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -118,35 +118,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 +411,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 +427,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 +445,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 +876,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 +1678,8 @@ 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 (std::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..aafe1130f1b 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.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"
@@ -814,14 +815,18 @@ 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
+		      (std::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
+		      (std::make_unique<shift_count_overflow_diagnostic>
+			 (assign,
+			  // FIXME: is this correct?
+			  int (TYPE_PRECISION (TREE_TYPE (rhs1))),
+			  rhs2_cst));
 		}
 	  }
 
@@ -1039,8 +1044,8 @@ 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 (std::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 +1234,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 (std::make_unique<dump_path_diagnostic> ());
 	  }
 	else if (is_special_named_call_p (call, "__analyzer_dump_region_model",
 					  0))
@@ -1763,9 +1768,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 std::make_unique<reason_attr_access>
+		(m_callee_fndecl, m_access);
 	    }
 	  private:
 	    tree m_callee_fndecl;
@@ -2573,7 +2579,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 (std::make_unique<poisoned_value_diagnostic>
+			      (ptr, pkind, NULL));
 	      }
 	  }
       }
@@ -2730,14 +2737,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 (std::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 (std::make_unique<write_to_const_diagnostic>
+		      (label_reg, label));
       }
       break;
     case RK_DECL:
@@ -2750,11 +2759,13 @@ 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 (std::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 (std::make_unique<write_to_string_literal_diagnostic>
+		    (dest_reg));
       break;
     }
 }
@@ -4683,9 +4694,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-file.cc b/gcc/analyzer/sm-file.cc
index f6cb29c7806..b54e5fd4269 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.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"
@@ -82,7 +83,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 +408,9 @@ 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));
+			       // C++14:
+			       std::make_unique<double_fclose>
+				(*this, diag_arg));
 		sm_ctxt->set_next_state (stmt, arg, m_stop);
 	      }
 	    return true;
@@ -474,10 +477,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 std::make_unique<file_leak> (*this, var);
 }
 
 } // anonymous namespace
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 553fcd80085..ea6e5b81613 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.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"
@@ -387,7 +388,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 +1730,9 @@ 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));
+					 // FIXME: C++14:
+					 std::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,9 @@ 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));
+					 // FIXME: C++14:
+					 std::make_unique<null_arg>
+					   (*this, diag_arg, callee_fndecl, i));
 			  sm_ctxt->set_next_state (stmt, arg, m_stop);
 			}
 		    }
@@ -1784,7 +1786,9 @@ 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));
+			     // FIXME: C++14:
+			     std::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 +1796,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_deref (*this, diag_arg));
+			     // FIXME: C++14:
+			     std::make_unique<null_deref> (*this, diag_arg));
 	      sm_ctxt->set_next_state (stmt, arg, m_stop);
 	    }
 	  else if (freed_p (state))
@@ -1800,8 +1805,9 @@ 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));
+			     // FIXME: C++14:
+			     std::make_unique<use_after_free>
+			       (*this, diag_arg, astate->m_deallocator));
 	      sm_ctxt->set_next_state (stmt, arg, m_stop);
 	    }
 	}
@@ -1853,8 +1859,9 @@ 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));
+		 // FIXME: C++14:
+		 std::make_unique<free_of_non_heap>
+		   (*this, diag_arg, freed_reg, d->m_name));
   sm_ctxt->set_next_state (call, arg, m_stop);
 }
 
@@ -1886,7 +1893,8 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt,
 	    = new mismatching_deallocation (*this, diag_arg,
 					    astate->m_deallocators,
 					    d);
-	  sm_ctxt->warn (node, call, arg, pd);
+	  sm_ctxt->warn (node, call, arg,
+			 std::unique_ptr<pending_diagnostic> (pd));
 	}
       sm_ctxt->set_next_state (call, arg, d->m_freed);
     }
@@ -1898,7 +1906,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));
+		     std::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 +1944,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,
+			 std::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 +1958,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"));
+		     std::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 +2040,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 std::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..061f79b57f1 100644
--- a/gcc/analyzer/sm-pattern-test.cc
+++ b/gcc/analyzer/sm-pattern-test.cc
@@ -21,6 +21,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"
@@ -143,8 +144,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,
+		     std::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..447d7047467 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -20,6 +20,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"
@@ -184,7 +185,9 @@ 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));
+		     // FIXME: C++14:
+		     std::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..0d559f309a1 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -21,6 +21,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"
@@ -357,8 +358,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));
+			     std::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 d2d03c3d602..2c2adebc5ff 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -21,6 +21,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"
@@ -811,7 +812,8 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
 	      {
 		tree diag_divisor = sm_ctxt->get_diagnostic_tree (divisor);
 		sm_ctxt->warn  (node, stmt, divisor,
-				new tainted_divisor (*this, diag_divisor, b));
+				std::make_unique<tainted_divisor>
+				  (*this, diag_divisor, b));
 		sm_ctxt->set_next_state (stmt, divisor, m_stop);
 	      }
 	  }
@@ -981,10 +983,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));
+			 std::make_unique<tainted_access_attrib_size>
+			   (*this, diag_size, b,
+			    callee_fndecl,
+			    access->sizarg,
+			    access_str));
 	}
     }
 }
@@ -1047,7 +1050,8 @@ 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 (std::make_unique<tainted_array_index>
+			    (taint_sm, arg, b));
 	    }
 	  }
 	  break;
@@ -1069,7 +1073,8 @@ 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 (std::make_unique<tainted_offset>
+			      (taint_sm, arg, b));
 	      }
 	  }
 	  break;
@@ -1094,7 +1099,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 (std::make_unique<tainted_size> (taint_sm, arg, b));
 	      }
 	  }
 	  break;
@@ -1140,7 +1145,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 (std::make_unique<tainted_allocation_size>
+		    (taint_sm, arg, b, mem_space));
     }
 }
 
diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
index 24c20b894cd..dab2ace3368 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"
@@ -40,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/program-point.h"
 #include "analyzer/store.h"
 #include "analyzer/svalue.h"
+#include "analyzer/pending-diagnostic.h"
 
 #if ENABLE_ANALYZER
 
@@ -122,6 +124,13 @@ state_machine::get_state_by_name (const char *name) const
   gcc_unreachable ();
 }
 
+/* FIXME.  */
+std::unique_ptr<pending_diagnostic>
+state_machine::on_leak (tree var ATTRIBUTE_UNUSED) const
+{
+  return std::unique_ptr<pending_diagnostic> (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 e80ef1fac37..06252c8bcce 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..a2999980351 100644
--- a/gcc/analyzer/varargs.cc
+++ b/gcc/analyzer/varargs.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"
@@ -217,7 +218,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 +559,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));
+		   std::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 +635,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 std::make_unique<va_list_leak> (*this, NULL, var);
 }
 
 } // anonymous namespace
@@ -989,17 +990,21 @@ 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));
+			// FIXME: C++14:
+			ctxt->warn (std::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));
+		    // FIXME: C++14:
+		    ctxt->warn (std::make_unique <va_list_exhausted>
+				  (va_list_tree, arg_reg));
 		  saw_problem = true;
 		}
 	    }
-- 
2.26.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-08 20:46 [RFC] Using std::unique_ptr and std::make_unique in our code David Malcolm
@ 2022-07-08 21:15 ` Gabriel Ravier
  2022-07-08 21:16 ` Jonathan Wakely
  2022-07-11 10:56 ` Pedro Alves
  2 siblings, 0 replies; 10+ messages in thread
From: Gabriel Ravier @ 2022-07-08 21:15 UTC (permalink / raw)
  To: David Malcolm, gcc

On 7/8/22 22:46, David Malcolm via Gcc 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.
>
> Is there:
> (a) an easy way to implement a std::make_unique replacement
>      (e.g. in system.h? what to call it?), or
> (b) some C++11-compatible way to do the same thing?
> without accidentally bringing in C++14 features.
>
> Thoughts?
> Dave
>
> ---
>   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                    | 37 ++++++++--------
>   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              | 44 +++++++++++--------
>   gcc/analyzer/region-model.h               | 31 +++++++-------
>   gcc/analyzer/region.cc                    |  1 +
>   gcc/analyzer/sm-file.cc                   | 11 +++--
>   gcc/analyzer/sm-malloc.cc                 | 51 +++++++++++++----------
>   gcc/analyzer/sm-pattern-test.cc           |  5 ++-
>   gcc/analyzer/sm-sensitive.cc              |  5 ++-
>   gcc/analyzer/sm-signal.cc                 |  5 ++-
>   gcc/analyzer/sm-taint.cc                  | 24 +++++++----
>   gcc/analyzer/sm.cc                        |  9 ++++
>   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                   | 25 ++++++-----
>   32 files changed, 194 insertions(+), 138 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..41d82f6e5dc 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, d->get_kind ()); // FIXME
>     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..1e437de029f 100644
> --- a/gcc/analyzer/engine.cc
> +++ b/gcc/analyzer/engine.cc
> @@ -118,35 +118,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 +411,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 +427,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 +445,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 +876,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 +1678,8 @@ 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 (std::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..aafe1130f1b 100644
> --- a/gcc/analyzer/region-model.cc
> +++ b/gcc/analyzer/region-model.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"
> @@ -814,14 +815,18 @@ 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
> +		      (std::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
> +		      (std::make_unique<shift_count_overflow_diagnostic>
> +			 (assign,
> +			  // FIXME: is this correct?
> +			  int (TYPE_PRECISION (TREE_TYPE (rhs1))),
> +			  rhs2_cst));
>   		}
>   	  }
>   
> @@ -1039,8 +1044,8 @@ 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 (std::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 +1234,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 (std::make_unique<dump_path_diagnostic> ());
>   	  }
>   	else if (is_special_named_call_p (call, "__analyzer_dump_region_model",
>   					  0))
> @@ -1763,9 +1768,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 std::make_unique<reason_attr_access>
> +		(m_callee_fndecl, m_access);
>   	    }
>   	  private:
>   	    tree m_callee_fndecl;
> @@ -2573,7 +2579,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 (std::make_unique<poisoned_value_diagnostic>
> +			      (ptr, pkind, NULL));
>   	      }
>   	  }
>         }
> @@ -2730,14 +2737,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 (std::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 (std::make_unique<write_to_const_diagnostic>
> +		      (label_reg, label));
>         }
>         break;
>       case RK_DECL:
> @@ -2750,11 +2759,13 @@ 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 (std::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 (std::make_unique<write_to_string_literal_diagnostic>
> +		    (dest_reg));
>         break;
>       }
>   }
> @@ -4683,9 +4694,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-file.cc b/gcc/analyzer/sm-file.cc
> index f6cb29c7806..b54e5fd4269 100644
> --- a/gcc/analyzer/sm-file.cc
> +++ b/gcc/analyzer/sm-file.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"
> @@ -82,7 +83,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 +408,9 @@ 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));
> +			       // C++14:
> +			       std::make_unique<double_fclose>
> +				(*this, diag_arg));
>   		sm_ctxt->set_next_state (stmt, arg, m_stop);
>   	      }
>   	    return true;
> @@ -474,10 +477,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 std::make_unique<file_leak> (*this, var);
>   }
>   
>   } // anonymous namespace
> diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
> index 553fcd80085..ea6e5b81613 100644
> --- a/gcc/analyzer/sm-malloc.cc
> +++ b/gcc/analyzer/sm-malloc.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"
> @@ -387,7 +388,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 +1730,9 @@ 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));
> +					 // FIXME: C++14:
> +					 std::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,9 @@ 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));
> +					 // FIXME: C++14:
> +					 std::make_unique<null_arg>
> +					   (*this, diag_arg, callee_fndecl, i));
>   			  sm_ctxt->set_next_state (stmt, arg, m_stop);
>   			}
>   		    }
> @@ -1784,7 +1786,9 @@ 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));
> +			     // FIXME: C++14:
> +			     std::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 +1796,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_deref (*this, diag_arg));
> +			     // FIXME: C++14:
> +			     std::make_unique<null_deref> (*this, diag_arg));
>   	      sm_ctxt->set_next_state (stmt, arg, m_stop);
>   	    }
>   	  else if (freed_p (state))
> @@ -1800,8 +1805,9 @@ 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));
> +			     // FIXME: C++14:
> +			     std::make_unique<use_after_free>
> +			       (*this, diag_arg, astate->m_deallocator));
>   	      sm_ctxt->set_next_state (stmt, arg, m_stop);
>   	    }
>   	}
> @@ -1853,8 +1859,9 @@ 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));
> +		 // FIXME: C++14:
> +		 std::make_unique<free_of_non_heap>
> +		   (*this, diag_arg, freed_reg, d->m_name));
>     sm_ctxt->set_next_state (call, arg, m_stop);
>   }
>   
> @@ -1886,7 +1893,8 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt,
>   	    = new mismatching_deallocation (*this, diag_arg,
>   					    astate->m_deallocators,
>   					    d);
> -	  sm_ctxt->warn (node, call, arg, pd);
> +	  sm_ctxt->warn (node, call, arg,
> +			 std::unique_ptr<pending_diagnostic> (pd));
>   	}
>         sm_ctxt->set_next_state (call, arg, d->m_freed);
>       }
> @@ -1898,7 +1906,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));
> +		     std::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 +1944,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,
> +			 std::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 +1958,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"));
> +		     std::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 +2040,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 std::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..061f79b57f1 100644
> --- a/gcc/analyzer/sm-pattern-test.cc
> +++ b/gcc/analyzer/sm-pattern-test.cc
> @@ -21,6 +21,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"
> @@ -143,8 +144,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,
> +		     std::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..447d7047467 100644
> --- a/gcc/analyzer/sm-sensitive.cc
> +++ b/gcc/analyzer/sm-sensitive.cc
> @@ -20,6 +20,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"
> @@ -184,7 +185,9 @@ 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));
> +		     // FIXME: C++14:
> +		     std::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..0d559f309a1 100644
> --- a/gcc/analyzer/sm-signal.cc
> +++ b/gcc/analyzer/sm-signal.cc
> @@ -21,6 +21,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"
> @@ -357,8 +358,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));
> +			     std::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 d2d03c3d602..2c2adebc5ff 100644
> --- a/gcc/analyzer/sm-taint.cc
> +++ b/gcc/analyzer/sm-taint.cc
> @@ -21,6 +21,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"
> @@ -811,7 +812,8 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
>   	      {
>   		tree diag_divisor = sm_ctxt->get_diagnostic_tree (divisor);
>   		sm_ctxt->warn  (node, stmt, divisor,
> -				new tainted_divisor (*this, diag_divisor, b));
> +				std::make_unique<tainted_divisor>
> +				  (*this, diag_divisor, b));
>   		sm_ctxt->set_next_state (stmt, divisor, m_stop);
>   	      }
>   	  }
> @@ -981,10 +983,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));
> +			 std::make_unique<tainted_access_attrib_size>
> +			   (*this, diag_size, b,
> +			    callee_fndecl,
> +			    access->sizarg,
> +			    access_str));
>   	}
>       }
>   }
> @@ -1047,7 +1050,8 @@ 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 (std::make_unique<tainted_array_index>
> +			    (taint_sm, arg, b));
>   	    }
>   	  }
>   	  break;
> @@ -1069,7 +1073,8 @@ 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 (std::make_unique<tainted_offset>
> +			      (taint_sm, arg, b));
>   	      }
>   	  }
>   	  break;
> @@ -1094,7 +1099,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 (std::make_unique<tainted_size> (taint_sm, arg, b));
>   	      }
>   	  }
>   	  break;
> @@ -1140,7 +1145,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 (std::make_unique<tainted_allocation_size>
> +		    (taint_sm, arg, b, mem_space));
>       }
>   }
>   
> diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
> index 24c20b894cd..dab2ace3368 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"
> @@ -40,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "analyzer/program-point.h"
>   #include "analyzer/store.h"
>   #include "analyzer/svalue.h"
> +#include "analyzer/pending-diagnostic.h"
>   
>   #if ENABLE_ANALYZER
>   
> @@ -122,6 +124,13 @@ state_machine::get_state_by_name (const char *name) const
>     gcc_unreachable ();
>   }
>   
> +/* FIXME.  */
> +std::unique_ptr<pending_diagnostic>
> +state_machine::on_leak (tree var ATTRIBUTE_UNUSED) const
> +{
> +  return std::unique_ptr<pending_diagnostic> (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 e80ef1fac37..06252c8bcce 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..a2999980351 100644
> --- a/gcc/analyzer/varargs.cc
> +++ b/gcc/analyzer/varargs.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"
> @@ -217,7 +218,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 +559,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));
> +		   std::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 +635,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 std::make_unique<va_list_leak> (*this, NULL, var);
>   }
>   
>   } // anonymous namespace
> @@ -989,17 +990,21 @@ 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));
> +			// FIXME: C++14:
> +			ctxt->warn (std::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));
> +		    // FIXME: C++14:
> +		    ctxt->warn (std::make_unique <va_list_exhausted>
> +				  (va_list_tree, arg_reg));
>   		  saw_problem = true;
>   		}
>   	    }

Provided there are no licensing issues, an implementation could be taken 
from:

- The paper proposing the addition of make_unique to C++14: 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3588.htm

- cppreference's example implementation: 
https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique

- A StackOverflow answer on this subject: 
https://stackoverflow.com/questions/17902405/how-to-implement-make-unique-function-in-c11

Otherwise it also seems simple enough to pick out the implementation 
from libstdc++ (over at libstdc++-v3/include/bits/unique_ptr.h, as of 
right now between approximately line 1034 and line 1096)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-08 20:46 [RFC] Using std::unique_ptr and std::make_unique in our code David Malcolm
  2022-07-08 21:15 ` Gabriel Ravier
@ 2022-07-08 21:16 ` Jonathan Wakely
  2022-07-11 10:56 ` Pedro Alves
  2 siblings, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2022-07-08 21:16 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

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.



> 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.


> (b) some C++11-compatible way to do the same thing?
> without accidentally bringing in C++14 features.

std::make_unique doesn't use any C++14 features.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-08 20:46 [RFC] Using std::unique_ptr and std::make_unique in our code David Malcolm
  2022-07-08 21:15 ` Gabriel Ravier
  2022-07-08 21:16 ` Jonathan Wakely
@ 2022-07-11 10:56 ` Pedro Alves
  2022-07-12  0:32   ` David Malcolm
  2022-07-12 10:21   ` Florian Weimer
  2 siblings, 2 replies; 10+ messages in thread
From: Pedro Alves @ 2022-07-11 10:56 UTC (permalink / raw)
  To: David Malcolm, gcc

Hi!

On 2022-07-08 9:46 p.m., David Malcolm via Gcc wrote:
> -				    pending_diagnostic *d,
> +				    std::unique_ptr<pending_diagnostic> d,

I see that you didn't add any typedef for std::unique_ptr<foo> in this patch.  It will be
inevitable that people will start adding them, for conciseness, IME, though.  To avoid diverging
naming styles for such typedefs in the codebase, GDB settled on using the "_up" suffix (for Unique Pointer)
quite early in the C++11 conversion, and we use such typedefs pervasively nowadays.  For example, for the type
above, we'd have:

  typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;

and then:

 -				    pending_diagnostic *d,
 +				    pending_diagnostic_up d,

I would suggest GCC have a similar guideline, before people start using foo_ptr,
bar_unp, quux_p, whatnot diverging styles.

And it would be nice if GCC followed the same nomenclature style as GDB, so we could
have one single guideline for the whole GNU toolchain, so people moving between codebases
only had to learn one guideline.

Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-11 10:56 ` Pedro Alves
@ 2022-07-12  0:32   ` David Malcolm
  2022-08-10  1:15     ` James K. Lowden
  2022-07-12 10:21   ` Florian Weimer
  1 sibling, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-07-12  0:32 UTC (permalink / raw)
  To: Pedro Alves, gcc

On Mon, 2022-07-11 at 11:56 +0100, Pedro Alves wrote:
> Hi!
> 
> On 2022-07-08 9:46 p.m., David Malcolm via Gcc wrote:
> > -                                   pending_diagnostic *d,
> > +                                  
> > std::unique_ptr<pending_diagnostic> d,
> 
> I see that you didn't add any typedef for std::unique_ptr<foo> in
> this patch.  It will be
> inevitable that people will start adding them, for conciseness, IME,
> though. 

Perhaps, but right now I prefer to spell out std::unique_ptr<T>, since
I'm not as comfortable with C++11 as I might be.


>  To avoid diverging
> naming styles for such typedefs in the codebase, GDB settled on using
> the "_up" suffix (for Unique Pointer)
> quite early in the C++11 conversion, and we use such typedefs
> pervasively nowadays.  For example, for the type
> above, we'd have:
> 
>   typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;
> 
> and then:
> 
>  -                                  pending_diagnostic *d,
>  +                                  pending_diagnostic_up d,
> 
> I would suggest GCC have a similar guideline, before people start
> using foo_ptr,
> bar_unp, quux_p, whatnot diverging styles.

Thanks for the info.  I suspect the gdb community is much more
comfortable with C++ (and C++11) than the gcc community.

The recommendation sounds reasonable for if/when we start adding such
typedefs, but, as I said, for now I think I want to spell out
std::unique_ptr<T> in the few places I'm using it.

Hope this makes sense, and these are just my opinions, of course
Dave


> 
> And it would be nice if GCC followed the same nomenclature style as
> GDB, so we could
> have one single guideline for the whole GNU toolchain, so people
> moving between codebases
> only had to learn one guideline.
> 
> Pedro Alves
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-11 10:56 ` Pedro Alves
  2022-07-12  0:32   ` David Malcolm
@ 2022-07-12 10:21   ` Florian Weimer
  2022-07-12 10:45     ` Jonathan Wakely
  2022-07-12 11:00     ` Pedro Alves
  1 sibling, 2 replies; 10+ messages in thread
From: Florian Weimer @ 2022-07-12 10:21 UTC (permalink / raw)
  To: Pedro Alves; +Cc: David Malcolm, gcc

* Pedro Alves:

> For example, for the type above, we'd have:
>
>   typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;
>
> and then:
>
>  -				    pending_diagnostic *d,
>  +				    pending_diagnostic_up d,
>
> I would suggest GCC have a similar guideline, before people start
> using foo_ptr, bar_unp, quux_p, whatnot diverging styles.

This doesn't seem to provide much benefit over writing

  uP<pending_diagnostic> d;

and with that construct, you don't need to worry about the actual
relationship between pending_diagnostic and pending_diagnostic_up.

I think the GDB situation is different because many of the types do not
have proper destructors, so std::unique_ptr needs a custom deleter.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-12 10:21   ` Florian Weimer
@ 2022-07-12 10:45     ` Jonathan Wakely
  2022-07-12 11:01       ` Pedro Alves
  2022-07-12 11:00     ` Pedro Alves
  1 sibling, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2022-07-12 10:45 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Pedro Alves, gcc

On Tue, 12 Jul 2022 at 11:22, Florian Weimer via Gcc <gcc@gcc.gnu.org> wrote:
>
> * Pedro Alves:
>
> > For example, for the type above, we'd have:
> >
> >   typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;
> >
> > and then:
> >
> >  -                                pending_diagnostic *d,
> >  +                                pending_diagnostic_up d,
> >
> > I would suggest GCC have a similar guideline, before people start
> > using foo_ptr, bar_unp, quux_p, whatnot diverging styles.
>
> This doesn't seem to provide much benefit over writing
>
>   uP<pending_diagnostic> d;
>
> and with that construct, you don't need to worry about the actual
> relationship between pending_diagnostic and pending_diagnostic_up.
>
> I think the GDB situation is different because many of the types do not
> have proper destructors, so std::unique_ptr needs a custom deleter.


A fairly common idiom is for the type to define the typedef itself:

struct pending_diagnostic {
  using ptr = std::unique_ptr<pending_diagnostic>;
  // ...
};

Then you use pending_diagnostic::ptr. If you want a custom deleter for
the type, you add it to the typedef.

Use a more descriptive name like uptr or uniq_ptr instead of "ptr" if
you prefer.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-12 10:21   ` Florian Weimer
  2022-07-12 10:45     ` Jonathan Wakely
@ 2022-07-12 11:00     ` Pedro Alves
  1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2022-07-12 11:00 UTC (permalink / raw)
  To: Florian Weimer; +Cc: David Malcolm, gcc

On 2022-07-12 11:21 a.m., Florian Weimer wrote:
> * Pedro Alves:
> 
>> For example, for the type above, we'd have:
>>
>>   typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;
>>
>> and then:
>>
>>  -				    pending_diagnostic *d,
>>  +				    pending_diagnostic_up d,
>>
>> I would suggest GCC have a similar guideline, before people start
>> using foo_ptr, bar_unp, quux_p, whatnot diverging styles.
> 
> This doesn't seem to provide much benefit over writing
> 
>   uP<pending_diagnostic> d;
> 
> and with that construct, you don't need to worry about the actual
> relationship between pending_diagnostic and pending_diagnostic_up.

Given the guideline, nobody ever worries about that.  When you see "_up",
you just know it's a unique pointer.

And as you point out, there's the custom deleters case to consider too.

> 
> I think the GDB situation is different because many of the types do not
> have proper destructors, so std::unique_ptr needs a custom deleter.

Yes, there are a few cases like but it's not "many" as you suggest,
and most are types for which we have no control, like 3rd party library
types, like debuginfod, curses, python.  Most of the rest of the custom deleter cases
are instead because of intrusive refcounting.  I.e., the deleter decrements the
object's refcount, instead of deleting the object straight away.

These are valid cases, not "GDB is doing it wrong, so GCC won't have to
bother".  I would suspect that GCC will end up with a good number of
custom deleters as well.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-12 10:45     ` Jonathan Wakely
@ 2022-07-12 11:01       ` Pedro Alves
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2022-07-12 11:01 UTC (permalink / raw)
  To: Jonathan Wakely, Florian Weimer; +Cc: gcc

On 2022-07-12 11:45 a.m., Jonathan Wakely wrote:
> On Tue, 12 Jul 2022 at 11:22, Florian Weimer via Gcc <gcc@gcc.gnu.org> wrote:
>>
>> * Pedro Alves:
>>
>>> For example, for the type above, we'd have:
>>>
>>>   typedef std::unique_ptr<pending_diagnostic> pending_diagnostic_up;
>>>
>>> and then:
>>>
>>>  -                                pending_diagnostic *d,
>>>  +                                pending_diagnostic_up d,
>>>
>>> I would suggest GCC have a similar guideline, before people start
>>> using foo_ptr, bar_unp, quux_p, whatnot diverging styles.
>>
>> This doesn't seem to provide much benefit over writing
>>
>>   uP<pending_diagnostic> d;
>>
>> and with that construct, you don't need to worry about the actual
>> relationship between pending_diagnostic and pending_diagnostic_up.
>>
>> I think the GDB situation is different because many of the types do not
>> have proper destructors, so std::unique_ptr needs a custom deleter.
> 
> 
> A fairly common idiom is for the type to define the typedef itself:
> 
> struct pending_diagnostic {
>   using ptr = std::unique_ptr<pending_diagnostic>;
>   // ...
> };
> 
> Then you use pending_diagnostic::ptr. If you want a custom deleter for
> the type, you add it to the typedef.
> 
> Use a more descriptive name like uptr or uniq_ptr instead of "ptr" if
> you prefer.
> 

Only works if you can change the type, though.  Sometimes you can't,
as it comes from a library.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] Using std::unique_ptr and std::make_unique in our code
  2022-07-12  0:32   ` David Malcolm
@ 2022-08-10  1:15     ` James K. Lowden
  0 siblings, 0 replies; 10+ messages in thread
From: James K. Lowden @ 2022-08-10  1:15 UTC (permalink / raw)
  To: David Malcolm via Gcc

On Mon, 11 Jul 2022 20:32:07 -0400
David Malcolm via Gcc <gcc@gcc.gnu.org> wrote:

> Perhaps, but right now I prefer to spell out std::unique_ptr<T>, since
> I'm not as comfortable with C++11 as I might be.

Hi David, [off list]

You might be interested to know Bjarne Stroustrup observes that during
the development of C++, new features tend to be introduced with long,
explicit syntax that no one can misinterpret.  Then, over time, as the
community becomes comfortable with the new idea (and tired of typing
it) it gets briefer.  

	"The prefix template<...> syntax was not my first choice when I
designed templates. It was forced upon me by people worried that
templates would be misused by less competent programmers, leading to
confusion and errors. The heavy syntax for exception handling, try
{ ... } catch( ... ) { ... }, was a similar story [Stroustrup 2007]. It
seems that for every new feature many people demand a LOUD syntax to
protect against real and imagined potential problems. After a while,
they then complain about verbosity."

https://dl.acm.org/doi/pdf/10.1145/3386320?utm_source=ZHShareTargetIDMore&utm_medium=social&utm_oi=54584470929408

At the very least, you have company.  :-)  

Regards, 

--jkl

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-10  1:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-08 20:46 [RFC] Using std::unique_ptr and std::make_unique in our code David Malcolm
2022-07-08 21:15 ` Gabriel Ravier
2022-07-08 21:16 ` Jonathan Wakely
2022-07-11 10:56 ` Pedro Alves
2022-07-12  0:32   ` David Malcolm
2022-08-10  1:15     ` James K. Lowden
2022-07-12 10:21   ` Florian Weimer
2022-07-12 10:45     ` Jonathan Wakely
2022-07-12 11:01       ` Pedro Alves
2022-07-12 11:00     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).