| Message ID | orpo43vgzj.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PR,c++/84789] do not resolve typename into template-independent | expand |
On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > resolve_typename_type may peek into template types that might still be > specialized. In some cases, e.g. g++.dg/template/friend48.C or > g++.dg/template/decl2.C, that is exactly the right thing to do. In > others, like the newly-added testcase g++.dg/template/pr84789.C, it > isn't, and if the qualifying scope happens to resolve to a non-template > type, we resolve to that and then fail the assert that checks we still > have a template-dependent scope. We're looking inside them because we're trying to parse a declarator; the tentative parse will fail in this case, because we aren't in a declarator, but that doesn't mean it's wrong to peek. Though I'm not sure why cp_parser_parse_and_diagnose_invalid_type_name is passing true for declarator_p to cp_parser_id_expression. I'm going to try changing that to false and see if anything breaks. > It appears to me that, in cases in which the assert would fail, we are > missing the typename keyword, and we ought to report an error; if we > just return the incoming type unchanged, that's exactly what we get. > So, I'm turning such failed asserts into early returns, so that the > parser can recover and report an error. I disagree; it seems to me that the assert should allow the case where the scope was originally dependent, but got resolved earlier in the function. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> resolve_typename_type may peek into template types that might still be >> specialized. In some cases, e.g. g++.dg/template/friend48.C or >> g++.dg/template/decl2.C, that is exactly the right thing to do. In >> others, like the newly-added testcase g++.dg/template/pr84789.C, it >> isn't, and if the qualifying scope happens to resolve to a non-template >> type, we resolve to that and then fail the assert that checks we still >> have a template-dependent scope. > We're looking inside them because we're trying to parse a declarator; > the tentative parse will fail in this case, because we aren't in a > declarator, but that doesn't mean it's wrong to peek. Huh? We're referencing members of an unrelated template that AFAIK needs not even be defined at that point, and even if it is, it could still be specialized afterwards. How can it possibly be right to short-circuit such nested names? How would template instantiation/specialization get a chance to do the proper mapping? template<typename> struct B : A {}; // would /*: A {}*/ make any diff? template<typename T> struct C : B<T> // would /* : B<T>*/ make any diff? { B<T>::A::I::I i; // { dg-error "typename" } }; Is it by any chance the fact that B<T> is a base class for C<T> that makes it correct to peek into it? I don't recall any exception of this sort. > I disagree; it seems to me that the assert should allow the case where > the scope was originally dependent, but got resolved earlier in the > function. Doesn't the function always take dependent scopes? I for some reason thought that was the case.
On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Fri, Mar 16, 2018 at 5:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> resolve_typename_type may peek into template types that might still be >>> specialized. In some cases, e.g. g++.dg/template/friend48.C or >>> g++.dg/template/decl2.C, that is exactly the right thing to do. In >>> others, like the newly-added testcase g++.dg/template/pr84789.C, it >>> isn't, and if the qualifying scope happens to resolve to a non-template >>> type, we resolve to that and then fail the assert that checks we still >>> have a template-dependent scope. > >> We're looking inside them because we're trying to parse a declarator; >> the tentative parse will fail in this case, because we aren't in a >> declarator, but that doesn't mean it's wrong to peek. > > Huh? We're referencing members of an unrelated template that AFAIK > needs not even be defined at that point, and even if it is, it could > still be specialized afterwards. How can it possibly be right to > short-circuit such nested names? How would template > instantiation/specialization get a chance to do the proper mapping? > > template<typename> struct B : A {}; // would /*: A {}*/ make any diff? > template<typename T> struct C : B<T> // would /* : B<T>*/ make any diff? > { > B<T>::A::I::I i; // { dg-error "typename" } > }; > Is it by any chance the fact that B<T> is a base class for C<T> that > makes it correct to peek into it? I don't recall any exception of this > sort. No, we look inside when we're trying to parse the qualified-id as the name of a declaration; in a declarator-id we can look into uninstantiated classes, otherwise there would be no way to define members of class templates. void X<T>::N::f() { } // looks inside X<T> >> I disagree; it seems to me that the assert should allow the case where >> the scope was originally dependent, but got resolved earlier in the >> function. > > Doesn't the function always take dependent scopes? I for some reason > thought that was the case. Yes, as the comment says, a TYPENAME_TYPE should always have a dependent scope. In this case, the dependent scope was "typename B<T>::A", but just above we resolved it to just A, making it no longer dependent. Jason
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >>> that doesn't mean it's wrong to peek. >> Huh? We're referencing members of an unrelated template that AFAIK >> needs not even be defined at that point, and even if it is, it could >> still be specialized afterwards. How can it possibly be right to >> short-circuit such nested names? >> template<typename> struct B : A {}; // would /*: A {}*/ make any diff? >> template<typename T> struct C : B<T> // would /* : B<T>*/ make any diff? >> { >> B<T>::A::I::I i; // { dg-error "typename" } >> }; > No, we look inside when we're trying to parse the qualified-id as the > name of a declaration Yeah, but that's not the case at hand. I guess we're miscommunicating. I understood you were saying it was ok to peek in this case. Would you please state, for clarity, what your stance is on peeking in this case, specifically, in the B<T>::A::I::I within the definition of C above? > in a declarator-id we can look into uninstantiated classes, otherwise > there would be no way to define members of class templates. > void X<T>::N::f() { } // looks inside X<T> Of course, but then, we wouldn't get to a template-independent member. A member of a template-dependent name is still template-dependent. It's only when a qualified name maps to an external name that it might become template-independent, and when this happens, I believe the qualified-id is not one that can be used to define a member. Specifically: struct K { struct L { static double j; }; }; template <typename T> struct M { struct N { static int i; }; }; template <typename T> struct O { typedef M<T> P; typedef K Q; }; template <typename T> int O<T>::P::N::i = 42; template <typename T> double O<T>::Q::L::j = 42.0; if we remap O<T>::P to M<T> and O<T>::Q to K, how will we realize the given type-ids are not appropriate? Where will the template parmlist belong when the qualified-id is taken as equivalent to K::L::j? FWIW, we silently swallow the definition of i above (I think we shouldn't), and we fail the same assert in resolve_typename_type when parsing the definition of j. So, you see, we do have problems with declarators, and we have problems with types. AFAICT, we shouldn't simplify qualified-ids in declarators, otherwise we'd fail to issue errors for the definitions above, and we shouldn't simplify qualified-ids in types involving templates other than the one we'd in. So it's not clear to me what purpose the resolver is supposed to serve, considering that simplifying the types seem to almost never be wanted. But one issue is whether we can peek into dependent types, another is whether we can and should simplify them. The answers to these questions don't seem to be very clear to me. What is clear to me is that refraining from simplifying from dependent to non-dependent enables us to report errors that we were missing in both declarators and types, and that simplifying from dependent to unrelated dependent prevents us from reporting the error we should in the definition of i above. But is there any case in which *not* simplifying causes problems? >>> I disagree; it seems to me that the assert should allow the case where >>> the scope was originally dependent, but got resolved earlier in the >>> function. >> >> Doesn't the function always take dependent scopes? I for some reason >> thought that was the case. > Yes, as the comment says, a TYPENAME_TYPE should always have a > dependent scope. In this case, the dependent scope was "typename > B<T>::A", but just above we resolved it to just A, making it no longer > dependent. Then you got me thoroughly confused: what did you mean by 'the assert should allow' a case that's a given?
On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >>>> that doesn't mean it's wrong to peek. > >>> Huh? We're referencing members of an unrelated template that AFAIK >>> needs not even be defined at that point, and even if it is, it could >>> still be specialized afterwards. How can it possibly be right to >>> short-circuit such nested names? > >>> template<typename> struct B : A {}; // would /*: A {}*/ make any diff? >>> template<typename T> struct C : B<T> // would /* : B<T>*/ make any diff? >>> { >>> B<T>::A::I::I i; // { dg-error "typename" } >>> }; > >> No, we look inside when we're trying to parse the qualified-id as the >> name of a declaration > > Yeah, but that's not the case at hand. I guess we're miscommunicating. > I understood you were saying it was ok to peek in this case. Would you > please state, for clarity, what your stance is on peeking in this case, > specifically, in the B<T>::A::I::I within the definition of C above? It's OK when we're tentatively trying to parse it as a declarator, not when we're trying to parse it as a type-specifier. We seem to be talking past each other on this point: We already don't peek when parsing a type-specifier, we only call resolve_typename_type with false for only_current_p in the context of trying to parse a declarator. The parser tries both interpretations for B<T>::A::I::I. The type interpretation is ill-formed because of missing 'typename', so it tries again as a declarator, and therefore tries to resolve the TYPENAME_TYPE that it built while trying for a type, and trips the assert. >> in a declarator-id we can look into uninstantiated classes, otherwise >> there would be no way to define members of class templates. > >> void X<T>::N::f() { } // looks inside X<T> > > Of course, but then, we wouldn't get to a template-independent member. > A member of a template-dependent name is still template-dependent. It's > only when a qualified name maps to an external name that it might become > template-independent, and when this happens, I believe the qualified-id > is not one that can be used to define a member. Specifically: > > struct K { struct L { static double j; }; }; > template <typename T> struct M { struct N { static int i; }; }; > template <typename T> struct O { typedef M<T> P; typedef K Q; }; > template <typename T> int O<T>::P::N::i = 42; > template <typename T> double O<T>::Q::L::j = 42.0; > > if we remap O<T>::P to M<T> and O<T>::Q to K, how will we realize the > given type-ids are not appropriate? Where will the template parmlist > belong when the qualified-id is taken as equivalent to K::L::j? Agreed, these look bizarre because the template parm is used in O<T>, which is not an enclosing class of i or j. And j is clearly ill-formed because it declares a non-template as a template. But it's not clear to me that i is ill-formed; we allow struct A { static int i; }; struct B { typedef ::A A; }; int B::A::i = 0; and the i example seems analogous. I wouldn't argue against making it ill-formed, but I don't see a rule that would make it so in the current draft standard. And even resolving the scope to be non-dependent doesn't necessarily mean the declaration will be, if the template parameter list ends up being for a member template: struct K { struct L { template <typename T> static void f(T); }; }; template <typename T> struct O { typedef K Q; }; template <typename T> void O<T>::Q::L::f(T) { } >>>> I disagree; it seems to me that the assert should allow the case where >>>> the scope was originally dependent, but got resolved earlier in the >>>> function. >>> >>> Doesn't the function always take dependent scopes? I for some reason >>> thought that was the case. > >> Yes, as the comment says, a TYPENAME_TYPE should always have a >> dependent scope. In this case, the dependent scope was "typename >> B<T>::A", but just above we resolved it to just A, making it no longer >> dependent. > > Then you got me thoroughly confused: what did you mean by 'the assert > should allow' a case that's a given? asserts are often used to verify that things we think of as givens are actually true. :) I suppose we could remove the assert, but I'd probably move it up above where we start messing with 'scope'. Jason
On Mar 21, 2018, Jason Merrill <jason@redhat.com> wrote: > On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> I understood you were saying it was ok to peek in this case. Would you >> please state, for clarity, what your stance is on peeking in this case, >> specifically, in the B<T>::A::I::I within the definition of C above? > It's OK when we're tentatively trying to parse it as a declarator, not > when we're trying to parse it as a type-specifier. Thanks for the clarification; it had seemed to me that you were rejecting my understanding that we shouldn't look up within the unrelated template type when parsing the type id. Now I understand you were only rejecting the notion that we were parsing it as a type id. Once I understood that, and realized why you were speaking of declarators although the testcase at hand had a template-dependent type id that was obviously (to me) not a declarator, it all made sense ;-) Sorry for being so dense. > I wouldn't argue against making it ill-formed, but I don't see a rule > that would make it so in the current draft standard. I have just spent some time looking for such a rule and failed to find it too. So I put in all of the bizarre declarators from this conversation into new testcases, but expecting them to be accepted. >>>>> I disagree; it seems to me that the assert should allow the case where >>>>> the scope was originally dependent, but got resolved earlier in the >>>>> function. >>>> Doesn't the function always take dependent scopes? I for some reason >>>> thought that was the case. >>> Yes, as the comment says, a TYPENAME_TYPE should always have a >>> dependent scope. In this case, the dependent scope was "typename >>> B<T>::A", but just above we resolved it to just A, making it no longer >>> dependent. >> Then you got me thoroughly confused: what did you mean by 'the assert >> should allow' a case that's a given? > asserts are often used to verify that things we think of as givens are > actually true. :) Yeah, but the wording 'allow' rather than 'require'/'verify' set me off. We used to require the opposite of X, so allowing X amounted to assert (!X || X), which didn't sound like something you'd mean. > I suppose we could remove the assert, but I'd probably move it up > above where we start messing with 'scope'. Now that makes sense to me, and it works, but I was not happy with the errors we got, so I went ahead and arranged for the type name to be reparsed as a non-declarator for the diagnostic, so that we get a better diagnostic. Regstrapping, after a successful check-g++ of a non-bootstrapped x86_64-linux-gnu build. Ok to install if the regstrap passes? [PR c++/84789] do not fail to resolve typename into template-independent Although resolve_typename_type always takes a template-dependent type-id, and it usually resolves it to another template-dependent type-id, it is not correct to require the latter: in declarators, template-dependent scopes may turn out to name template-independent types, as in the pr84789-2.C and pr84789-3.C testcases. The ill-formed testcase pr84789.C trips the same too-strict assert, and also gets fixed by removing the assertion on the simplified scope. However, whereas when the dependent type cannot be resolved, we get an error that suggests 'typename' is missing: pr84789.C:12:3: error: need ‘typename’ before ‘typename B<T>::A::I::I’ because ‘typename B<T>::A::I’ is a dependent scope B<T>::A::I::I i; ^~~~ when it can, we got errors that did not point at that possibility, which may be confusing: pr84789.C:9:15: error: ‘A::I’ {aka ‘int’} is not a class type B<T>::A::I::I i; // { dg-error "typename" } ^ pr84789.C:9:15: error: ‘I’ in ‘A::I’ {aka ‘int’} does not name a type Changing the parser diagnostic code that reports an invalid type name so that it does not attempt to reparse the name as a declarator gets us the superior diagnostic of a missing 'typename' keyword. for gcc/cp/ChangeLog PR c++/84789 * pt.c (resolve_typename_type): Drop assert that stopped simplification to template-independent types. Add assert to verify the initial scope is template dependent. * parser.c (cp_parser_parse_and_diagnose_invalid_type_name): Reparse the id expression as a type-name, not a declarator. for gcc/testsuite/ChangeLog PR c++/84789 * g++.dg/template/pr84789.C: New. * g++.dg/template/pr84789-2.C: New. * g++.dg/template/pr84789-3.C: New. * g++.dg/parse/dtor11.C: Accept alternate error message. --- gcc/cp/parser.c | 2 +- gcc/cp/pt.c | 5 +++-- gcc/testsuite/g++.dg/parse/dtor11.C | 2 +- gcc/testsuite/g++.dg/template/pr84789-2.C | 26 ++++++++++++++++++++++++ gcc/testsuite/g++.dg/template/pr84789-3.C | 31 +++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/template/pr84789.C | 13 ++++++++++++ 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/pr84789-2.C create mode 100644 gcc/testsuite/g++.dg/template/pr84789-3.C create mode 100644 gcc/testsuite/g++.dg/template/pr84789.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 0f7df14ce148..cf4865148849 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -3455,7 +3455,7 @@ cp_parser_parse_and_diagnose_invalid_type_name (cp_parser *parser) /*template_keyword_p=*/false, /*check_dependency_p=*/true, /*template_p=*/NULL, - /*declarator_p=*/true, + /*declarator_p=*/false, /*optional_p=*/false); /* If the next token is a (, this is a function with no explicit return type, i.e. constructor, destructor or conversion op. */ diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index d7c0c7bec81b..5293c2b5491b 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -25249,6 +25249,9 @@ resolve_typename_type (tree type, bool only_current_p) gcc_assert (TREE_CODE (type) == TYPENAME_TYPE); scope = TYPE_CONTEXT (type); + /* We shouldn't have built a TYPENAME_TYPE with a non-dependent scope. */ + gcc_checking_assert (uses_template_parms (scope)); + /* Usually the non-qualified identifier of a TYPENAME_TYPE is TYPE_IDENTIFIER (type). But when 'type' is a typedef variant of a TYPENAME_TYPE node, then TYPE_NAME (type) is set to the TYPE_DECL representing @@ -25285,8 +25288,6 @@ resolve_typename_type (tree type, bool only_current_p) /* scope is either the template itself or a compatible instantiation like X<T>, so look up the name in the original template. */ scope = CLASSTYPE_PRIMARY_TEMPLATE_TYPE (scope); - /* We shouldn't have built a TYPENAME_TYPE with a non-dependent scope. */ - gcc_checking_assert (uses_template_parms (scope)); /* If scope has no fields, it can't be a current instantiation. Check this before currently_open_class to avoid infinite recursion (71515). */ if (!TYPE_FIELDS (scope)) diff --git a/gcc/testsuite/g++.dg/parse/dtor11.C b/gcc/testsuite/g++.dg/parse/dtor11.C index 63ffb60bac10..83fd93489f11 100644 --- a/gcc/testsuite/g++.dg/parse/dtor11.C +++ b/gcc/testsuite/g++.dg/parse/dtor11.C @@ -8,5 +8,5 @@ struct A struct B { - A::~B B(); // { dg-error "as member of" } + A::~B B(); // { dg-error "as member of|as a type" } }; diff --git a/gcc/testsuite/g++.dg/template/pr84789-2.C b/gcc/testsuite/g++.dg/template/pr84789-2.C new file mode 100644 index 000000000000..0b42148ef3e4 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84789-2.C @@ -0,0 +1,26 @@ +// { dg-do compile } + +struct K { + struct L { + static double j; + }; +}; + +template <typename T> +struct M { + struct N { + static int i; + }; +}; + +template <typename T> +struct O { + typedef M<T> P; + typedef K Q; +}; + +template <typename T> +int O<T>::P::N::i = 42; // This is obfuscated, but apparently ok. + +template <typename T> +double O<T>::Q::L::j = 42.0; // { dg-error "non-template" } diff --git a/gcc/testsuite/g++.dg/template/pr84789-3.C b/gcc/testsuite/g++.dg/template/pr84789-3.C new file mode 100644 index 000000000000..bc38c1544ba9 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84789-3.C @@ -0,0 +1,31 @@ +// { dg-do compile } + +struct A +{ + static int i; +}; +struct B +{ + typedef ::A A; +}; +int B::A::i = 0; + +struct K +{ + struct L + { + template <typename T> + static void f(T); + }; +}; + +template <typename T> +struct O +{ + typedef K Q; +}; + +template <typename T> +void O<T>::Q::L::f(T) +{ +} diff --git a/gcc/testsuite/g++.dg/template/pr84789.C b/gcc/testsuite/g++.dg/template/pr84789.C new file mode 100644 index 000000000000..bc1567f3fe77 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84789.C @@ -0,0 +1,13 @@ +// { dg-do compile } + +struct A +{ + typedef int I; +}; + +template<typename> struct B : A {}; + +template<typename T> struct C : B<T> +{ + B<T>::A::I::I i; // { dg-error "typename" } +};
OK, thanks. On Wed, Mar 21, 2018 at 11:48 AM, Jason Merrill <jason@redhat.com> wrote: > On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >> >>> On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >>>> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote: >>>>> that doesn't mean it's wrong to peek. >> >>>> Huh? We're referencing members of an unrelated template that AFAIK >>>> needs not even be defined at that point, and even if it is, it could >>>> still be specialized afterwards. How can it possibly be right to >>>> short-circuit such nested names? >> >>>> template<typename> struct B : A {}; // would /*: A {}*/ make any diff? >>>> template<typename T> struct C : B<T> // would /* : B<T>*/ make any diff? >>>> { >>>> B<T>::A::I::I i; // { dg-error "typename" } >>>> }; >> >>> No, we look inside when we're trying to parse the qualified-id as the >>> name of a declaration >> >> Yeah, but that's not the case at hand. I guess we're miscommunicating. >> I understood you were saying it was ok to peek in this case. Would you >> please state, for clarity, what your stance is on peeking in this case, >> specifically, in the B<T>::A::I::I within the definition of C above? > > It's OK when we're tentatively trying to parse it as a declarator, not > when we're trying to parse it as a type-specifier. > > We seem to be talking past each other on this point: We already don't > peek when parsing a type-specifier, we only call resolve_typename_type > with false for only_current_p in the context of trying to parse a > declarator. The parser tries both interpretations for B<T>::A::I::I. > The type interpretation is ill-formed because of missing 'typename', > so it tries again as a declarator, and therefore tries to resolve the > TYPENAME_TYPE that it built while trying for a type, and trips the > assert. > >>> in a declarator-id we can look into uninstantiated classes, otherwise >>> there would be no way to define members of class templates. >> >>> void X<T>::N::f() { } // looks inside X<T> >> >> Of course, but then, we wouldn't get to a template-independent member. >> A member of a template-dependent name is still template-dependent. It's >> only when a qualified name maps to an external name that it might become >> template-independent, and when this happens, I believe the qualified-id >> is not one that can be used to define a member. Specifically: >> >> struct K { struct L { static double j; }; }; >> template <typename T> struct M { struct N { static int i; }; }; >> template <typename T> struct O { typedef M<T> P; typedef K Q; }; >> template <typename T> int O<T>::P::N::i = 42; >> template <typename T> double O<T>::Q::L::j = 42.0; >> >> if we remap O<T>::P to M<T> and O<T>::Q to K, how will we realize the >> given type-ids are not appropriate? Where will the template parmlist >> belong when the qualified-id is taken as equivalent to K::L::j? > > Agreed, these look bizarre because the template parm is used in O<T>, > which is not an enclosing class of i or j. And j is clearly > ill-formed because it declares a non-template as a template. > > But it's not clear to me that i is ill-formed; we allow > > struct A { static int i; }; > struct B { typedef ::A A; }; > int B::A::i = 0; > > and the i example seems analogous. I wouldn't argue against making it > ill-formed, but I don't see a rule that would make it so in the > current draft standard. And even resolving the scope to be > non-dependent doesn't necessarily mean the declaration will be, if the > template parameter list ends up being for a member template: > > struct K { struct L { template <typename T> static void f(T); }; }; > template <typename T> struct O { typedef K Q; }; > template <typename T> void O<T>::Q::L::f(T) { } > >>>>> I disagree; it seems to me that the assert should allow the case where >>>>> the scope was originally dependent, but got resolved earlier in the >>>>> function. >>>> >>>> Doesn't the function always take dependent scopes? I for some reason >>>> thought that was the case. >> >>> Yes, as the comment says, a TYPENAME_TYPE should always have a >>> dependent scope. In this case, the dependent scope was "typename >>> B<T>::A", but just above we resolved it to just A, making it no longer >>> dependent. >> >> Then you got me thoroughly confused: what did you mean by 'the assert >> should allow' a case that's a given? > > asserts are often used to verify that things we think of as givens are > actually true. :) > > I suppose we could remove the assert, but I'd probably move it up > above where we start messing with 'scope'. > > Jason
On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote:
> OK, thanks.
Note the testcase FAILs with -fconcepts when I do make check-c++-all,
FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12)
FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors)
Excess errors:
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type
/usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' in 'A::I' {aka 'int'} does not name a type
In all other standard modes it is fine.
Jakub
On Mar 23, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >> OK, thanks. > Note the testcase FAILs with -fconcepts when I do make check-c++-all, Hmm, I don't get that with check or check-g++. Should we expand the default std_list in g++-dg.exp, or should I manually test these additional variants? Anyway, thanks for the report, I'll try to figure out why this fails with concepts.
On Fri, Mar 23, 2018 at 01:45:01PM -0300, Alexandre Oliva wrote: > On Mar 23, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > > > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: > >> OK, thanks. > > > Note the testcase FAILs with -fconcepts when I do make check-c++-all, > > Hmm, I don't get that with check or check-g++. Should we expand the > default std_list in g++-dg.exp, or should I manually test these > additional variants? I think we should add the -std=c++17 mode to the list of standards cycled by default, now that it is a released standard. -fconcepts is experimental and so is -std=c++2a, so perhaps those can stay out of the default and be just checked occassionally. The full set of check-c++-all FAILs I'm getting that don't show up with check-g++ is: FAIL: g++.dg/spellcheck-macro-ordering-2.C -std=c++17 -fconcepts (test for errors, line 10) FAIL: g++.dg/spellcheck-macro-ordering-2.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for errors, line 6) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for warnings, line 6) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for warnings, line 15) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12) FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++17 (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++2a (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++17 -fconcepts (test for excess errors) Jakub
On Mar 23, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >> OK, thanks. > Note the testcase FAILs with -fconcepts when I do make check-c++-all, > FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12) > FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors) > Excess errors: > /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: > 'A::I' {aka 'int'} is not a class type > /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' > in 'A::I' {aka 'int'} does not name a type Is this ok to install? [PR c++/84789] adjust testcase for -fconcepts When compiling with -fconcepts, cp_parser_template_declaration_after_export calls cp_parser_template_introduction and that preparses qualified-ids not preceded by typename in such a way that, when we get to cp_parser_parse_and_diagnose_invalid_type_name and then cp_parser_diagnose_invalid_type_name, the nested name specifier no longer carries the previous template-dependent context, so we don't stand a chance to suggest the use of 'typename' any more. Thus, tolerate in the testcase the poorer error messages we get. for gcc/testsuite/ChangeLog PR c++/84789 * g++.dg/template/pr84789.C: Adjust for testing with -fconcepts too. --- gcc/testsuite/g++.dg/template/pr84789.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/g++.dg/template/pr84789.C b/gcc/testsuite/g++.dg/template/pr84789.C index bc1567f3fe77..63b9832fecf8 100644 --- a/gcc/testsuite/g++.dg/template/pr84789.C +++ b/gcc/testsuite/g++.dg/template/pr84789.C @@ -9,5 +9,5 @@ template<typename> struct B : A {}; template<typename T> struct C : B<T> { - B<T>::A::I::I i; // { dg-error "typename" } + B<T>::A::I::I i; // { dg-error "not a class type|does not name a type|typename" } };
OK. On Fri, Mar 23, 2018 at 3:17 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 23, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > >> On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >>> OK, thanks. > >> Note the testcase FAILs with -fconcepts when I do make check-c++-all, >> FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12) >> FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors) >> Excess errors: >> /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: >> 'A::I' {aka 'int'} is not a class type >> /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' >> in 'A::I' {aka 'int'} does not name a type > > Is this ok to install? > > [PR c++/84789] adjust testcase for -fconcepts > > When compiling with -fconcepts, > cp_parser_template_declaration_after_export calls > cp_parser_template_introduction and that preparses qualified-ids not > preceded by typename in such a way that, when we get to > cp_parser_parse_and_diagnose_invalid_type_name and then > cp_parser_diagnose_invalid_type_name, the nested name specifier no > longer carries the previous template-dependent context, so we don't > stand a chance to suggest the use of 'typename' any more. Thus, > tolerate in the testcase the poorer error messages we get. > > for gcc/testsuite/ChangeLog > > PR c++/84789 > * g++.dg/template/pr84789.C: Adjust for testing with > -fconcepts too. > --- > gcc/testsuite/g++.dg/template/pr84789.C | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/g++.dg/template/pr84789.C b/gcc/testsuite/g++.dg/template/pr84789.C > index bc1567f3fe77..63b9832fecf8 100644 > --- a/gcc/testsuite/g++.dg/template/pr84789.C > +++ b/gcc/testsuite/g++.dg/template/pr84789.C > @@ -9,5 +9,5 @@ template<typename> struct B : A {}; > > template<typename T> struct C : B<T> > { > - B<T>::A::I::I i; // { dg-error "typename" } > + B<T>::A::I::I i; // { dg-error "not a class type|does not name a type|typename" } > }; > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 89024c10fe2b..067221fa78ea 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -25195,8 +25195,23 @@ resolve_typename_type (tree type, bool only_current_p) /* scope is either the template itself or a compatible instantiation like X<T>, so look up the name in the original template. */ scope = CLASSTYPE_PRIMARY_TEMPLATE_TYPE (scope); - /* We shouldn't have built a TYPENAME_TYPE with a non-dependent scope. */ - gcc_checking_assert (uses_template_parms (scope)); + /* We shouldn't have built a TYPENAME_TYPE with a non-dependent + scope. However, it might be a dependent scope that's being + resolved to a non-dependent scope just because we're looking up + scopes we shouldn't, e.g. + + template <typename> class B { typedef int I; }; + template <typename T> class C : B<T> { B<T>::I i; }; + + We need 'typename' before C<T>::i's type, because we can't enter + the scope of B<T> for an unbound template parameter T to tell I + identifies a type (that's why we need typename), but the parser + attempts to do so, presumably so that it can produce better error + messages. However, we'd skip necessary errors if we resolved a + template-dependent type to a template-independent one, so don't + do that. */ + if (!uses_template_parms (scope)) + return type; /* If scope has no fields, it can't be a current instantiation. Check this before currently_open_class to avoid infinite recursion (71515). */ if (!TYPE_FIELDS (scope)) diff --git a/gcc/testsuite/g++.dg/template/pr84789.C b/gcc/testsuite/g++.dg/template/pr84789.C new file mode 100644 index 000000000000..bc1567f3fe77 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84789.C @@ -0,0 +1,13 @@ +// { dg-do compile } + +struct A +{ + typedef int I; +}; + +template<typename> struct B : A {}; + +template<typename T> struct C : B<T> +{ + B<T>::A::I::I i; // { dg-error "typename" } +};