[Clang][Sema] Diagnose indeterminately sequenced accesses

[rZhBoYao]

Summary

Add -Windeterminately-sequenced and fix -Wunsequenced according to P0400R0 and CWG2571.

Fixes #63935

[rZhBoYao]

Put -Windeterminately-sequenced under -Wsequence-point's control

[Endill]

Thank you for working on this!

clang/test/SemaCXX/warn-unsequenced.cpp

3–4

Checking for cxx17 prefix in C++23 mode is misleading. I suggest to rename is to since-cxx17, and have a new run line for C++23 mode, leaving check in C++17 mode intact.

[Endill]

I'm sorry for hand-waving, but I also remember @aaron.ballman saying that code related to -Wunsequenced or around it was missing checks for C++17.
I can't say whether you addressed that or not, so just leaving it here for you and reviewers.

[rZhBoYao]

Improve the test.

Not sure what the lack of C++17 checks was referring to in your conversation with Aaron.
Can @aaron.ballman confirm whether it is addressed by this patch?

[rZhBoYao]

BTW, I am not sure if CWG2571 is implemented by @cor3ntin? If so, can we mark it as done on https://clang.llvm.org/cxx_dr_status.html#2571? This patch handles the warning around it tho.

[Endill]

In D156057#4527787, @rZhBoYao wrote:

BTW, I am not sure if CWG2571 is implemented by @cor3ntin? If so, can we mark it as done on https://clang.llvm.org/cxx_dr_status.html#2571? This patch handles the warning around it tho.

As a reminder, we don't change cxx_dr_status.html directly. Instead, we add a special comment in clang/test/CXX/drs/dr25xx.cpp (for the case of CWG2571), which should be accompanied by test case(s) if possible. You can follow the pattern of existing tests. After that clang/www/make_cxx_dr_status script should be run to update the HTML.

[rZhBoYao]

Thanks for the reminder. I am aware of that. Browsing through 762672a73a1e and a560ccf2af7a, I believe the indeterminately sequenced requirement is met, neither of which test the codegen so I might just put the example shown in the release note in dr25xx.cpp.

In D156057#4527821, @Endill wrote:

In D156057#4527787, @rZhBoYao wrote:

BTW, I am not sure if CWG2571 is implemented by @cor3ntin? If so, can we mark it as done on https://clang.llvm.org/cxx_dr_status.html#2571? This patch handles the warning around it tho.

As a reminder, we don't change cxx_dr_status.html directly. Instead, we add a special comment in clang/test/CXX/drs/dr25xx.cpp (for the case of CWG2571), which should be accompanied by test case(s) if possible. You can follow the pattern of existing tests. After that clang/www/make_cxx_dr_status script should be run to update the HTML.

[rZhBoYao]

Refactor VisitCXXOperatorCallExpr and mark CWG2571 as done in Clang 15 (deliberately the same version P2128R6 was implemented) since this patch only fix the diagnostics around it.

[cor3ntin]

@aaron.ballman that's been waiting for a while - i don't feel comfortable reviewing it, mind looking at it?

clang/docs/ReleaseNotes.rst

 - ``-Wformat`` cast fix-its will now suggest ``static_cast`` instead of C-style casts
   for C++ code.
 - ``-Wformat`` will no longer suggest a no-op fix-it for fixing scoped enum format
   warnings. Instead, it will suggest casting the enum object to the type specified
   in the format string.
 - Clang now emits ``-Wconstant-logical-operand`` warning even when constant logical
   operand is on left side.
   (`#37919 <https://github.com/llvm/llvm-project/issues/37919>`_)
