[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-20 Thread Erich Keane via cfe-commits

erichkeane wrote:

FWIW, I've been working actively on Open ACC Sema, and I found that the split 
so far has made development much more organized/easier, and makes naming of 
things a lot less awkward.  I realize splitting out actual language features is 
concerning to some, but I'm cautiously optimistic with how successful I think 
the split of some of the dialects have been.



https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-20 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> Given the concerns raised here, it might make sense to put this change on 
> hold, do a few more similar experiments for other parts of C++, and see what 
> sort of principled design approach makes sense to take here.

It's not clear to me what do we want to clarify or try out before going forward 
with this PR. I moved concepts out of `Sema` in #92672, and that also went well 
in terms of resistance I felt while trying to get declarations out of `Sema`. 
We need to start somewhere, and I believe concepts and coroutines are the right 
first steps. Alternative is to come up with some kind of a grand plan for core 
C and C++ parts (Expr, ExprCXX, Decl, DeclCXX, etc.), but prior discussions on 
the topic suggested that it's not feasible at this point.

> My current thinking (which could be off-base!) is that for language features 
> strongly tied to language semantics (like coroutines), if we do split them 
> then it probably makes sense to split those off in a language-based approach 
> (getSema().CXX().Coroutines().whatever) whereas more language-agnostic 
> concepts (like constant expression evaluation) might make sense to split off 
> into their own component whose language-specific behavior can be controlled 
> by a traits object of some kind (getSema().ConstExpr().whatever).

Whatever shape `Sema` is going to take in the future, splitting `Sema` into a 
flat list of components moves us forward, because merging components back into 
bigger components is trivial compared to splitting them up. One of the primary 
goals of this effort is not as much transforming `Sema` according to some 
vision, as it is highlighting and learning how various parts of it are 
connected. That should make our future design decisions more informed.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-20 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> What is the goal here?

The eventual goal is for Sema to have appropriate layering instead of being a 
monolithic design. "Appropriate layering" can be handled in many different 
ways, but the goal when this was discussed extensively in the past was to split 
by "category" where some categories are language-based (splitting OpenACC from 
HLSL from Objective-C, etc) and some categories are feature-based (splitting 
attributes from coroutines from constant expression evaluation).

Clang is designed to be a library and our semantic layer is... not. So the goal 
is to eventually partition these into separable concerns so it is easier to 
maintain and so some parts can be reused (for example, constant expression 
evaluation could be helpful for clang-tidy while still not needing to expose 
the entirety of Sema to achieve it). This is obviously a very long-term goal, 
and also a bit of a research effort in terms of where to draw the line for such 
a monolithic part of the compiler.

> We had buy-in to split languages out and I think that has been successful 
> (with painful churns).

FWIW, I have not received any reports about churn being painful from 
downstreams. This was discussed at a previous Clang Language WG meeting and the 
only pain this has caused that I'm aware of has been the usual "it's hard to 
track against ToT" variety that we get with any significant maintenance of the 
project. If there are worse problems than that, it would be good to get details 
about them.

We need the ability to maintain and evolve Clang, so fear of churn without 
evidence of harm is not a blocking concern. That said, if there's some concrete 
reason the churn outweighs the benefits of the split, that is a different 
situation.

> Splitting out individual C++ features or even C from C++ is a different 
> proposition all together because these things are naturally interweaved, and 
> we quickly get diminishing returns (and increased churns), at the cost of 
> more clumsy APIs
(ie things like SemaAccess() everywhere and thing harder to find or organized)

I think this is part of the investigative work and I disagree with a blanket 
prohibition against splitting out features, at least at this early of a stage. 
IMO, we should evaluate the benefits for each component on their own merits and 
push back splitting up where it doesn't make sense to do so.

Specific to splitting out coroutines, I can see arguments in either direction. 
It's pretty trivial to split out because it's built "on top" of Sema rather 
than deeply integrated with it like C and C++ support are. However, it's not 
clear that it can ever be a reusable component and it's also not clear whether 
we want to go as far as what [Chuanqi's 
comment](https://github.com/llvm/llvm-project/pull/92645#issuecomment-2119530823)
 suggests in terms of layering. Given the concerns raised here, it might make 
sense to put this change on hold, do a few more similar experiments for other 
parts of C++, and see what sort of principled design approach makes sense to 
take here. My current thinking (which could be off-base!) is that for language 
features strongly tied to language semantics (like coroutines), if we do split 
them then it probably makes sense to split those off in a language-based 
approach (`getSema().CXX().Coroutines().whatever`) whereas more 
language-agnostic concepts (like constant expression evaluation) might make 
sense to split off into their own component whose language-specific behavior 
can be controlled by a traits object of some kind 
(`getSema().ConstExpr().whatever`).

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-20 Thread via cfe-commits

cor3ntin wrote:

> It was quite extensively discussed in 
> https://github.com/llvm/llvm-project/pull/84184, but it wasn't deemed 
> necessary to go through RFC process for a non-functional change that doesn't 
> affect stable interfaces.

We had buy-in to split languages out and I think that has been successful (with 
painful churns).
Splitting out individual C++ features or even C from C++ is a different 
proposition all together because these things are naturally interweaved, and we 
quickly get diminishing returns (and increased churns), at the cost of more 
clumsy APIs
(ie things like `SemaAccess()`  _everywhere_ and thing harder to find or 
organized)

I think there are a lots of opportunities to remove some utilities objects out 
of Sema.h, move pragma handling etc which should be a bit more self contained 
before considering doing more surgery on C++ (and at some point we should pause 
and consider the impact of already done work)
There are a lot of architectures specific functions that we might be able to 
move, if we want to?




https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-19 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> While I am not against the idea about splitting Sema, the implementation 
> detail makes me slightly concerning. What I had in mind is to split SemaC, 
> SemaCXX (this can be derived class of SemaCXX), SemaObjC and other dialects 
> (e.g., ACC or CUDA) out of Sema.

Inheritance is problematic, because it can't work off forward declarations of 
base classes. I believe there is still hope that we can reduce the blast radius 
of header changes in terms of TUs being recompiled.

Also note that language dialects you have in mind have already been moved 
outside of `Sema`.

> Then big features of C++ can be member of SemaCXX as SemaCoroutine, 
> SemaConcept...

This would mean even more hierarchy than just `Sema` containing a flat list of 
references to its parts. I'm not particularly opposed to it, but it's not on my 
radar at the moment. It would impact usage side, e.g. 
`Actions.CXX().Coroutine().ActOnCoawaitExpr()`, and I can see why people would 
be opposed to it.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-19 Thread Chuanqi Xu via cfe-commits

ChuanqiXu9 wrote:

While I am not against the idea about splitting Sema, the implementation detail 
makes me slightly concerning. What I had in mind is to split `SemaC`, `SemaCXX` 
(this can be derived class of `SemaCXX`), `SemaObjC` and other dialects (e.g., 
ACC or CUDA) out of `Sema`. Then big features of C++ can be member of `SemaCXX` 
as `SemaCoroutine`, `SemaConcept`...

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-19 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> Was there ever an RFC on this whole 'splitting up Sema' project?

It was quite extensively discussed in #84184, but it wasn't deemed necessary to 
go through RFC process for a non-functional change that doesn't affect stable 
interfaces.

> I think starting up from the easier bits is a risky proposition, as we might 
> realize this whole thing will fail when we get to the harder parts.

`Sema` has grown so big and interconnected that it's virtually impossible to 
wrap a head around it to solve hard problems first. 

> I also think this can lead in the future to the same sort of problems 
> overusing const got us in the first place: needing large amount of repetitive 
> changes all around when a commonly used or deeply nested component starts 
> depending on a larger chunk of Sema.

I recently looked into our `const` situation, and I definitely agree that it's 
overused. I believe one of the reasons it's hard to address systematically at 
the `Sema` level is again its size and tight coupling of everything to 
everything, which makes it hard to figure out "leaves" of call chains to start 
to rectify the situation without viral changes.

> Also, this will further cement current design lines, without considering 
> where we want to be in the future.

I believe cemented design line between `Sema` and `Parser` served us well, even 
if it's limiting sometimes. That said, changes I'm making doesn't really cement 
anything yet. Everything is still available from everywhere (save for static 
functions). The difference is that dependencies between parts of `Sema` become 
more visible.

> And lastly, it seems this will lead to large amounts of code churn without 
> proportional benefit, impacting in-tree development as well as external 
> projects.

I believe that we're significantly improving maintainability here, and the 
churn is worth it in the long run. For instance, #82217 took me whopping 20-25 
hours of non-stop work to finish. Doing that in one step was requested by 
downstreams.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-19 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

Was there ever an RFC on this whole 'splitting up Sema' project?

I have my own reservations as well.

I think starting up from the easier bits is a risky proposition, as we might 
realize this whole thing will fail when we get to the harder parts.

I also think this can lead in the future to the same sort of problems overusing 
const got us in the first place: needing large amount of repetitive changes all 
around when a commonly used or deeply nested component starts depending on a 
larger chunk of Sema.

Also, this will further cement current design lines, without considering where 
we want to be in the future.

And lastly, it seems this will lead to large amounts of code churn without 
proportional benefit, impacting in-tree development as well as external 
projects.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-19 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll updated 
https://github.com/llvm/llvm-project/pull/92645

>From 4fe09a3411e54561857d2f9d8c1808cb2728e299 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Sat, 18 May 2024 13:33:04 +0300
Subject: [PATCH 1/3] [clang] Introduce `SemaCoroutine`

---
 clang/include/clang/Sema/Sema.h  |  58 +-
 clang/include/clang/Sema/SemaCoroutine.h |  73 
 clang/lib/Parse/ParseExpr.cpp|   3 +-
 clang/lib/Parse/ParseExprCXX.cpp |   3 +-
 clang/lib/Parse/ParseStmt.cpp|   3 +-
 clang/lib/Sema/Sema.cpp  |   4 +-
 clang/lib/Sema/SemaCoroutine.cpp | 218 ---
 clang/lib/Sema/SemaDecl.cpp  |  16 +-
 clang/lib/Sema/SemaStmt.cpp  |   9 +-
 clang/lib/Sema/TreeTransform.h   |  25 +--
 10 files changed, 227 insertions(+), 185 deletions(-)
 create mode 100644 clang/include/clang/Sema/SemaCoroutine.h

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d4d4a82525a02..1d0fbeacfe061 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -168,6 +168,7 @@ class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
 class SemaCodeCompletion;
+class SemaCoroutine;
 class SemaCUDA;
 class SemaHLSL;
 class SemaObjC;
@@ -989,6 +990,11 @@ class Sema final : public SemaBase {
 return *CodeCompletionPtr;
   }
 
+  SemaCoroutine () {
+assert(CoroutinePtr);
+return *CoroutinePtr;
+  }
+
   SemaCUDA () {
 assert(CUDAPtr);
 return *CUDAPtr;
@@ -1050,6 +1056,7 @@ class Sema final : public SemaBase {
   mutable IdentifierInfo *Ident_super;
 
   std::unique_ptr CodeCompletionPtr;
+  std::unique_ptr CoroutinePtr;
   std::unique_ptr CUDAPtr;
   std::unique_ptr HLSLPtr;
   std::unique_ptr ObjCPtr;
@@ -2267,57 +2274,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name C++ Coroutines
-  /// Implementations are in SemaCoroutine.cpp
-  ///@{
-
-public:
-  /// The C++ "std::coroutine_traits" template, which is defined in
-  /// \
-  ClassTemplateDecl *StdCoroutineTraitsCache;
-
-  bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
-   StringRef Keyword);
-  ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
-
-  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
-  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
-  UnresolvedLookupExpr *Lookup);
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-  Expr *Awaiter, bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-UnresolvedLookupExpr *Lookup);
-  ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
-  StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
-   bool IsImplicit = false);
-  StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
-  bool buildCoroutineParameterMoves(SourceLocation Loc);
-  VarDecl *buildCoroutinePromise(SourceLocation Loc);
-  void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
-
-  // As a clang extension, enforces that a non-coroutine function must be 
marked
-  // with [[clang::coro_wrapper]] if it returns a type marked with
-  // [[clang::coro_return_type]].
-  // Expects that FD is not a coroutine.
-  void CheckCoroutineWrapper(FunctionDecl *FD);
-  /// Lookup 'coroutine_traits' in std namespace and std::experimental
-  /// namespace. The namespace found is recorded in Namespace.
-  ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
-   SourceLocation FuncLoc);
-  /// Check that the expression co_await promise.final_suspend() shall not be
-  /// potentially-throwing.
-  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
-
-  ///@}
-
-  //
-  //
-  // -
-  //
-  //
-
   /// \name C++ Scope Specifiers
   /// Implementations are in SemaCXXScopeSpec.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaCoroutine.h 
