[clang] [clang][Parser] "Better" error messages for invalid template template (PR #95726)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 17 05:02:00 PDT 2024
https://github.com/Veeloxfire updated https://github.com/llvm/llvm-project/pull/95726
>From 44620330cd5238de549d3d77ddc447cd3bc51e60 Mon Sep 17 00:00:00 2001
From: Veeloxfire <58116051+Veeloxfire at users.noreply.github.com>
Date: Mon, 17 Jun 2024 01:20:32 +0100
Subject: [PATCH 1/5] [clang][Parser] "Better" error messages for invalid
template template
For the somewhat easy mistake of `template<template A> ...` clang outputs a partially cryptic error
Change this to assume the programmer intended `typename` in cases where `template` is illegal, and emit diagnostics (and forward parsing) accordingly
This mirrors the behaviour of `typedef` handling
---
clang/lib/Parse/ParseTemplate.cpp | 17 +++++++++++++++++
clang/test/CXX/drs/cwg1xx.cpp | 2 ++
clang/test/Parser/cxx-template-decl.cpp | 2 +-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index a5130f56600e5..e5308d9edac5f 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -787,6 +787,23 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
unsigned Position) {
assert(Tok.is(tok::kw_template) && "Expected 'template' keyword");
+ if (Token ahead = GetLookAheadToken(1);
+ ahead.isOneOf(tok::identifier, tok::ellipsis,
+ tok::equal, tok::comma,
+ tok::greater, tok::greatergreater, tok::greatergreatergreater)) {
+ // Maybe they intended `typename` instead of `template` (given thats more common)
+ // Error early, to add a fixit hint
+
+ Diag(ahead.getLocation(), diag::err_expected_less_after) << "template";
+
+ Diag(Tok.getLocation(), diag::note_meant_to_use_typename)
+ << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Tok.getLocation()),
+ "typename");
+
+ Tok.setKind(tok::kw_typename);
+ return ParseTypeParameter(Depth, Position);
+ }
+
// Handle the template <...> part.
SourceLocation TemplateLoc = ConsumeToken();
SmallVector<NamedDecl*,8> TemplateParams;
diff --git a/clang/test/CXX/drs/cwg1xx.cpp b/clang/test/CXX/drs/cwg1xx.cpp
index e7dddd1ea9278..72b3ff40152d5 100644
--- a/clang/test/CXX/drs/cwg1xx.cpp
+++ b/clang/test/CXX/drs/cwg1xx.cpp
@@ -1145,8 +1145,10 @@ namespace cwg181 { // cwg181: yes
namespace X {
template <template X<class T> > struct A { };
// expected-error at -1 +{{}}
+ // expected-note at -2 {{did you mean to use 'typename'?}}
template <template X<class T> > void f(A<X>) { }
// expected-error at -1 +{{}}
+ // expected-note at -2 {{did you mean to use 'typename'?}}
}
namespace Y {
diff --git a/clang/test/Parser/cxx-template-decl.cpp b/clang/test/Parser/cxx-template-decl.cpp
index 734438069b9ae..69b9ab012b478 100644
--- a/clang/test/Parser/cxx-template-decl.cpp
+++ b/clang/test/Parser/cxx-template-decl.cpp
@@ -22,7 +22,7 @@ template<template<int+>> struct x3; // expected-error {{expected ',' or '>' in t
cpp14-error {{template template parameter requires 'class' after the parameter list}} \
cpp17-error {{template template parameter requires 'class' or 'typename' after the parameter list}}
template <template X> struct Err1; // expected-error {{expected '<' after 'template'}} \
-// expected-error{{extraneous}}
+// expected-note{{did you mean to use 'typename'?}}
template <template <typename> > struct Err2; // cpp14-error {{template template parameter requires 'class' after the parameter list}}
// cpp17-error at -1{{template template parameter requires 'class' or 'typename' after the parameter list}}
template <template <typename> Foo> struct Err3; // cpp14-error {{template template parameter requires 'class' after the parameter list}}
>From 35840e8fd58d3dfa0db424d55eedc1d666d10505 Mon Sep 17 00:00:00 2001
From: Veeloxfire <58116051+Veeloxfire at users.noreply.github.com>
Date: Mon, 17 Jun 2024 12:18:48 +0100
Subject: [PATCH 2/5] [clang][Parser] Check for actual failure condition
`ahead.isNot(tok::less)` is the actual failure condition of the parse, so check for that instead of trying to match all the possible alternatives
---
clang/lib/Parse/ParseTemplate.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index e5308d9edac5f..381c57536e755 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -787,10 +787,7 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
unsigned Position) {
assert(Tok.is(tok::kw_template) && "Expected 'template' keyword");
- if (Token ahead = GetLookAheadToken(1);
- ahead.isOneOf(tok::identifier, tok::ellipsis,
- tok::equal, tok::comma,
- tok::greater, tok::greatergreater, tok::greatergreatergreater)) {
+ if (Token ahead = GetLookAheadToken(1); ahead.isNot(tok::less)) {
// Maybe they intended `typename` instead of `template` (given thats more common)
// Error early, to add a fixit hint
>From 5ae501f81a7659865cc9aa709e8d1ffd8f068eda Mon Sep 17 00:00:00 2001
From: Veeloxfire <58116051+Veeloxfire at users.noreply.github.com>
Date: Mon, 17 Jun 2024 12:22:33 +0100
Subject: [PATCH 3/5] [clang][Parser] Reformat
---
clang/lib/Parse/ParseTemplate.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 381c57536e755..f2b1096ebeb32 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -788,14 +788,14 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
assert(Tok.is(tok::kw_template) && "Expected 'template' keyword");
if (Token ahead = GetLookAheadToken(1); ahead.isNot(tok::less)) {
- // Maybe they intended `typename` instead of `template` (given thats more common)
- // Error early, to add a fixit hint
+ // Maybe they intended `typename` instead of `template` (given thats more
+ // common) Error early, to add a fixit hint
Diag(ahead.getLocation(), diag::err_expected_less_after) << "template";
-
+
Diag(Tok.getLocation(), diag::note_meant_to_use_typename)
- << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Tok.getLocation()),
- "typename");
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(Tok.getLocation()), "typename");
Tok.setKind(tok::kw_typename);
return ParseTypeParameter(Depth, Position);
>From a9c1ca4c1ddc6c7c0c23babe8195af9198b4c3cc Mon Sep 17 00:00:00 2001
From: Veeloxfire <58116051+Veeloxfire at users.noreply.github.com>
Date: Mon, 17 Jun 2024 12:32:24 +0100
Subject: [PATCH 4/5] [clang][Parser] Reformat 2
---
clang/test/CXX/drs/cwg1xx.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/CXX/drs/cwg1xx.cpp b/clang/test/CXX/drs/cwg1xx.cpp
index 72b3ff40152d5..6810a1952504e 100644
--- a/clang/test/CXX/drs/cwg1xx.cpp
+++ b/clang/test/CXX/drs/cwg1xx.cpp
@@ -1145,10 +1145,10 @@ namespace cwg181 { // cwg181: yes
namespace X {
template <template X<class T> > struct A { };
// expected-error at -1 +{{}}
- // expected-note at -2 {{did you mean to use 'typename'?}}
+ // expected-note at -2 {{did you mean to use 'typename'?}}
template <template X<class T> > void f(A<X>) { }
// expected-error at -1 +{{}}
- // expected-note at -2 {{did you mean to use 'typename'?}}
+ // expected-note at -2 {{did you mean to use 'typename'?}}
}
namespace Y {
>From be06449927133c1e345876a1adba95bdab541d30 Mon Sep 17 00:00:00 2001
From: Veeloxfire <58116051+Veeloxfire at users.noreply.github.com>
Date: Mon, 17 Jun 2024 13:01:43 +0100
Subject: [PATCH 5/5] [clang][Parser] Suggested changes and cleanup
---
clang/lib/Parse/ParseTemplate.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index f2b1096ebeb32..33030a0095c04 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -787,11 +787,11 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
unsigned Position) {
assert(Tok.is(tok::kw_template) && "Expected 'template' keyword");
- if (Token ahead = GetLookAheadToken(1); ahead.isNot(tok::less)) {
- // Maybe they intended `typename` instead of `template` (given thats more
- // common) Error early, to add a fixit hint
+ if (Token Next = NextToken(); Next.isNot(tok::less)) {
+ // `template` may have been a typo for `typename`, given that the
+ // latter is more common.
- Diag(ahead.getLocation(), diag::err_expected_less_after) << "template";
+ Diag(Next.getLocation(), diag::err_expected_less_after) << "template";
Diag(Tok.getLocation(), diag::note_meant_to_use_typename)
<< FixItHint::CreateReplacement(
More information about the cfe-commits
mailing list