Re: [C++ Patch] PR 81054 ("[7/8 Regression] ICE with volatile variable in constexpr function") [Take 2]

[Paolo Carlini <paolo dot carlini at oracle dot com>]

Hi,

On 17/01/2018 15:58, Jason Merrill wrote:
> On Tue, Jan 16, 2018 at 4:40 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> > On 16/01/2018 22:35, Jason Merrill wrote:
> > > On Tue, Jan 16, 2018 at 3:32 PM, Paolo Carlini <paolo.carlini@oracle.com>
> > > wrote:
> > > > thus I figured out what was badly wrong in my first try: I misread
> > > > ensure_literal_type_for_constexpr_object and missed that it can return
> > > > NULL_TREE without emitting an hard error. Thus my first try even caused
> > > > miscompilations :( Anyway, when DECL_DECLARED_CONSTEXPR_P is true we are
> > > > safe and indeed we want to clear it as matter of error recovery. Then, in
> > > > this safe case the only change in the below is returning early, thus
> > > > avoiding any internal inconsistencies later and also the redundant /
> > > > misleading diagnostic which I already mentioned.
> > > I can't see how this could be right.  In the cases where we don't give
> > > an error (e.g. because we're dealing with an instantiation of a
> > > variable template) there is no error, so we need to proceed with the
> > > rest of cp_finish_decl as normal.
> > The cases where we don't give an error all fall under
> > DECL_DECLARED_CONSTEXPR_P == false, thus aren't affected at all.
> Ah, true.  Though that's a bit subtle; maybe change ensure_... to
> return error_mark_node in the error case?
Agreed. The below does that and I'm finishing testing it (in libstdc++ 
at the moment, so far so good and I checked separately for those nasty 
breakages I had yesterday). Note, however, this isn't exactly equivalent 
to my previous patch: it's definitely cleaner and less subtle IMO but 
more aggressive because we immediately bail out of cp_finish_decl in all 
cases of error, not just when DECL_DECLARED_CONSTEXPR_P == true. Ok if 
it passes?

Thanks!
Paolo.

////////////////////////

Attachment: CL_81054_4
Description: Text document

Attachment: patch_81054_4
Description: Text document