merge main into amd-staging

Author: ronlieb

clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp

@@ -66,8 +66,11 @@ void UnconventionalAssignOperatorCheck::registerMatchers(
                                 hasArgument(0, cxxThisExpr())),
             cxxOperatorCallExpr(
                 hasOverloadedOperatorName("="),
-                hasArgument(
-                    0, unaryOperator(hasOperatorName("*"),
+                hasArgument(0, unaryOperator(hasOperatorName("*"),
+                                             hasUnaryOperand(cxxThisExpr())))),
+            binaryOperator(
+                hasOperatorName("="),
+                hasLHS(unaryOperator(hasOperatorName("*"),
                                      hasUnaryOperand(cxxThisExpr())))))))));
   const auto IsGoodAssign = cxxMethodDecl(IsAssign, HasGoodReturnType);
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -266,6 +266,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check by providing additional
   examples and fixing some macro related false positives.
 
+- Improved :doc:`misc-unconventional-assign-operator
+  <clang-tidy/checks/misc/unconventional-assign-operator>` check by fixing
+  false positives when copy assignment operator function in a template class
+  returns the result of another assignment to ``*this`` (``return *this=...``).
+
 - Improved :doc:`misc-unused-using-decls
   <clang-tidy/checks/misc/unused-using-decls>` check by fixing false positives
   on ``operator""`` with template parameters.

clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp

@@ -163,3 +163,16 @@ struct TemplateTypeAlias {
   Alias3<TypeAlias::Alias> &operator=(double) { return *this; }
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'TemplateTypeAlias&' [misc-unconventional-assign-operator]
 };
+
+namespace gh143237 {
+template<typename T>
+struct TemplateAssignment {
+  explicit TemplateAssignment(int) {
+  }
+
+  TemplateAssignment& operator=(int n) {
+    // No warning
+    return *this = TemplateAssignment(n);
+  }
+};
+}

clang/lib/AST/ByteCode/Compiler.cpp

@@ -5545,7 +5545,6 @@ bool Compiler<Emitter>::visitCXXForRangeStmt(const CXXForRangeStmt *S) {
   const Stmt *BeginStmt = S->getBeginStmt();
   const Stmt *RangeStmt = S->getRangeStmt();
   const Stmt *EndStmt = S->getEndStmt();
-  const VarDecl *LoopVar = S->getLoopVariable();
 
   LabelTy EndLabel = this->getLabel();
   LabelTy CondLabel = this->getLabel();
@@ -5570,7 +5569,7 @@ bool Compiler<Emitter>::visitCXXForRangeStmt(const CXXForRangeStmt *S) {
   if (!this->jumpFalse(EndLabel))
     return false;
 
-  if (!this->visitVarDecl(LoopVar))
+  if (!this->visitDeclStmt(S->getLoopVarStmt(), /*EvaluateConditionDecl=*/true))
     return false;
 
   // Body.

clang/lib/AST/ByteCode/Interp.h

@@ -1930,8 +1930,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
   const Pointer &Ptr = S.Stk.peek<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
-  if (Ptr.canBeInitialized())
+  if (Ptr.canBeInitialized()) {
     Ptr.initialize();
+    Ptr.activate();
+  }
   if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
   else
@@ -1945,8 +1947,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
   const Pointer &Ptr = S.Stk.pop<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
-  if (Ptr.canBeInitialized())
+  if (Ptr.canBeInitialized()) {
     Ptr.initialize();
+    Ptr.activate();
+  }
   if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
   else

clang/lib/AST/ByteCode/InterpBuiltin.cpp

@@ -861,7 +861,9 @@ static bool interp__builtin_overflowop(InterpState &S, CodePtr OpPC,
 
   // Write Result to ResultPtr and put Overflow on the stack.
   assignInteger(S, ResultPtr, ResultT, Result);
-  ResultPtr.initialize();
+  if (ResultPtr.canBeInitialized())
+    ResultPtr.initialize();
+
   assert(Call->getDirectCallee()->getReturnType()->isBooleanType());
   S.Stk.push<Boolean>(Overflow);
   return true;

clang/lib/AST/ByteCode/Pointer.cpp

@@ -505,8 +505,19 @@ void Pointer::activate() const {
   auto activate = [](Pointer &P) -> void {
     P.getInlineDesc()->IsActive = true;
   };
-  auto deactivate = [](Pointer &P) -> void {
+
+  std::function<void(Pointer &)> deactivate;
+  deactivate = [&deactivate](Pointer &P) -> void {
     P.getInlineDesc()->IsActive = false;
+
+    if (const Record *R = P.getRecord()) {
+      for (const Record::Field &F : R->fields()) {
+        Pointer FieldPtr = P.atField(F.Offset);
+        if (FieldPtr.getInlineDesc()->IsActive)
+          deactivate(FieldPtr);
+      }
+      // FIXME: Bases?
+    }
   };
 
   // Unions might be nested etc., so find the topmost Pointer that's
@@ -516,21 +527,31 @@ void Pointer::activate() const {
     UnionPtr = UnionPtr.getBase();
 
   assert(UnionPtr.getFieldDesc()->isUnion());
-
   const Record *UnionRecord = UnionPtr.getRecord();
+
+  // The direct child pointer of the union that's on the path from
+  // this pointer to the union.
+  Pointer ChildPtr = *this;
+  assert(ChildPtr != UnionPtr);
+  while (true) {
+    if (ChildPtr.getBase() == UnionPtr)
+      break;
+    ChildPtr = ChildPtr.getBase();
+  }
+  assert(ChildPtr.getBase() == UnionPtr);
+
   for (const Record::Field &F : UnionRecord->fields()) {
     Pointer FieldPtr = UnionPtr.atField(F.Offset);
-    if (FieldPtr == *this) {
+    if (FieldPtr == ChildPtr) {
+      // No need to deactivate, will be activated in the next loop.
     } else {
       deactivate(FieldPtr);
-      // FIXME: Recurse.
     }
   }
 
   Pointer B = *this;
   while (B != UnionPtr) {
     activate(B);
-    // FIXME: Need to de-activate other fields of parent records.
     B = B.getBase();
   }
 }

clang/test/AST/ByteCode/builtin-functions.cpp

@@ -1753,4 +1753,12 @@ namespace I128Mul {
 }
 #endif
 
+namespace InitParam {
+  constexpr int foo(int a) {
+      __builtin_mul_overflow(20, 10, &a);
+      return a;
+  }
+  static_assert(foo(10) == 200);
+}
+
 #endif