Skip to content

[clang] NFC: introduce UnsignedOrNone as a replacement for std::optional<unsigned>#134142

Merged
mizvekov merged 1 commit intomainfrom
users/mizvekov/clang-unsigned-or-none
Apr 3, 2025
Merged

[clang] NFC: introduce UnsignedOrNone as a replacement for std::optional<unsigned>#134142
mizvekov merged 1 commit intomainfrom
users/mizvekov/clang-unsigned-or-none

Conversation

@mizvekov
Copy link
Contributor

@mizvekov mizvekov commented Apr 2, 2025

This introduces a new class 'UnsignedOrNone', which models a lite version of std::optional<unsigned>, but has the same size as 'unsigned'.

This replaces most uses of std::optional<unsigned>, and similar schemes utilizing 'int' and '-1' as sentinel.

Besides the smaller size advantage, this is simpler to serialize, as its internal representation is a single unsigned int as well.

@mizvekov mizvekov self-assigned this Apr 2, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Apr 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 2, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Matheus Izvekov (mizvekov)

Changes

This introduces a new class 'UnsignedOrNone', which models a lite version of std::optional<unsigned>, but has the same size as 'unsigned'.

This replaces most uses of std::optional<unsigned>, and similar schemes utilizing 'int' and '-1' as sentinel.

Besides the smaller size advantage, this is simpler to serialize, as its internal representation is a single unsigned int as well.


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

60 Files Affected:

  • (modified) clang/include/clang/AST/ASTConcept.h (+5-7)
  • (modified) clang/include/clang/AST/ASTContext.h (+9-11)
  • (modified) clang/include/clang/AST/ASTImporter.h (+1-1)
  • (modified) clang/include/clang/AST/ASTStructuralEquivalence.h (+1-2)
  • (modified) clang/include/clang/AST/Decl.h (+5-5)
  • (modified) clang/include/clang/AST/DeclTemplate.h (+12-24)
  • (modified) clang/include/clang/AST/ExprCXX.h (+13-20)
  • (modified) clang/include/clang/AST/ExprObjC.h (+1-1)
  • (modified) clang/include/clang/AST/Mangle.h (+2-2)
  • (modified) clang/include/clang/AST/PropertiesBase.td (+5-9)
  • (modified) clang/include/clang/AST/TemplateBase.h (+5-8)
  • (modified) clang/include/clang/AST/TemplateName.h (+6-7)
  • (modified) clang/include/clang/AST/Type.h (+11-14)
  • (modified) clang/include/clang/AST/TypeProperties.td (+2-2)
  • (added) clang/include/clang/Basic/UnsignedOrNone.h (+53)
  • (modified) clang/include/clang/Sema/Sema.h (+29-30)
  • (modified) clang/include/clang/Sema/SemaLambda.h (+1-1)
  • (modified) clang/include/clang/Sema/SemaOpenACC.h (+1-1)
  • (modified) clang/include/clang/Sema/Template.h (+2-3)
  • (modified) clang/include/clang/Sema/TemplateDeduction.h (+1-1)
  • (modified) clang/include/clang/Serialization/ASTReader.h (+1-1)
  • (modified) clang/include/clang/Serialization/ASTRecordReader.h (+4)
  • (modified) clang/include/clang/Serialization/ASTRecordWriter.h (+4)
  • (modified) clang/lib/AST/ASTContext.cpp (+18-21)
  • (modified) clang/lib/AST/ASTImporter.cpp (+4-5)
  • (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+3-3)
  • (modified) clang/lib/AST/ComputeDependence.cpp (+1-1)
  • (modified) clang/lib/AST/Decl.cpp (+1-1)
  • (modified) clang/lib/AST/DeclTemplate.cpp (+6-6)
  • (modified) clang/lib/AST/Expr.cpp (+1-1)
  • (modified) clang/lib/AST/ExprCXX.cpp (+6-7)
  • (modified) clang/lib/AST/ItaniumMangle.cpp (+3-3)
  • (modified) clang/lib/AST/JSONNodeDumper.cpp (+1-1)
  • (modified) clang/lib/AST/TemplateBase.cpp (+3-6)
  • (modified) clang/lib/AST/TemplateName.cpp (+6-4)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+1-1)
  • (modified) clang/lib/AST/Type.cpp (+5-4)
  • (modified) clang/lib/Sema/Sema.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaConcept.cpp (+10-11)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-5)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+4-4)
  • (modified) clang/lib/Sema/SemaLambda.cpp (+11-14)
  • (modified) clang/lib/Sema/SemaOpenACC.cpp (+3-2)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+2-3)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+5-7)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+32-28)
  • (modified) clang/lib/Sema/SemaTemplateDeductionGuide.cpp (+10-18)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+49-51)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+29-30)
  • (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+13-13)
  • (modified) clang/lib/Sema/SemaType.cpp (+6-8)
  • (modified) clang/lib/Sema/TreeTransform.h (+77-78)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+2-2)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+6-7)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+1-1)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+1-1)
  • (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+3-6)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+1-1)
  • (modified) clang/unittests/AST/ASTImporterTest.cpp (+6-6)
diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h
index f89899c3ea7b1..078e1e848f393 100644
--- a/clang/include/clang/AST/ASTConcept.h
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -18,6 +18,7 @@
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallVector.h"
@@ -229,15 +230,14 @@ class TypeConstraint {
   /// type-constraint.
   Expr *ImmediatelyDeclaredConstraint = nullptr;
   ConceptReference *ConceptRef;
-  int ArgumentPackSubstitutionIndex;
+  UnsignedOrNone ArgPackSubstIndex;
 
 public:
   TypeConstraint(ConceptReference *ConceptRef,
                  Expr *ImmediatelyDeclaredConstraint,
-                 int ArgumentPackSubstitutionIndex)
+                 UnsignedOrNone ArgPackSubstIndex)
       : ImmediatelyDeclaredConstraint(ImmediatelyDeclaredConstraint),
-        ConceptRef(ConceptRef),
-        ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
+        ConceptRef(ConceptRef), ArgPackSubstIndex(ArgPackSubstIndex) {}
 
   /// \brief Get the immediately-declared constraint expression introduced by
   /// this type-constraint, that is - the constraint expression that is added to
@@ -248,9 +248,7 @@ class TypeConstraint {
 
   ConceptReference *getConceptReference() const { return ConceptRef; }
 
-  int getArgumentPackSubstitutionIndex() const {
-    return ArgumentPackSubstitutionIndex;
-  }
+  UnsignedOrNone getArgPackSubstIndex() const { return ArgPackSubstIndex; }
 
   // FIXME: Instead of using these concept related functions the callers should
   // directly work with the corresponding ConceptReference.
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index a24f30815e6b9..e0635f0136c6c 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1795,10 +1795,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
       QualType Wrapped, QualType Contained,
       const HLSLAttributedResourceType::Attributes &Attrs);
 
-  QualType
-  getSubstTemplateTypeParmType(QualType Replacement, Decl *AssociatedDecl,
-                               unsigned Index,
-                               std::optional<unsigned> PackIndex) const;
+  QualType getSubstTemplateTypeParmType(QualType Replacement,
+                                        Decl *AssociatedDecl, unsigned Index,
+                                        UnsignedOrNone PackIndex) const;
   QualType getSubstTemplateTypeParmPackType(Decl *AssociatedDecl,
                                             unsigned Index, bool Final,
                                             const TemplateArgument &ArgPack);
@@ -1853,8 +1852,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   ///        expansion is used in a context where the arity is inferred from
   ///        elsewhere, such as if the pattern contains a placeholder type or
   ///        if this is the canonical type of another pack expansion type.
