date:20210410

[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

2021-04-10 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added a comment.

@hamzasood, I suppose this is still worth pursuing. Could you address the 
comment?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50763

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

[PATCH] D37797: Fix crash in Sema when wrongly assuming VarDecl init is not value dependent.

2021-04-10 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added a comment.

@teemperor, should we land this?


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

https://reviews.llvm.org/D37797

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

[clang] 6270b3a - Temporairly revert "[CGCall] Annotate `this` argument with alignment"

2021-04-10 Thread Roman Lebedev via cfe-commits


Author: Roman Lebedev
Date: 2021-04-10T10:43:16+03:00
New Revision: 6270b3a1eafaba4279e021418c5a2c5a35abc002

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

LOG: Temporairly revert "[CGCall] Annotate `this` argument with alignment"

As per @jyknight, "It seems like there's a bug with vtable thunks getting the 
wrong information."
See https://reviews.llvm.org/D99790#2680857, https://godbolt.org/z/MxhYMe1q7

This reverts commit 0aa0458f1429372038ca6a4edc7e94c96cd9a753.

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGen/attr-nomerge.cpp
clang/test/CodeGenCXX/this-nonnull.cpp
clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp
clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm
clang/test/OpenMP/irbuilder_for_iterator.cpp
clang/test/OpenMP/irbuilder_for_rangefor.cpp
clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 06f9253e6bfb1..1d71148d67e67 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2295,7 +2295,7 @@ void CodeGenModule::ConstructAttributeList(
 llvm::AttributeSet::get(getLLVMContext(), Attrs);
   }
 
-  // Apply `nonnull`, `dereferencable(N)` and `align N` to the `this` argument.
+  // Apply `nonnull` and `dereferencable(N)` to the `this` argument.
   if (FI.isInstanceMethod() && !IRFunctionArgs.hasInallocaArg() &&
   !FI.arg_begin()->type->isVoidPointerType()) {
 auto IRArgs = IRFunctionArgs.getIRArgs(0);
@@ -2304,13 +2304,13 @@ void CodeGenModule::ConstructAttributeList(
 
 llvm::AttrBuilder Attrs;
 
-QualType ThisTy =
-FI.arg_begin()->type.castAs()->getPointeeType();
-
 if (!CodeGenOpts.NullPointerIsValid &&
 getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
   Attrs.addAttribute(llvm::Attribute::NonNull);
-  Attrs.addDereferenceableAttr(getMinimumObjectSize(ThisTy).getQuantity());
+  Attrs.addDereferenceableAttr(
+  getMinimumObjectSize(
+  FI.arg_begin()->type.castAs()->getPointeeType())
+  .getQuantity());
 } else {
   // FIXME dereferenceable should be correct here, regardless of
   // NullPointerIsValid. However, dereferenceable currently does not always
@@ -2322,12 +2322,6 @@ void CodeGenModule::ConstructAttributeList(
   .getQuantity());
 }
 
-llvm::Align Alignment =
-getNaturalTypeAlignment(ThisTy, /*BaseInfo=*/nullptr,
-/*TBAAInfo=*/nullptr, /*forPointeeType=*/true)
-.getAsAlign();
-Attrs.addAlignmentAttr(Alignment);
-
 ArgAttrs[IRArgs.first] = llvm::AttributeSet::get(getLLVMContext(), Attrs);
   }
 

diff  --git a/clang/test/CodeGen/attr-nomerge.cpp 
b/clang/test/CodeGen/attr-nomerge.cpp
index da0c33bb384c7..fc26af379fdb7 100644
--- a/clang/test/CodeGen/attr-nomerge.cpp
+++ b/clang/test/CodeGen/attr-nomerge.cpp
@@ -77,7 +77,7 @@ void something_else_again() {
 // CHECK: %[[AG:.*]] = load void (%class.A*)*, void (%class.A*)**
 // CHECK-NEXT: call void %[[AG]](%class.A* {{.*}}) #[[ATTR0]]
 // CHECK: %[[BG:.*]] = load void (%class.B*)*, void (%class.B*)**
-// CHECK-NEXT: call void %[[BG]](%class.B* nonnull align {{.*}} dereferenceable
+// CHECK-NEXT: call void %[[BG]](%class.B* nonnull dereferenceable
 // CHECK: call void @_ZN1AC1Ev({{.*}}) #[[ATTR0]]
 // CHECK: call void @_ZN1A1fEv({{.*}}) #[[ATTR0]]
 // CHECK: call void @_ZN1A1gEv({{.*}}) #[[ATTR0]]

diff  --git a/clang/test/CodeGenCXX/this-nonnull.cpp 
b/clang/test/CodeGenCXX/this-nonnull.cpp
index 7f146b6613991..63873388b0707 100644
--- a/clang/test/CodeGenCXX/this-nonnull.cpp
+++ b/clang/test/CodeGenCXX/this-nonnull.cpp
@@ -11,10 +11,10 @@ struct Struct {
 void TestReturnsVoid(Struct &s) {
   s.ReturnsVoid();
 
-  // CHECK-YES: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
align 4 dereferenceable(12) %0)
+  // CHECK-YES: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
dereferenceable(12) %0)
   /// FIXME Use dereferenceable after dereferenceable respects 
NullPointerIsValid.
-  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* align 4 
dereferenceable_or_null(12) %0)
+  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable_or_null(12) %0)
 }
 
-// CHECK-YES: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
align 4 dereferenceable(12))
-// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* align 4 
dereferenceable_or_null(12))
+// CHECK-YES: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
dereferenceable(12))
+// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable_or

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

2021-04-10 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri reopened this revision.
lebedev.ri added a comment.

Temporarily reverted in 6270b3a1eafaba4279e021418c5a2c5a35abc002 
.
Eagerly awaiting fix for that bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99790

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

[PATCH] D100161: Redistribute energy for Corpus

2021-04-10 Thread taotao gu via Phabricator via cfe-commits

gtt1995 added a comment.

In D100161#2679922 , @morehouse wrote:

> Thanks for the patch!  Would you mind sharing the experimental data/results 
> you obtained for this patch?
>
> Additionally, could you submit this patch to FuzzBench 
>  for an independent evaluation?
>
> Thanks,
> Matt

Hello,How can i share the experiment data?
It seems that  FuzzBench does not accept this parallel mode evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100161

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

[PATCH] D100161: Redistribute energy for Corpus

2021-04-10 Thread taotao gu via Phabricator via cfe-commits

gtt1995 added a comment.

F16230733: experiment.tar 

This is part of raw data , the object from oss-fuzz project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100161

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

[PATCH] D100161: Redistribute energy for Corpus

2021-04-10 Thread taotao gu via Phabricator via cfe-commits

gtt1995 added a comment.

In D100161#2681045 , @gtt1995 wrote:

> F16230733: experiment.tar 
>
> This is part of raw data , the object from oss-fuzz project.

The patched version data in average dir,  data of libfuzzer dir  is from 
original libfuzzer.
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100161

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

[PATCH] D60456: [RISCV] Hard float ABI support

2021-04-10 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, 
sameer.abuasal, s.egerton.

Your use of CoerceAndExpand seems fine; thanks for pinging me on it


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60456

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

[clang] 71ab6c9 - [Matrix] Implement C-style explicit type conversions for matrix types.

2021-04-10 Thread Florian Hahn via cfe-commits


Author: Saurabh Jha
Date: 2021-04-10T11:48:41+01:00
New Revision: 71ab6c98a0d1b5590ed12f955dea6dfc714e9490

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

LOG: [Matrix] Implement C-style explicit type conversions for matrix types.

This implements C-style type conversions for matrix types, as specified
in clang/docs/MatrixTypes.rst.

Fixes PR47141.

Reviewed By: fhahn

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

Added: 
clang/test/CodeGen/matrix-cast.c
clang/test/Sema/matrix-cast.c
clang/test/SemaCXX/matrix-casts.cpp

Modified: 
clang/include/clang/AST/OperationKinds.def
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGExprComplex.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Edit/RewriteObjCFoundationAPI.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

Removed: 




diff  --git a/clang/include/clang/AST/OperationKinds.def 
b/clang/include/clang/AST/OperationKinds.def
index 7c82ab6e57eff..b05b9d81569eb 100644
--- a/clang/include/clang/AST/OperationKinds.def
+++ b/clang/include/clang/AST/OperationKinds.def
@@ -181,6 +181,9 @@ CAST_OPERATION(PointerToBoolean)
 ///(void) malloc(2048)
 CAST_OPERATION(ToVoid)
 
