[clang] [NFC] Factor out common parts of ArraySections into its own class (PR #89639)

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 11:54:42 PDT 2024


https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/89639

>From b95304b22915217ad149c1b22a6517c585067acf Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Mon, 22 Apr 2024 10:35:13 -0700
Subject: [PATCH 1/3] [NFC] Factor out common parts of ArraySections into its
 own class

OpenACC is going to need an array sections implementation that is a
simpler version/more restrictive version of the OpenMP version.  This
patch extracts the things that should be common between the two into a
template base class. This will allow a followup patch to implement an
OpenACC array sections expression type.
---
 clang/include/clang/AST/Expr.h       | 105 +++++++++++++++++++++++++++
 clang/include/clang/AST/ExprOpenMP.h |  74 ++++---------------
 2 files changed, 119 insertions(+), 60 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 2bfefeabc348be..3fc0170d1a0d8e 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6610,6 +6610,111 @@ class TypoExpr : public Expr {
 
 };
 
+// This is a sub-class for OpenMP and OpenACC array sections. OpenACC uses a
+// very small subset of the functionality, so this class only exposes the things
+// the two have in common. This type is not expected to be used directly,
+// instead it is inherited from by the OMP and OpenACC variants.
+//
+// This type does not capture a 'stride', only a lower-bound and length (plus
+// the base expression), but provides room via a template argument to get
+// additional ones.
+template<unsigned NumSubExprs, bool AllowNullExprs>
+class ArraySectionExprBase : public Expr {
+  enum {BASE, LOWER_BOUND, LENGTH, END_EXPR = NumSubExprs};
+  Stmt *SubExprs[END_EXPR];
+  SourceLocation ColonLocFirst;
+  SourceLocation RBracketLoc;
+
+  protected:
+    ArraySectionExprBase(StmtClass SC, Expr *Base, Expr *LowerBound,
+                         Expr *Length, QualType Type, ExprValueKind VK,
+                         ExprObjectKind OK, SourceLocation ColonLocFirst,
+                         SourceLocation RBracketLoc)
+        : Expr(SC, Type, VK, OK), ColonLocFirst(ColonLocFirst),
+          RBracketLoc(RBracketLoc) {
+      setBase(Base);
+      setLowerBound(LowerBound);
+      setLength(Length);
+    }
+
+    explicit ArraySectionExprBase(StmtClass SC, EmptyShell Shell)
+        : Expr(SC, Shell) {}
+
+    void setSubExpr(unsigned Idx, Expr *SubExpr) {
+      assert(Idx > LENGTH &&
+             "setting sub expression owned by ArraySectionExprBase: Should be "
+             "using the direct 'setter' functions");
+      assert((SubExpr || AllowNullExprs) && "Null expression when not allowed");
+      SubExprs[Idx] = SubExpr;
+    }
+
+    Expr *getSubExpr(unsigned Idx) {
+      assert(Idx > LENGTH &&
+             "getting sub expression owned by ArraySectionExprBase: Should be "
+             "using the direct 'getter' functions");
+      return cast_or_null<Expr>(SubExprs[Idx]);
+    }
+    const Expr *getSubExpr(unsigned Idx) const {
+      assert(Idx > LENGTH &&
+             "getting sub expression owned by ArraySectionExprBase: Should be "
+             "using the direct 'getter' functions");
+      return cast_or_null<Expr>(SubExprs[Idx]);
+    }
+
+  public:
+  /// Get base of the array section.
+  Expr *getBase() { return cast<Expr>(SubExprs[BASE]); }
+  const Expr *getBase() const { return cast<Expr>(SubExprs[BASE]); }
+  /// Set base of the array section.
+  void setBase(Expr *E) {
+    assert((E || AllowNullExprs) && "Null expression when not allowed");
+    SubExprs[BASE] = E;
+  }
+
+  /// Get lower bound of array section.
+  Expr *getLowerBound() { return cast_or_null<Expr>(SubExprs[LOWER_BOUND]); }
+  const Expr *getLowerBound() const {
+    return cast_or_null<Expr>(SubExprs[LOWER_BOUND]);
+  }
+  /// Set lower bound of the array section.
+  void setLowerBound(Expr *E) {
+    assert((E || AllowNullExprs) && "Null expression when not allowed");
+    SubExprs[LOWER_BOUND] = E;
+  }
+
+  /// Get length of array section.
+  Expr *getLength() { return cast_or_null<Expr>(SubExprs[LENGTH]); }
+  const Expr *getLength() const { return cast_or_null<Expr>(SubExprs[LENGTH]); }
+  /// Set length of the array section.
+  void setLength(Expr *E) {
+    assert((E || AllowNullExprs) && "Null expression when not allowed");
+    SubExprs[LENGTH] = E;
+  }
+
+  SourceLocation getBeginLoc() const LLVM_READONLY {
+    return getBase()->getBeginLoc();
+  }
+
+  SourceLocation getEndLoc() const LLVM_READONLY { return RBracketLoc; }
+
+  SourceLocation getColonLocFirst() const { return ColonLocFirst; }
+  void setColonLocFirst(SourceLocation L) { ColonLocFirst = L; }
+
+  SourceLocation getRBracketLoc() const { return RBracketLoc; }
+  void setRBracketLoc(SourceLocation L) { RBracketLoc = L; }
+
+  SourceLocation getExprLoc() const LLVM_READONLY {
+    return getBase()->getExprLoc();
+  }
+    child_range children() {
+      return child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
+    }
+
+    const_child_range children() const {
+      return const_child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
+    }
+};
+
 /// Frontend produces RecoveryExprs on semantic errors that prevent creating
 /// other well-formed expressions. E.g. when type-checking of a binary operator
 /// fails, we cannot produce a BinaryOperator expression. Instead, we can choose
diff --git a/clang/include/clang/AST/ExprOpenMP.h b/clang/include/clang/AST/ExprOpenMP.h
index be5b1f3fdd112f..c410c82d9469a3 100644
--- a/clang/include/clang/AST/ExprOpenMP.h
+++ b/clang/include/clang/AST/ExprOpenMP.h
@@ -53,92 +53,46 @@ namespace clang {
 /// When the length is absent it defaults to ⌈(size − lower-bound)/stride⌉,
 /// where size is the size of the array dimension. When the lower-bound is
 /// absent it defaults to 0.
-class OMPArraySectionExpr : public Expr {
+namespace OMPArraySectionIndices {
   enum { BASE, LOWER_BOUND, LENGTH, STRIDE, END_EXPR };
-  Stmt *SubExprs[END_EXPR];
-  SourceLocation ColonLocFirst;
+}
+class OMPArraySectionExpr
+    : public ArraySectionExprBase<OMPArraySectionIndices::END_EXPR,
+                                  /*AllowNullExprs=*/true> {
   SourceLocation ColonLocSecond;
-  SourceLocation RBracketLoc;
 
 public:
   OMPArraySectionExpr(Expr *Base, Expr *LowerBound, Expr *Length, Expr *Stride,
                       QualType Type, ExprValueKind VK, ExprObjectKind OK,
                       SourceLocation ColonLocFirst,
                       SourceLocation ColonLocSecond, SourceLocation RBracketLoc)
-      : Expr(OMPArraySectionExprClass, Type, VK, OK),
-        ColonLocFirst(ColonLocFirst), ColonLocSecond(ColonLocSecond),
-        RBracketLoc(RBracketLoc) {
-    SubExprs[BASE] = Base;
-    SubExprs[LOWER_BOUND] = LowerBound;
-    SubExprs[LENGTH] = Length;
-    SubExprs[STRIDE] = Stride;
+      : ArraySectionExprBase(OMPArraySectionExprClass, Base, LowerBound, Length,
+                             Type, VK, OK, ColonLocFirst, RBracketLoc) {
+    setSubExpr(OMPArraySectionIndices::STRIDE, Stride);
     setDependence(computeDependence(this));
   }
 
   /// Create an empty array section expression.
   explicit OMPArraySectionExpr(EmptyShell Shell)
-      : Expr(OMPArraySectionExprClass, Shell) {}
-
-  /// An array section can be written only as Base[LowerBound:Length].
-
-  /// Get base of the array section.
-  Expr *getBase() { return cast<Expr>(SubExprs[BASE]); }
-  const Expr *getBase() const { return cast<Expr>(SubExprs[BASE]); }
-  /// Set base of the array section.
-  void setBase(Expr *E) { SubExprs[BASE] = E; }
+      : ArraySectionExprBase(OMPArraySectionExprClass, Shell) {}
 
   /// Return original type of the base expression for array section.
   static QualType getBaseOriginalType(const Expr *Base);
 
-  /// Get lower bound of array section.
-  Expr *getLowerBound() { return cast_or_null<Expr>(SubExprs[LOWER_BOUND]); }
-  const Expr *getLowerBound() const {
-    return cast_or_null<Expr>(SubExprs[LOWER_BOUND]);
-  }
-  /// Set lower bound of the array section.
-  void setLowerBound(Expr *E) { SubExprs[LOWER_BOUND] = E; }
-
-  /// Get length of array section.
-  Expr *getLength() { return cast_or_null<Expr>(SubExprs[LENGTH]); }
-  const Expr *getLength() const { return cast_or_null<Expr>(SubExprs[LENGTH]); }
-  /// Set length of the array section.
-  void setLength(Expr *E) { SubExprs[LENGTH] = E; }
-
   /// Get stride of array section.
-  Expr *getStride() { return cast_or_null<Expr>(SubExprs[STRIDE]); }
-  const Expr *getStride() const { return cast_or_null<Expr>(SubExprs[STRIDE]); }
-  /// Set length of the array section.
-  void setStride(Expr *E) { SubExprs[STRIDE] = E; }
-
-  SourceLocation getBeginLoc() const LLVM_READONLY {
-    return getBase()->getBeginLoc();
+  Expr *getStride() { return getSubExpr(OMPArraySectionIndices::STRIDE); }
+  const Expr *getStride() const {
+    return getSubExpr(OMPArraySectionIndices::STRIDE);
   }
-  SourceLocation getEndLoc() const LLVM_READONLY { return RBracketLoc; }
-
-  SourceLocation getColonLocFirst() const { return ColonLocFirst; }
-  void setColonLocFirst(SourceLocation L) { ColonLocFirst = L; }
+  /// Set length of the array section.
+  void setStride(Expr *E) { setSubExpr(OMPArraySectionIndices::STRIDE, E); }
 
   SourceLocation getColonLocSecond() const { return ColonLocSecond; }
   void setColonLocSecond(SourceLocation L) { ColonLocSecond = L; }
 
-  SourceLocation getRBracketLoc() const { return RBracketLoc; }
-  void setRBracketLoc(SourceLocation L) { RBracketLoc = L; }
-
-  SourceLocation getExprLoc() const LLVM_READONLY {
-    return getBase()->getExprLoc();
-  }
-
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == OMPArraySectionExprClass;
   }
-
-  child_range children() {
-    return child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
-  }
-
-  const_child_range children() const {
-    return const_child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
-  }
 };
 
 /// An explicit cast in C or a C-style cast in C++, which uses the syntax

>From e4739614794889d45058d899ab55ed80088290d2 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Mon, 22 Apr 2024 11:18:20 -0700
Subject: [PATCH 2/3] Move to protected + clang-format

---
 clang/include/clang/AST/Expr.h       | 108 ++++++++++++++-------------
 clang/include/clang/AST/ExprOpenMP.h |   2 +-
 2 files changed, 57 insertions(+), 53 deletions(-)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 3fc0170d1a0d8e..c4aa56519b21ad 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6618,79 +6618,83 @@ class TypoExpr : public Expr {
 // This type does not capture a 'stride', only a lower-bound and length (plus
 // the base expression), but provides room via a template argument to get
 // additional ones.
-template<unsigned NumSubExprs, bool AllowNullExprs>
+template <unsigned NumSubExprs, bool AllowNullExprs>
 class ArraySectionExprBase : public Expr {
-  enum {BASE, LOWER_BOUND, LENGTH, END_EXPR = NumSubExprs};
+  friend class ASTStmtReader;
+
+  enum { BASE, LOWER_BOUND, LENGTH, END_EXPR = NumSubExprs };
   Stmt *SubExprs[END_EXPR];
   SourceLocation ColonLocFirst;
   SourceLocation RBracketLoc;
 
-  protected:
-    ArraySectionExprBase(StmtClass SC, Expr *Base, Expr *LowerBound,
-                         Expr *Length, QualType Type, ExprValueKind VK,
-                         ExprObjectKind OK, SourceLocation ColonLocFirst,
-                         SourceLocation RBracketLoc)
-        : Expr(SC, Type, VK, OK), ColonLocFirst(ColonLocFirst),
-          RBracketLoc(RBracketLoc) {
-      setBase(Base);
-      setLowerBound(LowerBound);
-      setLength(Length);
-    }
+protected:
+  ArraySectionExprBase(StmtClass SC, Expr *Base, Expr *LowerBound, Expr *Length,
+                       QualType Type, ExprValueKind VK, ExprObjectKind OK,
+                       SourceLocation ColonLocFirst, SourceLocation RBracketLoc)
+      : Expr(SC, Type, VK, OK), ColonLocFirst(ColonLocFirst),
+        RBracketLoc(RBracketLoc) {
+    setBase(Base);
+    setLowerBound(LowerBound);
+    setLength(Length);
+  }
 
-    explicit ArraySectionExprBase(StmtClass SC, EmptyShell Shell)
-        : Expr(SC, Shell) {}
+  explicit ArraySectionExprBase(StmtClass SC, EmptyShell Shell)
+      : Expr(SC, Shell) {}
 
-    void setSubExpr(unsigned Idx, Expr *SubExpr) {
-      assert(Idx > LENGTH &&
-             "setting sub expression owned by ArraySectionExprBase: Should be "
-             "using the direct 'setter' functions");
-      assert((SubExpr || AllowNullExprs) && "Null expression when not allowed");
-      SubExprs[Idx] = SubExpr;
-    }
+  void setSubExpr(unsigned Idx, Expr *SubExpr) {
+    assert(Idx > LENGTH &&
+           "setting sub expression owned by ArraySectionExprBase: Should be "
+           "using the direct 'setter' functions");
+    assert((SubExpr || AllowNullExprs) && "Null expression when not allowed");
+    SubExprs[Idx] = SubExpr;
+  }
 
-    Expr *getSubExpr(unsigned Idx) {
-      assert(Idx > LENGTH &&
-             "getting sub expression owned by ArraySectionExprBase: Should be "
-             "using the direct 'getter' functions");
-      return cast_or_null<Expr>(SubExprs[Idx]);
-    }
-    const Expr *getSubExpr(unsigned Idx) const {
-      assert(Idx > LENGTH &&
-             "getting sub expression owned by ArraySectionExprBase: Should be "
-             "using the direct 'getter' functions");
-      return cast_or_null<Expr>(SubExprs[Idx]);
-    }
+  Expr *getSubExpr(unsigned Idx) {
+    assert(Idx > LENGTH &&
+           "getting sub expression owned by ArraySectionExprBase: Should be "
+           "using the direct 'getter' functions");
+    return cast_or_null<Expr>(SubExprs[Idx]);
+  }
+  const Expr *getSubExpr(unsigned Idx) const {
+    assert(Idx > LENGTH &&
+           "getting sub expression owned by ArraySectionExprBase: Should be "
+           "using the direct 'getter' functions");
+    return cast_or_null<Expr>(SubExprs[Idx]);
+  }
 
-  public:
-  /// Get base of the array section.
-  Expr *getBase() { return cast<Expr>(SubExprs[BASE]); }
-  const Expr *getBase() const { return cast<Expr>(SubExprs[BASE]); }
   /// Set base of the array section.
   void setBase(Expr *E) {
     assert((E || AllowNullExprs) && "Null expression when not allowed");
     SubExprs[BASE] = E;
   }
 
-  /// Get lower bound of array section.
-  Expr *getLowerBound() { return cast_or_null<Expr>(SubExprs[LOWER_BOUND]); }
-  const Expr *getLowerBound() const {
-    return cast_or_null<Expr>(SubExprs[LOWER_BOUND]);
-  }
   /// Set lower bound of the array section.
   void setLowerBound(Expr *E) {
     assert((E || AllowNullExprs) && "Null expression when not allowed");
     SubExprs[LOWER_BOUND] = E;
   }
 
-  /// Get length of array section.
-  Expr *getLength() { return cast_or_null<Expr>(SubExprs[LENGTH]); }
-  const Expr *getLength() const { return cast_or_null<Expr>(SubExprs[LENGTH]); }
   /// Set length of the array section.
   void setLength(Expr *E) {
     assert((E || AllowNullExprs) && "Null expression when not allowed");
     SubExprs[LENGTH] = E;
   }
 
+public:
+  /// Get base of the array section.
+  Expr *getBase() { return cast<Expr>(SubExprs[BASE]); }
+  const Expr *getBase() const { return cast<Expr>(SubExprs[BASE]); }
+
+  /// Get lower bound of array section.
+  Expr *getLowerBound() { return cast_or_null<Expr>(SubExprs[LOWER_BOUND]); }
+  const Expr *getLowerBound() const {
+    return cast_or_null<Expr>(SubExprs[LOWER_BOUND]);
+  }
+
+  /// Get length of array section.
+  Expr *getLength() { return cast_or_null<Expr>(SubExprs[LENGTH]); }
+  const Expr *getLength() const { return cast_or_null<Expr>(SubExprs[LENGTH]); }
+
   SourceLocation getBeginLoc() const LLVM_READONLY {
     return getBase()->getBeginLoc();
   }
@@ -6706,13 +6710,13 @@ class ArraySectionExprBase : public Expr {
   SourceLocation getExprLoc() const LLVM_READONLY {
     return getBase()->getExprLoc();
   }
-    child_range children() {
-      return child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
-    }
+  child_range children() {
+    return child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
+  }
 
-    const_child_range children() const {
-      return const_child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
-    }
+  const_child_range children() const {
+    return const_child_range(&SubExprs[BASE], &SubExprs[END_EXPR]);
+  }
 };
 
 /// Frontend produces RecoveryExprs on semantic errors that prevent creating
diff --git a/clang/include/clang/AST/ExprOpenMP.h b/clang/include/clang/AST/ExprOpenMP.h
index c410c82d9469a3..f013f92cca430a 100644
--- a/clang/include/clang/AST/ExprOpenMP.h
+++ b/clang/include/clang/AST/ExprOpenMP.h
@@ -54,7 +54,7 @@ namespace clang {
 /// where size is the size of the array dimension. When the lower-bound is
 /// absent it defaults to 0.
 namespace OMPArraySectionIndices {
-  enum { BASE, LOWER_BOUND, LENGTH, STRIDE, END_EXPR };
+enum { BASE, LOWER_BOUND, LENGTH, STRIDE, END_EXPR };
 }
 class OMPArraySectionExpr
     : public ArraySectionExprBase<OMPArraySectionIndices::END_EXPR,

>From c8a9094b640c87d98074d5b1f67523e4204cdaef Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Mon, 22 Apr 2024 11:54:27 -0700
Subject: [PATCH 3/3] Correctly initialize second-colon-loc so it prints
 correctly

---
 clang/include/clang/AST/ExprOpenMP.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/AST/ExprOpenMP.h b/clang/include/clang/AST/ExprOpenMP.h
index f013f92cca430a..930b9e3603b7c2 100644
--- a/clang/include/clang/AST/ExprOpenMP.h
+++ b/clang/include/clang/AST/ExprOpenMP.h
@@ -67,7 +67,8 @@ class OMPArraySectionExpr
                       SourceLocation ColonLocFirst,
                       SourceLocation ColonLocSecond, SourceLocation RBracketLoc)
       : ArraySectionExprBase(OMPArraySectionExprClass, Base, LowerBound, Length,
-                             Type, VK, OK, ColonLocFirst, RBracketLoc) {
+                             Type, VK, OK, ColonLocFirst, RBracketLoc),
+        ColonLocSecond(ColonLocSecond) {
     setSubExpr(OMPArraySectionIndices::STRIDE, Stride);
     setDependence(computeDependence(this));
   }



More information about the cfe-commits mailing list