[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
aaron.ballman added inline comments.
Comment at: clang/lib/Parse/ParseTentative.cpp:91
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
v.g.vassilev wrote:
> aaron.ballman wrote:
> > Are you sure you can remove this? Wouldn't this be used for a case like:
> > ```
> > struct Foo {
> > struct Bar {
> > struct Baz {
> > ~Baz();
> > };
> > };
> > };
> >
> > Foo::Bar::Baz::~Baz() {}
> > ```
> > (I could be reading the code wrong, but I thought we had a reason to check
> > for `annot_cxxscope` -- seems we missed test coverage for it!)
> It looks like we do not need the `annot_cxxscope` as it seems it was
> processed above (likely one of the recent additions such as
> `ParseOptionalCXXScopeSpecifier`). I added a test case for the case you
> proposed in rG87ae74692456
Ah, good to know, and thank you for the additional test case!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148425/new/
https://reviews.llvm.org/D148425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
v.g.vassilev marked an inline comment as done.
v.g.vassilev added inline comments.
Comment at: clang/lib/Parse/ParseTentative.cpp:91
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
aaron.ballman wrote:
> Are you sure you can remove this? Wouldn't this be used for a case like:
> ```
> struct Foo {
> struct Bar {
> struct Baz {
> ~Baz();
> };
> };
> };
>
> Foo::Bar::Baz::~Baz() {}
> ```
> (I could be reading the code wrong, but I thought we had a reason to check
> for `annot_cxxscope` -- seems we missed test coverage for it!)
It looks like we do not need the `annot_cxxscope` as it seems it was processed
above (likely one of the recent additions such as
`ParseOptionalCXXScopeSpecifier`). I added a test case for the case you
proposed in rG87ae74692456
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148425/new/
https://reviews.llvm.org/D148425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
aaron.ballman added inline comments.
Comment at: clang/lib/Parse/ParseTentative.cpp:91
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
Are you sure you can remove this? Wouldn't this be used for a case like:
```
struct Foo {
struct Bar {
struct Baz {
~Baz();
};
};
};
Foo::Bar::Baz::~Baz() {}
```
(I could be reading the code wrong, but I thought we had a reason to check for
`annot_cxxscope` -- seems we missed test coverage for it!)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148425/new/
https://reviews.llvm.org/D148425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a9abe846617: [clang-repl] Correctly disambiguate dtor
declarations from statements. (authored by v.g.vassilev).
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148425/new/
https://reviews.llvm.org/D148425
Files:
clang/lib/Parse/ParseTentative.cpp
clang/test/Interpreter/disambiguate-decl-stmt.cpp
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -26,6 +26,10 @@
x.I::~I();
x = 20;
+struct Dtor1 {~Dtor1();};
+Dtor1::~Dtor1() { printf("Dtor1\n"); }
+Dtor1 d1;
+
// Ctors
// Deduction guide
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -88,10 +88,8 @@
}
case tok::kw_operator:
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
- return true;
-break;
+ case tok::tilde:
+return true;
default:
break;
}
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -26,6 +26,10 @@
x.I::~I();
x = 20;
+struct Dtor1 {~Dtor1();};
+Dtor1::~Dtor1() { printf("Dtor1\n"); }
+Dtor1 d1;
+
// Ctors
// Deduction guide
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -88,10 +88,8 @@
}
case tok::kw_operator:
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
- return true;
-break;
+ case tok::tilde:
+return true;
default:
break;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
v.g.vassilev added a comment. Let's rely on a post-commit review here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148425/new/ https://reviews.llvm.org/D148425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148425: [clang-repl] Correctly disambiguate dtor declarations from statements
v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, aaron.ballman.
Herald added a project: All.
v.g.vassilev requested review of this revision.
Repository:
rC Clang
https://reviews.llvm.org/D148425
Files:
clang/lib/Parse/ParseTentative.cpp
clang/test/Interpreter/disambiguate-decl-stmt.cpp
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -26,6 +26,10 @@
x.I::~I();
x = 20;
+struct Dtor1 {~Dtor1();};
+Dtor1::~Dtor1() { printf("Dtor1\n"); }
+Dtor1 d1;
+
// Ctors
// Deduction guide
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -88,10 +88,8 @@
}
case tok::kw_operator:
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
- return true;
-break;
+ case tok::tilde:
+return true;
default:
break;
}
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -26,6 +26,10 @@
x.I::~I();
x = 20;
+struct Dtor1 {~Dtor1();};
+Dtor1::~Dtor1() { printf("Dtor1\n"); }
+Dtor1 d1;
+
// Ctors
// Deduction guide
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -88,10 +88,8 @@
}
case tok::kw_operator:
return true;
- case tok::annot_cxxscope: // Check if this is a dtor.
-if (NextToken().is(tok::tilde))
- return true;
-break;
+ case tok::tilde:
+return true;
default:
break;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
