On Tue, Apr 3, 2018 at 11:25 PM, Alexandre Oliva <aol...@redhat.com> wrote:
> On Apr 3, 2018, Alexandre Oliva <aol...@redhat.com> wrote:
>
>> On Apr 2, 2018, Jason Merrill <ja...@redhat.com> wrote:
>>> On Sat, Mar 31, 2018 at 7:12 AM, Alexandre Oliva <aol...@redhat.com> wrote:
>>>> struct a {
>>>> static int const z, i = __builtin_offsetof(struct b { int j; }, j);
>>>> b c;
>>>> };
>>>> int const a::z = __builtin_offsetof(struct d { int k; }, k);
>>>> d e;
>
>>>> Since d is visible, I suppose the most conservative solution would be to
>>>> name the global namespace as the context for type d, rather than
>>>> defining it as a member of a. Right?
>
>>> The global namespace would be a rather arbitrary choice; it seems to
>>> me that using the current scope is a natural interpretation.
>
>>> About d in the example, I'm not sure how you mean the global namespace
>>> is the current scope; we should be pushed into a when parsing the
>>> initializer for a::z.
>
>> I was just describing observed behavior. The code above compiles.
>
>> The explanation is in do_pushtag. It starts with a loop that, among
>> other things, skips COMPLETE_TYPE_P class scopes. we then record the
>> new type in the namespace named by the resulting scope. As the comment
>> says, this is meant to allow for types to be introduced in initializers
>> of static data members in spite of the class being already complete.
>
>> The problem, as I see it, is that we don't adjust the context to match,
>> so we introduce the type in one scope, but claim it to belong to
>> another. Which sort of works for named types, but comes down in flames
>> (err, reenters like Tiangong-1? ;-) if the type is anonymous.
>
>>> But I would think we should reject the definition of d because a is
>>> already complete, so it's too late to add members to it.
>
>> The existing code in GCC sort of disagrees with your proposal, so, for
>> the sake of avoiding breaking code that we used to compile (like the
>> definition 'd e' above), I'll insist on something along the lines of the
>> following patch:
>
> That patch breaks various cases of lambdas in data member initializers
> that reference members of the class containing the data member.
>
> This suggested to me that we already have infrastructure for and
> expectations of introducing types within a class after the class is
> complete. Using similar infrastructure to make types introduced within
> __builtin_offsetof, or even in type-casts under the extension I
> attempted to adjust in the proposed patch (the extension that Nathan
> mentioned he also ran into).
>
> I still think we could attempt to retain the extension as it is, parsing
> types introduced in data member initializers within the scope of the
> class containing the data member, like we do, but, when the class is
> already complete, recording it if not in TYPE_FIELDS, in some additional
> field consulted for name mangling purposes and, in order to retain
> compatibility, if the type is not a closure or anonymous, also recording
> it in the enclosing namespace, so that it is found by lookup as in the
> quoted snippet.
>
> Is that a terrible idea?
It sounds like a lot of work to support a very questionable pattern.
Perhaps we should just disallow defining a type in offsetof if the
current scope is a class.
Jason
