On 12/20/18 6:50 PM, Martin Sebor wrote: > On 12/20/18 10:46 AM, Martin Sebor wrote: >> On 12/17/18 7:58 AM, Jason Merrill wrote: >>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>> this patch implements an error message, for non-static initialization of a >>>> flexible array member. >>>> This duplicates the existing error message from the C-FE, to avoid ICE and >>>> wrong code generation >>>> issues, as pointed out in the PR. >>>> >>>> It is a bit funny that a non-functional feature like that has already >>>> rather much test coverage. >>>> The most easy adjustment seems to change the existing test cases to use >>>> static declarations. >>> >>> Martin, thoughts? >> >> Our high-level goal when tightening up how flexible array members >> are handled in C++ was to accept what's accepted in standard C mode >> and reject (or, at a minimum, warn for) C++ extensions that could >> be relied on in existing code. > > I meant "reject what couldn't be relied on" and "warn for that could > be." >
I believe the problem here is effectively that initializing non-static flexible array is not supported by the middle-end. All examples where flexible array members are initialized on automatic variable work only as long as they are simple enough that they are optimized away so that they do not survive until expansion. Take as example gcc/testsuite/g++.dg/ext/flexary13.C, it compiles and runs successfully, but the assertions start to fail if Ax is declared volatile, and at the same time, we know that the automatic variables are allocated in a way that they can overlap and crash at any time. My impression is that the existing C error made the middle-end kind of rely on this behavior. So I think the right thing to do is duplicate the existing C error in the C++ FE. I do not see any automatic variable with initialized flexible data members where it would be safe to only warn about them. > (Sorry for the delay, by the way. I've been migrating to a new machine > this week and things aren't yet working quite like I'm used to.) > >> >> The flexarray tests I added back then were for features that looked >> like intentional extensions and that seemed to work for at least >> some use cases as far as I could tell. What I noticed didn't work >> I created bugs for: 69338, 69696, and 69338 look related, but there >> are others. >> >> I think all these bugs should all be reviewed and a decision made >> about what's intended to work and what continues to be accepted as >> an accident and should be rejected. After that, we can adjust >> the existing tests. >> I would not rule out the possibility that there can be more bugs. But I think the existing tests need to avoid the case which evokes the new error. The question is, if changing from automatic to static objects prevents those tests to test what they were originally written for. I believe this is not the case, but I do probably not know all the background here. Thanks Bernd.
