erik.pilkington added inline comments.
================
Comment at: clang/test/SemaCXX/using-if-exists.cpp:11
+
+using NotNS::x UIE; // expected-error{{use of undeclared identifier 'NotNS'}}
+} // test_basic
----------------
Quuxplusone wrote:
> Do you also have a test for `using NS::NotNS::x UIE;`?
> (Both of these `NotNS` tests produce hard errors; `[[using_if_exists]]` does
> the silent-ignore thing only when the tippymost identifier is nonexistent,
> not if some intermediate name is nonexistent. This seems reasonable enough.)
Yeah, this is only about the tippymost name :). New patch adds a test for
non-existent names in the middle, which was previously untested.
================
Comment at: clang/test/SemaCXX/using-if-exists.cpp:60
+ typedef int mt;
+};
+
----------------
Quuxplusone wrote:
> An inheritance case not yet covered here errors out with `error: no matching
> constructor for initialization of 'Derived<Base>'`:
> ```
> struct Base { Base(int); };
> template<class T> struct Derived : T {
> using T::T [[clang::using_if_exists]];
> };
> Derived<Base> d(1);
> ```
> Does this make sense? Should this attribute perhaps give a hard error if you
> try to use it to inherit a constructor overload set like this, instead of
> (apparently) just no-opping the construct?
Hmm, yeah, I don't think that inheriting constructors really make sense with
this attribute. New patch adds an error.
================
Comment at: clang/test/SemaCXX/using-if-exists.cpp:79
+ using B::mf UIE; // expected-note {{using declaration annotated with
'using_if_exists' here}}
+ using typename B::mt UIE; // expected-note 2 {{using declaration annotated
with 'using_if_exists' here}}
+
----------------
Quuxplusone wrote:
> I notice there's a hard `error: 'using_if_exists' attribute cannot be applied
> to types` on
>
> using B::operator int UIE;
>
> Any thoughts on how to disambiguate that grammar?
Hmm, that's an interesting case. I'm not sure what the right approach is. I
think it would be a bit awkward to decide where to attach the attribute
depending on what kind of entities the attribute could potentially apply to.
FWIW this isn't a semantic restriction, since you can always just use the
prefix syntax: `UIE using B::operator int;`. Maybe @aaron.ballman has some
thoughts here?
================
Comment at: clang/test/SemaCXX/using-if-exists.cpp:107
+ using typename Ts::x... UIE; // expected-error 2 {{target of using
declaration conflicts with declaration already in scope}}
expected-note{{conflicting declaration}} expected-note{{target of using
declaration}}
+};
+
----------------
Quuxplusone wrote:
> This is a very good test, but it feels to me as if
> ```
> struct B { static int f(int); };
> struct C { };
> template<class... Ts> struct D : Ts... {
> using Ts::f... [[clang::using_if_exists]];
> };
> int main() { D<B, C>::f(1); }
> ```
> ought to be //at least// as well-formed as
> ```
> struct B { static int f(int); };
> struct C { static void f(); };
> template<class... Ts> struct D : Ts... {
> using Ts::f... [[clang::using_if_exists]];
> };
> int main() { D<B, C>::f(1); }
> ```
> I guess what's going on here is that an "unresolved using decl" is considered
> a separate kind of entity, i.e. `using Ts::f...` will work if //all//
> `Ts::f...` are variables, or if //all// of them are functions, or if //all//
> of them are unresolved, but it doesn't work if you have a mix of variables
> and functions, or variables and unresolveds, or functions and unresolveds. Is
> that basically correct, and intended?
> I guess what's going on here is that an "unresolved using decl" is considered
> a separate kind of entity, i.e. using Ts::f... will work if all Ts::f... are
> variables, or if all of them are functions, or if all of them are unresolved,
> but it doesn't work if you have a mix of variables and functions, or
> variables and unresolveds, or functions and unresolveds. Is that basically
> correct, and intended?
Yeah, exactly. My mental model for how this attribute works is that an
unresolved using-declaration introduces an entity that isn't a function, value,
or type, and is incompatible with all of those. Sometimes, that isn't what you
want for overloaded functions, like Louis ran into an issue while testing this
where `remove(const char *)` in `<cstdio>` being unresolved lead to a compile
error on the declaration of `remove(iter, iter, val)` in `<algorithm>`. That
seems to basically be the same issue you're seeing in your first test case.
Personally, my inclination is to conservatively just disallow any interplay
with an unresolved using-declaration and a function, and if we run into a lot
of problems in practice then we can come up with a more nuanced approach for
this case without breaking backwards compatibility. WDYT?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90188/new/
https://reviews.llvm.org/D90188
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
