[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

2021-04-10 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

@hamzasood, I suppose this is still worth pursuing. Could you address the 
comment?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50763/new/

https://reviews.llvm.org/D50763

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

2019-07-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.
Herald added a project: clang.



Comment at: lib/Parse/ParseCXXInlineMethods.cpp:266
+public:
+  UseCachedTokensRAII(Parser , CachedTokens , const void *Data)
+  : Self(Self), Data(Data) {

Can you pass ownership of the cached tokens into here instead of passing a 
mutable reference? It makes me uncomfortable for this object to be modifying a 
list of tokens that it doesn't own (by adding an EOF token) -- that's not 
something I'd ever expect a utility with this name to do!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50763/new/

https://reviews.llvm.org/D50763



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

2018-08-15 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood created this revision.
hamzasood added reviewers: erichkeane, v.g.vassilev, malcolm.parsons, rsmith.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, kristof.beyls, eraman.

This patch extracts the eof/stop token pattern used in late-parsing to a 
re-usable RAII class. This simplifies a lot of the late-parsing code.

A few related bugs were fixed in the process:

- Late-parsing a method with default arguments but without an exception 
specification would result in an invalid union access. It was mostly harmless 
but it's technically undefined behaviour.
- When an inherited late-parsed parameter is encountered, the associated 
declaration is force-casted to FunctionDecl. This caused a crash when the 
declaration is a template.


Repository:
  rC Clang

https://reviews.llvm.org/D50763

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp

Index: test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
===
--- test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
+++ test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
@@ -61,15 +61,24 @@
 struct A {
   struct B {
 static void Foo (int = 0);
+
+template
+static void TFoo (T = 0);
   };
   
   // should not hide default args
   friend void B::Foo (int);
+
+  // Same as above, but with templates!
+  // Should not crash the compiler.
+  template
+  friend void B::TFoo (T);
 };
 
 void Test ()
 {
   A::B::Foo ();
+  A::B::TFoo ();
 }
 
 } // namespace
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1116,10 +1116,10 @@
   this->Enabled = true;
 }
 
-
-Sema::CXXThisScopeRAII::~CXXThisScopeRAII() {
+void Sema::CXXThisScopeRAII::Exit() {
   if (Enabled) {
 S.CXXThisTypeOverride = OldCXXThisTypeOverride;
+Enabled = false;
   }
 }
 
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -2097,43 +2097,42 @@
 /// delayed, e.g., default arguments or an exception-specification, create a
 /// late-parsed method declaration record to handle the parsing at the end of
 /// the class definition.
-void Parser::HandleMemberFunctionDeclDelays(Declarator& DeclaratorInfo,
+void Parser::HandleMemberFunctionDeclDelays(Declarator ,
 Decl *ThisDecl) {
-  DeclaratorChunk::FunctionTypeInfo 
-= DeclaratorInfo.getFunctionTypeInfo();
-  // If there was a late-parsed exception-specification, we'll need a
-  // late parse
-  bool NeedLateParse = FTI.getExceptionSpecType() == EST_Unparsed;
-
-  if (!NeedLateParse) {
-// Look ahead to see if there are any default args
-for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx) {
-  auto Param = cast(FTI.Params[ParamIdx].Param);
-  if (Param->hasUnparsedDefaultArg()) {
-NeedLateParse = true;
-break;
-  }
-}
-  }
+  DeclaratorChunk::FunctionTypeInfo  = DeclaratorInfo.getFunctionTypeInfo();
+  auto ParamInfoRange = llvm::make_range(FTI.Params,
+ FTI.Params + FTI.NumParams);
 
-  if (NeedLateParse) {
-// Push this method onto the stack of late-parsed method
-// declarations.
-auto LateMethod = new LateParsedMethodDeclaration(this, ThisDecl);
-getCurrentClass().LateParsedDeclarations.push_back(LateMethod);
-LateMethod->TemplateScope = getCurScope()->isTemplateParamScope();
+  const bool LateParseDefaultArgs =
+  llvm::any_of(ParamInfoRange, [](const DeclaratorChunk::ParamInfo )
+{ return cast(PI.Param)->hasUnparsedDefaultArg(); });
 
-// Stash the exception-specification tokens in the late-pased method.
-LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
-FTI.ExceptionSpecTokens = nullptr;
+  const bool LateParseExceptionSpec =
+  FTI.getExceptionSpecType() == EST_Unparsed;
+
+  // If we didn't delay parsing for any parts of this function, then we're done.
+  if (!LateParseDefaultArgs && !LateParseExceptionSpec)
+return;
 
-// Push tokens for each parameter.  Those that do not have
-// defaults will be NULL.
-LateMethod->DefaultArgs.reserve(FTI.NumParams);
-for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
-  LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
-  FTI.Params[ParamIdx].Param,
-  std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
+  // Push this method onto the stack of late-parsed method declarations.
+  auto LateMethod = new LateParsedMethodDeclaration(this, ThisDecl);
+  getCurrentClass().LateParsedDeclarations.push_back(LateMethod);
+  LateMethod->TemplateScope =