rjmccall added inline comments.
================
Comment at: test/SemaCXX/address-space-method-overloading.cpp:28
+ //inas4.bar();
+ noas.bar(); // expected-error{{call to member function 'bar' is ambiguous}}
+}
----------------
ebevhan wrote:
> rjmccall wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > ebevhan wrote:
> > > > > Just as an aside (I think I mentioned this on one of the earlier
> > > > > patches as well); treating a non-AS-qualified object as being in a
> > > > > 'generic' AS even for normal C++ isn't a great idea IMO. The object
> > > > > initialization code should be checking if the ASes of the actual
> > > > > object and the desired object type are compatible, and only if so,
> > > > > permit the overload.
> > > > I think changing this would require changing AS compatibility rules in
> > > > general, not just for overloading but for example for conversions too.
> > > > This seems like a wider scope change that might affect backwards
> > > > compatibility. We might need to start an RFC on this first. John has
> > > > suggested adding a target setting for this on the other review. I
> > > > think that should work. Another idea could be to add some compiler
> > > > directives to specify what generic address space is (if any).
> > > >
> > > > Using default (no address space) as generic has a lot of advantages
> > > > since it doesn't need many parser changes. In OpenCL we weren't lucky
> > > > that initial implementation was added that used default for private
> > > > memory and therefore when generic was introduced we had to map it to a
> > > > new lang addr space value. That required a lot of changes in the
> > > > parser. But once we fix those actually, anyone should be able to map
> > > > generic to anything. I initially thought it has no other use cases than
> > > > in OpenCL but now I feel there might be a value to map default case (no
> > > > address space) for something specific and then use generic to specify a
> > > > placeholder address space on pointers or references. We could just
> > > > expose generic address space generically and also have a mode with no
> > > > generic address space. The latter means that conversions between
> > > > different address spaces is never valid. Would this make sense?
> > > The big problem with the address space support in Clang is that it isn't
> > > really very well formalized unless you're on OpenCL. There's no real way
> > > for a target to express the relations between its address spaces; most of
> > > the relations that exist are hard-coded.
> > >
> > > The support should probably be generalized for both targets and for
> > > LangASes like OpenCL's. Maybe:
> > >
> > > * the ASes would be defined in a TableGen file which specifies which ASes
> > > exist, which ones are compatible/subsets/supersets, etc,
> > > * or, just have a target hook that lets a target express that a
> > > conversion from AS A to AS B is prohibited/permitted explicitly/permitted
> > > implicitly.
> > >
> > > Just some loose ideas; an RFC for this is possibly the right way forward.
> > >
> > > The reason I bring all of this up is primarily because in our target, the
> > > 'default' AS is disjoint from our other ones, so there's no 'generic' AS
> > > to be had. Both implicit and explicit conversion between these ASes is
> > > prohibited. Since Clang currently doesn't allow you to express that ASes
> > > are truly incompatible, we have a flag that blocks such conversions
> > > unconditionally. Ideally it would be target-expressible.
> > >
> > > -----
> > >
> > > > I think changing this would require changing AS compatibility rules in
> > > > general, not just for overloading but for example for conversions too.
> > > > This seems like a wider scope change that might affect backwards
> > > > compatibility. We might need to start an RFC on this first. John has
> > > > suggested adding a target setting for this on the other review. I think
> > > > that should work. Another idea could be to add some compiler directives
> > > > to specify what generic address space is (if any).
> > >
> > > I'm unsure whether any changes to the current semantics really need to be
> > > done, at least for the overloading problem.
> > >
> > > For example, explicit conversion from a non-address space qualified
> > > pointer type to an address space qualified pointer type (and vice versa)
> > > is permitted, but implicit conversion is an error in both C and C++:
> > > https://godbolt.org/z/UhOC0g
> > >
> > > For an overload candidate to be viable, there has to be an implicit
> > > conversion sequence for the implicit object argument to the implicit
> > > object parameter type. But no such implicit conversion sequence exists
> > > for types in different ASes, even today, or the implicit example in the
> > > godbolt would pass. So, an overload candidate with a different AS
> > > qualification than the object should not be viable.
> > >
> > > This is clearly not compatible with OpenCL's notion of the `__generic`
> > > AS, but OpenCL already has logic for `__generic` in Qualifiers to handle
> > > that case (`isAddressSpaceSupersetOf`). Arguably, that behavior should
> > > probably not be hard coded, but that's how it is today.
> > >
> > > (Also, just because an AS is a superset of another AS does not
> > > necessarily mean that the language should permit an implicit conversion
> > > between them, but it's a reasonable behavior, I guess.)
> > >
> > > -----
> > >
> > > Ultimately, whether or not a conversion of a pointer/reference from
> > > address space A to address space B is permitted should both be a function
> > > of what the target and the language allows, but that support doesn't
> > > exist. I also don't think that exposing a 'generic' AS is the right
> > > approach. There are ASes, and some conversions from ASes to other ASes
> > > are legal, while others are not. There's also the question of the
> > > difference between implicit and explicit conversions; blocking all
> > > implicit conversions between ASes would not work for OpenCL, so there
> > > must be a way to express that as well.
> > >
> > > -----
> > >
> > > These are all just loose thoughts from my head. I'm not terribly familiar
> > > with OpenCL except for what I've seen around the Clang code, so there
> > > might be details there that I'm not aware of.
> > > just have a target hook that lets a target express that a conversion from
> > > AS A to AS B is prohibited/permitted explicitly/permitted implicitly.
> >
> > I think it has to be this, given the possibility of target-specific
> > subspacing rules in numbered address spaces. We can't make targets use
> > disjoint sets of numbered address spaces.
> >
> > I agree that, in general, the rule has to be that we check whether the
> > object type is convertible to the expected address space of the method; we
> > can't assume that there's a generic address space that works for
> > everything. That's not even true in OpenCL, since `constant` is not a
> > subspace of the generic address space.
> >
> > > John has suggested adding a target setting for this on the other review.
> >
> > The suggestion is just that we recognize some way to control what the
> > default address space for methods is.
> >
> > This is difficult with member pointers because a function type can be
> > turned into a member pointer type arbitrarily later; that is, we need to be
> > able to canonically distinguish a function type that was written without
> > any address space qualifier (and hence is `generic` if turned into a member
> > pointer) from one that was written with our representationally-default
> > address space qualifier (`private`). We can deal with this later, I think,
> > but it might require rethinking some things.
> > I think it has to be this, given the possibility of target-specific
> > subspacing rules in numbered address spaces. We can't make targets use
> > disjoint sets of numbered address spaces.
>
> I'm in favor of this option as well.
>
> > This is difficult with member pointers because a function type can be
> > turned into a member pointer type arbitrarily later; that is, we need to be
> > able to canonically distinguish a function type that was written without
> > any address space qualifier (and hence is generic if turned into a member
> > pointer) from one that was written with our representationally-default
> > address space qualifier (private). We can deal with this later, I think,
> > but it might require rethinking some things.
>
> Oh, now I get the problem with those. That is tricky.
>
> So when a function type is used as the type of a pointer-to-member and it is
> method-qualified with Default, it should be changed to be method-qualified
> with __generic instead?
If it isn't method-qualified at all, yes. If that's representationally the
same as being method-qualified with `private`, we have a problem — we can fix
that in the short term by banning method qualification with `private`, but we
do need a real fix.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57464/new/
https://reviews.llvm.org/D57464
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
