On Mon, Jan 23, 2017 at 7:21 PM, Alexandre Oliva <aol...@redhat.com> wrote:
> On Jan 23, 2017, Jason Merrill <ja...@redhat.com> wrote:
>
>>>>> + If the newly-created namespace is to be an inline namespace, after
>>>>> + push_namespace, get the nested namespace decl with
>>>>> + get_current_binding_level, pop back to the enclosing namespace,
>>>>> + call using_namespace with INLINE_P, and then push to the inline
>>>>> + namespace again. */
>>>
>>>> This seems like unnecessary contortions to expect of the caller
>>>> (i.e. GDB). Let's add a new front end function to handle this.
>>>
>>> Heh, it's really nothing compared with some other contortions required
>>> to navigate binding levels, e.g. to reenter a function scope.
>>>
>>> I didn't want to add redundant functionality to the libcc1 API, and we'd
>>> need the ability to separate the introduction or reentry in a namespace,
>>> from a using directive with attribute strong. And since inline
>>> namespaces use the same implementation as that, I figured it made sense
>>> to use the same interface for both.
>
>>> Besides, push_namespace is called a lot more often than using_namespace,
>>> so it makes sense to keep it simpler, and leave the extra parameter for
>>> the less common operation.
>>>
>>> Another argument to keep things as they are is that the inline-ness of a
>>> namespace is not a property related with entering the namespace, but
>>> rather with how names from it are brought into the other namespace.
>
>> Well, it's a property of the namespace.
>
> Even in the case of 'strong' using directives? I didn't think so.
'strong' using directives should really go away, they were a brief
stop on the way to inline namespaces. We really shouldn't introduce
more functionality based on them. I'll add a deprecated warning
now...
>>>>> +/* Pop the namespace last entered with push_namespace, or class last
>>>>> + entered with push_class, or function last entered with
>>>>> + push_function, restoring the binding level in effect before the
>>>>> + matching push_* call. */
>>>>> +
>>>>> +GCC_METHOD0 (int /* bool */, pop_namespace)
>>>
>>>> This should have a different name, perhaps pop_last_entered_scope?
>>>
>>> It was introduced very early on, long before the need for exposing
>>> function scopes was realized and implemented. GDB (branch) already had
>>> plenty of code using this primitive, so I didn't want to rename it, but
>>> if you insist in such spelling changes, I won't mind, and I guess GDB
>>> folks won't even. It's not too late yet ;-)
>
>> Please.
>
> How about just pop_binding_level, to align it with the nomenclature used
> in the comments? (I'd have suggested just pop_scope otherwise)
Sure.
>>>>> + The TARGET decl names the qualifying scope (foo:: above) and the
>>>>> + identifier (bar), but that does not mean that only TARGET will be
>>>>> + brought into the current scope: all bindings of TARGET's identifier
>>>>> + in the qualifying scope will be brought in.
>>>
>>>> This seems wrong; for namespace-scope using-declarations, only the
>>>> declarations in scope at the point of the using-declaration are
>>>> imported. Since DWARF represents each imported declaration
>>>> individually, I would think that we would want the plugin to import
>>>> them one at a time.
>>>
>>> What you say is true, but GCC doesn't offer functionality for that
>>> AFAICT, and the caller can always arrange for only the bindings that
>>> should be brought in by the using declarations to be defined at the time
>>> the using declaration is introduced, so I left it at that so as to
>>> minimize the changes to GCC proper.
>
>> Fair enough, but then why pass a decl? Passing scope and identifier
>> would seem cleaner if that's what you mean.
>
> Oh, hysterical raisins IIRC. 'scope' was not so easy to pass. The API
> makes gcc_decl and gcc_type distinct, unrelated types and, although
> clients would get gcc_decls for functions and (when needed) namespaces,
> they'd only get gcc_types for classes. Later on, start_class_definition
> was split so as to make forward declarations possible, and clients now
> necessarily get a gcc_decl for a class. (IIRC there weren't
> get_current_binding_level or type_decl either at that point)
>
> Now that the decl for a class is easily available, it makes some sense
> ot change the API to take a decl to specify the scope, but then a string
> might not be enough to name a member: operators are kind of a separate
> namespace, and we'd have to add a flag or some other means to denote
> them. It can be done, but it feels like a lot of pain for the debatable
> benefit of introducing additional possibilities of erroneous uses, such
> as passing invalid contexts and whatnot (besides the minor inconvenience
> of having to convert classes from decls to types, as DECL_CONTEXT
> wants).
>
> OTOH, passing a context explicitly, even if optional, and using a decl
> to convey name (sidestepping the operator name encoding issues) and
> context (if not explicitly stated) might enable the correct handling of
> some cases in which the client might have to issue multiple calls to
> accomplish a single using declaration that (indirectly) brings in names
> from multiple scopes. Now, I don't see how the client would get that
> information. Consider:
>
> namespace A {
> int f(int);
> }
>
> namespace B {
> void f(void *);
> }
>
> namespace C {
> using namespace A;
> using namespace B;
> }
>
> namespace D {
> using C::f;
> }
>
> There isn't any declaration in namespace C that the libcc1 client could
> use to convey the notion that we're bringing in all occurrences of C::f,
> in a way that would enable the client to use the current API for this
> purpose. However, the information that the client gets from debug
> information is that namespace D uses A::f and B::f, without any
> reference to C, so in the end, taking A::f and B::f won't just get the
> expected results: it offers the most convenient interface for the
> client, considering that what they get from debug info is a reference to
> the DIE of the imported declaration.
OK.
>>>>> For operator functions, set GCC_CP_FLAG_SPECIAL_FUNCTION in
>>>>> + SYM_KIND, in addition to any other applicable flags, and pass as
>>>>> + NAME a string starting with the two-character mangling for operator
>>>>> + name
>>>
>>>> Perhaps the compiler side should handle this transparently?
>>>
>>> Sorry, I don't understand what you're suggesting. Can you please
>>> elaborate?
>
>> I mean have the compiler recognize that the name is magic and infer
>> that it's a special function.
>
> I'd rather not have accidental namespace collisions: there are several
> layers with different conventions at play, so I decided to mark such
> things explicitly so as to be able to catch misuses. Consider that we
> have GCC's internal identifier representation, mangled identifier
> representation, names exported in debug information, and GDB's internal
> representation. When we get a special name, we want to be sure there
> was not a failure to convert between one's internal representation and
> libcc1's conventions. That's what motivated me to mark them explicitly.
It sounds like you're saying that you want the user to provide the
same information two different ways in order to make sure they're
getting it right. That seems kind of cumbersome.
> Adopting the name mangling conventions, rather than some enumeration or
> so, is justified by the need for some arbitrary convention, and reusing
> an existing one (that is NOT present in debug information names)
Isn't it present in DW_AT_linkage_name?
Jason