+/// CK_MatrixCast - A cast between matrix types of the same dimensions.
+CAST_OPERATION(MatrixCast)
+
 /// CK_VectorSplat - A conversion from an arithmetic type to a
 /// vector of that element type.  Fills all elements ("splats") with
 /// the source value.

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 2085904b7f01e..97cd903f3d6f4 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -518,7 +518,7 @@ class alignas(void *) Stmt {
 
 unsigned : NumExprBits;
 
-unsigned Kind : 6;
+unsigned Kind : 7;
 unsigned PartOfExplicitCast : 1; // Only set for ImplicitCastExpr.
 
 /// True if the call expression has some floating-point features.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0e14f8204f566..5c27902da2502 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8580,6 +8580,12 @@ let CategoryName = "Inline Assembly Issue" in {
 
 let CategoryName = "Semantic Issue" in {
 
+def err_invalid_conversion_between_matrixes : Error<
+  "conversion between matrix types%
diff { $ and $|}0,1 of 
diff erent size is not allowed">;
+
+def err_invalid_conversion_between_matrix_and_type : Error<
+  "conversion between matrix type %0 and incompatible type %1 is not allowed">;
+
 def err_invalid_conversion_between_vectors : Error<
   "invalid conversion between vector type%
diff { $ and $|}0,1 of 
diff erent "
   "size">;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ebf63764be1aa..77f716153feee 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11660,6 +11660,8 @@ class Sema final {
 
   bool isValidSveBitcast(QualType srcType, QualType destType);
 
+  bool areMatrixTypesOfTheSameDimension(QualType srcTy, QualType destTy);
+
   bool areLaxCompatibleVectorTypes(QualType srcType, QualType destType);
   bool isLaxVectorConversion(QualType srcType, QualType destType);
 
@@ -11718,6 +11720,13 @@ class Sema final {
   ExprResult checkUnknownAnyArg(SourceLocation callLoc,
 Expr *result, QualType ¶mType);
 
+  // CheckMatrixCast - Check type constraints for matrix casts.
+  // We allow casting between matrixes of the same dimensions i.e. when they
+  // have the same number of rows and column. Returns true if the cast is
+  // invalid.
+  bool CheckMatrixCast(SourceRange R, QualType DestTy, QualType SrcTy,
+   CastKind &Kind);
+
   // CheckVectorCast - check type constraints for vectors.
   // Since vectors are an extension, there are no C standard reference for 
this.
   // We allow casting between vectors and integer datatypes of the same size.

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index adb33036a1684..eccb42eeb7e74 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1708,6 +1708,7 @@ bool CastExpr::CastConsistency() const {
   case CK_FixedPointCast:
   case CK_FixedPointToIntegral:
   case CK_IntegralToFixedPoint:
+  case CK_MatrixCast:
 assert(!getType()->isBooleanType() && "unheralded conversion to bool");
 goto CheckNoBas

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

2021-04-10 Thread Florian Hahn via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71ab6c98a0d1: [Matrix] Implement C-style explicit type 
conversions for matrix types. (authored by SaurabhJha, committed by fhahn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++11 -fenable-matrix -fsyntax-only -verify %s
+
+template 
+
+using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+template 
+
+using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
+
+typedef struct test_struct {
+} test_struct;
+
+typedef int vec __attribute__((vector_size(4)));
+
+void f1() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_4_4 m2;
+  matrix_4_4 m3;
+  matrix_5_5 m4;
+  int i;
+  vec v;
+  test_struct *s;
+
+  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
+4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m3;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+
+  (int)m3;// expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
+4)))') to 'int'}}
+  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
+matrix_type(4, 4)))') is not allowed}}
+
+  (vec) m2;// expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
+to 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
+(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+
+  (test_struct *)m1;// expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
+  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') is not allowed}}'
+}
+
+void f2() {
+  // TODO: Update this test once the support of C-style casts for C++ is implemented.
+  matrix_4_4 m1;
+  matrix_5_5 m2;
+  matrix_5_5 m3;
+  matrix_4_4 m4;
+  float f;
+
+  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
+((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (matrix_5_5)m2;// expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
+  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
+is not allowed}}
+  (matrix_4_4)m4;  // expected-error {{C-style cast from 'matrix_4_4' (aka 'unsigned int \
+__attribute__((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not \
+allowed}}
+}
Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef struct test_struct {
+} test_struct;
+
+void f1() {
+  cx4x4 m

[PATCH] D77598: Integral template argument suffix and cast printing

2021-04-10 Thread Pratyush Das via Phabricator via cfe-commits

reikdas updated this revision to Diff 336604.
reikdas added a comment.

Address @rsmith comments.


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

https://reviews.llvm.org/D77598

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/StmtDataCollectors.td
  clang/include/clang/AST/TemplateBase.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/NestedNameSpecifier.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p12.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p13.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-ast-print.cpp
  clang/test/SemaCXX/cxx1z-ast-print.cpp
  clang/test/SemaCXX/matrix-type-builtins.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaTemplate/address_space-dependent.cpp
  clang/test/SemaTemplate/delegating-constructors.cpp
  clang/test/SemaTemplate/matrix-type.cpp
  clang/test/SemaTemplate/temp_arg_enum_printing.cpp
  clang/test/SemaTemplate/temp_arg_nontype.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
  clang/tools/libclang/CIndex.cpp
  
clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
@@ -20,7 +20,7 @@
 llvm::raw_string_ostream Stream(ArgStr);
 const TemplateArgument &Arg = ArgLoc.getArgument();
 
-Arg.print(Context->getPrintingPolicy(), Stream);
+Arg.print(Context->getPrintingPolicy(), Stream, /*IncludeType*/ true);
 Match(Stream.str(), ArgLoc.getLocation());
 return ExpectedLocationVisitor::
   TraverseTemplateArgumentLoc(ArgLoc);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5185,8 +5185,9 @@
 SmallString<128> Str;
 llvm::raw_svector_ostream OS(Str);
 OS << *ClassSpec;
-printTemplateArgumentList(OS, ClassSpec->getTemplateArgs().asArray(),
-  Policy);
+printTemplateArgumentList(
+OS, ClassSpec->getTemplateArgs().asArray(), Policy,
+ClassSpec->getSpecializedTemplate()->getTemplateParameters());
 return cxstring::createDup(OS.str());
   }
 
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -528,3 +528,33 @@
 x1.f(x2);
   }
 }