+- Added a new warning group ``-Windeterminately-sequenced`` to diagnose indeterminately
+  sequenced accesses amongst the initializations of each function parameter, as is the
+  case from C++17 onwards. Accordingly, ``-Wunsequenced`` no longer falsely diagnoses
+  such accesses as unsequenced.
+  (`#63935 <https://github.com/llvm/llvm-project/issues/63935>`_)
+
+  *Example C++23 Code*:
+
+  .. code-block:: c++
+
+    struct S { int operator[](auto...); };
+    int c{}, d{}, f(int, int = {}),
+        a = f(c++, (++d, f)(!c, !d)),
+        b = S()[f(f(c++)), c++];
+
+  *BEFORE*:
+
+  .. code-block:: text
+
+    <source>:3:12: warning: unsequenced modification and access to 'c' [-Wunsequenced]
+        3 |     a = f(c++, (++d, f)(!c, !d)),
+          |            ^             ~
+    <source>:4:18: warning: multiple unsequenced modifications to 'c' [-Wunsequenced]
+        4 |     b = S()[f(f(c++)), c++];
+          |                  ^      ~~
+
+  *AFTER*:
+
+  .. code-block:: text
+
+    <source>:3:12: warning: indeterminately sequenced modification and access to 'c' [-Windeterminately-sequenced]
+        3 |     a = f(c++, (++d, f)(!c, !d)),
+          |            ^             ~
+    <source>:4:18: warning: multiple indeterminately sequenced modifications to 'c' [-Windeterminately-sequenced]
+        4 |     b = S()[f(f(c++)), c++];
+          |                  ^      ~~
 
 Bug Fixes in This Version
 -------------------------
 
 - Added a new diagnostic warning group
   ``-Wdeprecated-redundant-constexpr-static-def``, under the existing
   ``-Wdeprecated`` group. This controls warnings about out-of-line definitions
   of 'static constexpr' data members that are unnecessary from C++17 onwards.

clang/include/clang/Basic/DiagnosticGroups.td

 def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
 def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;
 def GNUUnionCast : DiagGroup<"gnu-union-cast">;
 def GNUVariableSizedTypeNotAtEnd : DiagGroup<"gnu-variable-sized-type-not-at-end">;
 def Varargs : DiagGroup<"varargs">;
 def XorUsedAsPow : DiagGroup<"xor-used-as-pow">;
 
 def Unsequenced : DiagGroup<"unsequenced">;
+def IndeterminatelySequenced : DiagGroup<"indeterminately-sequenced">;
 // GCC name for -Wunsequenced
 def : DiagGroup<"sequence-point", [Unsequenced]>;
 
 // Preprocessor warnings.
 def AmbiguousMacro : DiagGroup<"ambiguous-macro">;
 def KeywordAsMacro : DiagGroup<"keyword-macro">;
 def ReservedIdAsMacro : DiagGroup<"reserved-macro-identifier">;
 def ReservedIdAsMacroAlias : DiagGroup<"reserved-id-macro", [ReservedIdAsMacro]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def warn_uninit_const_reference : Warning<
   "variable %0 is uninitialized when passed as a const reference argument "
   "here">, InGroup<UninitializedConstReference>, DefaultIgnore;
 
 def warn_unsequenced_mod_mod : Warning<
   "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
 def warn_unsequenced_mod_use : Warning<
   "unsequenced modification and access to %0">, InGroup<Unsequenced>;
+def warn_indeterminately_sequenced_mod_mod : Warning<
+  "multiple indeterminately sequenced modifications to %0">,
+  InGroup<IndeterminatelySequenced>;
+def warn_indeterminately_sequenced_mod_use : Warning<
+  "indeterminately sequenced modification and access to %0">,
+  InGroup<IndeterminatelySequenced>;
 
 def select_initialized_entity_kind : TextSubstitution<
   "%select{copying variable|copying parameter|initializing template parameter|"
   "returning object|initializing statement expression result|"
   "throwing object|copying member subobject|copying array element|"
   "allocating object|copying temporary|initializing base subobject|"
   "initializing vector element|capturing value}0">;
 

clang/lib/Sema/SemaChecking.cpp