-  QualType getPackExpansionType(QualType Pattern,
-                                std::optional<unsigned> NumExpansions,
+  QualType getPackExpansionType(QualType Pattern, UnsignedOrNone NumExpansions,
                                 bool ExpectPackInType = true) const;
 
   QualType getObjCInterfaceType(const ObjCInterfaceDecl *Decl,
@@ -1898,7 +1896,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   QualType getPackIndexingType(QualType Pattern, Expr *IndexExpr,
                                bool FullySubstituted = false,
                                ArrayRef<QualType> Expansions = {},
-                               int Index = -1) const;
+                               UnsignedOrNone Index = std::nullopt) const;
 
   /// Unary type transforms
   QualType getUnaryTransformType(QualType BaseType, QualType UnderlyingType,
@@ -2393,10 +2391,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   TemplateName
   getDependentTemplateName(const DependentTemplateStorage &Name) const;
 
-  TemplateName
-  getSubstTemplateTemplateParm(TemplateName replacement, Decl *AssociatedDecl,
-                               unsigned Index,
-                               std::optional<unsigned> PackIndex) const;
+  TemplateName getSubstTemplateTemplateParm(TemplateName replacement,
+                                            Decl *AssociatedDecl,
+                                            unsigned Index,
+                                            UnsignedOrNone PackIndex) const;
   TemplateName getSubstTemplateTemplateParmPack(const TemplateArgument &ArgPack,
                                                 Decl *AssociatedDecl,
                                                 unsigned Index,
diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h
index a2550716e3c7f..c40b92666a2ff 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/clang/include/clang/AST/ASTImporter.h
@@ -592,7 +592,7 @@ class TypeSourceInfo;
     /// F should be a field (or indirect field) declaration.
     /// \returns The index of the field in its parent context (starting from 0).
     /// On error `std::nullopt` is returned (parent context is non-record).
-    static std::optional<unsigned> getFieldIndex(Decl *F);
+    static UnsignedOrNone getFieldIndex(Decl *F);
   };
 
 } // namespace clang
diff --git a/clang/include/clang/AST/ASTStructuralEquivalence.h b/clang/include/clang/AST/ASTStructuralEquivalence.h
index 67aa0023c25d0..b0caded2f49a6 100644
--- a/clang/include/clang/AST/ASTStructuralEquivalence.h
+++ b/clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -123,8 +123,7 @@ struct StructuralEquivalenceContext {
   ///
   /// FIXME: This is needed by ASTImporter and ASTStructureEquivalence. It
   /// probably makes more sense in some other common place then here.
-  static std::optional<unsigned>
-  findUntaggedStructOrUnionIndex(RecordDecl *Anon);
+  static UnsignedOrNone findUntaggedStructOrUnionIndex(RecordDecl *Anon);
 
   // If ErrorOnTagTypeMismatch is set, return the error, otherwise get the
   // relevant warning for the input error diagnostic.
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index adf3634d205bc..4946ccce53a9d 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -33,6 +33,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "clang/Basic/Visibility.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -82,14 +83,13 @@ enum class ImplicitParamKind;
 // expanded.
 struct AssociatedConstraint {
   const Expr *ConstraintExpr = nullptr;
-  int ArgumentPackSubstitutionIndex = -1;
+  UnsignedOrNone ArgPackSubstIndex = std::nullopt;
 
   constexpr AssociatedConstraint() = default;
 
   explicit AssociatedConstraint(const Expr *ConstraintExpr,
-                                int ArgumentPackSubstitutionIndex = -1)
-      : ConstraintExpr(ConstraintExpr),
-        ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
+                                UnsignedOrNone ArgPackSubstIndex = std::nullopt)
+      : ConstraintExpr(ConstraintExpr), ArgPackSubstIndex(ArgPackSubstIndex) {}
 
   explicit operator bool() const { return ConstraintExpr != nullptr; }
 };
@@ -2538,7 +2538,7 @@ class FunctionDecl : public DeclaratorDecl,
   /// If this function is an allocation/deallocation function that takes
   /// the `std::nothrow_t` tag, return true through IsNothrow,
   bool isReplaceableGlobalAllocationFunction(
-      std::optional<unsigned> *AlignmentParam = nullptr,
+      UnsignedOrNone *AlignmentParam = nullptr,
       bool *IsNothrow = nullptr) const;
 
   /// Determine if this function provides an inline implementation of a builtin.
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 37fe0acf5d4d5..a8100b642e04c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1198,13 +1198,8 @@ class TemplateTypeParmDecl final : public TypeDecl,
   /// type constraint.
   bool TypeConstraintInitialized : 1;
 
-  /// Whether this type template parameter is an "expanded"
-  /// parameter pack, meaning that its type is a pack expansion and we
-  /// already know the set of types that expansion expands to.
-  bool ExpandedParameterPack : 1;
-
-  /// The number of type parameters in an expanded parameter pack.
-  unsigned NumExpanded = 0;
+  /// The number of type parameters in an expanded parameter pack, if any.
+  UnsignedOrNone NumExpanded = std::nullopt;
 
   /// The default template argument, if any.
   using DefArgStorage =
@@ -1213,19 +1208,17 @@ class TemplateTypeParmDecl final : public TypeDecl,
 
   TemplateTypeParmDecl(DeclContext *DC, SourceLocation KeyLoc,
                        SourceLocation IdLoc, IdentifierInfo *Id, bool Typename,
-                       bool HasTypeConstraint,
-                       std::optional<unsigned> NumExpanded)
+                       bool HasTypeConstraint, UnsignedOrNone NumExpanded)
       : TypeDecl(TemplateTypeParm, DC, IdLoc, Id, KeyLoc), Typename(Typename),
         HasTypeConstraint(HasTypeConstraint), TypeConstraintInitialized(false),
-        ExpandedParameterPack(NumExpanded),
-        NumExpanded(NumExpanded.value_or(0)) {}
+        NumExpanded(NumExpanded) {}
 
 public:
   static TemplateTypeParmDecl *
   Create(const ASTContext &C, DeclContext *DC, SourceLocation KeyLoc,
          SourceLocation NameLoc, unsigned D, unsigned P, IdentifierInfo *Id,
          bool Typename, bool ParameterPack, bool HasTypeConstraint = false,
-         std::optional<unsigned> NumExpanded = std::nullopt);
+         UnsignedOrNone NumExpanded = std::nullopt);
   static TemplateTypeParmDecl *CreateDeserialized(const ASTContext &C,
                                                   GlobalDeclID ID);
   static TemplateTypeParmDecl *CreateDeserialized(const ASTContext &C,
@@ -1327,13 +1320,8 @@ class TemplateTypeParmDecl final : public TypeDecl,
   /// expanded parameter pack. For example, instantiating
   /// \c X<int, unsigned int> results in \c Convertibles being an expanded
   /// parameter pack of size 2 (use getNumExpansionTypes() to get this number).
-  bool isExpandedParameterPack() const { return ExpandedParameterPack; }
-
-  /// Retrieves the number of parameters in an expanded parameter pack.
-  unsigned getNumExpansionParameters() const {
-    assert(ExpandedParameterPack && "Not an expansion parameter pack");
-    return NumExpanded;
-  }
+  /// Retrieves the number of parameters in an expanded parameter pack, if any.
+  UnsignedOrNone getNumExpansionParameters() const { return NumExpanded; }
 
   /// Returns the type constraint associated with this template parameter (if
   /// any).
@@ -1344,7 +1332,7 @@ class TemplateTypeParmDecl final : public TypeDecl,
 
   void setTypeConstraint(ConceptReference *CR,
                          Expr *ImmediatelyDeclaredConstraint,
-                         int ArgumentPackSubstitutionIndex);
+                         UnsignedOrNone ArgPackSubstIndex);
 
   /// Determine whether this template parameter has a type-constraint.
   bool hasTypeConstraint() const {
@@ -1360,7 +1348,7 @@ class TemplateTypeParmDecl final : public TypeDecl,
       llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
     if (HasTypeConstraint)
       AC.emplace_back(getTypeConstraint()->getImmediatelyDeclaredConstraint(),
-                      getTypeConstraint()->getArgumentPackSubstitutionIndex());
+                      getTypeConstraint()->getArgPackSubstIndex());
   }
 
   SourceRange getSourceRange() const override LLVM_READONLY;
