On Mon, Apr 16, 2018, 3:20 PM Alexandre Oliva <aol...@redhat.com> wrote:
> On Apr 16, 2018, Jason Merrill <ja...@redhat.com> wrote:
>
> > On Fri, Apr 13, 2018, 5:19 PM Alexandre Oliva <aol...@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?
>
This change looks good. One other nit: get_decl_maybe can also return a
list, so the name seems wrong. And this line
+ if (obj->list_p () && obj->get_decl_maybe ())
could be
if (tree_list_p ())
I think, and then get_decl_maybe wouldn't need to return a list anymore?
Jason
--
> 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
>
