[OpenACC] Implement Sema work for OpenACC Clauses

[erichkeane]

Now that we have AST nodes for OpenACC Clauses, this patch adds their creation to Sema and makes the Parser call all the required functions. This also redoes TreeTransform to work with the clauses/make sure they are transformed.

Much of this is NFC, since there is no clause we can test this behavior with. However, there IS one noticable change; we are now no longer diagnosing that a clause is 'not implemented' unless it there was no errors parsing its parameters. This is because it cleans up how we create and diagnose clauses.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

Now that we have AST nodes for OpenACC Clauses, this patch adds their creation to Sema and makes the Parser call all the required functions. This also redoes TreeTransform to work with the clauses/make sure they are transformed.

Much of this is NFC, since there is no clause we can test this behavior with. However, there IS one noticable change; we are now no longer diagnosing that a clause is 'not implemented' unless it there was no errors parsing its parameters. This is because it cleans up how we create and diagnose clauses.

---

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

9 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
• (modified) clang/include/clang/Parse/Parser.h (+28-6)
• (modified) clang/include/clang/Sema/SemaOpenACC.h (+37-2)
• (modified) clang/lib/Parse/ParseOpenACC.cpp (+89-53)
• (modified) clang/lib/Sema/SemaOpenACC.cpp (+53-16)
• (modified) clang/lib/Sema/TreeTransform.h (+64-15)
• (modified) clang/test/ParserOpenACC/parse-clauses.c (+223-350)
• (modified) clang/test/ParserOpenACC/parse-clauses.cpp (+3-6)
• (modified) clang/test/ParserOpenACC/parse-wait-clause.c (+41-61)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index df57f5e6ce11ba..238931770da3a2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12252,6 +12252,8 @@ def warn_acc_clause_unimplemented
 def err_acc_construct_appertainment
     : Error<"OpenACC construct '%0' cannot be used here; it can only "
             "be used in a statement context">;
+def err_acc_clause_appertainment
+    : Error<"OpenACC '%1' clause not valid on '%0' directive">;
 def err_acc_branch_in_out_compute_construct
     : Error<"invalid %select{branch|return|throw}0 %select{out of|into}1 "
             "OpenACC Compute Construct">;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 580bf2a5d79df5..8bc929b1dfe4bb 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -41,6 +41,7 @@ namespace clang {
   class InMessageExpressionRAIIObject;
   class PoisonSEHIdentifiersRAIIObject;
   class OMPClause;
+  class OpenACCClause;
   class ObjCTypeParamList;
   struct OMPTraitProperty;
   struct OMPTraitSelector;
@@ -3594,11 +3595,26 @@ class Parser : public CodeCompletionHandler {
     OpenACCDirectiveKind DirKind;
     SourceLocation StartLoc;
     SourceLocation EndLoc;
-    // TODO OpenACC: Add Clause list here once we have a type for that.
+    SmallVector<OpenACCClause *> Clauses;
     // TODO OpenACC: As we implement support for the Atomic, Routine, Cache, and
     // Wait constructs, we likely want to put that information in here as well.
   };
 
+  /// Represents the 'error' state of parsing an OpenACC Clause, and stores
+  /// whether we can continue parsing, or should give up on the directive.
+  enum class OpenACCParseCanContinue { Cannot = 0, Can = 1 };
+
+  /// A type to represent the state of parsing an OpenACC Clause. Situations
+  /// that result in an OpenACCClause pointer are a success and can continue
+  /// parsing, however some other situations can also continue.
+  /// FIXME: This is better represented as a std::expected when we get C++23.
+  using OpenACCClauseParseResult =
+      llvm::PointerIntPair<OpenACCClause *, 1, OpenACCParseCanContinue>;
+
+  OpenACCClauseParseResult OpenACCCanContinue();
+  OpenACCClauseParseResult OpenACCCannotContinue();
+  OpenACCClauseParseResult OpenACCSuccess(OpenACCClause *Clause);
+
   /// Parses the OpenACC directive (the entire pragma) including the clause
   /// list, but does not produce the main AST node.
   OpenACCDirectiveParseInfo ParseOpenACCDirective();
@@ -3613,12 +3629,18 @@ class Parser : public CodeCompletionHandler {
   bool ParseOpenACCClauseVarList(OpenACCClauseKind Kind);
   /// Parses any parameters for an OpenACC Clause, including required/optional
   /// parens.
-  bool ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
-                                OpenACCClauseKind Kind);
-  /// Parses a single clause in a clause-list for OpenACC.
-  bool ParseOpenACCClause(OpenACCDirectiveKind DirKind);
+  OpenACCClauseParseResult
+  ParseOpenACCClauseParams(ArrayRef<const OpenACCClause *> ExistingClauses,
+                           OpenACCDirectiveKind DirKind, OpenACCClauseKind Kind,
+                           SourceLocation ClauseLoc);
+  /// Parses a single clause in a clause-list for OpenACC. Returns nullptr on
+  /// error.
+  OpenACCClauseParseResult
+  ParseOpenACCClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                     OpenACCDirectiveKind DirKind);
   /// Parses the clause-list for an OpenACC directive.