b/clang/include/clang/Sema/SemaCoroutine.h
new file mode 100644
index 0..20f1fffc970f8
--- /dev/null
+++ b/clang/include/clang/Sema/SemaCoroutine.h
@@ -0,0 +1,73 @@
+//===- SemaCUDA.h - Semantic Analysis for C++20 coroutines 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file declares semantic 

[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll updated 
https://github.com/llvm/llvm-project/pull/92645

>From 4fe09a3411e54561857d2f9d8c1808cb2728e299 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Sat, 18 May 2024 13:33:04 +0300
Subject: [PATCH 1/3] [clang] Introduce `SemaCoroutine`

---
 clang/include/clang/Sema/Sema.h  |  58 +-
 clang/include/clang/Sema/SemaCoroutine.h |  73 
 clang/lib/Parse/ParseExpr.cpp|   3 +-
 clang/lib/Parse/ParseExprCXX.cpp |   3 +-
 clang/lib/Parse/ParseStmt.cpp|   3 +-
 clang/lib/Sema/Sema.cpp  |   4 +-
 clang/lib/Sema/SemaCoroutine.cpp | 218 ---
 clang/lib/Sema/SemaDecl.cpp  |  16 +-
 clang/lib/Sema/SemaStmt.cpp  |   9 +-
 clang/lib/Sema/TreeTransform.h   |  25 +--
 10 files changed, 227 insertions(+), 185 deletions(-)
 create mode 100644 clang/include/clang/Sema/SemaCoroutine.h

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d4d4a82525a02..1d0fbeacfe061 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -168,6 +168,7 @@ class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
 class SemaCodeCompletion;
+class SemaCoroutine;
 class SemaCUDA;
 class SemaHLSL;
 class SemaObjC;
@@ -989,6 +990,11 @@ class Sema final : public SemaBase {
 return *CodeCompletionPtr;
   }
 
+  SemaCoroutine () {
+assert(CoroutinePtr);
+return *CoroutinePtr;
+  }
+
   SemaCUDA () {
 assert(CUDAPtr);
 return *CUDAPtr;
@@ -1050,6 +1056,7 @@ class Sema final : public SemaBase {
   mutable IdentifierInfo *Ident_super;
 
   std::unique_ptr CodeCompletionPtr;
+  std::unique_ptr CoroutinePtr;
   std::unique_ptr CUDAPtr;
   std::unique_ptr HLSLPtr;
   std::unique_ptr ObjCPtr;
@@ -2267,57 +2274,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name C++ Coroutines
-  /// Implementations are in SemaCoroutine.cpp
-  ///@{
-
-public:
-  /// The C++ "std::coroutine_traits" template, which is defined in
-  /// \
-  ClassTemplateDecl *StdCoroutineTraitsCache;
-
-  bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
-   StringRef Keyword);
-  ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
-
-  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
-  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
-  UnresolvedLookupExpr *Lookup);
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-  Expr *Awaiter, bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-UnresolvedLookupExpr *Lookup);
-  ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
-  StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
-   bool IsImplicit = false);
-  StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
-  bool buildCoroutineParameterMoves(SourceLocation Loc);
-  VarDecl *buildCoroutinePromise(SourceLocation Loc);
-  void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
-
-  // As a clang extension, enforces that a non-coroutine function must be 
marked
-  // with [[clang::coro_wrapper]] if it returns a type marked with
-  // [[clang::coro_return_type]].
-  // Expects that FD is not a coroutine.
-  void CheckCoroutineWrapper(FunctionDecl *FD);
-  /// Lookup 'coroutine_traits' in std namespace and std::experimental
-  /// namespace. The namespace found is recorded in Namespace.
-  ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
-   SourceLocation FuncLoc);
-  /// Check that the expression co_await promise.final_suspend() shall not be
-  /// potentially-throwing.
-  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
-
-  ///@}
-
-  //
-  //
-  // -
-  //
-  //
-
   /// \name C++ Scope Specifiers
   /// Implementations are in SemaCXXScopeSpec.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaCoroutine.h 