class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
   class SequenceTree {
     struct Value {
       explicit Value(unsigned Parent) : Parent(Parent), Merged(false) {}
       unsigned Parent : 31;
       unsigned Merged : 1;
     };
     SmallVector<Value, 8> Values;
 
+    const SequenceChecker &SC;
+
   public:
     /// A region within an expression which may be sequenced with respect
     /// to some other region.
     class Seq {
       friend class SequenceTree;
 
       unsigned Index;
 
       explicit Seq(unsigned N) : Index(N) {}
 
     public:
       Seq() : Index(0) {}
     };
 
-    SequenceTree() { Values.push_back(Value(0)); }
+    SequenceTree(const SequenceChecker &SC) : SC(SC) {
+      Values.push_back(Value(0));
+    }
     Seq root() const { return Seq(0); }
 
     /// Create a new sequence of operations, which is an unsequenced
     /// subset of \p Parent. This sequence of operations is sequenced with
     /// respect to other children of \p Parent.
     Seq allocate(Seq Parent) {
       Values.push_back(Value(Parent.Index));
       return Seq(Values.size() - 1);
bool isUnsequenced(Seq Cur, Seq Old) {
       while (C >= Target) {
         if (C == Target)
           return true;
         C = Values[C].Parent;
       }
       return false;
     }
 
+    /// Determine whether two operations are indeterminately sequenced. This
+    /// operation is asymmetric: \p Cur should be the more recent sequence, and
+    /// \p Old should have been merged into its parent as appropriate.
+    bool isIndeterminatelySequenced(Seq Cur, Seq Old) {
+      unsigned C = representative(Cur.Index);
+      unsigned Target = representative(Old.Index);
+      for (Seq seq : llvm::iterator_range(SC.CallPostfixExprs.rbegin(),
+                                          SC.CallPostfixExprs.rend())) {
+        unsigned Sibling = seq.Index;
+        unsigned Parent = Values[Sibling].Parent;
+        if (Target <= Sibling)
+          continue;
+        if ((Target = Values[Target].Parent) != Parent)
+          continue;
+        while (C >= Target) {
+          if (C == Target)
+            return true;
+          C = Values[C].Parent;
+        }
+      }
+      return false;
+    }
+
   private:
     /// Pick a representative for a sequence.
     unsigned representative(unsigned K) {
       if (Values[K].Merged)
         // Perform path compression as we go.
         return Values[K].Parent = representative(Values[K].Parent);
       return K;
     }
struct UsageInfo {
 
     UsageInfo() : Diagnosed(false) {}
   };
   using UsageInfoMap = llvm::SmallDenseMap<Object, UsageInfo, 16>;
 
   Sema &SemaRef;
 
   /// Sequenced regions within the expression.
-  SequenceTree Tree;
+  SequenceTree Tree = *this;
 
   /// Declaration modifications and references which we have seen.
   UsageInfoMap UsageMap;
 
   /// The region we are currently within.
   SequenceTree::Seq Region;
 
   /// Filled in with declarations which were modified as a side-effect
   /// (that is, post-increment operations).
   SmallVectorImpl<std::pair<Object, Usage>> *ModAsSideEffect = nullptr;
 
-  /// Expressions to check later. We defer checking these to reduce
-  /// stack usage.
-  SmallVectorImpl<const Expr *> &WorkList;
-
   /// RAII object wrapping the visitation of a sequenced subexpression of an
   /// expression. At the end of this process, the side-effects of the evaluation
   /// become sequenced with respect to the value computation of the result, so
   /// we downgrade any UK_ModAsSideEffect within the evaluation to
   /// UK_ModAsValue.
   struct SequencedSubexpression {
     SequencedSubexpression(SequenceChecker &Self)
       : Self(Self), OldModAsSideEffect(Self.ModAsSideEffect) {
public:
     }
 
   private:
     SequenceChecker &Self;
     EvaluationTracker *Prev;
     bool EvalOK = true;
   } *EvalTracker = nullptr;
 
+  /// RAII object tracking the visitation of CallExpr and that of its
+  /// subexpressions.
+  struct InCallExprArgs {
+    InCallExprArgs(SequenceChecker &SC) : SC(SC) {
+      SC.CallPostfixExprs.push_back(SC.Region);
+    };
+
+    ~InCallExprArgs() { SC.CallPostfixExprs.pop_back(); }
+
+  private:
+    SequenceChecker &SC;
+  };
+
+  /// Only modified by InCallExprArgs
+  SmallVector<SequenceTree::Seq, 4> CallPostfixExprs;
+
   /// Find the object which is produced by the specified expression,
   /// if any.
   Object getObject(const Expr *E, bool Mod) const {
     E = E->IgnoreParenCasts();
     if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
       if (Mod && (UO->getOpcode() == UO_PreInc || UO->getOpcode() == UO_PreDec))
         return getObject(UO->getSubExpr(), Mod);
     } else if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
   }
 
   /// Note that an object \p O was modified or used by an expression
   /// \p UsageExpr with usage kind \p UK. \p UI is the \p UsageInfo for
   /// the object \p O as obtained via the \p UsageMap.
   void addUsage(Object O, UsageInfo &UI, const Expr *UsageExpr, UsageKind UK) {
     // Get the old usage for the given object and usage kind.
     Usage &U = UI.Uses[UK];
-    if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq)) {
+    bool FoundUnsequenced = U.UsageExpr && Tree.isUnsequenced(Region, U.Seq);
+    bool FoundIndeterminatelySequenced =
+        U.UsageExpr && SemaRef.LangOpts.CPlusPlus17 &&
+        Tree.isIndeterminatelySequenced(Region, U.Seq);
+    if (!FoundUnsequenced && !FoundIndeterminatelySequenced) {
       // If we have a modification as side effect and are in a sequenced
       // subexpression, save the old Usage so that we can restore it later
       // in SequencedSubexpression::~SequencedSubexpression.
       if (UK == UK_ModAsSideEffect && ModAsSideEffect)
         ModAsSideEffect->push_back(std::make_pair(O, U));
       // Then record the new usage with the current sequencing region.
       U.UsageExpr = UsageExpr;
       U.Seq = Region;
     }
   }
 
   /// Check whether a modification or use of an object \p O in an expression
   /// \p UsageExpr conflicts with a prior usage of kind \p OtherKind. \p UI is
   /// the \p UsageInfo for the object \p O as obtained via the \p UsageMap.
   /// \p IsModMod is true when we are checking for a mod-mod unsequenced
   /// usage and false we are checking for a mod-use unsequenced usage.
   void checkUsage(Object O, UsageInfo &UI, const Expr *UsageExpr,
                   UsageKind OtherKind, bool IsModMod) {
     if (UI.Diagnosed)
       return;
 
     const Usage &U = UI.Uses[OtherKind];
-    if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq))
+    bool FoundUnsequenced = U.UsageExpr && Tree.isUnsequenced(Region, U.Seq);
+    bool FoundIndeterminatelySequenced =
+        U.UsageExpr && SemaRef.LangOpts.CPlusPlus17 &&
+        Tree.isIndeterminatelySequenced(Region, U.Seq);
+    if (!FoundUnsequenced && !FoundIndeterminatelySequenced)
       return;
 
     const Expr *Mod = U.UsageExpr;
     const Expr *ModOrUse = UsageExpr;
     if (OtherKind == UK_Use)
       std::swap(Mod, ModOrUse);
 
     SemaRef.DiagRuntimeBehavior(
         Mod->getExprLoc(), {Mod, ModOrUse},
-        SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod
+        SemaRef.PDiag(!FoundUnsequenced && FoundIndeterminatelySequenced
+                          ? IsModMod
+                                ? diag::warn_indeterminately_sequenced_mod_mod
+                                : diag::warn_indeterminately_sequenced_mod_use
+                      : IsModMod ? diag::warn_unsequenced_mod_mod
                                  : diag::warn_unsequenced_mod_use)
             << O << SourceRange(ModOrUse->getExprLoc()));
     UI.Diagnosed = true;
   }
 
   // A note on note{Pre, Post}{Use, Mod}:
   //
   // (It helps to follow the algorithm with an expression such as
   //  "((++k)++, k) = k" or "k = (k++, k++)". Both contain unsequenced
class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
   void notePostMod(Object O, const Expr *ModExpr, UsageKind UK) {
     UsageInfo &UI = UsageMap[O];
     checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect,
                /*IsModMod=*/true);
     addUsage(O, UI, ModExpr, /*UsageKind=*/UK);
   }
 
 public:
-  SequenceChecker(Sema &S, const Expr *E,
-                  SmallVectorImpl<const Expr *> &WorkList)
-      : Base(S.Context), SemaRef(S), Region(Tree.root()), WorkList(WorkList) {
+  SequenceChecker(Sema &S, const Expr *E)
+      : Base(S.Context), SemaRef(S), Region(Tree.root()) {
     Visit(E);
-    // Silence a -Wunused-private-field since WorkList is now unused.
-    // TODO: Evaluate if it can be used, and if not remove it.
-    (void)this->WorkList;
   }
 
   void VisitStmt(const Stmt *S) {
     // Skip all statements which aren't expressions for now.
   }
 
   void VisitExpr(const Expr *E) {
     // By default, just recurse to evaluated subexpressions.
void VisitCallExpr(const CallExpr *CE) {
     // C++11 [intro.execution]p15:
     //   When calling a function [...], every value computation and side effect
     //   associated with any argument expression, or with the postfix expression
     //   designating the called function, is sequenced before execution of every
     //   expression or statement in the body of the function [and thus before
     //   the value computation of its result].
     SequencedSubexpression Sequenced(*this);
     SemaRef.runWithSufficientStackSpace(CE->getExprLoc(), [&] {
+      if (!SemaRef.getLangOpts().CPlusPlus17)
+        return Base::VisitCallExpr(CE);
+
       // C++17 [expr.call]p5
       //   The postfix-expression is sequenced before each expression in the
       //   expression-list and any default argument. [...]
-      SequenceTree::Seq CalleeRegion;
-      SequenceTree::Seq OtherRegion;
-      if (SemaRef.getLangOpts().CPlusPlus17) {
-        CalleeRegion = Tree.allocate(Region);
-        OtherRegion = Tree.allocate(Region);
-      } else {
-        CalleeRegion = Region;
-        OtherRegion = Region;
-      }
-      SequenceTree::Seq OldRegion = Region;
-
-      // Visit the callee expression first.
+      SequenceTree::Seq Parent = Region;
+      SequenceTree::Seq CalleeRegion = Tree.allocate(Parent);
       Region = CalleeRegion;
-      if (SemaRef.getLangOpts().CPlusPlus17) {
+      {
         SequencedSubexpression Sequenced(*this);
-        Visit(CE->getCallee());
-      } else {
-        Visit(CE->getCallee());
+        Visit(isa<CXXOperatorCallExpr>(CE) ? CE->getArg(0) : CE->getCallee());
       }
 
-      // Then visit the argument expressions.
-      Region = OtherRegion;
-      for (const Expr *Argument : CE->arguments())
+      // C++17 [expr.call]p5
+      //   [...] The initialization of a parameter, including every associated
+      //   value computation and side effect, is indeterminately sequenced with
+      //   respect to that of any other parameter.
+      SmallVector<SequenceTree::Seq, 16> Args;
+      auto ArgRange =
+          isa<CXXOperatorCallExpr>(CE)
+              ? llvm::iterator_range(CE->arg_begin() + 1, CE->arg_end())
+              : CE->arguments();
+      InCallExprArgs _(*this);
+      for (const Expr *Argument : ArgRange) {
+        SequencedSubexpression Sequenced(*this);
+        Region = Tree.allocate(Parent);
+        Args.push_back(Region);
         Visit(Argument);
+      }
 
-      Region = OldRegion;
-      if (SemaRef.getLangOpts().CPlusPlus17) {
-        Tree.merge(CalleeRegion);
-        Tree.merge(OtherRegion);
-      }
+      Region = Parent;
+      for (SequenceTree::Seq region : Args)
+        Tree.merge(region);
     });
   }
 
   void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *CXXOCE) {
+    if (!SemaRef.getLangOpts().CPlusPlus17)
+      return VisitCallExpr(CXXOCE);
+
     // C++17 [over.match.oper]p2:
     //   [...] the operator notation is first transformed to the equivalent
     //   function-call notation as summarized in Table 12 (where @ denotes one
     //   of the operators covered in the specified subclause). However, the
     //   operands are sequenced in the order prescribed for the built-in
     //   operator (Clause 8).
     //
     // From the above only overloaded binary operators and overloaded call
     // operators have sequencing rules in C++17 that we need to handle
     // separately.
-    if (!SemaRef.getLangOpts().CPlusPlus17 ||
-        (CXXOCE->getNumArgs() != 2 && CXXOCE->getOperator() != OO_Call))
-      return VisitCallExpr(CXXOCE);
-
     enum {
       NoSequencing,
       LHSBeforeRHS,
       RHSBeforeLHS,
-      LHSBeforeRest
+      IndeterminatelySequenced
     } SequencingKind;
     switch (CXXOCE->getOperator()) {
     case OO_Equal:
     case OO_PlusEqual:
     case OO_MinusEqual:
     case OO_StarEqual:
     case OO_SlashEqual:
     case OO_PercentEqual:
     case OO_CaretEqual:
     case OO_AmpEqual:
     case OO_PipeEqual:
     case OO_LessLessEqual:
     case OO_GreaterGreaterEqual:
       SequencingKind = RHSBeforeLHS;
       break;
-
     case OO_LessLess:
     case OO_GreaterGreater:
     case OO_AmpAmp:
     case OO_PipePipe:
     case OO_Comma:
     case OO_ArrowStar:
-    case OO_Subscript:
       SequencingKind = LHSBeforeRHS;
       break;
-
+    case OO_Subscript:
+      SequencingKind = SemaRef.getLangOpts().CPlusPlus23
+                           ? IndeterminatelySequenced // CWG2571
+                           : LHSBeforeRHS;
+      break;
     case OO_Call:
-      SequencingKind = LHSBeforeRest;
+      SequencingKind = IndeterminatelySequenced;
       break;
-
     default:
       SequencingKind = NoSequencing;
       break;
     }
 
-    if (SequencingKind == NoSequencing)
-      return VisitCallExpr(CXXOCE);
+    if (SequencingKind == NoSequencing) {
+      SequenceTree::Seq Parent = Region;
+      SequenceTree::Seq CalleeRegion = Tree.allocate(Parent);
+      SequenceTree::Seq OtherRegion = Tree.allocate(Parent);
+
+      // Visit the callee expression first.
+      Region = CalleeRegion;
+      {
+        SequencedSubexpression Sequenced(*this);
+        Visit(CXXOCE->getCallee());
+      }
+
+      // Then visit the argument expressions.
+      Region = OtherRegion;
+      for (const Expr *Argument : CXXOCE->arguments())
+        Visit(Argument);
+
+      Region = Parent;
+      Tree.merge(CalleeRegion);
+      Tree.merge(OtherRegion);
+      return;
+    }
 
     // This is a call, so all subexpressions are sequenced before the result.
     SequencedSubexpression Sequenced(*this);
 
     SemaRef.runWithSufficientStackSpace(CXXOCE->getExprLoc(), [&] {
       assert(SemaRef.getLangOpts().CPlusPlus17 &&
              "Should only get there with C++17 and above!");
-      assert((CXXOCE->getNumArgs() == 2 || CXXOCE->getOperator() == OO_Call) &&
-             "Should only get there with an overloaded binary operator"
-             " or an overloaded call operator!");
-
-      if (SequencingKind == LHSBeforeRest) {
-        assert(CXXOCE->getOperator() == OO_Call &&
-               "We should only have an overloaded call operator here!");
-
-        // This is very similar to VisitCallExpr, except that we only have the
-        // C++17 case. The postfix-expression is the first argument of the
-        // CXXOperatorCallExpr. The expressions in the expression-list, if any,
-        // are in the following arguments.
-        //
-        // Note that we intentionally do not visit the callee expression since
-        // it is just a decayed reference to a function.
-        SequenceTree::Seq PostfixExprRegion = Tree.allocate(Region);
-        SequenceTree::Seq ArgsRegion = Tree.allocate(Region);
-        SequenceTree::Seq OldRegion = Region;
 
+      if (SequencingKind == IndeterminatelySequenced) {
+        assert((CXXOCE->getOperator() == OO_Subscript ||
+                CXXOCE->getOperator() == OO_Call) &&
+               "We should only have an overloaded call or subscript operator "
+               "here!");
         assert(CXXOCE->getNumArgs() >= 1 &&
-               "An overloaded call operator must have at least one argument"
-               " for the postfix-expression!");
-        const Expr *PostfixExpr = CXXOCE->getArgs()[0];
-        llvm::ArrayRef<const Expr *> Args(CXXOCE->getArgs() + 1,
-                                          CXXOCE->getNumArgs() - 1);
-
-        // Visit the postfix-expression first.
-        {
-          Region = PostfixExprRegion;
-          SequencedSubexpression Sequenced(*this);
-          Visit(PostfixExpr);
-        }
-
-        // Then visit the argument expressions.
-        Region = ArgsRegion;
-        for (const Expr *Arg : Args)
-          Visit(Arg);
-
-        Region = OldRegion;
-        Tree.merge(PostfixExprRegion);
-        Tree.merge(ArgsRegion);
-      } else {
-        assert(CXXOCE->getNumArgs() == 2 &&
-               "Should only have two arguments here!");
-        assert((SequencingKind == LHSBeforeRHS ||
-                SequencingKind == RHSBeforeLHS) &&
-               "Unexpected sequencing kind!");
+               "An overloaded call or subscript operator must have at least "
+               "one argument for the postfix-expression!");
+        return VisitCallExpr(CXXOCE);
+      assert(
+          (SequencingKind == LHSBeforeRHS || SequencingKind == RHSBeforeLHS) &&
 
-        // We do not visit the callee expression since it is just a decayed
-        // reference to a function.
-        const Expr *E1 = CXXOCE->getArg(0);
-        const Expr *E2 = CXXOCE->getArg(1);
-        if (SequencingKind == RHSBeforeLHS)
-          std::swap(E1, E2);
 
-        return VisitSequencedExpressions(E1, E2);
-      }
     });
   }
 
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
     // This is a call, so all subexpressions are sequenced before the result.
     SequencedSubexpression Sequenced(*this);
 
     if (!CCE->isListInitialization())
void VisitInitListExpr(const InitListExpr *ILE) {
     for (unsigned I = 0; I < Elts.size(); ++I)
       Tree.merge(Elts[I]);
   }
 };
 
 } // namespace
 
 void Sema::CheckUnsequencedOperations(const Expr *E) {
-  SmallVector<const Expr *, 8> WorkList;
-  WorkList.push_back(E);
-  while (!WorkList.empty()) {
-    const Expr *Item = WorkList.pop_back_val();
-    SequenceChecker(*this, Item, WorkList);
-  }
+  SequenceChecker(*this, E);
 }
 
 void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
                               bool IsConstexpr) {
   llvm::SaveAndRestore ConstantContext(isConstantEvaluatedOverride,
                                        IsConstexpr || isa<ConstantExpr>(E));
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())

