Hi again,
thus today I had to revert my first try at resolving this error recovery
issue because it caused c++/71465. In retrospect it was a bit heavy
handed anyway, the zeroing of the type served us well for many years...
Today I tried to investigate the issue a bit more. I remind you that we
are crashing on the gcc_unreachable () at the end of
build_simple_base_path while, during error recovery, we are processing
the initialization of the final 'a' in:
class A
{
virtual void foo () { }
};
class B : public A, A { }; // { dg-error "duplicate base type" }
B b1, &b2 = b1;
A a = b2;
Now the first comment I have about this, about which I'd like to have
some feedback, is that the gcc_unreachable () at the end of
build_simple_base_path without a return seems a bit weird to me, because
the function is *supposed to return a tree*, not void. I guess some
front-ends would even warn on this?!? Anyway, if we change:
/* */
gcc_unreachable ();
}
to:
/* */
gcc_assert (seen_error ());
return error_mark_node;
}
we get something which, by and large is equally safe in terms of
miscompilations and helps a lot error recovery, ie, we would not have
c++/70202. It's also true however that we would not make much progress
in trying to understand if we could solve the specific problem at issue
in a more insightful way.
Then my next observation is the following: if we change the testcase to:
class A
{
virtual void foo () { }
};
class B : public A, A { }; // { dg-error "duplicate base type" }
B b1, &b2 = b1;
A a = b1;
thus everything is the same but at the end we have 'b1', not 'b2', we
also don't crash. We are saved not by something particularly smart ;)
but by this check at the beginning of ocp_convert:
if (error_operand_p (e) || type == error_mark_node)
return error_mark_node;
that is, the TREE_TYPE of 'e', the VAR_DECL for 'b1', is error_mark_node
and we return early the error_mark_node, we don't reach the problematic
build_base_path. With 'b2' we have a more complicated REFERENCE_REF
which we don't catch, but otherwise everything is the same, we reach
build_base_path and we crash on the gcc_unreachable. In practice, if I
add at the beginning of ocp_convert:
+ if (REFERENCE_REF_P (expr)
+ && VAR_P (TREE_OPERAND (expr, 0))
+ && DECL_INITIAL (TREE_OPERAND (expr, 0)) == error_mark_node)
+ return error_mark_node;
it also passes testing. Then the final observation is that we also don't
crash if the type of 'A' is simpler, thus we don't have the declaration of
virtual void foo () { }
Then I gather that when the type is "complex" enough we are not cleaning
things up completely enough when we encounter it as a duplicated base.
Maybe as optimal error recovery when we see a duplicated base we should
clean up everything having to do with bases end up with something
equivalent to an incomplete class B; or something like that?!? I don't
know what we want to try short / medium /long term...
Thanks!
Paolo.