b/clang/include/clang/Sema/SemaCoroutine.h
new file mode 100644
index 0..20f1fffc970f8
--- /dev/null
+++ b/clang/include/clang/Sema/SemaCoroutine.h
@@ -0,0 +1,73 @@
+//===- SemaCUDA.h - Semantic Analysis for C++20 coroutines 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file declares semantic 

[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread via cfe-commits

https://github.com/cor3ntin requested changes to this pull request.

As said in previous comments, I don't think this is the right direction

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> What is the goal here? there is a lot of code reuse in C++ and C. and having 
> a file per feature is going to make thing worse (maintenance, unboarding, 
> compile times).

Maintenance: one big blob of code which is `Sema` makes it virtually impossible 
to approach long-standing const-correctness issue in a systematic way, because 
enormous flat list of member functions doesn't help you identify the leaves of 
dependency chains to start there.

Onboarding: this effort started with an impression I had that it's almost 
impossible to wrap your head around `Sema` because of its size. This was an 
onboarding problem for me.

Compile times: per measurements taken in 
https://github.com/llvm/llvm-project/pull/84184#issuecomment-1984528598, 
indirection that splitting of `Sema` introduces doesn't have a noticeable 
impact on both our compile times and runtime performance.

> There are a lot more low-hamging fruits we should do before considering that 
> sort of split.

> Wasm, AMD, Builtins, pragmas, random objects which could be in headers ( 
> NestedNameSpecInfo, ContextRAII, NameClassification, AccessCheckingSFINAE, 
> InstantiatingTemplate, SynthesizedFunctionScope, etc - I would recommend 
> using Aliases so they are still functionally in Sema)

Time for that will come. Irrespective of the order we move stuff out of `Sema`, 
the hard parts (basically all the code for C++98 and, likely, C++11) will 
remain, because we haven't come with a plan for them yet. And those will be 
minor wins in terms of volume of code, because OpenMP miracle is unlikely to 
manifest again (when I was able to reduce `Sema.h` by almost 1k LoC).

C++ features not present in C++98 were identified as potential low-hanging 
fruits, because they are implemented on top of what was already there, and 
coroutines were indeed rather easy to factor out. C++20 modules are definitely 
not a low-hanging fruit, though.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread via cfe-commits

cor3ntin wrote:

What is the goal here? there is a lot of code reuse in C++ and C. and having a 
file per feature is going to make thing worse (maintenance, unboarding, compile 
times).
There are a lot more low-hamging fruits we should do before considering that 
sort of split.

Wasm, AMD, Builtins,  pragmas, random objects which could be in headers ( 
NestedNameSpecInfo, ContextRAII, NameClassification, AccessCheckingSFINAE,  
InstantiatingTemplate, SynthesizedFunctionScope, etc - I would recommend using 
Aliases so they are still functionally in Sema)

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

As @erichkeane described it, we're clearing the woods of "core" C++ 
functionality before dealing with the hard parts, like `SemaDeclCXX.cpp` or 
`SemaChecking.cpp`.
Actual LoC wins are secondary, as they have been all along.

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread via cfe-commits

cor3ntin wrote:

Thanks! However, I am fairly against moving C and C++ features out of Sema, 
especially when the outcome is 50 Loc kess, at the cost of not knowing where to 
put or find things in the long term (as we add coroutines adjacent features, or 
code related to both coroutines and something else).

The previous sema changes made sense (despite the inevitable churn) - I am not 
sure this one does

https://github.com/llvm/llvm-project/pull/92645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll updated 
https://github.com/llvm/llvm-project/pull/92645

>From 4fe09a3411e54561857d2f9d8c1808cb2728e299 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Sat, 18 May 2024 13:33:04 +0300
Subject: [PATCH 1/3] [clang] Introduce `SemaCoroutine`

---
 clang/include/clang/Sema/Sema.h  |  58 +-
 clang/include/clang/Sema/SemaCoroutine.h |  73 
 clang/lib/Parse/ParseExpr.cpp|   3 +-
 clang/lib/Parse/ParseExprCXX.cpp |   3 +-
 clang/lib/Parse/ParseStmt.cpp|   3 +-
 clang/lib/Sema/Sema.cpp  |   4 +-
 clang/lib/Sema/SemaCoroutine.cpp | 218 ---
 clang/lib/Sema/SemaDecl.cpp  |  16 +-
 clang/lib/Sema/SemaStmt.cpp  |   9 +-
 clang/lib/Sema/TreeTransform.h   |  25 +--
 10 files changed, 227 insertions(+), 185 deletions(-)
 create mode 100644 clang/include/clang/Sema/SemaCoroutine.h

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d4d4a82525a02..1d0fbeacfe061 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -168,6 +168,7 @@ class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
 class SemaCodeCompletion;
+class SemaCoroutine;
 class SemaCUDA;
 class SemaHLSL;
 class SemaObjC;
@@ -989,6 +990,11 @@ class Sema final : public SemaBase {
 return *CodeCompletionPtr;
   }
 
+  SemaCoroutine () {
+assert(CoroutinePtr);
+return *CoroutinePtr;
+  }
+
   SemaCUDA () {
 assert(CUDAPtr);
 return *CUDAPtr;
@@ -1050,6 +1056,7 @@ class Sema final : public SemaBase {
   mutable IdentifierInfo *Ident_super;
 
   std::unique_ptr CodeCompletionPtr;
+  std::unique_ptr CoroutinePtr;
   std::unique_ptr CUDAPtr;
   std::unique_ptr HLSLPtr;
   std::unique_ptr ObjCPtr;
@@ -2267,57 +2274,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name C++ Coroutines
-  /// Implementations are in SemaCoroutine.cpp
-  ///@{
-
-public:
-  /// The C++ "std::coroutine_traits" template, which is defined in
-  /// \
-  ClassTemplateDecl *StdCoroutineTraitsCache;
-
-  bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
-   StringRef Keyword);
-  ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
-
-  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
-  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
-  UnresolvedLookupExpr *Lookup);
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-  Expr *Awaiter, bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-UnresolvedLookupExpr *Lookup);
-  ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
-  StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
-   bool IsImplicit = false);
-  StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
-  bool buildCoroutineParameterMoves(SourceLocation Loc);
-  VarDecl *buildCoroutinePromise(SourceLocation Loc);
-  void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
-
-  // As a clang extension, enforces that a non-coroutine function must be 
marked
-  // with [[clang::coro_wrapper]] if it returns a type marked with
-  // [[clang::coro_return_type]].
-  // Expects that FD is not a coroutine.
-  void CheckCoroutineWrapper(FunctionDecl *FD);
-  /// Lookup 'coroutine_traits' in std namespace and std::experimental
-  /// namespace. The namespace found is recorded in Namespace.
-  ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
-   SourceLocation FuncLoc);
-  /// Check that the expression co_await promise.final_suspend() shall not be
-  /// potentially-throwing.
-  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
-
-  ///@}
-
-  //
-  //
-  // -
-  //
-  //
-
   /// \name C++ Scope Specifiers
   /// Implementations are in SemaCXXScopeSpec.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaCoroutine.h 
