[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103314#2810795 , @martong wrote:

> I have the first measurements results in the attached zip file. The file 
> contains the html file generated by csa-testbench. It's name contains `CTU` 
> but actually it was a regular non-CTU analysis. The most interesting is 
> probably the run-times, where we can notice a small increase:
> F17327338: image.png 
> Other than that, the number of the warnings seems to be unchanged. The most 
> notable change in the statistics is in the number of paths explored by the 
> analyzer: in some cases (e.g. twin) it increased with 2-3 %. F17327375: 
> CTU_20results_20on_20open_20projects_201.zip 
> 

This sounds amazing! Great job!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103314/new/

https://reviews.llvm.org/D103314

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bf20631 - [clang] Implement P2266 Simpler implicit move

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2021-06-13T12:10:56+02:00
New Revision: bf20631782183cd19e0bb7219e908c2bbb01a75f

URL: 
https://github.com/llvm/llvm-project/commit/bf20631782183cd19e0bb7219e908c2bbb01a75f
DIFF: 
https://github.com/llvm/llvm-project/commit/bf20631782183cd19e0bb7219e908c2bbb01a75f.diff

LOG: [clang] Implement P2266 Simpler implicit move

This Implements 
[[http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2266r1.html|P2266 
Simpler implicit move]].

Signed-off-by: Matheus Izvekov 

Reviewed By: Quuxplusone

Differential Revision: https://reviews.llvm.org/D99005

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/constant-expression-cxx14.cpp
clang/test/SemaCXX/coroutine-rvo.cpp
clang/test/SemaCXX/coroutines.cpp
clang/test/SemaCXX/deduced-return-type-cxx14.cpp
clang/test/SemaCXX/return-stack-addr.cpp
clang/test/SemaCXX/warn-return-std-move.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f7ec89a33e00c..db389922ae3a1 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4763,7 +4763,7 @@ class Sema final {
 bool isMoveEligible() const { return S != None; };
 bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
   };
-  NamedReturnInfo getNamedReturnInfo(const Expr *E, bool ForceCXX20 = false);
+  NamedReturnInfo getNamedReturnInfo(Expr *&E, bool ForceCXX2b = false);
   NamedReturnInfo getNamedReturnInfo(const VarDecl *VD,
  bool ForceCXX20 = false);
   const VarDecl *getCopyElisionCandidate(NamedReturnInfo &Info,

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 187e7a0516d10..cec80436d575e 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -994,22 +994,10 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, 
Expr *E,
 E = R.get();
   }
 
-  // Move the return value if we can
-  NamedReturnInfo NRInfo = getNamedReturnInfo(E, /*ForceCXX20=*/true);
-  if (NRInfo.isMoveEligible()) {
-InitializedEntity Entity = InitializedEntity::InitializeResult(
-Loc, E->getType(), NRInfo.Candidate);
-ExprResult MoveResult = PerformMoveOrCopyInitialization(Entity, NRInfo, E);
-if (MoveResult.get())
-  E = MoveResult.get();
-  }
-
-  // FIXME: If the operand is a reference to a variable that's about to go out
-  // of scope, we should treat the operand as an xvalue for this overload
-  // resolution.
   VarDecl *Promise = FSI->CoroutinePromise;
   ExprResult PC;
   if (E && (isa(E) || !E->getType()->isVoidType())) {
+getNamedReturnInfo(E, /*ForceCXX2b=*/true);
 PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
   } else {
 E = MakeFullDiscardedValueExpr(E).get();

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e9ab2bcd67c2e..20925f95f18e7 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -854,10 +854,6 @@ ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr 
*Ex,
 Diag(OpLoc, diag::err_omp_simd_region_cannot_use_stmt) << "throw";
 
   if (Ex && !Ex->isTypeDependent()) {
-QualType ExceptionObjectTy = Context.getExceptionObjectType(Ex->getType());
-if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
-  return ExprError();
-
 // Initialize the exception result.  This implicitly weeds out
 // abstract types or types with inaccessible copy constructors.
 
@@ -876,6 +872,10 @@ ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr 
*Ex,
 NamedReturnInfo NRInfo =
 IsThrownVarInScope ? getNamedReturnInfo(Ex) : NamedReturnInfo();
 
+QualType ExceptionObjectTy = Context.getExceptionObjectType(Ex->getType());
+if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
+  return ExprError();
+
 InitializedEntity Entity = InitializedEntity::InitializeException(
 OpLoc, ExceptionObjectTy,
 /*NRVO=*/NRInfo.isCopyElidable());

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 136e39198c728..afea878b299a6 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3312,15 +3312,16 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope 
*CurScope) {
 /// without considering function return type, if applicable.
 ///
 /// \param E The expression being retur

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Matheus Izvekov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf2063178218: [clang] Implement P2266 Simpler implicit move 
(authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99005/new/

https://reviews.llvm.org/D99005

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
 
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 // RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
@@ -217,8 +217,8 @@
 }
 
 // But if the return type is a reference type, then moving would be wrong.
-Derived& testRetRef1(Derived&& d) { return d; }
-Base& testRetRef2(Derived&& d) { return d; }
+Derived &testRetRef1(Derived &&d) { return d; } // cxx2b-error {{non-const lvalue reference to type 'Derived' cannot bind to a temporary of type 'Derived'}}
+Base &testRetRef2(Derived &&d) { return d; }// cxx2b-error {{non-const lvalue reference to type 'Base' cannot bind to a temporary of type 'Derived'}}
 #if __cplusplus >= 201402L
 auto&& testRetRef3(Derived&& d) { return d; }
 decltype(auto) testRetRef4(Derived&& d) { return (d); }
Index: clang/test/SemaCXX/return-stack-addr.cpp
===
--- clang/test/SemaCXX/return-stack-addr.cpp
+++ clang/test/SemaCXX/return-stack-addr.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected   %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected   %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11 %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx2b  %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx11_20   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11 %s
 
 int* ret_local() {
   int x = 1;
@@ -29,7 +29,8 @@
 
 int& ret_local_ref() {
   int x = 1;
-  return x;  // expected-warning {{reference to stack memory}}
+  return x; // cxx11_20-warning {{reference to stack memory}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
 }
 
 int* ret_local_addrOf() {
@@ -154,8 +155,10 @@
   (void) [&]() -> int& { return b; };
   (void) [=]() mutable -> int& { return a; };
   (void) [=]() mutable -> int& { return b; };
-  (void) [&]() -> int& { int a; return a; }; // expected-warning {{reference to stack}}
-  (void) [=]() -> int& { int a; return a; }; // expected-warning {{reference to stack}}
+  (void) [&]() -> int& { int a; return a; }; // cxx11_20-warning {{reference to stack}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
+  (void) [=]() -> int& { int a; return a; }; // cxx11_20-warning {{reference to stack}}
+  // cxx2b-error@-1 {{non-const lvalue reference to type '

[clang] 7d7e913 - SValExplainer.h - get APSInt values by const reference instead of value. NFCI.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2021-06-13T13:05:17+01:00
New Revision: 7d7e913e096a915038dd41d0bfe5dd8827da1f60

URL: 
https://github.com/llvm/llvm-project/commit/7d7e913e096a915038dd41d0bfe5dd8827da1f60
DIFF: 
https://github.com/llvm/llvm-project/commit/7d7e913e096a915038dd41d0bfe5dd8827da1f60.diff

LOG: SValExplainer.h - get APSInt values by const reference instead of value. 
NFCI.

Avoid unnecessary copies.

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h 
b/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
index 0f33909daec0d..31a4ed50a7230 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
@@ -65,7 +65,7 @@ class SValExplainer : public FullSValVisitor {
   }
 
   std::string VisitLocConcreteInt(loc::ConcreteInt V) {
-llvm::APSInt I = V.getValue();
+const llvm::APSInt &I = V.getValue();
 std::string Str;
 llvm::raw_string_ostream OS(Str);
 OS << "concrete memory address '" << I << "'";
@@ -77,7 +77,7 @@ class SValExplainer : public FullSValVisitor {
   }
 
   std::string VisitNonLocConcreteInt(nonloc::ConcreteInt V) {
-llvm::APSInt I = V.getValue();
+const llvm::APSInt &I = V.getValue();
 std::string Str;
 llvm::raw_string_ostream OS(Str);
 OS << (I.isSigned() ? "signed " : "unsigned ") << I.getBitWidth()



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 7ff3a89 - [clang][NFC] Add IsAnyDestructorNoReturn field to CXXRecord instead of calculating it on demand

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2021-06-13T14:48:27+02:00
New Revision: 7ff3a89a7b94193638cb13f8a0a1ef70094c8263

URL: 
https://github.com/llvm/llvm-project/commit/7ff3a89a7b94193638cb13f8a0a1ef70094c8263
DIFF: 
https://github.com/llvm/llvm-project/commit/7ff3a89a7b94193638cb13f8a0a1ef70094c8263.diff

LOG: [clang][NFC] Add IsAnyDestructorNoReturn field to CXXRecord instead of 
calculating it on demand

This patch addresses a performance issue I noticed when using clang-12 to 
compile projects of mine. Even though the files weren't too large (around 1k 
cpp), the compiler was taking more than a minute to compile the source file, 
much longer than either GCC or MSVC.

Using a profiler it turned out the issue was the isAnyDestructorNoReturn 
function in CXXRecordDecl. In particular it being recursive, recalculating the 
property for every invocation, for every field and base class. This showed up 
in tracebacks in the profiler.

This patch instead adds IsAnyDestructorNoReturn as a Field to the data inside 
of CXXRecord and updates when a new base class, destructor, or record field 
member is added.

After this patch the problematic file of mine went from a compile time of 81s, 
down to 12s.

The patch itself should not change any functionality, just improve performance.

Differential Revision: https://reviews.llvm.org/D104182

Added: 


Modified: 
clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
clang/include/clang/AST/DeclCXX.h
clang/lib/AST/DeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def 
b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
index d15d6698860f4..9b270682f8cf0 100644
--- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -242,4 +242,8 @@ FIELD(HasDeclaredCopyConstructorWithConstParam, 1, MERGE_OR)
 /// const-qualified reference parameter or a non-reference parameter.
 FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
 
+/// Whether the destructor is no-return. Either explicitly, or if any
+/// base classes or fields have a no-return destructor
+FIELD(IsAnyDestructorNoReturn, 1, NO_MERGE)
+
 #undef FIELD

diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index e9f9da6bd4bc4..0d5ad40fc19e7 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1480,7 +1480,7 @@ class CXXRecordDecl : public RecordDecl {
 
   /// Returns true if the class destructor, or any implicitly invoked
   /// destructors are marked noreturn.
-  bool isAnyDestructorNoReturn() const;
+  bool isAnyDestructorNoReturn() const { return 
data().IsAnyDestructorNoReturn; }
 
   /// If the class is a local class [class.local], returns
   /// the enclosing function declaration.

diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index bcff72ccadeab..aeee35d9c74f6 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -108,7 +108,8 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl 
*D)
   ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
   ImplicitCopyAssignmentHasConstParam(true),
   HasDeclaredCopyConstructorWithConstParam(false),
-  HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+  HasDeclaredCopyAssignmentWithConstParam(false),
+  IsAnyDestructorNoReturn(false), IsLambda(false),
   IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
   HasODRHash(false), Definition(D) {}
 
@@ -424,6 +425,9 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const 
*Bases,
 if (!BaseClassDecl->hasIrrelevantDestructor())
   data().HasIrrelevantDestructor = false;
 
+if (BaseClassDecl->isAnyDestructorNoReturn())
+  data().IsAnyDestructorNoReturn = true;
+
 // C++11 [class.copy]p18:
 //   The implicitly-declared copy assignment operator for a class X will
 //   have the form 'X& X::operator=(const X&)' if each direct base class B
@@ -836,6 +840,9 @@ void CXXRecordDecl::addedMember(Decl *D) {
   data().HasTrivialSpecialMembers &= ~SMF_Destructor;
   data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
 }
+
+if (DD->isNoReturn())
+  data().IsAnyDestructorNoReturn = true;
   }
 
   // Handle member functions.
@@ -1233,6 +1240,8 @@ void CXXRecordDecl::addedMember(Decl *D) {
   data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
 if (!FieldRec->hasIrrelevantDestructor())
   data().HasIrrelevantDestructor = false;
+if (FieldRec->isAnyDestructorNoReturn())
+  data().IsAnyDestructorNoReturn = true;
 if (FieldRec->hasObjectMember())
   setHasObjectMember(true);
 if (FieldRec->hasVolatileMember())
@@ -1888,29 +1897,6 @@ CXXDestructorDecl *CXXRecordDecl::getDestructor() const {
   return R.empty() ? nullptr : dyn_c

[PATCH] D104182: [clang][NFC] Add IsAnyDestructorNoReturn field to CXXRecord instead of calculating it on demand

[Markus Böck via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ff3a89a7b94: [clang][NFC] Add IsAnyDestructorNoReturn field 
to CXXRecord instead of… (authored by zero9178).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104182/new/

https://reviews.llvm.org/D104182

Files:
  clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/DeclCXX.cpp


Index: clang/lib/AST/DeclCXX.cpp
===
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -108,7 +108,8 @@
   ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
   ImplicitCopyAssignmentHasConstParam(true),
   HasDeclaredCopyConstructorWithConstParam(false),
-  HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+  HasDeclaredCopyAssignmentWithConstParam(false),
+  IsAnyDestructorNoReturn(false), IsLambda(false),
   IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
   HasODRHash(false), Definition(D) {}
 
@@ -424,6 +425,9 @@
 if (!BaseClassDecl->hasIrrelevantDestructor())
   data().HasIrrelevantDestructor = false;
 
+if (BaseClassDecl->isAnyDestructorNoReturn())
+  data().IsAnyDestructorNoReturn = true;
+
 // C++11 [class.copy]p18:
 //   The implicitly-declared copy assignment operator for a class X will
 //   have the form 'X& X::operator=(const X&)' if each direct base class B
@@ -836,6 +840,9 @@
   data().HasTrivialSpecialMembers &= ~SMF_Destructor;
   data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
 }
+
+if (DD->isNoReturn())
+  data().IsAnyDestructorNoReturn = true;
   }
 
   // Handle member functions.
@@ -1233,6 +1240,8 @@
   data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
 if (!FieldRec->hasIrrelevantDestructor())
   data().HasIrrelevantDestructor = false;
+if (FieldRec->isAnyDestructorNoReturn())
+  data().IsAnyDestructorNoReturn = true;
 if (FieldRec->hasObjectMember())
   setHasObjectMember(true);
 if (FieldRec->hasVolatileMember())
@@ -1888,29 +1897,6 @@
   return R.empty() ? nullptr : dyn_cast(R.front());
 }
 
-bool CXXRecordDecl::isAnyDestructorNoReturn() const {
-  // Destructor is noreturn.
-  if (const CXXDestructorDecl *Destructor = getDestructor())
-if (Destructor->isNoReturn())
-  return true;
-
-  // Check base classes destructor for noreturn.
-  for (const auto &Base : bases())
-if (const CXXRecordDecl *RD = Base.getType()->getAsCXXRecordDecl())
-  if (RD->isAnyDestructorNoReturn())
-return true;
-
-  // Check fields for noreturn.
-  for (const auto *Field : fields())
-if (const CXXRecordDecl *RD =
-Field->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl())
-  if (RD->isAnyDestructorNoReturn())
-return true;
-
-  // All destructors are not noreturn.
-  return false;
-}
-
 static bool isDeclContextInNamespace(const DeclContext *DC) {
   while (!DC->isTranslationUnit()) {
 if (DC->isNamespace())
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -1480,7 +1480,7 @@
 
   /// Returns true if the class destructor, or any implicitly invoked
   /// destructors are marked noreturn.
-  bool isAnyDestructorNoReturn() const;
+  bool isAnyDestructorNoReturn() const { return 
data().IsAnyDestructorNoReturn; }
 
   /// If the class is a local class [class.local], returns
   /// the enclosing function declaration.
Index: clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
===
--- clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -242,4 +242,8 @@
 /// const-qualified reference parameter or a non-reference parameter.
 FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
 
+/// Whether the destructor is no-return. Either explicitly, or if any
+/// base classes or fields have a no-return destructor
+FIELD(IsAnyDestructorNoReturn, 1, NO_MERGE)
+
 #undef FIELD


Index: clang/lib/AST/DeclCXX.cpp
===
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -108,7 +108,8 @@
   ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
   ImplicitCopyAssignmentHasConstParam(true),
   HasDeclaredCopyConstructorWithConstParam(false),
-  HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+  HasDeclaredCopyAssignmentWithConstParam(false),
+  IsAnyDestructorNoReturn(false), IsLambda(false),
   IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
   HasOD

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

[Darwin Xu via Phabricator via cfe-commits]

darwin added a comment.

In D104044#2813515 , @MyDeveloperDay 
wrote:

> Do we need a set options for when we want to insert/retain/add a newline 
> after various constructs? frankly I've been mulling over the concept of 
> adding a
>
>   NewLinesBetweenFunctions: 1
>
> I personally don't like code written like this as I find it hard to read, I'd 
> like to be able to mandate a single line between functions
>
>   void foo()
>   {
>   ...
>   }
>   void bar()
>   {
>   ...
>   }
>   void foobar()
>   {
>   ...
>   }
>
> I prefer when its written as:
>
>   void foo()
>   {
>   ...
>   }
>   
>   void bar()
>   {
>   ...
>   }
>   
>   void foobar()
>   {
>   ...
>   }
>
> Maybe we even need a more generalised mechanism that would allow alot of 
> flexibility letting people control their own specific style.
>
>   EmptyLineInsertionStyle: Custom
> AfterNameSpaceOpeningBrace: 1
> BeforeNameSpaceClosingBrace: 1
> BetweenFunctions: 2
> AfterClassOpeningBrace: 1
> BeforeClassClosingBrace: 1
> AfterAccessModifier : 1
> BeforeAccessModifier: 1

I totally agree with you on this part, but I think this is a new feature 
requirement. Maybe we can open a new bug and set the "Severity" to 
"enhancement".

In D104044#2813794 , @MyDeveloperDay 
wrote:

> My point being there is inconsistency between how different types of blocks 
> of code are handled, and rather than trying to fix another corner case maybe 
> we should take a more holistic approach, all these KeepEmptyLines and 
> EmptyLineAfterXXX options and what you'll need in order to fix this issue are 
> all addressing what is effectively the same issue, and that is that the 
> addition and/or removal of empty lines is a little hit or miss depending on 
> your combination and permutation of settings and the type of block

I agree that we should use a holistic approach to solve the problem as long as 
we can, but I think the namespace is different than the class based on those 
reasons:

- We indent in the class scope, but we almost never indent in the namespace 
scope. (I've checked all the predefined styles)
- The life-cycles of the objects in the class scope are associated with the 
class scope, but the life-cycles of the objects in a namespace is always global.

> I personally would prefer we took a step back and asked ourselves if we are 
> really facing a bug here or just different people desiring different 
> functionality?

My reason for this being a bug is very simple. If my original code is like this 
(original):

  01 namespace B
  02 {
  03
  04
  04 int j;
  05
  06
  07 }

Then I want the code to be formatted like this (expected):

  01 namespace B
  02 {
  03
  04 int j;
  05
  06 }

Not like this (actual):

  01 namespace B
  02 {
  03 int j;
  04
  05 }

> Whatever the rules are for an inner class, I don't particularly see they are 
> much different for a class in a namespace (which I why I picked that example 
> to demonstrate the point), we won't resolve that inconsistency in a way that 
> will satisfy everyone without having a more powerful mechanism.
>
> If you are thinking you want to just fix your bug then I'd be saying that it 
> SHOULD remove the empty lines (including the one prior to the } // namespace 
> MyLibrary, having said that I'm slightly struggling to understand why
>
>   class Foo {
>   
>   
>   
>   
>   class Bar {};
>   };
>
> isn't conforming to the setting of MaxEmptyLinesToKeep if I set it to 1 where

"it SHOULD remove the empty lines (including the one prior to the } // 
namespace MyLibrary", I don't get what you mean here, can you give me an 
example?

As for the code (original):

  darwin@Darwins-iMac temp % cat f.cpp 
  class Foo {
  
  
  
  
  class Bar {};
  };

It will be formatted into:

  darwin@Darwins-iMac temp % clang-format f.cpp -style="{MaxEmptyLinesToKeep: 
1}"
  class Foo {
  
class Bar {};
  };

I didn't see any problem here.

>   namespace MyLibrary {
>   
>   class Tool {};
>   }  // namespace MyLibrary
>
> is
>
> i.e. set MaxEmptyLinesToKeep  to 0 and
>
> it gets formatted as:
>
>   namespace MyLibrary {
>   class Tool {};
>   }  // namespace MyLibrary

The behavior is correct in this case. Notice the `{` is on the same line as the 
`namespace`. If I set `AfterNamespace` as true, then the behavior is different.

Orignal code, there are two empty lines before and after `class Tool {};`:

  darwin@Darwins-iMac temp % cat e.cpp 
  01 namespace MyLibrary {
  02
  03 
  04 class Tool {};
  05 
  06 
  07 } 

If I format it with `AfterNamespace` as true and `MaxEmptyLinesToKeep` as 0, 
all those empty lines are removed, which is correct:

  darwin@Darwins-iMac temp % clang-format e.cpp -style="{BasedOnStyle: google, 
BreakBeforeBraces: Custom, BraceWrapping: {AfterNamespace: true}, 
MaxEmptyLinesToKeep: 0}"
  01 namespace MyLibrary
  02 {
  03 class Tool {};
  04 }  //

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

[Darwin Xu via Phabricator via cfe-commits]

darwin added a comment.

In D104044#2814016 , 
@HazardyKnusperkeks wrote:

> Going the full way, to fix the number of empty lines after/before/between 
> elements would be real nice. But even nicer would be if you can state a range.
>
> But I think all those proposed options should not be added in one go.

Yes, adding a new option is a new feature requirement. What I am trying to do 
here is to keep the clang-format behaving reasonably.

> In D104044#2812399 , @darwin wrote:
>
>> About the issue, let me explain it. It isn't bound to the google style or 
>> LLVM style either, since both of them keep the first brace at the same line 
>> of the namespace.
>
> Then I would like to use the LLVM style in the tests, otherwise it suggests 
> that the issue is a result of using google style.

Wouldn't this suggest that this issue is a result of using LLVM style?


Repository:
  rZORG LLVM Github Zorg

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104044/new/

https://reviews.llvm.org/D104044

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

>> In D104044#2812399 , @darwin wrote:
>>
>>> About the issue, let me explain it. It isn't bound to the google style or 
>>> LLVM style either, since both of them keep the first brace at the same line 
>>> of the namespace.
>>
>> Then I would like to use the LLVM style in the tests, otherwise it suggests 
>> that the issue is a result of using google style.
>
> Wouldn't this suggest that this issue is a result of using LLVM style?

Not in my opinion, because all the tests I've seen and worked with are LLVM 
style, because that's the default. But use what ever style you want.


Repository:
  rZORG LLVM Github Zorg

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104044/new/

https://reviews.llvm.org/D104044

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha created this revision.
Herald added a subscriber: tschuett.
SaurabhJha requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds examples of compound assignment and type conversions for matrix 
types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104198

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,41 @@
 return a + b * c;
   }
 
+The matrix type extension supports compound assignments for addition, 
subtraction, and multiplication, provided
+their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  void f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are 
C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  void f1(ix5x5 i, fx5x5 f) {
+f = (fx5x5)i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,41 @@
 return a + b * c;
   }
 
+The matrix type extension supports compound assignments for addition, subtraction, and multiplication, provided
+their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  void f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  void f1(ix5x5 i, fx5x5 f) {
+f = (fx5x5)i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 351728.
SaurabhJha added a comment.

Forgot to add a colon in code-block header. Fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104198/new/

https://reviews.llvm.org/D104198

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,41 @@
 return a + b * c;
   }
 
+The matrix type extension supports compound assignments for addition, 
subtraction, and multiplication, provided
+their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  void f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are 
C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  void f1(ix5x5 i, fx5x5 f) {
+f = (fx5x5)i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,41 @@
 return a + b * c;
   }
 
+The matrix type extension supports compound assignments for addition, subtraction, and multiplication, provided
+their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  void f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  void f1(ix5x5 i, fx5x5 f) {
+f = (fx5x5)i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104192: [clang][RISCV] Change implicit ARCH for explicitly specified ABI

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The thing looks weird. My mental model is that `-march` can infer `-mabi`, not 
the other way around...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104192/new/

https://reviews.llvm.org/D104192

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 527a182 - DirectoryWatcher: also wait for the notifier thread

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2021-06-13T10:58:55-07:00
New Revision: 527a1821e6f8e115db3335a3341c7ac491725a0d

URL: 
https://github.com/llvm/llvm-project/commit/527a1821e6f8e115db3335a3341c7ac491725a0d
DIFF: 
https://github.com/llvm/llvm-project/commit/527a1821e6f8e115db3335a3341c7ac491725a0d.diff

LOG: DirectoryWatcher: also wait for the notifier thread

Ultimately the DirectoryWatcher is not ready until the notifier thread
is also active.  Failure to wait for the notifier thread may result in
loss of events.  While this is not catastrophic in practice, the tests
are sensitive to this as depending on the thread scheduler, the thread
may fail to being execution before the operations are completed by the
fixture.  Running this in a tight loop shows no regressions locally as
previously, but this failure mode was been sighted once on a builder.

Added: 


Modified: 
clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp

Removed: 




diff  --git a/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp 
b/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
index 6bcfb86e7f99..847d20bfbb6f 100644
--- a/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ b/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -40,6 +40,7 @@ class DirectoryWatcherWindows : public 
clang::DirectoryWatcher {
 
   std::mutex Mutex;
   bool WatcherActive = false;
+  bool NotifierActive = false;
   std::condition_variable Ready;
 
   class EventQueue {
@@ -117,7 +118,9 @@ DirectoryWatcherWindows::DirectoryWatcherWindows(
   });
 
   std::unique_lock lock(Mutex);
-  Ready.wait(lock, [this] { return this->WatcherActive; });
+  Ready.wait(lock, [this] {
+return this->WatcherActive && this->NotifierActive;
+  });
 }
 
 DirectoryWatcherWindows::~DirectoryWatcherWindows() {
@@ -227,6 +230,12 @@ void DirectoryWatcherWindows::NotifierThreadProc(bool 
WaitForInitialSync) {
   if (!WaitForInitialSync)
 this->InitialScan();
 
+  {
+std::unique_lock lock(Mutex);
+NotifierActive = true;
+  }
+  Ready.notify_one();
+
   while (true) {
 DirectoryWatcher::Event E = Q.pop_front();
 Callback(E, /*IsInitial=*/false);



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 673c5ba - [clang-format] Adds a formatter for aligning arrays of structs

[Björn Schäpers via cfe-commits]

Author: Fred Grim
Date: 2021-06-13T21:14:37+02:00
New Revision: 673c5ba58497298a684f8b8dfddbfb11cd89950e

URL: 
https://github.com/llvm/llvm-project/commit/673c5ba58497298a684f8b8dfddbfb11cd89950e
DIFF: 
https://github.com/llvm/llvm-project/commit/673c5ba58497298a684f8b8dfddbfb11cd89950e.diff

LOG: [clang-format] Adds a formatter for aligning arrays of structs

This adds a new formatter to arrange array of struct initializers into
neat columns.

Differential Revision: https://reviews.llvm.org/D101868

Added: 
clang/test/Format/struct-array-initializer.cpp

Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/TokenAnnotator.h
clang/lib/Format/WhitespaceManager.cpp
clang/lib/Format/WhitespaceManager.h
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 9686830c3bd3e..0d0c07fa350fd 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -204,6 +204,41 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+**AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``)
+  if not ``None``, when using initialization for an array of structs
+  aligns the fields into columns.
+
+  Possible values:
+
+  * ``AIAS_Left`` (in configuration: ``Left``)
+Align array column and left justify the columns e.g.:
+
+.. code-block:: c++
+
+  struct test demo[] =
+  {
+  {56, 23,"hello"},
+  {-1, 93463, "world"},
+  {7,  5, "!!"   }
+  };
+
+  * ``AIAS_Right`` (in configuration: ``Right``)
+Align array column and right justify the columns e.g.:
+
+.. code-block:: c++
+
+  struct test demo[] =
+  {
+  {56,23, "hello"},
+  {-1, 93463, "world"},
+  { 7, 5,"!!"}
+  };
+
+  * ``AIAS_None`` (in configuration: ``None``)
+Don't align array initializer columns.
+
+
+
 **AlignConsecutiveAssignments** (``AlignConsecutiveStyle``)
   Style of aligning consecutive assignments.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5835dd2f7c63..bddaea6e48461 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -265,6 +265,9 @@ clang-format
 - Makes ``PointerAligment: Right`` working with 
``AlignConsecutiveDeclarations``.
   (Fixes https://llvm.org/PR27353)
 
+- Option ``AlignArrayOfStructure`` has been added to allow for ordering 
array-like
+  initializers.
+
 libclang
 
 

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 503f0e446bff3..164765ca1a1a3 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -90,6 +90,35 @@ struct FormatStyle {
   /// brackets.
   BracketAlignmentStyle AlignAfterOpenBracket;
 
+  /// Different style for aligning array initializers.
+  enum ArrayInitializerAlignmentStyle {
+/// Align array column and left justify the columns e.g.:
+/// \code
+///   struct test demo[] =
+///   {
+///   {56, 23,"hello"},
+///   {-1, 93463, "world"},
+///   {7,  5, "!!"   }
+///   };
+/// \endcode
+AIAS_Left,
+/// Align array column and right justify the columns e.g.:
+/// \code
+///   struct test demo[] =
+///   {
+///   {56,23, "hello"},
+///   {-1, 93463, "world"},
+///   { 7, 5,"!!"}
+///   };
+/// \endcode
+AIAS_Right,
+/// Don't align array initializer columns.
+AIAS_None
+  };
+  /// if not ``None``, when using initialization for an array of structs
+  /// aligns the fields into columns.
+  ArrayInitializerAlignmentStyle AlignArrayOfStructures;
+
   /// Styles for alignment of consecutive tokens. Tokens can be assignment 
signs
   /// (see
   /// ``AlignConsecutiveAssignments``), bitfield member separators (see
@@ -3272,6 +3301,7 @@ struct FormatStyle {
   bool operator==(const FormatStyle &R) const {
 return AccessModifierOffset == R.AccessModifierOffset &&
AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
+   AlignArrayOfStructures == R.AlignArrayOfStructures &&
AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
AlignConsecutiveBitFields == R.AlignConsecutiveBitFields &&
AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&

diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 8d16f299d3aef..53cbbf66e85ab 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -143,6 +143,16 @@ template <> struct 
ScalarEnumerationTraits {
   }
 };
 
+template <>
+struct ScalarEnumera

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG673c5ba58497: [clang-format] Adds a formatter for aligning 
arrays of structs (authored by feg208, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101868/new/

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  

[PATCH] D104076: [clang-cl][sanitizer] Add -fsanitize-address-use-after-return to clang.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Note, clang-cl is an executable (`ninja clang-cl`), so using `clang-cl` in the 
subject can cause confusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104076/new/

https://reviews.llvm.org/D104076

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104106: [clang][deps] NFC: Stop using moved-from object

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104106/new/

https://reviews.llvm.org/D104106

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104104: [clang][deps] NFC: Handle `DependencyOutputOptions` only once

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104104/new/

https://reviews.llvm.org/D104104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104036: [clang][deps] Prevent unintended modifications of the original TU command-line

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM, with one suggestion inline.




Comment at: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:194
  CompilerInstance &I, DependencyConsumer &C,
- std::map>
- OriginalPrebuiltModuleFiles);
+ CompilerInvocation OriginalInvocation);
 

I wonder if it'd be better to take `CompilerInvocation&&` here. Then the caller 
is required to either pass `std::move` or make a deep copy at the call site, 
and it's perhaps more clear that there's a deep copy being made.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104036/new/

https://reviews.llvm.org/D104036

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

Okay, LGTM. (Sorry for the delay, I've been out.)

In D103461#2793254 , @jansvoboda11 
wrote:

> My reason for the FIXME is that we could get rid of bunch of Windows-specific 
> logic by adjusting `CompilerInvocation` instead.

Please add that motivation to the FIXME itself.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104012: [clang][deps] Move stripping of diagnostic serialization from `clang-scan-deps` to `DependencyScanning` library

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104012/new/

https://reviews.llvm.org/D104012

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104155: Add documentation for -fsanitize-address-use-after-return.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:3
   ---
   NOTE: This file is automatically generated by running clang-tblgen
   -gen-opt-docs. Do not edit this file by hand!!

This file is generated by `clang-tblgen -gen-opt-docs`.

You can edit `clang/docs/UsersManual.rst` and include the information that 
`=never` can reduce the object file size.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104155/new/

https://reviews.llvm.org/D104155

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103802: [clang][modules][pch] Allow loading PCH with different modules cache path

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103802/new/

https://reviews.llvm.org/D103802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103526: [clang][deps] Handle modular dependencies present in PCH

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103526/new/

https://reviews.llvm.org/D103526

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103807: [clang][deps] Ensure deterministic order of TU '-fmodule-file=' arguments

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I'm not sure the performance problems with std::map will matter in practice 
here, but have you considered sorting before emission rather than relying on 
the data structure's iteration order? (It'd make it easy to switch to StringMap 
in the future.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103807/new/

https://reviews.llvm.org/D103807

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] e03be2e - unwind: allow building with GCC

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2021-06-13T14:44:54-07:00
New Revision: e03be2efe564026ad3b04d459794c89c674e1ed9

URL: 
https://github.com/llvm/llvm-project/commit/e03be2efe564026ad3b04d459794c89c674e1ed9
DIFF: 
https://github.com/llvm/llvm-project/commit/e03be2efe564026ad3b04d459794c89c674e1ed9.diff

LOG: unwind: allow building with GCC

This was regressed in adf1561d6ce8.  Since gcc does not support
`__has_feature`, this adjusts the build to use the
`__SANITIZE_ADDRESS__` macro which GCC defines to identify if ASAN is
enabled (similar to `__has_feature`).  This allows building libunwind
with gcc again.

Patch by Daniel Levin!

Reviewed By: compnerd

Differential Revision: https://reviews.llvm.org/D104176

Added: 


Modified: 
libunwind/src/libunwind.cpp

Removed: 




diff  --git a/libunwind/src/libunwind.cpp b/libunwind/src/libunwind.cpp
index 9b3b92bdff099..1faf000ce44a9 100644
--- a/libunwind/src/libunwind.cpp
+++ b/libunwind/src/libunwind.cpp
@@ -16,7 +16,13 @@
 
 #include 
 
-#if __has_feature(address_sanitizer)
+// Define the __has_feature extension for compilers that do not support it so
+// that we can later check for the presence of ASan in a compiler-neutral way.
+#if !defined(__has_feature)
+#define __has_feature(feature) 0
+#endif
+
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
 #include 
 #endif
 
@@ -187,7 +193,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_get_proc_info, 
unw_get_proc_info)
 /// Resume execution at cursor position (aka longjump).
 _LIBUNWIND_HIDDEN int __unw_resume(unw_cursor_t *cursor) {
   _LIBUNWIND_TRACE_API("__unw_resume(cursor=%p)", static_cast(cursor));
-#if __has_feature(address_sanitizer)
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
   // Inform the ASan runtime that now might be a good time to clean stuff up.
   __asan_handle_no_return();
 #endif



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100139: [ifs][elfabi] Merge llvm-ifs/elfabi tools

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:18
 namespace ifstool {
 void Merger::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output, const InputInfoList &Inputs,

Ideally, this job would use the IFS library to construct `.ifs` or stub file 
directly without invoking an external tool. Can you please leave a `TODO` here 
along those lines?



Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:47
+} // end anonymous namespace
 
+// Command line flags:

Ideally we would use `OptTable` for option parsing, can you please leave a 
`TODO` comment here along those lines?



Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:244-245
+MemoryBuffer::getFile(FilePath)) {
+  // Compare IFS output with existing IFS file.
+  // If IFS file unchanged, abort updating.
+  if ((*BufOrError)->getBuffer() == IFSStr)





Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:414
+  IFSTarget HintTarget = parseTriple(OptTargetTripleHint);
+  if (Stub.Target.Arch.getValue() != HintTarget.Arch.getValue()) {
+fatalError(make_error(

No need for `{` and `}` here and below since each `if` has only one statement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100139/new/

https://reviews.llvm.org/D100139

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104192: [clang][RISCV] Change implicit ARCH for explicitly specified ABI

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D104192#2815757 , @MaskRay wrote:

> The thing looks weird. My mental model (ppc) is that `-march` can infer 
> `-mabi`, not the other way around...

The original way in Driver/Toolchains/Arch/RISCV.cpp is

  if (MABI.equals_lower("ilp32e"))
return "rv32e";
  else if (MABI.startswith_lower("ilp32"))
return "rv32imafdc";
  else if (MABI.startswith_lower("lp64"))
return "rv64imafdc";

So you think we should delete it entirely?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104192/new/

https://reviews.llvm.org/D104192

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104118: [OpenCL] Use DW_LANG_OpenCL language tag for OpenCL C

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D104118#2813358 , @stuart wrote:

> Note: there is currently no DWARF language code defined for //C++ for 
> OpenCL//, so we must use DW_LANG_C_plus_plus* if we wish to be able to 
> determine whether output has been generated from //C++ for OpenCL// source or 
> from //OpenCL C// source. I have raised DWARF issue 210514.1 
>  to add a dedicated //C++ 
> for OpenCL// code in the next version of DWARF, but for now I believe that it 
> is best to use DW_LANG_OpenCL for //OpenCL C// only, and not for //C++ for 
> OpenCL//.
>
> I could perhaps add a note regarding this to the commit message but am 
> concerned about overcomplicating the message.

NIT: It would be nice to have this as part of the commit summary/message itself.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104118/new/

https://reviews.llvm.org/D104118

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 092c303 - AMD k8 family does not support SSE4.x which are required by x86-64-v2+

[via cfe-commits]

Author: serge-sans-paille
Date: 2021-06-14T07:17:30+02:00
New Revision: 092c303955cd18be6c0b923b1c0a1b96e2c91893

URL: 
https://github.com/llvm/llvm-project/commit/092c303955cd18be6c0b923b1c0a1b96e2c91893
DIFF: 
https://github.com/llvm/llvm-project/commit/092c303955cd18be6c0b923b1c0a1b96e2c91893.diff

LOG: AMD k8 family does not support SSE4.x which are required by x86-64-v2+

So don't define __tune__k8__ for these micro architecture.

SSE, SSE2 and SSE3 appear in https://www.amd.com/system/files/TechDocs/25112.PDF
but not SSE4.x.

Differential Revision: https://reviews.llvm.org/D104116

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index ee5b6cb3c087f..8f7749bd60661 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -514,9 +514,6 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
&Opts,
   case CK_K8:
   case CK_K8SSE3:
   case CK_x86_64:
-  case CK_x86_64_v2:
-  case CK_x86_64_v3:
-  case CK_x86_64_v4:
 defineCPUMacros(Builder, "k8");
 break;
   case CK_AMDFAM10:



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104116: AMD k8 family does not support SSE4.x which are required by x86-64-v2+

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG092c303955cd: AMD k8 family does not support SSE4.x which 
are required by x86-64-v2+ (authored by serge-sans-paille).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104116/new/

https://reviews.llvm.org/D104116

Files:
  clang/lib/Basic/Targets/X86.cpp


Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -514,9 +514,6 @@
   case CK_K8:
   case CK_K8SSE3:
   case CK_x86_64:
-  case CK_x86_64_v2:
-  case CK_x86_64_v3:
-  case CK_x86_64_v4:
 defineCPUMacros(Builder, "k8");
 break;
   case CK_AMDFAM10:


Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -514,9 +514,6 @@
   case CK_K8:
   case CK_K8SSE3:
   case CK_x86_64:
-  case CK_x86_64_v2:
-  case CK_x86_64_v3:
-  case CK_x86_64_v4:
 defineCPUMacros(Builder, "k8");
 break;
   case CK_AMDFAM10:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b5b9489 - Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann via cfe-commits]

Author: Stephan Bergmann
Date: 2021-06-14T08:04:03+02:00
New Revision: b5b9489b2415dc48c39d4d7d4bae6197dc499f38

URL: 
https://github.com/llvm/llvm-project/commit/b5b9489b2415dc48c39d4d7d4bae6197dc499f38
DIFF: 
https://github.com/llvm/llvm-project/commit/b5b9489b2415dc48c39d4d7d4bae6197dc499f38.diff

LOG: Only consider built-in compound assignment operators for -Wunused-but-set-*

At least LibreOffice has, for mainly historic reasons that would be hard to
change now, a class Any with an overloaded operator >>= that semantically does
not assign to the LHS but rather extracts into the (by-reference) RHS.  Which
thus caused false positive -Wunused-but-set-parameter and
-Wunused-but-set-variable after those have been introduced recently.

This change is more conservative about the assumed semantics of overloaded
operators, excluding compound assignment operators but keeping plain operator =
ones.  At least for LibreOffice, that strikes a good balance of not producing
false positives but still finding lots of true ones.

(The change to the BinaryOperator case in MaybeDecrementCount is necessary
because e.g. the template f4 test code in warn-unused-but-set-variables-cpp.cpp
turns the += into a BinaryOperator.)

Differential Revision: https://reviews.llvm.org/D103949

Added: 


Modified: 
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 20925f95f18e..a57c5ad198e1 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7808,11 +7808,15 @@ static void MaybeDecrementCount(
 Expr *E, llvm::DenseMap &RefsMinusAssignments) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType()) {
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }

diff  --git a/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp 
b/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
index aa598408dee5..400e9d7681b3 100644
--- a/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ b/clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@ struct S {
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@ void f2() {
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5b9489b2415: Only consider built-in compound assignment 
operators for -Wunused-but-set-* (authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D103949?vs=350824&id=351778#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103949/new/

https://reviews.llvm.org/D103949

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7808,11 +7808,15 @@
 Expr *E, llvm::DenseMap &RefsMinusAssignments) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType()) {
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7808,11 +7808,15 @@
 Expr *E, llvm::DenseMap &RefsMinusAssignments) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType()) {
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits