On Mar 21, 2018, Jason Merrill <ja...@redhat.com> wrote: > On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva <aol...@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" } +}; -- 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