b/clang/include/clang/Sema/SemaCoroutine.h
new file mode 100644
index 0..20f1fffc970f8
--- /dev/null
+++ b/clang/include/clang/Sema/SemaCoroutine.h
@@ -0,0 +1,73 @@
+//===- SemaCUDA.h - Semantic Analysis for C++20 coroutines 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file declares semantic 

[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-coroutines

Author: Vlad Serebrennikov (Endilll)


Changes

This patch moves coroutines-specific `Sema` functions into the new 
`SemaCoroutine` class. This continues previous efforts to split `Sema` up. 
Additional context can be found in #84184.
As usual, in order to help reviewing this, formatting changes are split into a 
separate commit.

---

Patch is 48.46 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/92645.diff


10 Files Affected:

- (modified) clang/include/clang/Sema/Sema.h (+7-51) 
- (added) clang/include/clang/Sema/SemaCoroutine.h (+73) 
- (modified) clang/lib/Parse/ParseExpr.cpp (+3-1) 
- (modified) clang/lib/Parse/ParseExprCXX.cpp (+2-1) 
- (modified) clang/lib/Parse/ParseStmt.cpp (+3-1) 
- (modified) clang/lib/Sema/Sema.cpp (+4-2) 
- (modified) clang/lib/Sema/SemaCoroutine.cpp (+153-120) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+3-13) 
- (modified) clang/lib/Sema/SemaStmt.cpp (+6-5) 
- (modified) clang/lib/Sema/TreeTransform.h (+17-14) 


``diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d4d4a82525a02..1d0fbeacfe061 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -168,6 +168,7 @@ class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
 class SemaCodeCompletion;
+class SemaCoroutine;
 class SemaCUDA;
 class SemaHLSL;
 class SemaObjC;
@@ -989,6 +990,11 @@ class Sema final : public SemaBase {
 return *CodeCompletionPtr;
   }
 
+  SemaCoroutine () {
+assert(CoroutinePtr);
+return *CoroutinePtr;
+  }
+
   SemaCUDA () {
 assert(CUDAPtr);
 return *CUDAPtr;
@@ -1050,6 +1056,7 @@ class Sema final : public SemaBase {
   mutable IdentifierInfo *Ident_super;
 
   std::unique_ptr CodeCompletionPtr;
+  std::unique_ptr CoroutinePtr;
   std::unique_ptr CUDAPtr;
   std::unique_ptr HLSLPtr;
   std::unique_ptr ObjCPtr;
@@ -2267,57 +2274,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name C++ Coroutines
-  /// Implementations are in SemaCoroutine.cpp
-  ///@{
-
-public:
-  /// The C++ "std::coroutine_traits" template, which is defined in
-  /// \
-  ClassTemplateDecl *StdCoroutineTraitsCache;
-
-  bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
-   StringRef Keyword);
-  ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
-
-  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
-  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
-  UnresolvedLookupExpr *Lookup);
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-  Expr *Awaiter, bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-UnresolvedLookupExpr *Lookup);
-  ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
-  StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
-   bool IsImplicit = false);
-  StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
-  bool buildCoroutineParameterMoves(SourceLocation Loc);
-  VarDecl *buildCoroutinePromise(SourceLocation Loc);
-  void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
-
-  // As a clang extension, enforces that a non-coroutine function must be 
marked
-  // with [[clang::coro_wrapper]] if it returns a type marked with
-  // [[clang::coro_return_type]].
-  // Expects that FD is not a coroutine.
-  void CheckCoroutineWrapper(FunctionDecl *FD);
-  /// Lookup 'coroutine_traits' in std namespace and std::experimental
-  /// namespace. The namespace found is recorded in Namespace.
-  ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
-   SourceLocation FuncLoc);
-  /// Check that the expression co_await promise.final_suspend() shall not be
-  /// potentially-throwing.
-  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
-
-  ///@}
-
-  //
-  //
-  // -
-  //
-  //
-
   /// \name C++ Scope Specifiers
   /// Implementations are in SemaCXXScopeSpec.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaCoroutine.h 
