This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [PR c++/80290] recycle tinst garbage sooner


On Apr 16, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Fri, Apr 13, 2018, 5:19 PM Alexandre Oliva <aoliva@redhat.com> wrote:
>> +  tree get_node () const {
>> +    if (!split_list_p ()) return tldcl;
>> +    else return const_cast <tinst_level *>(this)->to_list ();
>> +  }

> This looks like a dangerous violation of const correctness, since it does
> in fact modify the object.

Well...  We know the object's actual type is not const, since we
allocate all objects of this type from GCable memory.  Conceptually, the
object is not modified, we're just lazily performing an expensive
rearrangement of the internal representation that we would have done
upfront if we weren't trying to save that expense.  This would be a
perfect case for mutable, if only gengtype didn't get confused by it.


OTOH, we could drop a tiny amount of constification in error.c and avoid
the problem altogether, as in the following incremental patch:

 gcc/cp/cp-tree.h |    4 ++--
 gcc/cp/error.c   |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e9d9bab879bc..26a50ac136dd 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5909,9 +5909,9 @@ struct GTY((chain_next ("%h.next"))) tinst_level {
   /* Return the original node; if it's a split list, make it a
      TREE_LIST first, so that it can be returned as a single tree
      object.  */
-  tree get_node () const {
+  tree get_node () {
     if (!split_list_p ()) return tldcl;
-    else return const_cast <tinst_level *>(this)->to_list ();
+    else return to_list ();
   }
 
   /* Return the original node if it's a DECL or a TREE_LIST, but do
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 983ffdfedbb2..95b8b84f34ab 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3475,7 +3475,7 @@ print_instantiation_full_context (diagnostic_context *context)
 
 static void
 print_instantiation_partial_context_line (diagnostic_context *context,
-					  const struct tinst_level *t,
+					  struct tinst_level *t,
 					  location_t loc, bool recursive_p)
 {
   if (loc == UNKNOWN_LOCATION)


Does this incremental change make it ok (pending retesting), or should I
look into adding mutable support to gengtype, or something else?

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]