This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR c++/84789] do not resolve typename into template-independent


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" }
+};


-- 
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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]