+
+namespace TypeSuffix {
+  template  struct A {};
+  template <> struct A<1> { using type = int; }; // expected-note {{'A<1>::type' declared here}}
+  A<1L>::type a; // expected-error {{no type named 'type' in 'TypeSuffix::A<1L>'; did you mean 'A<1>::type'?}}
+
+  template  struct B {};
+  template <> struct B<1> { using type = int; }; // expected-note {{'B<1>::type' declared here}}
+  B<2>::type b;  // expected-error {{no type named 'type' in 'TypeSuffix::B<2>'; did you mean 'B<1>::type'?}}
+
+  template  struct C {};
+  template <> struct C<'a'> { using type = signed char; }; // expected-note {{'C<'a'>::type' declared here}}
+  C<(signed char)'a'>::type c; // expected-error {{no type named 'type' in 'TypeSuffix::C<(signed char)'a'>'; did you mean 'C<'a'>::type'?}}
+
+  template  struct D {};
+  template <> struct D<'a'> { using type = signed char; }; // expected-note {{'D<'a'>::type' declared here}}
+  D<'b'>::type d;  // expected-error {{no type named 'type' in 'TypeSuffix::D<'b'>'; did you mean 'D<'a'>::type'?}}
+
+  template  struct E {};
+  template <> struct E<'a'> { using type = unsigned char; }; // expected-note {{'E<'a'>::type' declared here}}
+  E<(unsigned char)'a'>::type e; // expected-error {{no type named 'type' in 'TypeSuffix::E<(unsigned char)'a'>'; did you mean 'E<'a'>::type'?}}
+
+  template  struct F {};
+  template <> struct F<'a'> { using type = unsigned char; }; // expected-note {{'F<'a'>::type' declared here}}
+  F<'b'>::type f;

[PATCH] D77598: Integral template argument suffix and cast printing

2021-04-10 Thread Pratyush Das via Phabricator via cfe-commits

reikdas added inline comments.



Comment at: clang/lib/AST/TemplateBase.cpp:71-72
 
   if (T->isBooleanType() && !Policy.MSVCFormatting) {
 Out << (Val.getBoolValue() ? "true" : "false");
   } else if (T->isCharType()) {

rnk wrote:
> rsmith wrote:
> > rsmith wrote:
> > > rnk wrote:
> > > > rsmith wrote:
> > > > > reikdas wrote:
> > > > > > rsmith wrote:
> > > > > > > rsmith wrote:
> > > > > > > > It looks like `MSVCFormatting` wants `bool` values to be 
> > > > > > > > printed as `0` and `1`, and this patch presumably changes that 
> > > > > > > > (along with the printing of other builtin types). I wonder if 
> > > > > > > > this is a problem in practice (eg, if such things are used as 
> > > > > > > > keys for debug info or similar)...
> > > > > > > Do we need to suppress printing the suffixes below in 
> > > > > > > `MSVCFormatting` mode too?
> > > > > > @rsmith The tests pass, so that is reassuring at least. Is there 
> > > > > > any other way to find out whether we need to suppress printing the 
> > > > > > suffixes for other types in MSVCFormatting mode?
> > > > > @rnk Can you take a look at this? Per 
> > > > > rG60103383f097b6580ecb4519eeb87defdb7c05c9 and PR21528 it seems like 
> > > > > there might be specific requirements for how template arguments are 
> > > > > formatted for CodeView support; we presumably need to make sure we 
> > > > > still satisfy those requirements.
> > > > Probably. This change looks like it preserves behavior from before when 
> > > > `MSVCFormatting` is set, so I think this is OK. I checked, my version 
> > > > of MSVC still uses 1/0 in debug info for boolean template args.
> > > My concern is that we're changing the behavior for the other cases below 
> > > in `MSVCFormatting` mode, not that we're changing the behavior for 
> > > `bool`. If we have specific formatting requirements in `MSVCFormatting` 
> > > mode, they presumably apply to all types, not only to `bool`, so we 
> > > should be careful to not change the output in `MSVCFormatting` mode for 
> > > any type.
> > @rnk Ping.
> I think we do need to suppress the suffixes for MSVCFormatting. Consider this 
> visualizer I found in the stl.natvis file:
> ```
>   
>   {_MyRep} nanoseconds
>   
>   
> 
>   
>   {_MyRep} microseconds
>   
>   
> ```
> 
> Adding L or ULL after 100 has the potential to break the visualizer.
> 
> However, in general, I don't think we need to freeze this code in amber. It's 
> not like we put a lot of thought into making this code produce MSVC-idential 
> names when we started using it in CodeView debug info. We just used it and 
> debug issues as they come up.
> 
> I think a good rule of thumb for making changes is probably to ask if the 
> main STL data structure names include this template argument feature. So, 
> auto-typed non-type template parameters probably aren't an issue.
> 
> ---
> 
> Separately, I wish the stl.natvis file was part of the 
> github.com/microsoft/STL project so I could just link to it...
Just to clarify - is an MSVCFormatting specific change required in this patch? 
(do we need to suppress suffixes and casts for MSVCFormatting printing policy?)
And is there anyone we need to notify about this patch before it lands (to make 
sure nothing breaks)?


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

https://reviews.llvm.org/D77598

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

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

2021-04-10 Thread Anton Bikineev via Phabricator via cfe-commits

AntonBikineev added a comment.

friendly ping..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99808

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

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

2021-04-10 Thread Anton Bikineev via Phabricator via cfe-commits

AntonBikineev updated this revision to Diff 336612.
AntonBikineev added a comment.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99808

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-unsigned-char-zero-compare.cc

Index: clang/test/Sema/tautological-unsigned-char-zero-compare.cc
===
--- /dev/null
+++ clang/test/Sema/tautological-unsigned-char-zero-compare.cc
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-fno-signed-char \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=unsigned %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=signed %s
+
+void f(char c, unsigned char uc, signed char cc) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
+
+void ref(char &c, unsigned char &uc, signed char &cc) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11415,11 +11415,14 @@
 << OtherIsBooleanDespiteType << *Result
 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
   } else {
-unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
-? (HasEnumType(OriginalOther)
-   ? diag::warn_unsigned_enum_always_true_comparison
-   : diag::warn_unsigned_always_true_comparison)
-: diag::warn_tautological_constant_compare;
+bool IsCharTy = OtherT.withoutLocalFastQualifiers() == S.Context.CharTy;
+unsigned Diag =
+(isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+? (HasEnumType(OriginalOther)
+   ? diag::warn_unsigned_enum_always_true_comparison
+   : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
+  : diag::warn_unsigned_always_true_comparison)
+: diag::warn_tautological_constant_compare;
 
 S.Diag(E->getOperatorLoc(), Diag)
 << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6784,6 +6784,10 @@
   "result of comparison of %select{%3|unsigned expression}0 %2 "
   "%select{unsigned expression|%3}0 is always %4">,
   InGroup, DefaultIgnore;
+def warn_unsigned_char_always_true_comparison : Warning<
+  "result of comparison of %select{%3|char expression}0 %2 "
+  "%select{char expression|%3}0 is always %4, since char is interpreted as "
+  "unsigned">, InGroup, DefaultIgnore;
 def warn_unsigned_enum_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned enum expression}0 %2 "
   "%select{unsigned enum expression|%3}0 is always %4">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -591,11 +591,13 @@
 def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedCharZeroCompare : DiagGroup<"tautological-unsigned-char-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 // For compatibility with GCC. Tautological compari

[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes

2021-04-10 Thread Josh Junon via Phabricator via cfe-commits

Qix- updated this revision to Diff 336614.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99861

Files:
  clang/examples/CMakeLists.txt
  clang/examples/PrintAttributeTokens/CMakeLists.txt
  clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp
  clang/examples/PrintAttributeTokens/PrintAttributeTokens.exports
  clang/examples/PrintAttributeTokens/README.txt
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/ParsedAttr.cpp
  clang/test/Frontend/plugin-print-attr-tokens.cpp

Index: clang/test/Frontend/plugin-print-attr-tokens.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-print-attr-tokens.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -load %llvmshlibdir/PrintAttributeTokens%pluginext -fsyntax-only -ast-dump -verify %s
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+[[print_tokens(
+the, mitochondria,
+, Of::The($cell))]] void
+fn1a() {}
+[[plugin::print_tokens("a string")]] void fn1b() {}
+[[plugin::print_tokens()]] void fn1c() {}
+[[plugin::print_tokens(some_ident)]] void fn1d() {}
+[[plugin::print_tokens(int)]] void fn1e() {}
Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -45,10 +45,11 @@
   else if (HasParsedType)
 return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Token>(0, 0, 0, 1, 0, 0);
   return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+  detail::PropertyData, Token>(NumArgs, 0, 0, 0, 0,
+   NumTokens);
 }
 
 AttributeFactory::AttributeFactory() {
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4096,13 +4096,39 @@
   LO.CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x;
 
   // If the attribute isn't known, we will not attempt to parse any
-  // arguments.
+  // arguments. Instead, we just record the tokens and add the attribute
+  // directly. The recording happens here because this is the only place
+  // where user-defined (via plugins) attributes are parsed, and thus
+  // they care about the token stream directly.
   if (!hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
 AttrName, getTargetInfo(), getLangOpts())) {
-// Eat the left paren, then skip to the ending right paren.
+// Begin recording session.
+SmallVector RecordedTokens;
+assert(!PP.hasTokenRecorder());
+PP.setTokenRecorder(
+[&RecordedTokens](const Token &Tok) { RecordedTokens.push_back(Tok); });
+
+// Eat the left paren.
 ConsumeParen();
+
+// skip to the ending right paren.
 SkipUntil(tok::r_paren);
-return false;
+
+// End recording session.
+PP.setTokenRecorder(nullptr);
+
+// Add new attribute with the token list.
+// We assert that we have at least one token,
+// since we have to ignore the final r_paren.
+assert(RecordedTokens.size() > 0);
+Attrs.addNew(
+AttrName,
+SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc, AttrNameLoc),
+ScopeName, ScopeLoc, nullptr, 0,
+getLangOpts().CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x,
+RecordedTokens.data(), RecordedTokens.size() - 2);
+
+return true;
   }
 
   if (ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"))) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2828,7 +2828,7 @@
   ArgsVector ArgExprs;
   ArgExprs.push_back(ArgExpr.get());
   Attrs.addNew(KWName, KWLoc, nullptr, KWLoc, ArgExprs.data(), 1,
-   ParsedAttr::AS_Keyword, EllipsisLoc);
+   ParsedAttr::AS_Keyword, nullptr, 0, EllipsisLoc);
 }
 
 ExprResult Parser::ParseExtIntegerArgument() {
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -970,6 +970,9 @@
 if (OnToken)
   OnToken(Result);
   }
+
+  if (OnRecordedToken)
+OnRecordedToken(Result);
 }
 
 /// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -18,6 +18,7 @@
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "cla

[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes

2021-04-10 Thread Josh Junon via Phabricator via cfe-commits

Qix- added a comment.

@aaron.ballman updated with comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99861

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

[clang-tools-extra] 3b677b8 - [libtooling][clang-tidy] Fix diagnostics not highlighting fed SourceRanges

2021-04-10 Thread via cfe-commits


Author: Whisperity
Date: 2021-04-10T16:43:44+02:00
New Revision: 3b677b81cec7b3c5132aee8fccc30252d87deb69

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

LOG: [libtooling][clang-tidy] Fix diagnostics not highlighting fed SourceRanges

Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.

This patch allows Clang-Tidy checks to do

diag(X->getLocation(), "text") << Y->getSourceRange();

and get the highlight of `Y` as expected:

warning: text [blah-blah]
xxx(something)
^   ~

Reviewed-By: aaron.ballman, njames93

Differential Revision: http://reviews.llvm.org/D98635

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp

clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
clang/include/clang/Tooling/Core/Diagnostic.h
clang/include/clang/Tooling/DiagnosticsYaml.h
clang/lib/Tooling/Core/Diagnostic.cpp
clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp 
b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 88ba4bf63e09f..73d66b980a5e1 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -132,6 +132,8 @@ class ErrorReporter {
   }
   auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
   << Message.Message << Name;
+  for (const FileByteRange &FBR : Error.Message.Ranges)
+Diag << getRange(FBR);
   // FIXME: explore options to support interactive fix selection.
   const llvm::StringMap *ChosenFix;
   if (ApplyFixes != FB_NoFix &&
@@ -257,17 +259,13 @@ class ErrorReporter {
   for (const auto &Repl : FileAndReplacements.second) {
 if (!Repl.isApplicable())
   continue;
-SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
-Files.makeAbsolutePath(FixAbsoluteFilePath);
-SourceLocation FixLoc =
-getLocation(FixAbsoluteFilePath, Repl.getOffset());
-SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Repl.getLength());
-// Retrieve the source range for applicable fixes. Macro definitions
-// on the command line have locations in a virtual buffer and don't
-// have valid file paths and are therefore not applicable.
-CharSourceRange Range =
-CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
-Diag << FixItHint::CreateReplacement(Range, Repl.getReplacementText());
+FileByteRange FBR;
+FBR.FilePath = Repl.getFilePath().str();
+FBR.FileOffset = Repl.getOffset();
+FBR.Length = Repl.getLength();
+
+Diag << FixItHint::CreateReplacement(getRange(FBR),
+ Repl.getReplacementText());
   }
 }
   }
