[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

2017-11-28 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string )
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}

hokein wrote:
> Passing `std::string` object is fine here -- because we use std::move below 
> to avoid the extra copy.
> 
> Is the warning caught by the clang-tidy check?
Unfortunatelly, I haven't tested, but I believe it's checkcpp warning.


https://reviews.llvm.org/D40543



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

[PATCH] D39834: [clang] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-22 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 123914.

https://reviews.llvm.org/D39834

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/opt-record.c


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,8 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc 
-nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O 
-check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib 
%s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt 
-fno-save-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-FOPT-DISABLE
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -20,3 +22,4 @@
 // CHECK-EQ: "-cc1"
 // CHECK-EQ: "-opt-record-file" "BAR.txt"
 
+// CHECK-FOPT-DISABLE-NOT: "-fno-save-optimization-record"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4342,6 +4342,7 @@
 CmdArgs.push_back("-fapple-pragma-pack");
 
   if (Args.hasFlag(options::OPT_fsave_optimization_record,
+   options::OPT_foptimization_record_file_EQ,
options::OPT_fno_save_optimization_record, false)) {
 CmdArgs.push_back("-opt-record-file");
 


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,8 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -20,3 +22,4 @@
 // CHECK-EQ: "-cc1"
 // CHECK-EQ: "-opt-record-file" "BAR.txt"
 
+// CHECK-FOPT-DISABLE-NOT: "-fno-save-optimization-record"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4342,6 +4342,7 @@
 CmdArgs.push_back("-fapple-pragma-pack");
 
   if (Args.hasFlag(options::OPT_fsave_optimization_record,
+   options::OPT_foptimization_record_file_EQ,
options::OPT_fno_save_optimization_record, false)) {
 CmdArgs.push_back("-opt-record-file");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

2017-11-29 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk created this revision.

Motivation: https://bugs.llvm.org/show_bug.cgi?id=34870
I'm totally not sure this is correct


https://reviews.llvm.org/D40594

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins.c


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -253,13 +253,16 @@
   volatile long double resld;
 
   resf = __builtin_fmodf(F,F);
-  // CHECK: frem float
+  // CHECK: call float @fmodf(float %
+  // CHECK-NOT: readnone
 
   resd = __builtin_fmod(D,D);
-  // CHECK: frem double
+  // CHECK: call double @fmod(double %
+  // CHECK-NOT: readnone
 
   resld = __builtin_fmodl(LD,LD);
-  // CHECK: frem x86_fp80
+  // CHECK: call x86_fp80 @fmodl(x86_fp80 %
+  // CHECK-NOT: readnone
 
   resf = __builtin_fabsf(F);
   resd = __builtin_fabs(D);
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -899,14 +899,6 @@
   case Builtin::BI__builtin_fabsl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::fabs));
   }
-  case Builtin::BI__builtin_fmod:
-  case Builtin::BI__builtin_fmodf:
-  case Builtin::BI__builtin_fmodl: {
-Value *Arg1 = EmitScalarExpr(E->getArg(0));
-Value *Arg2 = EmitScalarExpr(E->getArg(1));
-Value *Result = Builder.CreateFRem(Arg1, Arg2, "fmod");
-return RValue::get(Result);
-  }
   case Builtin::BI__builtin_copysign:
   case Builtin::BI__builtin_copysignf:
   case Builtin::BI__builtin_copysignl: {


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -253,13 +253,16 @@
   volatile long double resld;
 
   resf = __builtin_fmodf(F,F);
-  // CHECK: frem float
+  // CHECK: call float @fmodf(float %
+  // CHECK-NOT: readnone
 
   resd = __builtin_fmod(D,D);
-  // CHECK: frem double
+  // CHECK: call double @fmod(double %
+  // CHECK-NOT: readnone
 
   resld = __builtin_fmodl(LD,LD);
-  // CHECK: frem x86_fp80
+  // CHECK: call x86_fp80 @fmodl(x86_fp80 %
+  // CHECK-NOT: readnone
 
   resf = __builtin_fabsf(F);
   resd = __builtin_fabs(D);
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -899,14 +899,6 @@
   case Builtin::BI__builtin_fabsl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::fabs));
   }