-  void ParseOpenACCClauseList(OpenACCDirectiveKind DirKind);
+  SmallVector<OpenACCClause *>
+  ParseOpenACCClauseList(OpenACCDirectiveKind DirKind);
   bool ParseOpenACCWaitArgument();
   /// Parses the clause of the 'bind' argument, which can be a string literal or
   /// an ID expression.
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index 7f50d7889ad79b..7eaf03f5be7bd8 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -28,6 +28,37 @@ class Sema;
 
 class SemaOpenACC {
 public:
+  /// A type to represent all the data for an OpenACC Clause that has been
+  /// parsed, but not yet created/semantically analyzed. This is effectively a
+  /// discriminated union on the 'Clause Kind', with all of the individual
+  /// clause details stored in a std::variant.
+  class OpenACCParsedClause {
+    OpenACCDirectiveKind DirKind;
+    OpenACCClauseKind ClauseKind;
+    SourceRange ClauseRange;
+    SourceLocation LParenLoc;
+
+    // TODO OpenACC: Add variant here to store details of individual clauses.
+
+  public:
+    OpenACCParsedClause(OpenACCDirectiveKind DirKind,
+                        OpenACCClauseKind ClauseKind, SourceLocation BeginLoc)
+        : DirKind(DirKind), ClauseKind(ClauseKind), ClauseRange(BeginLoc, {}) {}
+
+    OpenACCDirectiveKind getDirectiveKind() const { return DirKind; }
+
+    OpenACCClauseKind getClauseKind() const { return ClauseKind; }
+
+    SourceLocation getBeginLoc() const { return ClauseRange.getBegin(); }
+
+    SourceLocation getLParenLoc() const { return LParenLoc; }
+
+    SourceLocation getEndLoc() const { return ClauseRange.getEnd(); }
+
+    void setLParenLoc(SourceLocation EndLoc) { LParenLoc = EndLoc; }
+    void setEndLoc(SourceLocation EndLoc) { ClauseRange.setEnd(EndLoc); }
+  };
+
   SemaOpenACC(Sema &S);
 
   ASTContext &getASTContext() const;
@@ -37,7 +68,8 @@ class SemaOpenACC {
   Sema &SemaRef;
 
   /// Called after parsing an OpenACC Clause so that it can be checked.
-  bool ActOnClause(OpenACCClauseKind ClauseKind, SourceLocation StartLoc);
+  OpenACCClause *ActOnClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                             OpenACCParsedClause &Clause);
 
   /// Called after the construct has been parsed, but clauses haven't been
   /// parsed.  This allows us to diagnose not-implemented, as well as set up any
@@ -63,7 +95,10 @@ class SemaOpenACC {
   /// declaration group or associated statement.
   StmtResult ActOnEndStmtDirective(OpenACCDirectiveKind K,
                                    SourceLocation StartLoc,
-                                   SourceLocation EndLoc, StmtResult AssocStmt);
+                                   SourceLocation EndLoc,
+                                   MutableArrayRef<OpenACCClause *> Clauses,
+                                   StmtResult AssocStmt);
+
   /// Called after the directive has been completely parsed, including the
   /// declaration group or associated statement.
   DeclGroupRef ActOnEndDeclDirective();
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 07dd2ba0106a4e..374984735556d8 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/OpenACCClause.h"
 #include "clang/Basic/OpenACCKinds.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/Parser.h"
@@ -582,12 +583,26 @@ unsigned getOpenACCScopeFlags(OpenACCDirectiveKind DirKind) {
 
 } // namespace
 
+Parser::OpenACCClauseParseResult Parser::OpenACCCanContinue() {
+  return {nullptr, OpenACCParseCanContinue::Can};
+}
+
+Parser::OpenACCClauseParseResult Parser::OpenACCCannotContinue() {
+  return {nullptr, OpenACCParseCanContinue::Cannot};
+}
+
+Parser::OpenACCClauseParseResult Parser::OpenACCSuccess(OpenACCClause *Clause) {
+  return {Clause, OpenACCParseCanContinue::Can};
+}
+
 // OpenACC 3.3, section 1.7:
 // To simplify the specification and convey appropriate constraint information,
 // a pqr-list is a comma-separated list of pdr items. The one exception is a
 // clause-list, which is a list of one or more clauses optionally separated by
 // commas.
-void Parser::ParseOpenACCClauseList(OpenACCDirectiveKind DirKind) {
+SmallVector<OpenACCClause *>
+Parser::ParseOpenACCClauseList(OpenACCDirectiveKind DirKind) {
+  SmallVector<OpenACCClause *> Clauses;
   bool FirstClause = true;
   while (getCurToken().isNot(tok::annot_pragma_openacc_end)) {
     // Comma is optional in a clause-list.
@@ -595,13 +610,17 @@ void Parser::ParseOpenACCClauseList(OpenACCDirectiveKind DirKind) {
       ConsumeToken();
     FirstClause = false;
 
-    // Recovering from a bad clause is really difficult, so we just give up on
-    // error.
-    if (ParseOpenACCClause(DirKind)) {
+    OpenACCClauseParseResult Result = ParseOpenACCClause(Clauses, DirKind);
+    if (OpenACCClause *Clause = Result.getPointer()) {
+      Clauses.push_back(Clause);
+    } else if (Result.getInt() == OpenACCParseCanContinue::Cannot) {
+      // Recovering from a bad clause is really difficult, so we just give up on
+      // error.
       SkipUntilEndOfDirective(*this);
-      return;
+      return Clauses;
     }
   }
+  return Clauses;
 }
 
 ExprResult Parser::ParseOpenACCIntExpr() {
@@ -762,42 +781,48 @@ bool Parser::ParseOpenACCGangArgList() {
 // really have its owner grammar and each individual one has its own definition.
 // However, they all are named with a single-identifier (or auto/default!)
 // token, followed in some cases by either braces or parens.
-bool Parser::ParseOpenACCClause(OpenACCDirectiveKind DirKind) {
+Parser::OpenACCClauseParseResult
+Parser::ParseOpenACCClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                           OpenACCDirectiveKind DirKind) {
   // A number of clause names are actually keywords, so accept a keyword that
   // can be converted to a name.
   if (expectIdentifierOrKeyword(*this))
-    return true;
+    return OpenACCCannotContinue();
 
   OpenACCClauseKind Kind = getOpenACCClauseKind(getCurToken());
 
-  if (Kind == OpenACCClauseKind::Invalid)
-    return Diag(getCurToken(), diag::err_acc_invalid_clause)
-           << getCurToken().getIdentifierInfo();
+  if (Kind == OpenACCClauseKind::Invalid) {
+    Diag(getCurToken(), diag::err_acc_invalid_clause)
+        << getCurToken().getIdentifierInfo();
+    return OpenACCCannotContinue();
+  }
 
   // Consume the clause name.
   SourceLocation ClauseLoc = ConsumeToken();
 
-  bool Result = ParseOpenACCClauseParams(DirKind, Kind);
-  getActions().OpenACC().ActOnClause(Kind, ClauseLoc);
-  return Result;
+  return ParseOpenACCClauseParams(ExistingClauses, DirKind, Kind, ClauseLoc);
 }
 
-bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
-                                      OpenACCClauseKind Kind) {
+Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams(
+    ArrayRef<const OpenACCClause *> ExistingClauses,
+    OpenACCDirectiveKind DirKind, OpenACCClauseKind ClauseKind,
+    SourceLocation ClauseLoc) {
   BalancedDelimiterTracker Parens(*this, tok::l_paren,
                                   tok::annot_pragma_openacc_end);
+  SemaOpenACC::OpenACCParsedClause ParsedClause(DirKind, ClauseKind, ClauseLoc);
 
-  if (ClauseHasRequiredParens(DirKind, Kind)) {
+  if (ClauseHasRequiredParens(DirKind, ClauseKind)) {
+    ParsedClause.setLParenLoc(getCurToken().getLocation());
     if (Parens.expectAndConsume()) {
       // We are missing a paren, so assume that the person just forgot the
       // parameter.  Return 'false' so we try to continue on and parse the next
       // clause.
       SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openacc_end,
                 Parser::StopBeforeMatch);
-      return false;
+      return OpenACCCanContinue();
     }
 
-    switch (Kind) {
+    switch (ClauseKind) {
     case OpenACCClauseKind::Default: {
       Token DefKindTok = getCurToken();
 
@@ -818,34 +843,34 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
       // this clause list.
       if (CondExpr.isInvalid()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     }
     case OpenACCClauseKind::CopyIn:
       tryParseAndConsumeSpecialTokenKind(
-          *this, OpenACCSpecialTokenKind::ReadOnly, Kind);
-      if (ParseOpenACCClauseVarList(Kind)) {
+          *this, OpenACCSpecialTokenKind::ReadOnly, ClauseKind);
+      if (ParseOpenACCClauseVarList(ClauseKind)) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     case OpenACCClauseKind::Create:
     case OpenACCClauseKind::CopyOut:
       tryParseAndConsumeSpecialTokenKind(*this, OpenACCSpecialTokenKind::Zero,
-                                         Kind);
-      if (ParseOpenACCClauseVarList(Kind)) {
+                                         ClauseKind);
+      if (ParseOpenACCClauseVarList(ClauseKind)) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     case OpenACCClauseKind::Reduction:
       // If we're missing a clause-kind (or it is invalid), see if we can parse
       // the var-list anyway.
       ParseReductionOperator(*this);
-      if (ParseOpenACCClauseVarList(Kind)) {
+      if (ParseOpenACCClauseVarList(ClauseKind)) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     case OpenACCClauseKind::Self:
@@ -868,19 +893,19 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
     case OpenACCClauseKind::Present:
     case OpenACCClauseKind::Private:
     case OpenACCClauseKind::UseDevice:
-      if (ParseOpenACCClauseVarList(Kind)) {
+      if (ParseOpenACCClauseVarList(ClauseKind)) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     case OpenACCClauseKind::Collapse: {
       tryParseAndConsumeSpecialTokenKind(*this, OpenACCSpecialTokenKind::Force,
-                                         Kind);
+                                         ClauseKind);
       ExprResult NumLoops =
           getActions().CorrectDelayedTyposInExpr(ParseConstantExpression());
       if (NumLoops.isInvalid()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     }
@@ -888,7 +913,7 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
       ExprResult BindArg = ParseOpenACCBindClauseArgument();
       if (BindArg.isInvalid()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     }
@@ -900,7 +925,7 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
       ExprResult IntExpr = ParseOpenACCIntExpr();
       if (IntExpr.isInvalid()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     }
@@ -912,23 +937,28 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
         ConsumeToken();
       } else if (ParseOpenACCDeviceTypeList()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     case OpenACCClauseKind::Tile:
       if (ParseOpenACCSizeExprList()) {
         Parens.skipToEnd();
-        return false;
+        return OpenACCCanContinue();
       }
       break;
     default:
       llvm_unreachable("Not a required parens type?");
     }
 
-    return Parens.consumeClose();
-  } else if (ClauseHasOptionalParens(DirKind, Kind)) {
+    ParsedClause.setEndLoc(getCurToken().getLocation());
+
+    if (Parens.consumeClose())
+      return OpenACCCannotContinue();
+
+  } else if (ClauseHasOptionalParens(DirKind, ClauseKind)) {
+    ParsedClause.setLParenLoc(getCurToken().getLocation());
     if (!Parens.consumeOpen()) {
-      switch (Kind) {
+      switch (ClauseKind) {
       case OpenACCClauseKind::Self: {
         assert(DirKind != OpenACCDirectiveKind::Update);
         ExprResult CondExpr = ParseOpenACCConditionalExpr(*this);
@@ -936,21 +966,22 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
         // this clause list.
         if (CondExpr.isInvalid()) {
           Parens.skipToEnd();
-          return false;
+          return OpenACCCanContinue();
         }
         break;
       }
       case OpenACCClauseKind::Vector:
       case OpenACCClauseKind::Worker: {
         tryParseAndConsumeSpecialTokenKind(*this,
-                                           Kind == OpenACCClauseKind::Vector
+                                           ClauseKind ==
+                                                   OpenACCClauseKind::Vector
                                                ? OpenACCSpecialTokenKind::Length
                                                : OpenACCSpecialTokenKind::Num,
-                                           Kind);
+                                           ClauseKind);
         ExprResult IntExpr = ParseOpenACCIntExpr();
         if (IntExpr.isInvalid()) {
           Parens.skipToEnd();
-          return false;
+          return OpenACCCanContinue();
         }
         break;
       }
@@ -958,29 +989,32 @@ bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
         ExprResult AsyncArg = ParseOpenACCAsyncArgument();
         if (AsyncArg.isInvalid()) {
           Parens.skipToEnd();
-          return false;
+          return OpenACCCanContinue();
         }
         break;
       }
       case OpenACCClauseKind::Gang:
         if (ParseOpenACCGangArgList()) {
           Parens.skipToEnd();
-          return false;
+          return OpenACCCanContinue();
         }
         break;
       case OpenACCClauseKind::Wait:
         if (ParseOpenACCWaitArgument()) {
           Parens.skipToEnd();
-          return false;
+          return OpenACCCanContinue();
         }
         break;
       default:
         llvm_unreachable("Not an optional parens type?");
       }
-      Parens.consumeClose();
+      ParsedClause.setEndLoc(getCurToken().getLocation());
+      if (Parens.consumeClose())
+        return OpenACCCannotContinue();
     }
   }
-  return false;
+  return OpenACCSuccess(
+      Actions.OpenACC().ActOnClause(ExistingClauses, ParsedClause));
 }
 
 /// OpenACC 3.3 section 2.16:
@@ -1204,15 +1238,16 @@ Parser::OpenACCDirectiveParseInfo Parser::ParseOpenACCDirective() {
     Diag(Tok, diag::err_expected) << tok::l_paren;
   }
 
-  // Parses the list of clauses, if present.
-  ParseOpenACCClauseList(DirKind);
+  // Parses the list of clauses, if present, plus set up return value.
+  OpenACCDirectiveParseInfo ParseInfo{DirKind, StartLoc, SourceLocation{},
+                                      ParseOpenACCClauseList(DirKind)};
 
   assert(Tok.is(tok::annot_pragma_openacc_end) &&
          "Didn't parse all OpenACC Clauses");
-  SourceLocation EndLoc = ConsumeAnnotationToken();
-  assert(EndLoc.isValid());
+  ParseInfo.EndLoc = ConsumeAnnotationToken();
+  assert(ParseInfo.EndLoc.isValid());
 
-  return OpenACCDirectiveParseInfo{DirKind, StartLoc, EndLoc};
+  return ParseInfo;
 }
 
 // Parse OpenACC directive on a declaration.
@@ -1255,5 +1290,6 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
   }
 
   return getActions().OpenACC().ActOnEndStmtDirective(
-      DirInfo.DirKind, DirInfo.StartLoc, DirInfo.EndLoc, AssocStmt);
+      DirInfo.DirKind, DirInfo.StartLoc, DirInfo.EndLoc, DirInfo.Clauses,
+      AssocStmt);
 }
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 86ffa5ad74c130..cd91d4fb2b7925 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -37,6 +37,32 @@ bool diagnoseConstructAppertainment(SemaOpenACC &S, OpenACCDirectiveKind K,
   }
   return false;
 }
+
+bool doesClauseApplyToDirective(OpenACCDirectiveKind DirectiveKind,
+                                OpenACCClauseKind ClauseKind) {
+  switch (ClauseKind) {
+    // FIXME: For each clause as we implement them, we can add the
+    // 'legalization' list here.
+  default:
+    // Do nothing so we can go to the 'unimplemented' diagnostic instead.
+    return true;
+  }
+  llvm_unreachable("Invalid clause kind");
+}
+
+/// Destruct and deallocate any clauses that aren't going to be used because
+/// they don't have a Construct to attach to.
+void DestroyUnusedClauses(ASTContext &Ctx,
+                          MutableArrayRef<OpenACCClause *> Clauses) {
+  auto *Itr = Clauses.begin();
+  auto *End = Clauses.end();
+
+  for (; Itr != End; ++I...
[truncated]

[Endilll]

I hope #87634 will be merged soon, so you can get rid of SemaRef.Diag() here.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[alexey-bataev]

Why do you need to delete unused clauses? Maybe just keep them unused and just ignore?

[erichkeane]

It just seemed like a bad idea to 'leak' clauses that aren't added to the AST. Depending on how many 'bad' constructs there end up being, this could presumably be an unfortunate amount of memory.

[alexey-bataev]

Not sure this is a real issue. I think it is enough just to report the error (or warning) and just ignore the clauses in the codegen.

[erichkeane]

Well, there WOULD be no 'ignoring' in codegen, as these clauses won't be added to the AST at all. They'd be created, then just 'forgotten' if we don't add them to a construct.

[erichkeane]

I discussed this with Aaron who confirmed that because we're allocating in the ASTContext::Allocate pool that we're fine to skip destruction, so I'm just going to do that.

[alexey-bataev]

Do you have the test for this?

[erichkeane]

In this patch, no. The next patch (that actually adds a clause) does, but I wanted to split the patch up to make it easier for you to review. This one is already unfortunately large, despite it effectively being 'NFC'.

[alexey-bataev]

Better to move it to another patch, I think

[erichkeane]

The transforming of clauses? I can, it just makes the next patch 'larger'.

[erichkeane]

I'll remove the body of this from the next revision of this, just validating now.

[erichkeane]

Build failure is completely unrelated, but I think this does everything you've asked for @alexey-bataev . Let me know what you think!

[alexey-bataev]

LG with a nit

[alexey-bataev]

Add assertion message