b/clang/include/clang/Sema/SemaCoroutine.h
new file mode 100644
index 0..d4ca6cb0d860a
--- /dev/null
+++ b/clang/include/clang/Sema/SemaCoroutine.h
@@ -0,0 +1,73 @@
+//===- SemaCUDA.h - Semantic Analysis for C++20 coroutines 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH 

[clang] [clang] Introduce `SemaCoroutine` (PR #92645)

2024-05-18 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll created 
https://github.com/llvm/llvm-project/pull/92645

This patch moves coroutines-specific `Sema` functions into the new 
`SemaCoroutine` class. This continues previous efforts to split `Sema` up. 
Additional context can be found in #84184.
As usual, in order to help reviewing this, formatting changes are split into a 
separate commit.

>From 4fe09a3411e54561857d2f9d8c1808cb2728e299 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Sat, 18 May 2024 13:33:04 +0300
Subject: [PATCH 1/2] [clang] Introduce `SemaCoroutine`

---
 clang/include/clang/Sema/Sema.h  |  58 +-
 clang/include/clang/Sema/SemaCoroutine.h |  73 
 clang/lib/Parse/ParseExpr.cpp|   3 +-
 clang/lib/Parse/ParseExprCXX.cpp |   3 +-
 clang/lib/Parse/ParseStmt.cpp|   3 +-
 clang/lib/Sema/Sema.cpp  |   4 +-
 clang/lib/Sema/SemaCoroutine.cpp | 218 ---
 clang/lib/Sema/SemaDecl.cpp  |  16 +-
 clang/lib/Sema/SemaStmt.cpp  |   9 +-
 clang/lib/Sema/TreeTransform.h   |  25 +--
 10 files changed, 227 insertions(+), 185 deletions(-)
 create mode 100644 clang/include/clang/Sema/SemaCoroutine.h

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d4d4a82525a02..1d0fbeacfe061 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -168,6 +168,7 @@ class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
 class SemaCodeCompletion;
+class SemaCoroutine;
 class SemaCUDA;
 class SemaHLSL;
 class SemaObjC;
@@ -989,6 +990,11 @@ class Sema final : public SemaBase {
 return *CodeCompletionPtr;
   }
 
+  SemaCoroutine () {
+assert(CoroutinePtr);
+return *CoroutinePtr;
+  }
+
   SemaCUDA () {
 assert(CUDAPtr);
 return *CUDAPtr;
@@ -1050,6 +1056,7 @@ class Sema final : public SemaBase {
   mutable IdentifierInfo *Ident_super;
 
   std::unique_ptr CodeCompletionPtr;
+  std::unique_ptr CoroutinePtr;
   std::unique_ptr CUDAPtr;
   std::unique_ptr HLSLPtr;
   std::unique_ptr ObjCPtr;
@@ -2267,57 +2274,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name C++ Coroutines
-  /// Implementations are in SemaCoroutine.cpp
-  ///@{
-
-public:
-  /// The C++ "std::coroutine_traits" template, which is defined in
-  /// \
-  ClassTemplateDecl *StdCoroutineTraitsCache;
-
-  bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
-   StringRef Keyword);
-  ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
-  StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
-
-  ExprResult BuildOperatorCoawaitLookupExpr(Scope *S, SourceLocation Loc);
-  ExprResult BuildOperatorCoawaitCall(SourceLocation Loc, Expr *E,
-  UnresolvedLookupExpr *Lookup);
-  ExprResult BuildResolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-  Expr *Awaiter, bool IsImplicit = false);
-  ExprResult BuildUnresolvedCoawaitExpr(SourceLocation KwLoc, Expr *Operand,
-UnresolvedLookupExpr *Lookup);
-  ExprResult BuildCoyieldExpr(SourceLocation KwLoc, Expr *E);
-  StmtResult BuildCoreturnStmt(SourceLocation KwLoc, Expr *E,
-   bool IsImplicit = false);
-  StmtResult BuildCoroutineBodyStmt(CoroutineBodyStmt::CtorArgs);
-  bool buildCoroutineParameterMoves(SourceLocation Loc);
-  VarDecl *buildCoroutinePromise(SourceLocation Loc);
-  void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
-
-  // As a clang extension, enforces that a non-coroutine function must be 
marked
-  // with [[clang::coro_wrapper]] if it returns a type marked with
-  // [[clang::coro_return_type]].
-  // Expects that FD is not a coroutine.
-  void CheckCoroutineWrapper(FunctionDecl *FD);
-  /// Lookup 'coroutine_traits' in std namespace and std::experimental
-  /// namespace. The namespace found is recorded in Namespace.
-  ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
-   SourceLocation FuncLoc);
-  /// Check that the expression co_await promise.final_suspend() shall not be
-  /// potentially-throwing.
-  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
-
-  ///@}
-
-  //
-  //
-  // -
-  //
-  //
-
   /// \name C++ Scope Specifiers
   /// Implementations are in SemaCXXScopeSpec.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaCoroutine.h 
b/clang/include/clang/Sema/SemaCoroutine.h
new file mode 100644
index 0..20f1fffc970f8
--- /dev/null
+++ b/clang/include/clang/Sema/SemaCoroutine.h
@@ -0,0 +1,73 @@
+//===- SemaCUDA.h - Semantic Analysis for C++20 coroutines 
===//
+//
+// Part of the LLVM Project, under the Apache