@@ -277,9 +275,22 @@ class ErrorReporter {
 auto Diag =
 Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
 << Message.Message;
+for (const FileByteRange &FBR : Message.Ranges)
+  Diag << getRange(FBR);
 reportFix(Diag, Message.Fix);
   }
 
+  CharSourceRange getRange(const FileByteRange &Range) {
+SmallString<128> AbsoluteFilePath{Range.FilePath};
+Files.makeAbsolutePath(AbsoluteFilePath);
+SourceLocation BeginLoc = getLocation(AbsoluteFilePath, Range.FileOffset);
+SourceLocation EndLoc = BeginLoc.getLocWithOffset(Range.Length);
+// Retrieve the source range for applicable highlights and fixes. Macro
+// definition on the command line have locations in a virtual buffer and
+// don't have valid file paths and are therefore not applicable.
+return CharSourceRange::getCharRange(BeginLoc, EndLoc);
+  }
+
   FileManager Files;
   LangOptions LangOpts; // FIXME: use langopts from each original file
   IntrusiveRefCntPtr DiagOpts;

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 9e45f0aea798f..0590715562d08 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -25,6 +25,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/STLExtras.h"
@@ -62,15 +63,31 @@ class ClangTidyDiagnost

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3b677b81cec7: [libtooling][clang-tidy] Fix diagnostics not 
highlighting fed SourceRanges (authored by whisperity).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98635

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
  clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Core/Diagnostic.h
  clang/include/clang/Tooling/DiagnosticsYaml.h
  clang/lib/Tooling/Core/Diagnostic.cpp
  clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Index: clang/unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- clang/unittests/Tooling/DiagnosticsYamlTest.cpp
+++ clang/unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -20,14 +20,16 @@
 using namespace clang::tooling;
 using clang::tooling::Diagnostic;
 
-static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset,
- const std::string &FilePath,
- const StringMap &Fix) {
+static DiagnosticMessage
+makeMessage(const std::string &Message, int FileOffset,
+const std::string &FilePath, const StringMap &Fix,
+const SmallVector &Ranges) {
   DiagnosticMessage DiagMessage;
   DiagMessage.Message = Message;
   DiagMessage.FileOffset = FileOffset;
   DiagMessage.FilePath = FilePath;
   DiagMessage.Fix = Fix;
+  DiagMessage.Ranges = Ranges;
   return DiagMessage;
 }
 
@@ -47,8 +49,8 @@
  const StringMap &Fix,
  const SmallVector &Ranges) {
   return Diagnostic(DiagnosticName,
-makeMessage(Message, FileOffset, FilePath, Fix), {},
-Diagnostic::Warning, "path/to/build/directory", Ranges);
+makeMessage(Message, FileOffset, FilePath, Fix, Ranges), {},
+Diagnostic::Warning, "path/to/build/directory");
 }
 
 static const char *YAMLContent =
@@ -77,12 +79,12 @@
 "  Offset:  62\n"
 "  Length:  2\n"
 "  ReplacementText: 'replacement #2'\n"
+"  Ranges:\n"
+"- FilePath:'path/to/source.cpp'\n"
+"  FileOffset:  10\n"
+"  Length:  10\n"
 "Level:   Warning\n"
 "BuildDirectory:  'path/to/build/directory'\n"
-"Ranges:\n"
-"  - FilePath:'path/to/source.cpp'\n"
-"FileOffset:  10\n"
-"Length:  10\n"
 "  - DiagnosticName:  'diagnostic#3'\n"
 "DiagnosticMessage:\n"
 "  Message: 'message #3'\n"
@@ -123,9 +125,9 @@
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72,
"path/to/source2.cpp", {}, {}));
   TUD.Diagnostics.back().Notes.push_back(
-  makeMessage("Note1", 88, "path/to/note1.cpp", {}));
+  makeMessage("Note1", 88, "path/to/note1.cpp", {}, {}));
   TUD.Diagnostics.back().Notes.push_back(
-  makeMessage("Note2", 99, "path/to/note2.cpp", {}));
+  makeMessage("Note2", 99, "path/to/note2.cpp", {}, {}));
 
   std::string YamlContent;
   raw_string_ostream YamlContentStream(YamlContent);
@@ -166,7 +168,7 @@
   EXPECT_EQ(100u, Fixes1[0].getOffset());
   EXPECT_EQ(12u, Fixes1[0].getLength());
   EXPECT_EQ("replacement #1", Fixes1[0].getReplacementText());
-  EXPECT_TRUE(D1.Ranges.empty());
+  EXPECT_TRUE(D1.Message.Ranges.empty());
 
   Diagnostic D2 = TUDActual.Diagnostics[1];
   EXPECT_EQ("diagnostic#2", D2.DiagnosticName);
@@ -179,10 +181,10 @@
   EXPECT_EQ(62u, Fixes2[0].getOffset());
   EXPECT_EQ(2u, Fixes2[0].getLength());
   EXPECT_EQ("replacement #2", Fixes2[0].getReplacementText());
-  EXPECT_EQ(1u, D2.Ranges.size());
-  EXPECT_EQ("path/to/source.cpp", D2.Ranges[0].FilePath);
-  EXPECT_EQ(10u, D2.Ranges[0].FileOffset);
-  EXPECT_EQ(10u, D2.Ranges[0].Length);
+  EXPECT_EQ(1u, D2.Message.Ranges.size());
+  EXPECT_EQ("path/to/source.cpp", D2.Message.Ranges[0].FilePath);
+  EXPECT_EQ(10u, D2.Message.Ranges[0].FileOffset);
+  EXPECT_EQ(10u, D2.Message.Ranges[0].Length);
 
   Diagnostic D3 = TUDActual.Diagnostics[2];
   EXPECT_EQ("diagnostic#3", D3.DiagnosticName);
@@ -198,5 +200,5 @@
   EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath);
   std::vector Fixes3 = getFixes(D3.Message.Fix);
   EXPECT_TRUE(Fixes3.empty());
-  EXPECT_TRUE(D3.Ranges.empty());
+  EXPECT_TRUE(D3.Message.Ranges.empty());
 }
Index: clang/lib/Tooling/Core/Diagnostic.cpp
===

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:1585-1586
   InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
-  ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType,
- this->ReturnValue);
+  ExprResult Res =
+  S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
   if (Res.isInvalid())