@@ -3379,10 +3367,10 @@ inline TemplateDecl *getAsTypeTemplateDecl(Decl *D) {
 ///
 /// In \c A<int,int>::B, \c NTs and \c TTs have expanded pack size 2, and \c Us
 /// is not a pack expansion, so returns an empty Optional.
-inline std::optional<unsigned> getExpandedPackSize(const NamedDecl *Param) {
+inline UnsignedOrNone getExpandedPackSize(const NamedDecl *Param) {
   if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
-    if (TTP->isExpandedParameterPack())
-      return TTP->getNumExpansionParameters();
+    if (UnsignedOrNone Num = TTP->getNumExpansionParameters())
+      return Num;
   }
 
   if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index bdc0eb81b5f02..265c29d59396c 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4210,7 +4210,7 @@ class PackExpansionExpr : public Expr {
 
 public:
   PackExpansionExpr(QualType T, Expr *Pattern, SourceLocation EllipsisLoc,
-                    std::optional<unsigned> NumExpansions)
+                    UnsignedOrNone NumExpansions)
       : Expr(PackExpansionExprClass, T, Pattern->getValueKind(),
              Pattern->getObjectKind()),
         EllipsisLoc(EllipsisLoc),
@@ -4233,7 +4233,7 @@ class PackExpansionExpr : public Expr {
 
   /// Determine the number of expansions that will be produced when
   /// this pack expansion is instantiated, if already known.
-  std::optional<unsigned> getNumExpansions() const {
+  UnsignedOrNone getNumExpansions() const {
     if (NumExpansions)
       return NumExpansions - 1;
 
@@ -4304,8 +4304,7 @@ class SizeOfPackExpr final
   /// the given parameter pack.
   SizeOfPackExpr(QualType SizeType, SourceLocation OperatorLoc, NamedDecl *Pack,
                  SourceLocation PackLoc, SourceLocation RParenLoc,
-                 std::optional<unsigned> Length,
-                 ArrayRef<TemplateArgument> PartialArgs)
+                 UnsignedOrNone Length, ArrayRef<TemplateArgument> PartialArgs)
       : Expr(SizeOfPackExprClass, SizeType, VK_PRValue, OK_Ordinary),
         OperatorLoc(OperatorLoc), PackLoc(PackLoc), RParenLoc(RParenLoc),
         Length(Length ? *Length : PartialArgs.size()), Pack(Pack) {
@@ -4325,7 +4324,7 @@ class SizeOfPackExpr final
   static SizeOfPackExpr *Create(ASTContext &Context, SourceLocation OperatorLoc,
                                 NamedDecl *Pack, SourceLocation PackLoc,
                                 SourceLocation RParenLoc,
-                                std::optional<unsigned> Length = std::nullopt,
+                                UnsignedOrNone Length = std::nullopt,
                                 ArrayRef<TemplateArgument> PartialArgs = {});
   static SizeOfPackExpr *CreateDeserialized(ASTContext &Context,
                                             unsigned NumPartialArgs);
@@ -4467,7 +4466,7 @@ class PackIndexingExpr final
 
   Expr *getIndexExpr() const { return cast<Expr>(SubExprs[1]); }
 
-  std::optional<unsigned> getSelectedIndex() const {
+  UnsignedOrNone getSelectedIndex() const {
     if (isInstantiationDependent())
       return std::nullopt;
     ConstantExpr *CE = cast<ConstantExpr>(getIndexExpr());
@@ -4477,7 +4476,7 @@ class PackIndexingExpr final
   }
 
   Expr *getSelectedExpr() const {
-    std::optional<unsigned> Index = getSelectedIndex();
+    UnsignedOrNone Index = getSelectedIndex();
     assert(Index && "extracting the indexed expression of a dependant pack");
     return getTrailingObjects<Expr *>()[*Index];
   }
@@ -4523,11 +4522,11 @@ class SubstNonTypeTemplateParmExpr : public Expr {
   SubstNonTypeTemplateParmExpr(QualType Ty, ExprValueKind ValueKind,
                                SourceLocation Loc, Expr *Replacement,
                                Decl *AssociatedDecl, unsigned Index,
-                               std::optional<unsigned> PackIndex, bool RefParam)
+                               UnsignedOrNone PackIndex, bool RefParam)
       : Expr(SubstNonTypeTemplateParmExprClass, Ty, ValueKind, OK_Ordinary),
         Replacement(Replacement),
         AssociatedDeclAndRef(AssociatedDecl, RefParam), Index(Index),
-        PackIndex(PackIndex ? *PackIndex + 1 : 0) {
+        PackIndex(PackIndex.toInternalRepresentation()) {
     assert(AssociatedDecl != nullptr);
     SubstNonTypeTemplateParmExprBits.NameLoc = Loc;
     setDependence(computeDependence(this));
@@ -4549,10 +4548,8 @@ class SubstNonTypeTemplateParmExpr : public Expr {
   /// This should match the result of `getParameter()->getIndex()`.
   unsigned getIndex() const { return Index; }
 
-  std::optional<unsigned> getPackIndex() const {
-    if (PackIndex == 0)
-      return std::nullopt;
-    return PackIndex - 1;
+  UnsignedOrNone getPackIndex() const {
+    return UnsignedOrNone::fromInternalRepresentation(PackIndex);
   }
 
   NonTypeTemplateParmDecl *getParameter() const;
@@ -4867,7 +4864,7 @@ class CXXFoldExpr : public Expr {
   SourceLocation RParenLoc;
   // When 0, the number of expansions is not known. Otherwise, this is one more
   // than the number of expansions.
-  unsigned NumExpansions;
+  UnsignedOrNone NumExpansions = std::nullopt;
   Stmt *SubExprs[SubExpr::Count];
   BinaryOperatorKind Opcode;
 
@@ -4875,7 +4872,7 @@ class CXXFoldExpr : public Expr {
   CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
               SourceLocation LParenLoc, Expr *LHS, BinaryOperatorKind Opcode,
               SourceLocation EllipsisLoc, Expr *RHS, SourceLocation RParenLoc,
-              std::optional<unsigned> NumExpansions);
+              UnsignedOrNone NumExpansions);
 
   CXXFoldExpr(EmptyShell Empty) : Expr(CXXFoldExprClass, Empty) {}
 
@@ -4904,11 +4901,7 @@ class CXXFoldExpr : public Expr {
   SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
   BinaryOperatorKind getOperator() const { return Opcode; }
 
-  std::optional<unsigned> getNumExpansions() const {
-    if (NumExpansions)
-      return NumExpansions - 1;
-    return std::nullopt;
-  }
+  UnsignedOrNone getNumExpansions() const { return NumExpansions; }
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
     if (LParenLoc.isValid())
diff --git a/clang/include/clang/AST/ExprObjC.h b/clang/include/clang/AST/ExprObjC.h
index 1fccc26069582..f87fa85569c44 100644
--- a/clang/include/clang/AST/ExprObjC.h
+++ b/clang/include/clang/AST/ExprObjC.h
@@ -271,7 +271,7 @@ struct ObjCDictionaryElement {
 
   /// The number of elements this pack expansion will expand to, if
   /// this is a pack expansion and is known.
-  std::optional<unsigned> NumExpansions;
+  UnsignedOrNone NumExpansions;
 
   /// Determines whether this dictionary element is a pack expansion.
   bool isPackExpansion() const { return EllipsisLoc.isValid(); }
diff --git a/clang/include/clang/AST/Mangle.h b/clang/include/clang/AST/Mangle.h
index 9ed8895cbfff1..a0162fb7125fe 100644
--- a/clang/include/clang/AST/Mangle.h
+++ b/clang/include/clang/AST/Mangle.h
@@ -182,8 +182,8 @@ class MangleContext {
 
 class ItaniumMangleContext : public MangleContext {
 public:
-  using DiscriminatorOverrideTy =
-      std::optional<unsigned> (*)(ASTContext &, const NamedDecl *);
+  using DiscriminatorOverrideTy = UnsignedOrNone (*)(ASTContext &,
+                                                     const NamedDecl *);
   explicit ItaniumMangleContext(ASTContext &C, DiagnosticsEngine &D,
                                 bool IsAux = false)
       : MangleContext(C, D, MK_Itanium, IsAux) {}
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 5171555008ac9..0b0eccfa6ebce 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -144,6 +144,7 @@ def TemplateNameKind : EnumPropertyType<"TemplateName::NameKind">;
 def TypeOfKind : EnumPropertyType<"TypeOfKind">;
 def UInt32 : CountPropertyType<"uint32_t">;
 def UInt64 : CountPropertyType<"uint64_t">;
+def UnsignedOrNone : PropertyType;
 def UnaryTypeTransformKind : EnumPropertyType<"UnaryTransformType::UTTKind">;
 def VectorKind : EnumPropertyType<"VectorKind">;
 def TypeCoupledDeclRefInfo : PropertyType;
@@ -727,7 +728,7 @@ let Class = PropertyTypeCase<TemplateName, "SubstTemplateTemplateParm"> in {
   def : Property<"index", UInt32> {
     let Read = [{ parm->getIndex() }];
   }
-  def : Property<"packIndex", Optional<UInt32>> {
+  def : Property<"packIndex", UnsignedOrNone> {
     let Read = [{ parm->getPackIndex() }];
  ...
[truncated]

@mizvekov mizvekov force-pushed the users/mizvekov/trailing_require_clause_expansion branch from 4c3fe79 to 1405944 Compare April 2, 2025 19:26
@mizvekov mizvekov force-pushed the users/mizvekov/clang-unsigned-or-none branch from fec4487 to 0214dc4 Compare April 2, 2025 19:26
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First: you're kinda my hero on this, thank you so much!

Second: i have mixed emotions on most of this patch. I'm horrified how prevalent this pattern was, and am thrilled at how much of an improvement this is.

Third: A couple questions/suggestions, but in addition: we MIGHT find uses for this (in the future, so nothing to do today) as a template that takes an arbitrary integral type, but that is easy enough for soemone to add in the future when we have a non-unsigned use of this.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good reasons to not do my suggestions, overall thank you a ton! This is a great improvement. Ship it.

@mizvekov mizvekov force-pushed the users/mizvekov/trailing_require_clause_expansion branch from 1405944 to 4a6c038 Compare April 2, 2025 19:50
@mizvekov mizvekov force-pushed the users/mizvekov/clang-unsigned-or-none branch from 0214dc4 to b280da4 Compare April 2, 2025 19:50
Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question otherwise LGTM!

Base automatically changed from users/mizvekov/trailing_require_clause_expansion to main April 3, 2025 15:36
@mizvekov mizvekov force-pushed the users/mizvekov/clang-unsigned-or-none branch from b280da4 to 226afb9 Compare April 3, 2025 16:23
…nsigned>

This introduces a new class 'UnsignedOrNone', which models a lite
version of std::optional<unsigned>, but has the same size as 'unsigned'.

This replaces most uses of std::optional<unsigned> and of
similar schemes utilizing 'int' and '-1' as sentinel.
@mizvekov mizvekov force-pushed the users/mizvekov/clang-unsigned-or-none branch from 226afb9 to bb8ed8a Compare April 3, 2025 16:25
@mizvekov mizvekov merged commit cfee056 into main Apr 3, 2025
11 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-unsigned-or-none branch April 3, 2025 17:27
AnthonyLatsis added a commit to AnthonyLatsis/swift that referenced this pull request Jun 17, 2025
@ChuanqiXu9
Copy link
Member

Just noticed that this is not using std::optional<unsigned>. If this is good, why don't we put this to llvm/ADT? I feel that is a better place.

@mizvekov
Copy link
Contributor Author

Just noticed that this is not using std::optional<unsigned>. If this is good, why don't we put this to llvm/ADT? I feel that is a better place.

I don't disagree in principle, we just don't need to preempt making this available for all of llvm, if no one has the time to make a patch to use it outside of clang.

@shafik
Copy link
Collaborator

shafik commented Sep 18, 2025

This is linked to the following regression: #159563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants