<br><br><div class="gmail_quote">On Thu, Jan 17, 2013 at 2:27 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Wed, Jan 16, 2013 at 5:17 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Wed Jan 16 19:17:56 2013<br>
> New Revision: 172690<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=172690&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=172690&view=rev</a><br>
> Log:<br>
> Add -Wunsequenced (with compatibility alias -Wsequence-point) to warn on<br>
> expressions which have undefined behavior due to multiple unsequenced<br>
> modifications or an unsequenced modification and use of a variable.<br>
<br>
</div>I believe this resolves PR4579. (I've marked it resolved on your behalf)<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br>This is quite an awesome warning.<br><br>Is there actually a list of all the undefined/implementation defined/unspecified behavior and their corresponding warnings (or lack, thereof) ?<br>
<br>-- Matthieu<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> Added:<br>
>     cfe/trunk/test/SemaCXX/warn-unsequenced.cpp<br>
> Modified:<br>
>     cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h<br>
>     cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
>     cfe/trunk/include/clang/Sema/Sema.h<br>
>     cfe/trunk/lib/Sema/SemaChecking.cpp<br>
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
>     cfe/trunk/lib/Sema/SemaExpr.cpp<br>
>     cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
>     cfe/trunk/test/SemaCXX/altivec.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h (original)<br>
> +++ cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h Wed Jan 16 19:17:56 2013<br>
> @@ -49,6 +49,9 @@<br>
>    }<br>
><br>
>    void VisitChooseExpr(ChooseExpr *E) {<br>
> +    // Don't visit either child expression if the condition is dependent.<br>
> +    if (E->getCond()->isValueDependent())<br>
> +      return;<br>
>      // Only the selected subexpression matters; the other one is not evaluated.<br>
>      return this->Visit(E->getChosenSubExpr(Context));<br>
>    }<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Jan 16 19:17:56 2013<br>
> @@ -198,7 +198,6 @@<br>
>  def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;<br>
>  def Sentinel : DiagGroup<"sentinel">;<br>
>  def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;<br>
> -def : DiagGroup<"sequence-point">;<br>
>  def Shadow : DiagGroup<"shadow">;<br>
>  def Shorten64To32 : DiagGroup<"shorten-64-to-32">;<br>
>  def : DiagGroup<"sign-promo">;<br>
> @@ -218,6 +217,10 @@<br>
>  def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;<br>
>  def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;<br>
><br>
> +def Unsequenced : DiagGroup<"unsequenced">;<br>
> +// GCC name for -Wunsequenced<br>
> +def : DiagGroup<"sequence-point", [Unsequenced]>;<br>
> +<br>
>  // Preprocessor warnings.<br>
>  def AmbiguousMacro : DiagGroup<"ambiguous-macro">;<br>
><br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jan 16 19:17:56 2013<br>
> @@ -1323,6 +1323,11 @@<br>
>    "is always %select{false|true}2">;<br>
>  def err_init_incomplete_type : Error<"initialization of incomplete type %0">;<br>
><br>
> +def warn_unsequenced_mod_mod : Warning<<br>
> +  "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;<br>
> +def warn_unsequenced_mod_use : Warning<<br>
> +  "unsequenced modification and access to %0">, InGroup<Unsequenced>;<br>
> +<br>
>  def err_temp_copy_no_viable : Error<<br>
>    "no viable constructor %select{copying variable|copying parameter|"<br>
>    "returning object|throwing object|copying member subobject|copying array "<br>
><br>
> Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Jan 16 19:17:56 2013<br>
> @@ -7319,6 +7319,11 @@<br>
>                              SourceLocation ReturnLoc);<br>
>    void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);<br>
>    void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());<br>
> +  void CheckUnsequencedOperations(Expr *E);<br>
> +<br>
> +  /// \brief Perform semantic checks on a completed expression. This will either<br>
> +  /// be a full-expression or a default argument expression.<br>
> +  void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation());<br>
><br>
>    void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,<br>
>                                     Expr *Init);<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -5171,6 +5171,424 @@<br>
>    AnalyzeImplicitConversions(*this, E, CC);<br>
>  }<br>
><br>
> +namespace {<br>
> +/// \brief Visitor for expressions which looks for unsequenced operations on the<br>
> +/// same object.<br>
> +class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {<br>
> +  /// \brief A tree of sequenced regions within an expression. Two regions are<br>
> +  /// unsequenced if one is an ancestor or a descendent of the other. When we<br>
> +  /// finish processing an expression with sequencing, such as a comma<br>
> +  /// expression, we fold its tree nodes into its parent, since they are<br>
> +  /// unsequenced with respect to nodes we will visit later.<br>
> +  class SequenceTree {<br>
> +    struct Value {<br>
> +      explicit Value(unsigned Parent) : Parent(Parent), Merged(false) {}<br>
> +      unsigned Parent : 31;<br>
> +      bool Merged : 1;<br>
> +    };<br>
> +    llvm::SmallVector<Value, 8> Values;<br>
> +<br>
> +  public:<br>
> +    /// \brief A region within an expression which may be sequenced with respect<br>
> +    /// to some other region.<br>
> +    class Seq {<br>
> +      explicit Seq(unsigned N) : Index(N) {}<br>
> +      unsigned Index;<br>
> +      friend class SequenceTree;<br>
> +    public:<br>
> +      Seq() : Index(0) {}<br>
> +    };<br>
> +<br>
> +    SequenceTree() { Values.push_back(Value(0)); }<br>
> +    Seq root() const { return Seq(0); }<br>
> +<br>
> +    /// \brief Create a new sequence of operations, which is an unsequenced<br>
> +    /// subset of \p Parent. This sequence of operations is sequenced with<br>
> +    /// respect to other children of \p Parent.<br>
> +    Seq allocate(Seq Parent) {<br>
> +      Values.push_back(Value(Parent.Index));<br>
> +      return Seq(Values.size() - 1);<br>
> +    }<br>
> +<br>
> +    /// \brief Merge a sequence of operations into its parent.<br>
> +    void merge(Seq S) {<br>
> +      Values[S.Index].Merged = true;<br>
> +    }<br>
> +<br>
> +    /// \brief Determine whether two operations are unsequenced. This operation<br>
> +    /// is asymmetric: \p Cur should be the more recent sequence, and \p Old<br>
> +    /// should have been merged into its parent as appropriate.<br>
> +    bool isUnsequenced(Seq Cur, Seq Old) {<br>
> +      unsigned C = representative(Cur.Index);<br>
> +      unsigned Target = representative(Old.Index);<br>
> +      while (C >= Target) {<br>
> +        if (C == Target)<br>
> +          return true;<br>
> +        C = Values[C].Parent;<br>
> +      }<br>
> +      return false;<br>
> +    }<br>
> +<br>
> +  private:<br>
> +    /// \brief Pick a representative for a sequence.<br>
> +    unsigned representative(unsigned K) {<br>
> +      if (Values[K].Merged)<br>
> +        // Perform path compression as we go.<br>
> +        return Values[K].Parent = representative(Values[K].Parent);<br>
> +      return K;<br>
> +    }<br>
> +  };<br>
> +<br>
> +  /// An object for which we can track unsequenced uses.<br>
> +  typedef NamedDecl *Object;<br>
> +<br>
> +  /// Different flavors of object usage which we track. We only track the<br>
> +  /// least-sequenced usage of each kind.<br>
> +  enum UsageKind {<br>
> +    /// A read of an object. Multiple unsequenced reads are OK.<br>
> +    UK_Use,<br>
> +    /// A modification of an object which is sequenced before the value<br>
> +    /// computation of the expression, such as ++n.<br>
> +    UK_ModAsValue,<br>
> +    /// A modification of an object which is not sequenced before the value<br>
> +    /// computation of the expression, such as n++.<br>
> +    UK_ModAsSideEffect,<br>
> +<br>
> +    UK_Count = UK_ModAsSideEffect + 1<br>
> +  };<br>
> +<br>
> +  struct Usage {<br>
> +    Usage() : Use(0), Seq() {}<br>
> +    Expr *Use;<br>
> +    SequenceTree::Seq Seq;<br>
> +  };<br>
> +<br>
> +  struct UsageInfo {<br>
> +    UsageInfo() : Diagnosed(false) {}<br>
> +    Usage Uses[UK_Count];<br>
> +    /// Have we issued a diagnostic for this variable already?<br>
> +    bool Diagnosed;<br>
> +  };<br>
> +  typedef llvm::SmallDenseMap<Object, UsageInfo, 16> UsageInfoMap;<br>
> +<br>
> +  Sema &SemaRef;<br>
> +  /// Sequenced regions within the expression.<br>
> +  SequenceTree Tree;<br>
> +  /// Declaration modifications and references which we have seen.<br>
> +  UsageInfoMap UsageMap;<br>
> +  /// The region we are currently within.<br>
> +  SequenceTree::Seq Region;<br>
> +  /// Filled in with declarations which were modified as a side-effect<br>
> +  /// (that is, post-increment operations).<br>
> +  llvm::SmallVectorImpl<std::pair<Object, Usage> > *ModAsSideEffect;<br>
> +<br>
> +  /// RAII object wrapping the visitation of a sequenced subexpression of an<br>
> +  /// expression. At the end of this process, the side-effects of the evaluation<br>
> +  /// become sequenced with respect to the value computation of the result, so<br>
> +  /// we downgrade any UK_ModAsSideEffect within the evaluation to<br>
> +  /// UK_ModAsValue.<br>
> +  struct SequencedSubexpression {<br>
> +    SequencedSubexpression(SequenceChecker &Self)<br>
> +      : Self(Self), OldModAsSideEffect(Self.ModAsSideEffect) {<br>
> +      Self.ModAsSideEffect = &ModAsSideEffect;<br>
> +    }<br>
> +    ~SequencedSubexpression() {<br>
> +      for (unsigned I = 0, E = ModAsSideEffect.size(); I != E; ++I) {<br>
> +        UsageInfo &U = Self.UsageMap[ModAsSideEffect[I].first];<br>
> +        U.Uses[UK_ModAsSideEffect] = ModAsSideEffect[I].second;<br>
> +        Self.addUsage(U, ModAsSideEffect[I].first,<br>
> +                      ModAsSideEffect[I].second.Use, UK_ModAsValue);<br>
> +      }<br>
> +      Self.ModAsSideEffect = OldModAsSideEffect;<br>
> +    }<br>
> +<br>
> +    SequenceChecker &Self;<br>
> +    llvm::SmallVector<std::pair<Object, Usage>, 4> ModAsSideEffect;<br>
> +    llvm::SmallVectorImpl<std::pair<Object, Usage> > *OldModAsSideEffect;<br>
> +  };<br>
> +<br>
> +  /// \brief Find the object which is produced by the specified expression,<br>
> +  /// if any.<br>
> +  Object getObject(Expr *E, bool Mod) const {<br>
> +    E = E->IgnoreParenCasts();<br>
> +    if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {<br>
> +      if (Mod && (UO->getOpcode() == UO_PreInc || UO->getOpcode() == UO_PreDec))<br>
> +        return getObject(UO->getSubExpr(), Mod);<br>
> +    } else if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {<br>
> +      if (BO->getOpcode() == BO_Comma)<br>
> +        return getObject(BO->getRHS(), Mod);<br>
> +      if (Mod && BO->isAssignmentOp())<br>
> +        return getObject(BO->getLHS(), Mod);<br>
> +    } else if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {<br>
> +      // FIXME: Check for more interesting cases, like "x.n = ++x.n".<br>
> +      if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenCasts()))<br>
> +        return ME->getMemberDecl();<br>
> +    } else if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))<br>
> +      // FIXME: If this is a reference, map through to its value.<br>
> +      return DRE->getDecl();<br>
> +    return 0;<br>
> +  }<br>
> +<br>
> +  /// \brief Note that an object was modified or used by an expression.<br>
> +  void addUsage(UsageInfo &UI, Object O, Expr *Ref, UsageKind UK) {<br>
> +    Usage &U = UI.Uses[UK];<br>
> +    if (!U.Use || !Tree.isUnsequenced(Region, U.Seq)) {<br>
> +      if (UK == UK_ModAsSideEffect && ModAsSideEffect)<br>
> +        ModAsSideEffect->push_back(std::make_pair(O, U));<br>
> +      U.Use = Ref;<br>
> +      U.Seq = Region;<br>
> +    }<br>
> +  }<br>
> +  /// \brief Check whether a modification or use conflicts with a prior usage.<br>
> +  void checkUsage(Object O, UsageInfo &UI, Expr *Ref, UsageKind OtherKind,<br>
> +                  bool IsModMod) {<br>
> +    if (UI.Diagnosed)<br>
> +      return;<br>
> +<br>
> +    const Usage &U = UI.Uses[OtherKind];<br>
> +    if (!U.Use || !Tree.isUnsequenced(Region, U.Seq))<br>
> +      return;<br>
> +<br>
> +    Expr *Mod = U.Use;<br>
> +    Expr *ModOrUse = Ref;<br>
> +    if (OtherKind == UK_Use)<br>
> +      std::swap(Mod, ModOrUse);<br>
> +<br>
> +    SemaRef.Diag(Mod->getExprLoc(),<br>
> +                 IsModMod ? diag::warn_unsequenced_mod_mod<br>
> +                          : diag::warn_unsequenced_mod_use)<br>
> +      << O << SourceRange(ModOrUse->getExprLoc());<br>
> +    UI.Diagnosed = true;<br>
> +  }<br>
> +<br>
> +  void notePreUse(Object O, Expr *Use) {<br>
> +    UsageInfo &U = UsageMap[O];<br>
> +    // Uses conflict with other modifications.<br>
> +    checkUsage(O, U, Use, UK_ModAsValue, false);<br>
> +  }<br>
> +  void notePostUse(Object O, Expr *Use) {<br>
> +    UsageInfo &U = UsageMap[O];<br>
> +    checkUsage(O, U, Use, UK_ModAsSideEffect, false);<br>
> +    addUsage(U, O, Use, UK_Use);<br>
> +  }<br>
> +<br>
> +  void notePreMod(Object O, Expr *Mod) {<br>
> +    UsageInfo &U = UsageMap[O];<br>
> +    // Modifications conflict with other modifications and with uses.<br>
> +    checkUsage(O, U, Mod, UK_ModAsValue, true);<br>
> +    checkUsage(O, U, Mod, UK_Use, false);<br>
> +  }<br>
> +  void notePostMod(Object O, Expr *Use, UsageKind UK) {<br>
> +    UsageInfo &U = UsageMap[O];<br>
> +    checkUsage(O, U, Use, UK_ModAsSideEffect, true);<br>
> +    addUsage(U, O, Use, UK);<br>
> +  }<br>
> +<br>
> +public:<br>
> +  SequenceChecker(Sema &S, Expr *E)<br>
> +    : EvaluatedExprVisitor(S.Context), SemaRef(S), Region(Tree.root()),<br>
> +      ModAsSideEffect(0) {<br>
> +    Visit(E);<br>
> +  }<br>
> +<br>
> +  void VisitStmt(Stmt *S) {<br>
> +    // Skip all statements which aren't expressions for now.<br>
> +  }<br>
> +<br>
> +  void VisitExpr(Expr *E) {<br>
> +    // By default, just recurse to evaluated subexpressions.<br>
> +    EvaluatedExprVisitor::VisitStmt(E);<br>
> +  }<br>
> +<br>
> +  void VisitCastExpr(CastExpr *E) {<br>
> +    Object O = Object();<br>
> +    if (E->getCastKind() == CK_LValueToRValue)<br>
> +      O = getObject(E->getSubExpr(), false);<br>
> +<br>
> +    if (O)<br>
> +      notePreUse(O, E);<br>
> +    VisitExpr(E);<br>
> +    if (O)<br>
> +      notePostUse(O, E);<br>
> +  }<br>
> +<br>
> +  void VisitBinComma(BinaryOperator *BO) {<br>
> +    // C++11 [expr.comma]p1:<br>
> +    //   Every value computation and side effect associated with the left<br>
> +    //   expression is sequenced before every value computation and side<br>
> +    //   effect associated with the right expression.<br>
> +    SequenceTree::Seq LHS = Tree.allocate(Region);<br>
> +    SequenceTree::Seq RHS = Tree.allocate(Region);<br>
> +    SequenceTree::Seq OldRegion = Region;<br>
> +<br>
> +    {<br>
> +      SequencedSubexpression SeqLHS(*this);<br>
> +      Region = LHS;<br>
> +      Visit(BO->getLHS());<br>
> +    }<br>
> +<br>
> +    Region = RHS;<br>
> +    Visit(BO->getRHS());<br>
> +<br>
> +    Region = OldRegion;<br>
> +<br>
> +    // Forget that LHS and RHS are sequenced. They are both unsequenced<br>
> +    // with respect to other stuff.<br>
> +    Tree.merge(LHS);<br>
> +    Tree.merge(RHS);<br>
> +  }<br>
> +<br>
> +  void VisitBinAssign(BinaryOperator *BO) {<br>
> +    // The modification is sequenced after the value computation of the LHS<br>
> +    // and RHS, so check it before inspecting the operands and update the<br>
> +    // map afterwards.<br>
> +    Object O = getObject(BO->getLHS(), true);<br>
> +    if (!O)<br>
> +      return VisitExpr(BO);<br>
> +<br>
> +    notePreMod(O, BO);<br>
> +<br>
> +    // C++11 [expr.ass]p7:<br>
> +    //   E1 op= E2 is equivalent to E1 = E1 op E2, except that E1 is evaluated<br>
> +    //   only once.<br>
> +    //<br>
> +    // Therefore, for a compound assignment operator, O is considered used<br>
> +    // everywhere except within the evaluation of E1 itself.<br>
> +    if (isa<CompoundAssignOperator>(BO))<br>
> +      notePreUse(O, BO);<br>
> +<br>
> +    Visit(BO->getLHS());<br>
> +<br>
> +    if (isa<CompoundAssignOperator>(BO))<br>
> +      notePostUse(O, BO);<br>
> +<br>
> +    Visit(BO->getRHS());<br>
> +<br>
> +    notePostMod(O, BO, UK_ModAsValue);<br>
> +  }<br>
> +  void VisitCompoundAssignOperator(CompoundAssignOperator *CAO) {<br>
> +    VisitBinAssign(CAO);<br>
> +  }<br>
> +<br>
> +  void VisitUnaryPreInc(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }<br>
> +  void VisitUnaryPreDec(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }<br>
> +  void VisitUnaryPreIncDec(UnaryOperator *UO) {<br>
> +    Object O = getObject(UO->getSubExpr(), true);<br>
> +    if (!O)<br>
> +      return VisitExpr(UO);<br>
> +<br>
> +    notePreMod(O, UO);<br>
> +    Visit(UO->getSubExpr());<br>
> +    notePostMod(O, UO, UK_ModAsValue);<br>
> +  }<br>
> +<br>
> +  void VisitUnaryPostInc(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }<br>
> +  void VisitUnaryPostDec(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }<br>
> +  void VisitUnaryPostIncDec(UnaryOperator *UO) {<br>
> +    Object O = getObject(UO->getSubExpr(), true);<br>
> +    if (!O)<br>
> +      return VisitExpr(UO);<br>
> +<br>
> +    notePreMod(O, UO);<br>
> +    Visit(UO->getSubExpr());<br>
> +    notePostMod(O, UO, UK_ModAsSideEffect);<br>
> +  }<br>
> +<br>
> +  /// Don't visit the RHS of '&&' or '||' if it might not be evaluated.<br>
> +  void VisitBinLOr(BinaryOperator *BO) {<br>
> +    // The side-effects of the LHS of an '&&' are sequenced before the<br>
> +    // value computation of the RHS, and hence before the value computation<br>
> +    // of the '&&' itself, unless the LHS evaluates to zero. We treat them<br>
> +    // as if they were unconditionally sequenced.<br>
> +    {<br>
> +      SequencedSubexpression Sequenced(*this);<br>
> +      Visit(BO->getLHS());<br>
> +    }<br>
> +<br>
> +    bool Result;<br>
> +    if (!BO->getLHS()->isValueDependent() &&<br>
> +        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context) &&<br>
> +        !Result)<br>
> +      Visit(BO->getRHS());<br>
> +  }<br>
> +  void VisitBinLAnd(BinaryOperator *BO) {<br>
> +    {<br>
> +      SequencedSubexpression Sequenced(*this);<br>
> +      Visit(BO->getLHS());<br>
> +    }<br>
> +<br>
> +    bool Result;<br>
> +    if (!BO->getLHS()->isValueDependent() &&<br>
> +        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context) &&<br>
> +        Result)<br>
> +      Visit(BO->getRHS());<br>
> +  }<br>
> +<br>
> +  // Only visit the condition, unless we can be sure which subexpression will<br>
> +  // be chosen.<br>
> +  void VisitAbstractConditionalOperator(AbstractConditionalOperator *CO) {<br>
> +    SequencedSubexpression Sequenced(*this);<br>
> +    Visit(CO->getCond());<br>
> +<br>
> +    bool Result;<br>
> +    if (!CO->getCond()->isValueDependent() &&<br>
> +        CO->getCond()->EvaluateAsBooleanCondition(Result, SemaRef.Context))<br>
> +      Visit(Result ? CO->getTrueExpr() : CO->getFalseExpr());<br>
> +  }<br>
> +<br>
> +  void VisitCXXConstructExpr(CXXConstructExpr *CCE) {<br>
> +    if (!CCE->isListInitialization())<br>
> +      return VisitExpr(CCE);<br>
> +<br>
> +    // In C++11, list initializations are sequenced.<br>
> +    llvm::SmallVector<SequenceTree::Seq, 32> Elts;<br>
> +    SequenceTree::Seq Parent = Region;<br>
> +    for (CXXConstructExpr::arg_iterator I = CCE->arg_begin(),<br>
> +                                        E = CCE->arg_end();<br>
> +         I != E; ++I) {<br>
> +      Region = Tree.allocate(Parent);<br>
> +      Elts.push_back(Region);<br>
> +      Visit(*I);<br>
> +    }<br>
> +<br>
> +    // Forget that the initializers are sequenced.<br>
> +    Region = Parent;<br>
> +    for (unsigned I = 0; I < Elts.size(); ++I)<br>
> +      Tree.merge(Elts[I]);<br>
> +  }<br>
> +<br>
> +  void VisitInitListExpr(InitListExpr *ILE) {<br>
> +    if (!SemaRef.getLangOpts().CPlusPlus11)<br>
> +      return VisitExpr(ILE);<br>
> +<br>
> +    // In C++11, list initializations are sequenced.<br>
> +    llvm::SmallVector<SequenceTree::Seq, 32> Elts;<br>
> +    SequenceTree::Seq Parent = Region;<br>
> +    for (unsigned I = 0; I < ILE->getNumInits(); ++I) {<br>
> +      Expr *E = ILE->getInit(I);<br>
> +      if (!E) continue;<br>
> +      Region = Tree.allocate(Parent);<br>
> +      Elts.push_back(Region);<br>
> +      Visit(E);<br>
> +    }<br>
> +<br>
> +    // Forget that the initializers are sequenced.<br>
> +    Region = Parent;<br>
> +    for (unsigned I = 0; I < Elts.size(); ++I)<br>
> +      Tree.merge(Elts[I]);<br>
> +  }<br>
> +};<br>
> +}<br>
> +<br>
> +void Sema::CheckUnsequencedOperations(Expr *E) {<br>
> +  SequenceChecker(*this, E);<br>
> +}<br>
> +<br>
> +void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc) {<br>
> +  CheckImplicitConversions(E, CheckLoc);<br>
> +  CheckUnsequencedOperations(E);<br>
> +}<br>
> +<br>
>  void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,<br>
>                                         FieldDecl *BitField,<br>
>                                         Expr *Init) {<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -252,7 +252,7 @@<br>
>      return true;<br>
>    Arg = Result.takeAs<Expr>();<br>
><br>
> -  CheckImplicitConversions(Arg, EqualLoc);<br>
> +  CheckCompletedExpr(Arg, EqualLoc);<br>
>    Arg = MaybeCreateExprWithCleanups(Arg);<br>
><br>
>    // Okay: add the default argument to the parameter<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -3534,7 +3534,7 @@<br>
>        return ExprError();<br>
><br>
>      Expr *Arg = Result.takeAs<Expr>();<br>
> -    CheckImplicitConversions(Arg, Param->getOuterLocStart());<br>
> +    CheckCompletedExpr(Arg, Param->getOuterLocStart());<br>
>      // Build the default argument expression.<br>
>      return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param, Arg));<br>
>    }<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -18,6 +18,7 @@<br>
>  #include "clang/AST/CXXInheritance.h"<br>
>  #include "clang/AST/CharUnits.h"<br>
>  #include "clang/AST/DeclObjC.h"<br>
> +#include "clang/AST/EvaluatedExprVisitor.h"<br>
>  #include "clang/AST/ExprCXX.h"<br>
>  #include "clang/AST/ExprObjC.h"<br>
>  #include "clang/AST/TypeLoc.h"<br>
> @@ -5496,7 +5497,7 @@<br>
>        return ExprError();<br>
>    }<br>
><br>
> -  CheckImplicitConversions(FullExpr.get(), CC);<br>
> +  CheckCompletedExpr(FullExpr.get(), CC);<br>
>    return MaybeCreateExprWithCleanups(FullExpr);<br>
>  }<br>
><br>
><br>
> Modified: cfe/trunk/test/SemaCXX/altivec.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/altivec.cpp?rev=172690&r1=172689&r2=172690&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/altivec.cpp?rev=172690&r1=172689&r2=172690&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/altivec.cpp (original)<br>
> +++ cfe/trunk/test/SemaCXX/altivec.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -62,7 +62,7 @@<br>
>    vector float vf;<br>
>    vf++;<br>
><br>
> -  ++vi=vi;<br>
> +  ++vi=vi; // expected-warning {{unsequenced}}<br>
>    (++vi)[1]=1;<br>
>    template_f(vi);<br>
>  }<br>
><br>
> Added: cfe/trunk/test/SemaCXX/warn-unsequenced.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unsequenced.cpp?rev=172690&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unsequenced.cpp?rev=172690&view=auto</a><br>

> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/warn-unsequenced.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/warn-unsequenced.cpp Wed Jan 16 19:17:56 2013<br>
> @@ -0,0 +1,91 @@<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-unused %s<br>
> +<br>
> +int f(int, int);<br>
> +<br>
> +struct A {<br>
> +  int x, y;<br>
> +};<br>
> +struct S {<br>
> +  S(int, int);<br>
> +};<br>
> +<br>
> +void test() {<br>
> +  int a;<br>
> +  int xs[10];<br>
> +  ++a = 0; // ok<br>
> +  a + ++a; // expected-warning {{unsequenced modification and access to 'a'}}<br>
> +  a = ++a; // ok<br>
> +  a + a++; // expected-warning {{unsequenced modification and access to 'a'}}<br>
> +  a = a++; // expected-warning {{multiple unsequenced modifications to 'a'}}<br>
> +  ++ ++a; // ok<br>
> +  (a++, a++); // ok<br>
> +  ++a + ++a; // expected-warning {{multiple unsequenced modifications to 'a'}}<br>
> +  a++ + a++; // expected-warning {{multiple unsequenced modifications}}<br>
> +  (a++, a) = 0; // ok, increment is sequenced before value computation of LHS<br>
> +  a = xs[++a]; // ok<br>
> +  a = xs[a++]; // expected-warning {{multiple unsequenced modifications}}<br>
> +  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access}}<br>
> +  a = (++a, ++a); // ok<br>
> +  a = (a++, ++a); // ok<br>
> +  a = (a++, a++); // expected-warning {{multiple unsequenced modifications}}<br>
> +  f(a, a); // ok<br>
> +  f(a = 0, a); // expected-warning {{unsequenced modification and access}}<br>
> +  f(a, a += 0); // expected-warning {{unsequenced modification and access}}<br>
> +  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications}}<br>
> +<br>
> +  // Compound assignment "A OP= B" is equivalent to "A = A OP B" except that A<br>
> +  // is evaluated only once.<br>
> +  (++a, a) = 1; // ok<br>
> +  (++a, a) += 1; // ok<br>
> +  a = ++a; // ok<br>
> +  a += ++a; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  A agg1 = { a++, a++ }; // ok<br>
> +  A agg2 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  S str1(a++, a++); // expected-warning {{multiple unsequenced modifications}}<br>
> +  S str2 = { a++, a++ }; // ok<br>
> +  S str3 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  (xs[2] && (a = 0)) + a; // ok<br>
> +  (0 && (a = 0)) + a; // ok<br>
> +  (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  (xs[3] || (a = 0)) + a; // ok<br>
> +  (0 || (a = 0)) + a; // expected-warning {{unsequenced modification and access}}<br>
> +  (1 || (a = 0)) + a; // ok<br>
> +<br>
> +  (xs[4] ? a : ++a) + a; // ok<br>
> +  (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access}}<br>
> +  (1 ? a : ++a) + a; // ok<br>
> +  (xs[5] ? ++a : ++a) + a; // FIXME: warn here<br>
> +<br>
> +  (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  // Here, the read of the fourth 'a' might happen before or after the write to<br>
> +  // the second 'a'.<br>
> +  a += (a++, a) + a; // expected-warning {{unsequenced modification and access}}<br>
> +<br>
> +  int *p = xs;<br>
> +  a = *(a++, p); // ok<br>
> +  a = a++ && a; // ok<br>
> +<br>
> +  A *q = &agg1;<br>
> +  (q = &agg2)->y = q->x; // expected-warning {{unsequenced modification and access to 'q'}}<br>
> +<br>
> +  // This has undefined behavior if a == 0; otherwise, the side-effect of the<br>
> +  // increment is sequenced before the value computation of 'f(a, a)', which is<br>
> +  // sequenced before the value computation of the '&&', which is sequenced<br>
> +  // before the assignment. We treat the sequencing in '&&' as being<br>
> +  // unconditional.<br>
> +  a = a++ && f(a, a);<br>
> +<br>
> +  // This has undefined behavior if a != 0. FIXME: We should diagnose this.<br>
> +  (a && a++) + a;<br>
> +<br>
> +  (xs[7] && ++a) * (!xs[7] && ++a); // ok<br>
> +<br>
> +  xs[0] = (a = 1, a); // ok<br>
> +  (a -= 128) &= 128; // ok<br>
> +  ++a += 1; // ok<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br>