mizvekov wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > Perhaps this should just be direct initialization? Can't really find 
> > > anything in the standard though.
> > So I just checked this again. Regarding our conversation on IRC, what I 
> > said initially was correct: `ReturnValue` is always a member function 
> > expression, built by `makeReturnObject` -> `buildPromiseCall` -> 
> > `buildMemberCall`. So implicit move would never trigger here, and as far as 
> > I see there is no reason for this code to have used 
> > PerformMoveOrCopyInitialization in the first place.
> Err I meant: member function *call* expression
Also, I don't think we could force direct initialization here, if the object 
returned by Gro is volatile for example.
With that said, copy elision from the sema action on the return statement of 
get_return_object should take care of removing this copy here, I think, I don't 
see any reason it would not work here just as well as for expressions coming 
from the parser.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:854
   if (Ex && !Ex->isTypeDependent()) {
+NRVOResult NRVORes = IsThrownVarInScope ? getNRVOResult(Ex) : NRVOResult();
+

mizvekov wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > Any reason why you've moved that away from the comment that wants to 
> > > explain it?
> > Yes, on C++2b this might modify Ex, and I moved it so this would happen 
> > before we do the check with the type of the expression just below here.
> But yeah I forgot to move the comment, good catch.
Actually, had my wires crossed there with the P2266 DR, I only need to move 
this line there.



Comment at: clang/lib/Sema/SemaStmt.cpp:3157-3159
+  // it would deduce to a reference.
+  if (AT->isDecltypeAuto() && Res.isParenthesized)
+return Res = NRVOResult(), void();

aaronpuchert wrote:
> Can't we just do this when we know what it deduces to? It sounds weird to 
> handle dependent contexts here because we shouldn't attempt any return value 
> initialization then.
I think there are problems regarding the limitations of the current NRVO 
algorithm, but I had not had much time to study it or think of improvements, so 
I follow the current approach of trying to use as much available information as 
possible in order to eliminate candidates early.

With that said, this is not well covered by the test suite and I could probably 
get away with a lot less without breaking any existing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D100046: [AArch64] ACLE: Fix issue for mismatching enum types with builtins.

2021-04-10 Thread Raul Tambre via Phabricator via cfe-commits

tambre added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:10953-10967
 if (unsigned BuiltinID = NewFD->getBuiltinID()) {
   ASTContext::GetBuiltinTypeError Error;
   LookupNecessaryTypesForBuiltin(S, BuiltinID);
   QualType T = Context.GetBuiltinType(BuiltinID, Error);
   // If the type of the builtin differs only in its exception
   // specification, that's OK.
   // FIXME: If the types do differ in this way, it would be better to

This whole block is unnecessary since D77491 and should've been removed, but 
seems I missed it. Reverting/forgetting builtins was problematic and the 
attribute-based approach solved those deficiencies.

Delete this whole block and also remove `forgetBuiltin()`. The builtin-related 
code in `ActOnFunctionDeclarator()` handles everything necessary. No tests 
break and the SVE ones will pass. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100046

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

[clang-tools-extra] 8fa3975 - [libtooling][clang-tidy] Fix off-by-one rendering issue with SourceRanges

2021-04-10 Thread via cfe-commits


Author: Whisperity
Date: 2021-04-10T18:52:55+02:00
New Revision: 8fa39752477b225294cde0967a3b4c9c492e699c

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

LOG: [libtooling][clang-tidy] Fix off-by-one rendering issue with SourceRanges

There was an off-by-one issue with calculating the *exact* end location
of token ranges (as given by SomeDecl->getSourceRange()) which resulted in:

  xxx(something)
  ^~~~   // Note the missing ~ under the last character.

In addition, a test is added to keep the behaviour in check in the future.

This patch hotfixes commit 3b677b81cec7b3c5132aee8fccc30252d87deb69.

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 0590715562d08..08d5c0d6704ba 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -68,11 +68,12 @@ class ClangTidyDiagnosticRenderer : public 
DiagnosticRenderer {
 // into a real CharRange for the diagnostic printer later.
 // Whatever we store here gets decoupled from the current SourceManager, so
 // we **have to** know the exact position and length of the highlight.
-const auto &ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) 
{
+auto ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) {
   if (SourceRange.isCharRange())
 return SourceRange;
+  assert(SourceRange.isTokenRange());
   SourceLocation End = Lexer::getLocForEndOfToken(
-  SourceRange.getEnd(), 1, Loc.getManager(), LangOpts);
+  SourceRange.getEnd(), 0, Loc.getManager(), LangOpts);
   return CharSourceRange::getCharRange(SourceRange.getBegin(), End);
 };
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
index af8272ffcc632..414c7967bc825 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -1,10 +1,11 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp
-// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes' 
-export-fixes=%t.yaml -- -Wmissing-prototypes > %t.msg 2>&1
+// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array'
 -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s 
-implicit-check-not='{{warning|error|note}}:'
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
 #define X(n) void n ## n() {}
 X(f)
 int a[-1];
+int b[0];
 
 // CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 
'ff' [clang-diagnostic-missing-prototypes]
 // CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
@@ -12,14 +13,14 @@ int a[-1];
 // CHECK-MESSAGES: -input.cpp:2:1: note: declare 'static' if the function is 
not intended to be used outside of this translation unit
 // CHECK-MESSAGES: -input.cpp:1:14: note: expanded from macro 'X'
 // CHECK-MESSAGES: -input.cpp:3:7: error: 'a' declared as an array with a 
negative size [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:4:7: warning: zero size arrays are an extension 
[clang-diagnostic-zero-length-array]
 
 // CHECK-YAML: ---
 // CHECK-YAML-NEXT: MainSourceFile:  '{{.*}}-input.cpp'
 // CHECK-YAML-NEXT: Diagnostics:
 // CHECK-YAML-NEXT:   - DiagnosticName:  clang-diagnostic-missing-prototypes
 // CHECK-YAML-NEXT: DiagnosticMessage:
-// CHECK-YAML-NEXT:   Message: 'no previous prototype for function
-// ''ff'''
+// CHECK-YAML-NEXT:   Message: 'no previous prototype for function 
''ff'''
 // CHECK-YAML-NEXT:   FilePath:'{{.*}}-input.cpp'
 // CHECK-YAML-NEXT:   FileOffset:  30
 // CHECK-YAML-NEXT:   Replacements:[]
@@ -55,7 +56,19 @@ int a[-1];
 // CHECK-YAML-NEXT:   Ranges:
 // CHECK-YAML-NEXT:- FilePath:'{{.*}}-input.cpp'
 // CHECK-YAML-NEXT:  FileOffset:  41
-// CHECK-YAML-NEXT:  Length:  1
+// CHECK-YAML-NEXT:  Length:  2
 // CHECK-YAML-NEXT: Level:   Error
 // CHECK-YAML-NEXT: BuildDirectory:  '{{.*}}'
+// CHECK-YAML-NEXT:   - DiagnosticName:  clang-di

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336618.
whisperity set the repository for this revision to rG LLVM Github Monorepo.
whisperity added a comment.

Rebased over D98635 , allowing us to properly 
highlight the found //"mixable adjacent parameter range"// (assuming it's on 
the same line, of course):

F16236992: UnderSquiggly.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69560

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -0,0 +1,148 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN:  ]}' -- -x c
+
+#define bool _Bool
+#define true 1
+#define false 0
+
+typedef bool MyBool;
+
+#define TheLogicalType bool
+
+void declVoid(void); // NO-WARN: Declaration only.
+void decl(); // NO-WARN: Declaration only.
+void oneParam(int I) {}  // NO-WARN: 1 parameter.
+void variadic(int I, ...) {} // NO-WARN: 1 visible parameter.
+
+void trivial(int I, int J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'trivial' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'J'
+
+void qualifier(int I, const int CI) {} // NO-WARN: Distinct types.
+
+void restrictQualifier(char *restrict CPR1, char *restrict CPR2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters of 'restrictQualifier' of similar type ('char *restrict')
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CPR1'
+// CHECK-MESSAGES: :[[@LINE-3]]:60: note: the last parameter in the range is 'CPR2'
+
+void pointer1(int *IP1, int *IP2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 2 adjacent parameters of 'pointer1' of similar type ('int *')
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: the first parameter in the range is 'IP1'
+// CHECK-MESSAGES: :[[@LINE-3]]:30: note: the last parameter in the range is 'IP2'
+
+void pointerConversion(int *IP, long *LP) {}
+// NO-WARN: Even though C can convert any T* to U* back and forth, compiler
+// warnings already exist for this.
+
+void testVariadicsCall() {
+  int IVal = 1;
+  decl(IVal); // NO-WARN: Particular calls to "variadics" are like template
+  // instantiations, and we do not model them.
+
+  variadic(IVal);  // NO-WARN.
+  variadic(IVal, 2, 3, 4); // NO-WARN.
+}
+
+struct S {};
+struct T {};
+
+void taggedTypes1(struct S SVar, struct T TVar) {} // NO-WARN: Distinct types.
+
+void taggedTypes2(struct S SVar1, struct S SVar2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent parameters of 'taggedTypes2' of similar type ('struct S')
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'SVar1'
+// CHECK-MESSAGES: :[[@LINE-3]]:44: note: the last parameter in the range is 'SVar2'
+
+void wrappers(struct { int I; } I1, struct { int I; } I2) {} // NO-WARN: Distinct anonymous types.
+
+void knr(I, J)
+  int I;
+  int J;
+{}
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 2 adjacent parameters of 'knr' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the last parameter in the range is 'J'
+
+void boolAsWritten(bool B1, bool B2) {} // NO-WARN: The type name is ignored.
+// Note that "bool" is a macro that expands to "_Bool" in

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336619.
whisperity added a comment.

**[NFC]** Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -111,20 +111,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+void typedefChain(int I, MyInt1 MI1, MyInt2 MI2, MyInt2b MI2b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 4 adjacent parameters of 'typedefChain' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:23: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: after resolving type aliases, 'int' and 'MyInt2' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: after resolving type aliases, 'int' and 'MyInt2b' are the same
 
 typedef long MyLong1;
 using MyLong2 = long;
 
-void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -138,18 +156,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+void qualifiedThroughTypedef2(CInt CI1, const int CI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type are
 // CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
 // CHECK-MESSAGES: :[[@LINE-3]]:51: note: the last parameter in the range is 'CI2'
-
-void reference1(int I, int &IR) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int &CIR) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &&IRR) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &&CIRR) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after res

[PATCH] D96355: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336620.
whisperity set the repository for this revision to rG LLVM Github Monorepo.
whisperity added a comment.

**[NFC]** Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96355

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1} \
+// RUN:  ]}' --
+
+typedef int MyInt1;
+typedef int MyInt2;
+using CInt = const int;
+using CMyInt1 = const MyInt1;
+using CMyInt2 = const MyInt2;
+
+void qualified1(int I, const int CI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified1' of similar type are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:34: note: the last parameter in the range is 'CI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: 'int' and 'const int' parameters accept and bind the same kind of values
+
+void qualified2(int I, volatile int VI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: the last parameter in the range is 'VI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: 'int' and 'volatile int' parameters accept and bind the same kind of values
+
+void qualified3(int I, const volatile int CVI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified3' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'CVI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: 'int' and 'const volatile int' parameters accept and bind the same kind of values
+
+void qualified4(int *IP, const int *CIP) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified4' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'IP'
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: the last parameter in the range is 'CIP'
+// CHECK-MESSAGES: :[[@LINE-4]]:26: note: 'int *' and 'const int *' parameters accept and bind the same kind of values
+
+void qualified5(const int CI, const long CL) {} // NO-WARN: Not the same type
+
+void qualifiedPtr1(int *IP, int *const IPC) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 2 adjacent parameters of 'qualifiedPtr1' of similar type are
+// CHECK-MESSAGES: :[

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336622.
whisperity added a comment.

Made sure that the parameter's name is highlighted promptly in the 
//"first|last parameter of the range is..."// note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69560

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -0,0 +1,148 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN:  ]}' -- -x c
+
+#define bool _Bool
+#define true 1
+#define false 0
+
+typedef bool MyBool;
+
+#define TheLogicalType bool
+
+void declVoid(void); // NO-WARN: Declaration only.
+void decl(); // NO-WARN: Declaration only.
+void oneParam(int I) {}  // NO-WARN: 1 parameter.
+void variadic(int I, ...) {} // NO-WARN: 1 visible parameter.
+
+void trivial(int I, int J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'trivial' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'J'
+
+void qualifier(int I, const int CI) {} // NO-WARN: Distinct types.
+
+void restrictQualifier(char *restrict CPR1, char *restrict CPR2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters of 'restrictQualifier' of similar type ('char *restrict')
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CPR1'
+// CHECK-MESSAGES: :[[@LINE-3]]:60: note: the last parameter in the range is 'CPR2'
+
+void pointer1(int *IP1, int *IP2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 2 adjacent parameters of 'pointer1' of similar type ('int *')
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: the first parameter in the range is 'IP1'
+// CHECK-MESSAGES: :[[@LINE-3]]:30: note: the last parameter in the range is 'IP2'
+
+void pointerConversion(int *IP, long *LP) {}
+// NO-WARN: Even though C can convert any T* to U* back and forth, compiler
+// warnings already exist for this.
+
+void testVariadicsCall() {
+  int IVal = 1;
+  decl(IVal); // NO-WARN: Particular calls to "variadics" are like template
+  // instantiations, and we do not model them.
+
+  variadic(IVal);  // NO-WARN.
+  variadic(IVal, 2, 3, 4); // NO-WARN.
+}
+
+struct S {};
+struct T {};
+
+void taggedTypes1(struct S SVar, struct T TVar) {} // NO-WARN: Distinct types.
+
+void taggedTypes2(struct S SVar1, struct S SVar2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent parameters of 'taggedTypes2' of similar type ('struct S')
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'SVar1'
+// CHECK-MESSAGES: :[[@LINE-3]]:44: note: the last parameter in the range is 'SVar2'
+
+void wrappers(struct { int I; } I1, struct { int I; } I2) {} // NO-WARN: Distinct anonymous types.
+
+void knr(I, J)
+  int I;
+  int J;
+{}
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 2 adjacent parameters of 'knr' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the last parameter in the range is 'J'
+
+void boolAsWritten(bool B1, bool B2) {} // NO-WARN: The type name is ignored.
+// Note that "bool" is a macro that expands to "_Bool" internally, but it is
+// only "bool" that is ignored from the two.
+
+void underscoreBoolAsWritten(_Bool B1, _Bool B2) {}
+// Even though it is "_Bool" that is written in the code, the diagnostic message
+// r

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336623.
whisperity set the repository for this revision to rG LLVM Github Monorepo.
whisperity added a comment.

- Fixed a few nits.
- Rebased, and added highlighting (under-squiggly) the parameter/return type of 
conversion operators for extra emphasis.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -3,7 +3,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
-// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
@@ -3,7 +3,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
 // RUN:  ]}' --
 
 typedef int MyInt1;
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
@@ -3,7 +3,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 3}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
 // RUN:  ]}' --
 
 int add(int Left, int Right) { return Left + Right; } // NO-WARN: Only 2 parameters.
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -3,7 +3,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.Ig

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

2021-04-10 Thread Whisperity via Phabricator via cfe-commits

whisperity updated this revision to Diff 336625.
whisperity edited the summary of this revision.
whisperity added a comment.

Rebase over D98635 . Highlighting only the 
parameter's name, not the entire range of the parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp
  clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp
@@ -0,0 +1,477 @@
+// RUN: %check_clang_tidy %s readability-suspicious-call-argument %t -- -- -std=c++11
+
+void foo_1(int aa, int bb) {}
+
+void foo_2(int source, int aa) {}
+
+void foo_3(int valToRet, int aa) {}
+
+void foo_4(int pointer, int aa) {}
+
+void foo_5(int aa, int bb, int cc, ...) {}
+
+void foo_6(const int dd, bool &ee) {}
+
+void foo_7(int aa, int bb, int cc, int ff = 7) {}
+
+void foo_8(int frobble1, int frobble2) {}
+
+// Test functions for convertible argument--parameter types.
+void fun(const int &m);
+void fun2() {
+  int m = 3;
+  fun(m);
+}
+
+// Test cases for parameters of const reference and value.
+void value_const_reference(int ll, const int &kk);
+
+void const_ref_value_swapped() {
+  const int &kk = 42;
+  const int &ll = 42;
+  value_const_reference(kk, ll);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'kk' (passed to 'll') looks like it might be swapped with the 2nd, 'll' (passed to 'kk') [readability-suspicious-call-argument]
+  // CHECK-MESSAGES: :[[@LINE-7]]:6: note: in the call to 'value_const_reference', declared here
+}
+
+// Const, non const references.
+void const_nonconst_parameters(const int &mm, int &nn);
+
+void const_nonconst_swap1() {
+  const int &nn = 42;
+  int mm;
+  // Do not check, because non-const reference parameter cannot bind to const reference argument.
+  const_nonconst_parameters(nn, mm);
+}
+
+void const_nonconst_swap3() {
+  const int nn = 42;
+  int m = 42;
+  int &mm = m;
+  // Do not check, const int does not bind to non const reference.
+  const_nonconst_parameters(nn, mm);
+}
+
+void const_nonconst_swap2() {
+  int nn;
+  int mm;
+  // Check for swapped arguments. (Both arguments are non-const.)
+  const_nonconst_parameters(nn, mm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nn' (passed to 'mm') looks like it might be swapped with the 2nd, 'mm' (passed to 'nn')
+}
+
+void const_nonconst_pointers(const int *mm, int *nn);
+void const_nonconst_pointers2(const int *mm, const int *nn);
+
+void const_nonconst_pointers_swapped() {
+  int *mm;
+  const int *nn;
+  const_nonconst_pointers(nn, mm);
+}
+
+void const_nonconst_pointers_swapped2() {
+  const int *mm;
+  int *nn;
+  const_nonconst_pointers2(nn, mm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nn' (passed to 'mm') looks like it might be swapped with the 2nd, 'mm' (passed to 'nn')
+}
+
+// Test cases for pointers and arrays.
+void pointer_array_parameters(
+int *pp, int qq[4]);
+
+void pointer_array_swap() {
+  int qq[5];
+  int *pp;
+  // Check for swapped arguments. An array implicitly converts to a pointer.
+  pointer_array_parameters(qq, pp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'qq' (passed to 'pp') looks like it might be swapped with the 2nd, 'pp' (passed to 'qq')
+}
+
+// Test cases for multilevel pointers.
+void multilevel_pointer_parameters(int *const **pp,
+   const int *const *volatile const *qq);
+void multilevel_pointer_parameters2(
+char *nn, char *volatile *const *const *const *const &mm);
+
+typedef float T;
+typedef T *S;
+typedef S *const volatile R;
+typedef R *Q;
+typedef Q *P;
+typedef P *O;
+void multilevel_pointer_parameters3(float **const volatile ***rr, O &ss);
+
+void multilevel_pointer_swap() {
+  int *const **qq;
+  int *const **pp;
+  multilevel_pointer_parameters(qq, pp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'qq' (passed to 'ppp

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 336626.
mizvekov added a comment.

- Addresses aaronpuchert's review points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/nrvo-tracking.cpp

Index: clang/test/CodeGen/nrvo-tracking.cpp
===
--- clang/test/CodeGen/nrvo-tracking.cpp
+++ clang/test/CodeGen/nrvo-tracking.cpp
@@ -29,8 +29,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(3, t, T);
@@ -45,8 +43,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(5, t, auto);
@@ -61,8 +57,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(7, t, decltype(auto));
@@ -113,8 +107,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(5, t, auto);
@@ -129,8 +121,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(7, t, decltype(auto));
@@ -152,7 +142,11 @@
   }; }()();\
 }
 
-//B(1, X); // Uncomment this line at your own peril ;)
+// CHECK-LABEL: define{{.*}} void @_Z2b1v
+// CHECK:   call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call void @llvm.lifetime.end
+// CHECK-NEXT:  ret void
+B(1, X);
 
 // CHECK-LABEL: define{{.*}} void @_Z2b2v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
@@ -164,8 +158,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(3, T);
@@ -180,8 +172,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(5, );
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1052,9 +1052,18 @@
  StartingScope, InstantiatingVarTemplate);
 
   if (D->isNRVOVariable()) {
-QualType ReturnType = cast(DC)->getReturnType();
-if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
-  Var->setNRVOVariable(true);
+QualType ReturnType;
+if (auto *F = dyn_cast(DC))
+  ReturnType = F->getReturnType();
+else if (auto *F = dyn_cast(DC))
+  // FIXME: get the return type here somehow...
+  ReturnType = SemaRef.Context.getAutoDeductType();
+else
+  llvm_unreachable("Unknown context type");
+
+Sema::NRVOResult Res = SemaRef.getNRVOResult(Var);
+SemaRef.updNRVOResultWithRetType(Res, ReturnType);
+Var->setNRVOVariable(Res.isCopyElidable());
   }
 
   Var->setImplicit(D->isImplicit());
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3036,99 +3036,152 @@
   return new (Context) BreakStmt(BreakLoc);
 }
 
-/// Determine whether the given expression is a candidate for
-/// copy elision in either a return statement or a throw expression.
+/// Determine whether the given expression might be move-eligible or
+/// copy-elidable in either a (co_)return statement or throw expression,
+/// without considering function return type, if applicable.
 ///
-/// \param ReturnType If we're determining the copy elision candidate for
-/// a return statement, this is the return type of the function. If we're
-/// determining the copy elision candidate for a throw expression, this will
-/// be a NULL type.
+/// \param E The expression being returned from the function or block,
+//

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov marked 8 inline comments as done.
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3058
 
-bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
-  CopyElisionSemanticsKind CESK) {
-  QualType VDType = VD->getType();
-  // - in a return statement in a function with ...
-  // ... a class return type ...
-  if (!ReturnType.isNull() && !ReturnType->isDependentType()) {
-if (!ReturnType->isRecordType())
-  return false;
-// ... the same cv-unqualified type as the function return type ...
-// When considering moving this expression out, allow dissimilar types.
-if (!(CESK & CES_AllowDifferentTypes) && !VDType->isDependentType() &&
-!Context.hasSameUnqualifiedType(ReturnType, VDType))
-  return false;
+  // (other than a function ... parameter)
+  if (VD->getKind() == Decl::ParmVar) {

aaronpuchert wrote:
> These comments seem to be separated from their context now...
This one comment here is talking about the line just below it.
Any other examples where it seems separated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D100251: [IR][sanitizer] Set nounwind on module ctor/dtor, additionally set uwtable if -fasynchronous-unwind-tables

2021-04-10 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay created this revision.
MaskRay added reviewers: Sanitizers, jdoerfert, joerg, nickdesaulniers, rnk.
Herald added subscribers: dexonsmith, hiraditya.
MaskRay requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

On ELF targets, if a function has uwtable or personality, or does not have
nounwind (`needsUnwindTableEntry`), it marks that `.eh_frame` is needed in the 
module.

Then, a function gets `.eh_frame` if `needsUnwindTableEntry` or `-g[123]` is 
specified.
(i.e. If -g[123], every function gets `.eh_frame`.
This behavior is strange but that is the status quo on GCC and Clang.)

Let's take asan as an example. Other sanitizers are similar.
`asan.module_[cd]tor` has no attribute. `needsUnwindTableEntry` returns true,
so every function gets `.eh_frame` if `-g[123]` is specified.
This is the root cause that
`-fno-exceptions -fno-asynchronous-unwind-tables -g` produces .debug_frame
while
`-fno-exceptions -fno-asynchronous-unwind-tables -g -fsanitize=address` 
produces .eh_frame.

This patch

- sets the nounwind attribute on sanitizer module ctor/dtor.
- let Clang emit a module flag metadata "uwtable" for 
-fasynchronous-unwind-tables. If "uwtable" is set, sanitizer module ctor/dtor 
additionally get the uwtable attribute.

The "uwtable" mechanism is generic: synthesized functions not cloned/specialized
from existing ones should consider `Function::createWithDefaultAttr` instead of
`Function::create` if they want to get some default attributes which
have more of module semantics.

Other candidates: "frame-pointer" 
(https://github.com/ClangBuiltLinux/linux/issues/955
https://github.com/ClangBuiltLinux/linux/issues/1238), dso_local, etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100251

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/asan-new-pm.ll
  clang/test/CodeGen/asan-no-globals-no-comdat.cpp
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/ModuleUtils.cpp
  llvm/test/Instrumentation/AddressSanitizer/basic.ll
  llvm/test/Instrumentation/AddressSanitizer/no-globals.ll
  llvm/test/Instrumentation/AddressSanitizer/uwtable.ll
  llvm/test/Instrumentation/HWAddressSanitizer/basic.ll
  llvm/test/Instrumentation/HWAddressSanitizer/with-calls.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  
llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
@@ -60,5 +60,7 @@
   ret void
 }
 
-; ELF-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() comdat {
-; MACHO-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() {
+; ELF-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() #2 comdat {
+; MACHO-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() #2 {
+
+; CHECK: attributes #2 = { nounwind }
Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
@@ -4,8 +4,8 @@
 ; Module ctors should have stable names across modules, not something like
 ; @sancov.module_ctor.3 that may cause duplicate ctors after linked together.
 
-; CHECK: define internal void @sancov.module_ctor_trace_pc_guard() comdat {
-; CHECK: define internal void @sancov.module_ctor_bool_flag() comdat {
+; CHECK: define internal void @sancov.module_ctor_trace_pc_guard() #[[#]] comdat {
+; CHECK: define internal void @sancov.module_ctor_bool_flag() #[[#]] comdat {
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
@@ -4,8 +4,8 @@
 ; Module ctors should have stable names across modules, not something like
 ; @sancov.module_ctor.3 that may cause duplicate ctors after linked together.
 
-; CHECK: define internal void @s

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3030-3032
+  Res.S = Res.S != Sema::NRVOResult::None && CanMove
+  ? Sema::NRVOResult::MoveEligible
+  : Sema::NRVOResult::None;

mizvekov wrote:
> aaronpuchert wrote:
> > The way you've designed the enumeration, couldn't you use `std::min` here?
> Yes exactly!
> The previous version of this patch I submitted used exactly that, but then 
> for some reason I thought people could find std::min a bit odd here, and so I 
> changed it.
> But now I think I was right the first time around :)
Perhaps you can just add a comment to the enumeration saying that the values 
are totally ordered with regards to implication so that ⇒ is >=.



Comment at: clang/lib/Sema/SemaStmt.cpp:3058
 
-bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
-  CopyElisionSemanticsKind CESK) {
-  QualType VDType = VD->getType();
-  // - in a return statement in a function with ...
-  // ... a class return type ...
-  if (!ReturnType.isNull() && !ReturnType->isDependentType()) {
-if (!ReturnType->isRecordType())
-  return false;
-// ... the same cv-unqualified type as the function return type ...
-// When considering moving this expression out, allow dissimilar types.
-if (!(CESK & CES_AllowDifferentTypes) && !VDType->isDependentType() &&
-!Context.hasSameUnqualifiedType(ReturnType, VDType))
-  return false;
+  // (other than a function ... parameter)
+  if (VD->getKind() == Decl::ParmVar) {

mizvekov wrote:
> aaronpuchert wrote:
> > These comments seem to be separated from their context now...
> This one comment here is talking about the line just below it.
> Any other examples where it seems separated?
I meant the context of the comment itself. Originally this was quoting a clause

```
  // - in a return statement in a function [where] ...
  // ... the expression is the name of a non-volatile automatic object ...
```

and then additional parts later. Now you're having that initial part in a 
different function and it's not immediately clear what you're quoting here.

To be clear, this was about "(other than a function ... parameter)" below.



Comment at: clang/lib/Sema/SemaStmt.cpp:3140
+/// \param ReturnType This is the return type of the function.
+void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) {
+  if (!Res.Candidate)

mizvekov wrote:
> aaronpuchert wrote:
> > `NRVOResult` seems to be small, why not make this a proper function and let 
> > it return the result?
> It does use the result parameter as input and output. This function can only 
> "downgrade" a result it received previously.
> I could make it receive a result and return the result, but then would 
> probably just use it like this:
> ```
> NRVORes = updNRVOResultWithRetType(NRVORes, ...);
> ```
> Do you think that looks clearer?
Yes, that would seem more natural to me. Technically a caller could decide to 
not use the same variable in both places, for example it could pass a temporary 
or pass the result on directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-04-10 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

In D97653#2675005 , @aaron.ballman 
wrote:

> In D97653#2593287 , @njames93 wrote:
>
>> Where would be a good place for testing refersToDefaultCapture support?
>
> Thank you for your patience while I thought about this a bit more, sorry for 
> the delay in reviewing it!
>
> I think it probably makes sense to add testing for it in `clang\test\AST` 
> with an AST dumping test (so the text node dumper would probably need to be 
> updated to mention that the `DeclRefExpr` is an implicit capture).

That sounds like a good place to start, thanks.




Comment at: clang/lib/Sema/SemaLambda.cpp:1619
+  if (Cap.isVariableCapture() && ImplicitCaptureLoc.isValid())
+cast(InitExpr)->setRefersToDefaultCapture(true);
   InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(

aaron.ballman wrote:
> njames93 wrote:
> > Correct me if I'm wrong, but there shouldn't be a case where the assert in 
> > cast fails.
> Hmmm, could it be an unresolved lookup expr in the case of a dependent 
> capture? Or a `MemberExpr` in the case of capturing an anonymous structure or 
> union member capture?
> 
> Because we're trying to determine if a `DeclRefExpr` was implicitly captured, 
> would it be better to use "implicit capture" in the identifier rather than 
> "default capture"?
Can't capture anonymous members so thats a non issue. I'm not entirely sure 
about dependent, but the codepaths where isVariableCapture is true always seem 
to result in either an ExprError or DeclRefExpr.

I do agree with the name though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97653

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

[PATCH] D97683: [clang-tidy] Add include to misc-uniqueptr-reset-release

2021-04-10 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.
Herald added a project: clang-tools-extra.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97683

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

[PATCH] D96286: [clangd] Change TidyProvider cache to use a linked list approach

2021-04-10 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.
Herald added a project: clang-tools-extra.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96286

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

[PATCH] D100252: Fix for "Bug 27113 - MSVC-compat __identifier implementation incomplete"

2021-04-10 Thread Melvin Fox via Phabricator via cfe-commits

super_concat created this revision.
super_concat added a reviewer: rsmith.
super_concat added a project: clang.
super_concat requested review of this revision.

this patch fixes Bug 27113  by 
adding support for string literals to the implementation of the MS extension 
`__identifier`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100252

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Parser/MicrosoftExtensions.cpp


Index: clang/test/Parser/MicrosoftExtensions.cpp
===
--- clang/test/Parser/MicrosoftExtensions.cpp
+++ clang/test/Parser/MicrosoftExtensions.cpp
@@ -270,8 +270,10 @@
   // FIXME: We should pick a friendlier display name for this token kind.
   __identifier(1) // expected-error {{cannot convert  token 
to an identifier}}
   __identifier(+) // expected-error {{cannot convert '+' token to an 
identifier}}
-  __identifier("foo") // expected-error {{cannot convert  
token to an identifier}}
   __identifier(;) // expected-error {{cannot convert ';' token to an 
identifier}}
+  __identifier("1"); // expected-error {{use of undeclared identifier '1'}}
+  __identifier("+"); // expected-error {{use of undeclared identifier '+'}}
+  __identifier(";"); // expected-error {{use of undeclared identifier ';'}}
 }
 
 class inline_definition_pure_spec {
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1813,7 +1813,11 @@
 
 if (!Tok.isAnnotation() && Tok.getIdentifierInfo())
   Tok.setKind(tok::identifier);
-else {
+else if (Tok.is(tok::string_literal)) {
+  const StringRef StrData(Tok.getLiteralData() + 1, Tok.getLength() - 2);
+  Tok.setIdentifierInfo(this->getIdentifierInfo(StrData));
+  Tok.setKind(tok::identifier);
+} else {
   Diag(Tok.getLocation(), diag::err_pp_identifier_arg_not_identifier)
 << Tok.getKind();
   // Don't walk past anything that's not a real token.


Index: clang/test/Parser/MicrosoftExtensions.cpp
===
--- clang/test/Parser/MicrosoftExtensions.cpp
+++ clang/test/Parser/MicrosoftExtensions.cpp
@@ -270,8 +270,10 @@
   // FIXME: We should pick a friendlier display name for this token kind.
   __identifier(1) // expected-error {{cannot convert  token to an identifier}}
   __identifier(+) // expected-error {{cannot convert '+' token to an identifier}}
-  __identifier("foo") // expected-error {{cannot convert  token to an identifier}}
   __identifier(;) // expected-error {{cannot convert ';' token to an identifier}}
+  __identifier("1"); // expected-error {{use of undeclared identifier '1'}}
+  __identifier("+"); // expected-error {{use of undeclared identifier '+'}}
+  __identifier(";"); // expected-error {{use of undeclared identifier ';'}}
 }
 
 class inline_definition_pure_spec {
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1813,7 +1813,11 @@
 
 if (!Tok.isAnnotation() && Tok.getIdentifierInfo())
   Tok.setKind(tok::identifier);
-else {
+else if (Tok.is(tok::string_literal)) {
+  const StringRef StrData(Tok.getLiteralData() + 1, Tok.getLength() - 2);
+  Tok.setIdentifierInfo(this->getIdentifierInfo(StrData));
+  Tok.setKind(tok::identifier);
+} else {
   Diag(Tok.getLocation(), diag::err_pp_identifier_arg_not_identifier)
 << Tok.getKind();
   // Don't walk past anything that's not a real token.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3140
+/// \param ReturnType This is the return type of the function.
+void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) {
+  if (!Res.Candidate)

aaronpuchert wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > `NRVOResult` seems to be small, why not make this a proper function and 
> > > let it return the result?
> > It does use the result parameter as input and output. This function can 
> > only "downgrade" a result it received previously.
> > I could make it receive a result and return the result, but then would 
> > probably just use it like this:
> > ```
> > NRVORes = updNRVOResultWithRetType(NRVORes, ...);
> > ```
> > Do you think that looks clearer?
> Yes, that would seem more natural to me. Technically a caller could decide to 
> not use the same variable in both places, for example it could pass a 
> temporary or pass the result on directly.
But for this function, I don't see how it could make sense to actually use it 
like that.

You are either using it in a context where you don't have a ReturnType (throw, 
co_return), and you only care about the initial NRVOResult and don't call this 
function at all, or you are in a return statement, where you will call this 
function passing the initial NRVOResult, which will then have no further 
utility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-04-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3140
+/// \param ReturnType This is the return type of the function.
+void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) {
+  if (!Res.Candidate)

mizvekov wrote:
> aaronpuchert wrote:
> > mizvekov wrote:
> > > aaronpuchert wrote:
> > > > `NRVOResult` seems to be small, why not make this a proper function and 
> > > > let it return the result?
> > > It does use the result parameter as input and output. This function can 
> > > only "downgrade" a result it received previously.
> > > I could make it receive a result and return the result, but then would 
> > > probably just use it like this:
> > > ```
> > > NRVORes = updNRVOResultWithRetType(NRVORes, ...);
> > > ```
> > > Do you think that looks clearer?
> > Yes, that would seem more natural to me. Technically a caller could decide 
> > to not use the same variable in both places, for example it could pass a 
> > temporary or pass the result on directly.
> But for this function, I don't see how it could make sense to actually use it 
> like that.
> 
> You are either using it in a context where you don't have a ReturnType 
> (throw, co_return), and you only care about the initial NRVOResult and don't 
> call this function at all, or you are in a return statement, where you will 
> call this function passing the initial NRVOResult, which will then have no 
> further utility.
I haven't looked at the code, but FWIW, I'm always in favor of more 
"function-style" functions. Which would you rather deal with — `void 
square(int& x)` or `int square(int x)`? The same rationale applies uniformly 
everywhere. Even if we are "consuming" the argument in this particular 
instance, that might change after another few years of maintenance; and 
regardless, it might be easier for the maintainer to understand 
"function-style" code even if they aren't going to change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-04-10 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 336645.
njames93 added a comment.

Use implicit capture instead of default capture.

@aaron.ballman, Unfortunately the AST dump tests don't work as implicit 
captures don't appear to show up in the textual representation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97653

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -599,6 +599,7 @@
   Record.push_back(E->hasTemplateKWAndArgsInfo());
   Record.push_back(E->hadMultipleCandidates());
   Record.push_back(E->refersToEnclosingVariableOrCapture());
+  Record.push_back(E->refersToImplicitCapture());
   Record.push_back(E->isNonOdrUse());
 
   if (E->hasTemplateKWAndArgsInfo()) {
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2281,6 +2281,8 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ExplicitTemplateArgs
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //HadMultipleCandidates
   Abv->Add(BitCodeAbbrevOp(0)); // RefersToEnclosingVariableOrCapture
+  Abv->Add(
+  BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // RefersToImplicitCapture
   Abv->Add(BitCodeAbbrevOp(0)); // NonOdrUseReason
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclRef
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Location
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -597,6 +597,7 @@
   E->DeclRefExprBits.HasTemplateKWAndArgsInfo = Record.readInt();
   E->DeclRefExprBits.HadMultipleCandidates = Record.readInt();
   E->DeclRefExprBits.RefersToEnclosingVariableOrCapture = Record.readInt();
+  E->DeclRefExprBits.RefersToImplicitCapture = Record.readInt();
   E->DeclRefExprBits.NonOdrUseReason = Record.readInt();
   unsigned NumTemplateArgs = 0;
   if (E->hasTemplateKWAndArgsInfo())
@@ -2809,12 +2810,14 @@
 
 case EXPR_DECL_REF:
   S = DeclRefExpr::CreateEmpty(
-Context,
-/*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
-/*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
-/*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
-/*NumTemplateArgs=*/Record[ASTStmtReader::NumExprFields + 2] ?
-  Record[ASTStmtReader::NumExprFields + 6] : 0);
+  Context,
+  /*HasQualifier=*/Record[ASTStmtReader::NumExprFields],
+  /*HasFoundDecl=*/Record[ASTStmtReader::NumExprFields + 1],
+  /*HasTemplateKWAndArgsInfo=*/Record[ASTStmtReader::NumExprFields + 2],
+  /*NumTemplateArgs=*/
+  Record[ASTStmtReader::NumExprFields + 2]
+  ? Record[ASTStmtReader::NumExprFields + 7]
+  : 0);
   break;
 
 case EXPR_INTEGER_LITERAL:
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1615,6 +1615,8 @@
 return ExprError();
 
   Expr *InitExpr = Init.get();
+  if (Cap.isVariableCapture() && ImplicitCaptureLoc.isValid())
+cast(InitExpr)->setRefersToImplicitCapture(true);
   InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
   Name, Cap.getCaptureType(), Loc);
   InitializationKind InitKind =
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -988,6 +988,8 @@
   case NOUR_Constant: OS << " non_odr_use_constant"; break;
   case NOUR_Discarded: OS << " non_odr_use_discarded"; break;
   }
+  if (Node->refersToImplicitCapture())
+OS << " implicit_capture";
 }
 
 void TextNodeDumper::VisitUnresolvedLookupExpr(
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -389,6 +389,7 @@
   DeclRefExprBits.HadMultipleCandidates = false;
   DeclRefExprBits.RefersToEnclosingVariableOrCapture =
   RefersToEnclosingVariableOrCapture;
+  DeclRefExprBits.RefersToImplici

38 matches

Mail list logo