-  case Builtin::BI__builtin_fmod:
-  case Builtin::BI__builtin_fmodf:
-  case Builtin::BI__builtin_fmodl: {
-Value *Arg1 = EmitScalarExpr(E->getArg(0));
-Value *Arg2 = EmitScalarExpr(E->getArg(1));
-Value *Result = Builder.CreateFRem(Arg1, Arg2, "fmod");
-return RValue::get(Result);
-  }
   case Builtin::BI__builtin_copysign:
   case Builtin::BI__builtin_copysignf:
   case Builtin::BI__builtin_copysignl: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

2017-11-28 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D40543

Files:
  include/clang/Tooling/Refactoring/Rename/RenamingAction.h


Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
 
 private:
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string )
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
 
   Expected


Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
 
 private:
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string )
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
 
   Expected
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39834: [clang] -foptimization-record-file= should imply -fsave-optimization-record

2017-12-19 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added a comment.

In https://reviews.llvm.org/D39834#959500, @JDevlieghere wrote:

> Thanks Dmitry, this LGTM!
>
> PS: Let me know if you don't have commit access and want me to commit it for 
> you.


I don't have commit access, please commit. Thanks for code review.


https://reviews.llvm.org/D39834



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

[PATCH] D39834: [clangd] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-20 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 123559.

https://reviews.llvm.org/D39834

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/opt-record.c


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,7 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc 
-nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O 
-check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib 
%s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4378,6 +4378,14 @@
 }
   }
 
+  if (!Args.hasFlag(options::OPT_fsave_optimization_record,
+options::OPT_fno_save_optimization_record, false)) {
+if (const Arg *A = 
Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
+  }
+
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
  options::OPT_fno_rewrite_imports, false);
   if (RewriteImports)


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,7 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4378,6 +4378,14 @@
 }
   }
 
+  if (!Args.hasFlag(options::OPT_fsave_optimization_record,
+options::OPT_fno_save_optimization_record, false)) {
+if (const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
+  }
+
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
  options::OPT_fno_rewrite_imports, false);
   if (RewriteImports)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39834: [clangd] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-20 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added a comment.

In https://reviews.llvm.org/D39834#930165, @malaperle wrote:

> Hi! You put "[clangd]" in the title, but I don't believe it's related to 
> clangd but rather just "clang"?


I thought patch to clang option is supposed to be marked as "clangd".


https://reviews.llvm.org/D39834



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

[PATCH] D39834: [ClangDriver] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-09 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk created this revision.
Herald added a subscriber: ilya-biryukov.

This is my first attempt to contribute to llvm. I'm trying to implement this 
https://bugs.llvm.org/show_bug.cgi?id=33670. I'm struggling with writing tests 
for this patch. I will be very thankful if somebody guides me trough writing 
tests for such thing. Thanks a lot.


https://reviews.llvm.org/D39834

Files:
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = 
Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39834: [clangd] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-09 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 122395.
Quolyk retitled this revision from "[clangd][WIP] -foptimization-record-file= 
should imply -fsave-optimization-record" to "[clangd] 
-foptimization-record-file= should imply -fsave-optimization-record".
Quolyk added a comment.

Added 1 test, I guess it's sufficient


https://reviews.llvm.org/D39834

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/opt-record.c


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,7 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc 
-nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O 
-check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib 
%s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = 
Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,


Index: test/Driver/opt-record.c
===
--- test/Driver/opt-record.c
+++ test/Driver/opt-record.c
@@ -9,6 +9,7 @@
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-01-15 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 181763.
Quolyk added a comment.

Patch is not yet fixed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56661

Files:
  clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -23,46 +23,46 @@
   return base + 3;
 }
 