clang/test/SemaCXX/warn-unsequenced.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify=cxx11 -std=c++11 -Wno-unused -Wno-uninitialized \
 // RUN:            -Wunsequenced -Wno-c++17-extensions -Wno-c++14-extensions %s
-// RUN: %clang_cc1 -fsyntax-only -verify=cxx17 -std=c++17 -Wno-unused -Wno-uninitialized \
-// RUN:            -Wunsequenced -Wno-c++17-extensions -Wno-c++14-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -verify=cxx17 -std=c++23 -Wno-unused -Wno-uninitialized \
+// RUN:            -Wunsequenced %s
 
 int f(int, int = 0);
 int g1();
 int g2(int);
 
 struct A {
   int x, y;
 };
void test() {
   a = xs[++a]; // ok
   a = xs[a++]; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
   (a ? xs[0] : xs[1]) = ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
   a = (++a, ++a); // ok
   a = (a++, ++a); // ok
   a = (a++, a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
   f(a, a); // ok
   f(a = 0, a); // cxx11-warning {{unsequenced modification and access to 'a'}}
-               // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+               // cxx17-warning@-1 {{indeterminately sequenced modification and access to 'a'}}
   f(a, a += 0); // cxx11-warning {{unsequenced modification and access to 'a'}}
-                // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+                // cxx17-warning@-1 {{indeterminately sequenced modification and access to 'a'}}
   f(a = 0, a = 0); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-                   // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
+                   // cxx17-warning@-1 {{multiple indeterminately sequenced modifications to 'a'}}
   a = f(++a); // ok
   a = f(a++); // ok
   a = f(++a, a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-                   // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
+                   // cxx17-warning@-1 {{multiple indeterminately sequenced modifications to 'a'}}
 
   // Compound assignment "A OP= B" is equivalent to "A = A OP B" except that A
   // is evaluated only once.
   (++a, a) = 1; // ok
   (++a, a) += 1; // ok
   a = ++a; // ok
   a += ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
 
void test() {
   i >>= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
 
   p[i++] = i; // cxx11-warning {{unsequenced modification and access to 'i'}}
   p[i++] = (i = 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
   p++[i++] = (i = p ? i++ : i++); // cxx11-warning {{unsequenced modification and access to 'p'}}
                                   // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
 
   (i++, f)(i++, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  f((++i, f)(i++, j++), ++j); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+                              // cxx11-warning@-1 {{multiple unsequenced modifications to 'j'}}
+                              // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'j'}}
   (i++ + i++, f)(42, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
                           // cxx17-warning@-1 {{multiple unsequenced modifications to 'i'}}
   int (*pf)(int, int);
   (pf = f)(pf != nullptr, pf != nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
   pf((pf = f) != nullptr, 42); // cxx11-warning {{unsequenced modification and access to 'pf'}}
   f((pf = f, 42), (pf = f, 42)); // cxx11-warning {{multiple unsequenced modifications to 'pf'}}
-                                 // cxx17-warning@-1 {{multiple unsequenced modifications to 'pf'}}
+                                 // cxx17-warning@-1 {{multiple indeterminately sequenced modifications to 'pf'}}
   pf((pf = f) != nullptr, pf == nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
-                                          // cxx17-warning@-1 {{unsequenced modification and access to 'pf'}}
+                                          // cxx17-warning@-1 {{indeterminately sequenced modification and access to 'pf'}}
 }
 
 namespace PR20819 {
   struct foo { void bar(int); };
   foo get_foo(int);
 
   void g() {
     int a = 0;
     get_foo(a).bar(a++);  // cxx11-warning {{unsequenced modification and access to 'a'}}
   }
 }
 
 namespace overloaded_operators {
   struct E {
     E &operator=(E &);
     E operator()(E);
     E operator()(E, E);
+#if __cplusplus >= 202302L
+    E operator[](auto...);
+#else
     E operator[](E);
+#endif
   } e;
   // Binary operators with unsequenced operands.
   E operator+(E,E);
   E operator-(E,E);
   E operator*(E,E);
   E operator/(E,E);
   E operator%(E,E);
   E operator^(E,E);
void test() {
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) <<= ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) >>= ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
 
     operator+=(((void)i++,e), ((void)i++,e));
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
-    // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}}
+    // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'i'}}
 
     // Binary operators where the LHS is sequenced before the RHS in C++17.
     ((void)i++,e) << ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) >> ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) || ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) && ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e) , ((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     ((void)i++,e)->*((void)i++,e);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
 
     operator<<(((void)i++,e), ((void)i++,e));
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
-    // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}}
+    // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'i'}}
 
+#if __cplusplus >= 202302L
+    ((void)i++,e)[((void)i++,e), ++i];
+    // cxx17-warning@-1 {{multiple indeterminately sequenced modifications to 'i'}}
+#else
     ((void)i++,e)[((void)i++,e)];
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
+#endif
 
     ((void)i++,e)(((void)i++,e));
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
     e(((void)i++,e), ((void)i++,e));
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
-    // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}}
+    // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'i'}}
 
     ((void)i++,e).operator()(((void)i++,e));
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
 
   }
 }
 
 namespace PR35340 {
   struct S {};
   S &operator<<(S &, int);
 
   void test() {
     S s;
     int i = 0;
     s << i++ << i++;
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
 
     operator<<(operator<<(s, i++), i++);
     // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
-    // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}}
+    // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'i'}}
   }
 }
 
 namespace members {
 
 struct S1 {
   unsigned bf1 : 2;
   unsigned bf2 : 2;
int Foo<X>::Run() {
   if ((num = bar.get()) < 5 || num < 10) { }
   if (static_cast<E>((num = bar.get()) < 5) || static_cast<E>(num < 10)) { }
 
   if (static_cast<E>((num = bar.get()) < 5) && static_cast<E>(num < 10)) { }
   // cxx11-warning@-1 {{unsequenced modification and access to 'num'}}
 
   foo(num++, num++);
   // cxx11-warning@-1 {{multiple unsequenced modifications to 'num'}}
-  // cxx17-warning@-2 {{multiple unsequenced modifications to 'num'}}
+  // cxx17-warning@-2 {{multiple indeterminately sequenced modifications to 'num'}}
   return 1;
 }
 
 int x = Foo<int>().Run();
 // cxx11-note@-1 {{in instantiation of member function 'templates::Foo<int>::Run'}}
 // cxx17-note@-2 {{in instantiation of member function 'templates::Foo<int>::Run'}}