-void f(std::array a, int pos) {
-  a [ pos / 2 /*comment*/] = 1;
+void f(std::array list, int pos) {
+  list [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
-  int j = a[pos - 1];
+  int j = list[pos - 1];
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
 
-  a.at(pos-1) = 2; // OK, at() instead of []
-  gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
+  list.at(pos-1) = 2; // OK, at() instead of []
+  gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of []
 
-  a[-1] = 3;
+  list[-1] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index]
-  a[10] = 4;
+  list[10] = 4;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index]
 
-  a[const_index(7)] = 3;
+  list[const_index(7)] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements)
 
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK, constant index and inside bounds
+  list[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
 void g() {
-  int a[10];
+  int list[10];
   for (int i = 0; i < 10; ++i) {
-a[i] = i;
+list[i] = i;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
-// CHECK-FIXES: gsl::at(a, i) = i;
-gsl::at(a, i) = i; // OK, gsl::at() instead of []
+// CHECK-FIXES: gsl::at(list, i) = i;
+gsl::at(list, i) = i; // OK, gsl::at() instead of []
   }
 
-  a[-1] = 3; // flagged by clang-diagnostic-array-bounds
-  a[10] = 4; // flagged by clang-diagnostic-array-bounds
-  a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
-
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[-1] = 3; // flagged by clang-diagnostic-array-bounds
+  list[10] = 4; // flagged by clang-diagnostic-array-bounds
+  list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
+
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK, constant index and inside bounds
+  list[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
 struct S {
Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -24,48 +24,48 @@
   return base + 3;
 }
 
-void f(std::array a, int pos) {
-  a [ pos / 2 /*comment*/] = 1;
+void f(std::array list, int pos) {
+  list [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
-  // CHECK-FIXES: gsl::at(a,  pos / 2 /*comment*/) = 1;
-  int j = a[pos - 1];
+  // CHECK-FIXES: gsl::at(list,  pos / 2 /*comment*/) = 1;
+  int j = list[pos - 1];
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
-  // CHECK-FIXES: int j = gsl::at(a, pos - 1);
+  //

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-12 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added a comment.

For now, I just added tests. I have several questions, as I'm beginner 
(`ContainerSizeEmptyCheck.cpp`).

1. Do I have to extend `ValidContainer`, so it recognises `::std::string` with 
`length()` method  as valid container too or we can assume `::std::string` as 
valid container by default?
2. If `::std::string` is valid container, I just add one more expression to 
match. Is it correct way?




Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-12 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added a comment.

In D56644#1355434 , @lebedev.ri wrote:

> Either this is a NFC change with just tests, or the actual fix is missing.


Right, it's WIP for now.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk marked an inline comment as done.
Quolyk added inline comments.



Comment at: test/clang-tidy/readability-container-size-empty.cpp:19-20
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;

lebedev.ri wrote:
> Does it still work if only one of these exists?
It works indeed, do you suggest adding test case for this?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-12 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk created this revision.
Quolyk added reviewers: Eugene.Zelenko, omtcyfz, aaron.ballman, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.

Extends readability-container-size-empty to check std::string length() similar 
to size().
Motivation: https://bugs.llvm.org/show_bug.cgi?id=38255.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56644

Files:
  test/clang-tidy/readability-container-size-empty.cpp


Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -17,6 +17,7 @@
   bool operator!=(const char *) const;
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;
 };
 
@@ -106,9 +107,13 @@
   std::string str2;
   std::wstring wstr;
   (void)(str.size() + 0);
+  (void)(str.length() + 0);
   (void)(str.size() - 0);
+  (void)(str.length() - 0);
   (void)(0 + str.size());
+  (void)(0 + str.length());
   (void)(0 - str.size());
+  (void)(0 - str.length());
   if (intSet.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be 
used to check for emptiness instead of 'size' [readability-container-size-empty]
@@ -125,10 +130,14 @@
   // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
 ;
+  if (str.length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
   if ((str + str2).size() == 0)
 ;
+  if ((str + str2).length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (str == "")
@@ -141,6 +150,8 @@
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (wstr.size() == 0)
 ;
+  if (wstr.length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   if (wstr == "")


Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -17,6 +17,7 @@
   bool operator!=(const char *) const;
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;
 };
 
@@ -106,9 +107,13 @@
   std::string str2;
   std::wstring wstr;
   (void)(str.size() + 0);
+  (void)(str.length() + 0);
   (void)(str.size() - 0);
+  (void)(str.length() - 0);
   (void)(0 + str.size());
+  (void)(0 + str.length());
   (void)(0 - str.size());
+  (void)(0 - str.length());
   if (intSet.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
@@ -125,10 +130,14 @@
   // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
 ;
+  if (str.length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
   if ((str + str2).size() == 0)
 ;
+  if ((str + str2).length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (str == "")
@@ -141,6 +150,8 @@
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (wstr.size() == 0)
 ;
+  if (wstr.length() == 0)
+;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   if (wstr == "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 181467.
Quolyk added a comment.

Update tests. Handle length.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -17,6 +17,7 @@
   bool operator!=(const char *) const;
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;
 };
 
@@ -106,30 +107,42 @@
   std::string str2;
   std::wstring wstr;
   (void)(str.size() + 0);
+  (void)(str.length() + 0);
   (void)(str.size() - 0);
+  (void)(str.length() - 0);
   (void)(0 + str.size());
+  (void)(0 + str.length());
   (void)(0 - str.size());
+  (void)(0 - str.length());
   if (intSet.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'set'::empty() defined here
   if (intSet == std::set())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'set'::empty() defined here
   if (s_func() == "")
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
+  if (str.length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
   if ((str + str2).size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
+  if ((str + str2).length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (str == "")
 ;
@@ -141,7 +154,11 @@
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (wstr.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
+  if (wstr.length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   if (wstr == "")
 ;
@@ -150,7 +167,7 @@
   std::vector vect;
   if (vect.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (vect.empty()){{$}}
   if (vect == std::vector())
 ;
@@ -423,17 +440,17 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :45:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :45:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-01-14 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk added a comment.

For now I just updated tests. The problem is in `BaseRange` definition, as it 
holds `EndLoc` and `BeginLoc` pointing to the beginning of ArrayExpression base 
https://github.com/llvm-mirror/clang-tools-extra/blob/e0441f6939da38f26bea9c1d75bb33024daa4e40/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp#L78.
 I'm investigating this.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56661



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

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-01-14 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk created this revision.
Quolyk added reviewers: aaron.ballman, alexfh, JonasToth, omtcyfz.
Herald added subscribers: cfe-commits, arphaman, kbarton, xazax.hun, nemanjai.

This patch fixes incorrect array name generation for a 
cppcoreguidelines-pro-bounds-constant-array-index warning.
Motivation: https://bugs.llvm.org/show_bug.cgi?id=38510.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56661

Files:
  
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -47,12 +47,12 @@
 }
 
 void g() {
-  int a[10];
+  int list[10];
   for (int i = 0; i < 10; ++i) {
-a[i] = i;
+list[i] = i;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript 
when the index is not an integer constant expression; use gsl::at() instead
-// CHECK-FIXES: gsl::at(a, i) = i;
-gsl::at(a, i) = i; // OK, gsl::at() instead of []
+// CHECK-FIXES: gsl::at(list, i) = i;
+gsl::at(list, i) = i; // OK, gsl::at() instead of []
   }
 
   a[-1] = 3; // flagged by clang-diagnostic-array-bounds
Index: 
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
===
--- 
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++ 
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -24,48 +24,48 @@
   return base + 3;
 }
 
-void f(std::array a, int pos) {
-  a [ pos / 2 /*comment*/] = 1;
+void f(std::array list, int pos) {
+  list [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when 
the index is not an integer constant expression; use gsl::at() instead 
[cppcoreguidelines-pro-bounds-constant-array-index]
-  // CHECK-FIXES: gsl::at(a,  pos / 2 /*comment*/) = 1;
-  int j = a[pos - 1];
+  // CHECK-FIXES: gsl::at(list,  pos / 2 /*comment*/) = 1;
+  int j = list[pos - 1];
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when 
the index is not an integer constant expression; use gsl::at() instead
-  // CHECK-FIXES: int j = gsl::at(a, pos - 1);
+  // CHECK-FIXES: int j = gsl::at(list, pos - 1);
 
-  a.at(pos-1) = 2; // OK, at() instead of []
-  gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
+  list.at(pos-1) = 2; // OK, at() instead of []
+  gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of []
 
-  a[-1] = 3;
+  list[-1] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is 
negative [cppcoreguidelines-pro-bounds-constant-array-index]
-  a[10] = 4;
+  list[10] = 4;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements) 
[cppcoreguidelines-pro-bounds-constant-array-index]
 
-  a[const_index(7)] = 3;
+  list[const_index(7)] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past 
the end of the array (which contains 10 elements)
 
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK, constant index and inside bounds
+  list[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
 void g() {
   int a[10];
   for (int i = 0; i < 10; ++i) {
-a[i] = i;
+list[i] = i;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript 
when the index is not an integer constant expression; use gsl::at() instead
-// CHECK-FIXES: gsl::at(a, i) = i;
-gsl::at(a, i) = i; // OK, gsl::at() instead of []
+// CHECK-FIXES: gsl::at(list, i) = i;
+gsl::at(list, i) = i; // OK, gsl::at() instead of []
   }
 
-  a[-1] = 3; // flagged by clang-diagnostic-array-bounds
-  a[10] = 4; // flagged by clang-diagnostic-array-bounds
-  a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
-
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[-1] = 3; // flagged by clang-diagnostic-array-bounds
+  list[10] = 4; // flagged by clang-diagnostic-array-bounds
+  list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
+
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK,

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-07-12 Thread Dmitry Venikov via Phabricator via cfe-commits 
Quolyk updated this revision to Diff 209485.
Quolyk added a comment.
Herald added a subscriber: wuzish.
Herald added a project: clang.

Update. Solution is not elegant, because DeclRef->BaseRange somehow has zero 
length.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56661

Files:
  clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -23,46 +23,46 @@
   return base + 3;
 }
 
-void f(std::array a, int pos) {
-  a [ pos / 2 /*comment*/] = 1;
+void f(std::array list, int pos) {
+  list [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
-  int j = a[pos - 1];
+  int j = list[pos - 1];
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
 
-  a.at(pos-1) = 2; // OK, at() instead of []
-  gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
+  list.at(pos-1) = 2; // OK, at() instead of []
+  gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of []
 
-  a[-1] = 3;
+  list[-1] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index]
-  a[10] = 4;
+  list[10] = 4;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index]
 
-  a[const_index(7)] = 3;
+  list[const_index(7)] = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements)
 
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK, constant index and inside bounds
+  list[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
 void g() {
-  int a[10];
+  int list[10];
   for (int i = 0; i < 10; ++i) {
-a[i] = i;
+list[i] = i;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
-// CHECK-FIXES: gsl::at(a, i) = i;
-gsl::at(a, i) = i; // OK, gsl::at() instead of []
+// CHECK-FIXES: gsl::at(list, i) = i;
+gsl::at(list, i) = i; // OK, gsl::at() instead of []
   }
 
-  a[-1] = 3; // flagged by clang-diagnostic-array-bounds
-  a[10] = 4; // flagged by clang-diagnostic-array-bounds
-  a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
-
-  a[0] = 3; // OK, constant index and inside bounds
-  a[1] = 3; // OK, constant index and inside bounds
-  a[9] = 3; // OK, constant index and inside bounds
-  a[const_index(6)] = 3; // OK, constant index and inside bounds
+  list[-1] = 3; // flagged by clang-diagnostic-array-bounds
+  list[10] = 4; // flagged by clang-diagnostic-array-bounds
+  list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
+
+  list[0] = 3; // OK, constant index and inside bounds
+  list[1] = 3; // OK, constant index and inside bounds
+  list[9] = 3; // OK, constant index and inside bounds
+  list[const_index(6)] = 3; // OK, constant index and inside bounds
 }
 
 struct S {
Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -25,48 +25,48 @@
   return base + 3;
 }
 
-void f(std::array a, int pos) {
-  a [ pos / 2 /*comment*/] = 1;
+void f(std::array list, int pos) {
+  list [ pos / 2 /*comment*/] = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
-  // CHECK-FIXES: gsl::at(a,  pos / 2 /*comment*/) = 1;
-  int j = a[pos - 1];
+  // CHECK-FIXES: gsl::at(list,  pos / 2 /*comment*/) = 1;
+  int j = list[pos - 1];
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when