[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf022a5a792fd: Adds fixit hints to the -Wrange-loop-analysis 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68913

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template 
 struct Iterator {
@@ -67,14 +68,17 @@
   for (const int &x : int_non_ref_container) {}
   // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
   // expected-note@-2 {{use non-reference type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
   // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
   // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
   // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
 
 void test1() {
@@ -83,6 +87,7 @@
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (int &x : A) {}
@@ -93,6 +98,7 @@
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (double &x : A) {}
@@ -103,6 +109,7 @@
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : A) {}
@@ -126,6 +133,7 @@
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   //for (double &x : B) {}
   // Binding error
@@ -135,6 +143,7 @@
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   //for (Bar &x : B) {}
   // Binding error
@@ -148,6 +157,7 @@
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : C) {}
@@ -158,6 +168,7 @@
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
   //for (int &x : C) {}
@@ -174,6 +185,7 @@
   for (const Bar x : D) {}
   // expected-warning@-1 {{creates a copy}}
   // expected-note@-2 {{'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
   for (Bar &x : D) {}
   // No warning
   for (Bar x : D) {}
@@ -182,6 +194,7 @@
   for (const int &x : D) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : D) {}
   // No warning
   //for (int &x : D) {}
@@ -196,6 +209,7 @@
   for (const Bar &x : E) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : E) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : E) {}
@@ -210,6 +224,7 @@
   for (const Bar &x : F) {}
   // expected-warning@-1 {{resulting in a c

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8dc7b982b455: [NFC] Fixes -Wrange-loop-analysis warnings 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71857

Files:
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  llvm/include/llvm/Analysis/LoopInfo.h
  llvm/include/llvm/Analysis/LoopInfoImpl.h
  llvm/include/llvm/Support/GenericDomTree.h
  llvm/lib/Analysis/DomTreeUpdater.cpp
  llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/InterleavedLoadCombinePass.cpp
  llvm/lib/CodeGen/RegAllocFast.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
  llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
  llvm/lib/IR/TypeFinder.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/MC/XCOFFObjectWriter.cpp
  llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
  llvm/lib/MCA/Stages/InstructionTables.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
  llvm/lib/Support/CommandLine.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -132,7 +132,7 @@
 }
 
 AMDGPU::GPUKind llvm::AMDGPU::parseArchAMDGCN(StringRef CPU) {
-  for (const auto C : AMDGCNGPUs) {
+  for (const auto &C : AMDGCNGPUs) {
 if (CPU == C.Name)
   return C.Kind;
   }
@@ -141,7 +141,7 @@
 }
 
 AMDGPU::GPUKind llvm::AMDGPU::parseArchR600(StringRef CPU) {
-  for (const auto C : R600GPUs) {
+  for (const auto &C : R600GPUs) {
 if (CPU == C.Name)
   return C.Kind;
   }
@@ -163,12 +163,12 @@
 
 void AMDGPU::fillValidArchListAMDGCN(SmallVectorImpl &Values) {
   // XXX: Should this only report unique canonical names?
-  for (const auto C : AMDGCNGPUs)
+  for (const auto &C : AMDGCNGPUs)
 Values.push_back(C.Name);
 }
 
 void AMDGPU::fillValidArchListR600(SmallVectorImpl &Values) {
-  for (const auto C : R600GPUs)
+  for (const auto &C : R600GPUs)
 Values.push_back(C.Name);
 }
 
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -187,7 +187,7 @@
 // If we're adding this to all sub-commands, add it to the ones that have
 // already been registered.
 if (SC == &*AllSubCommands) {
-  for (const auto &Sub : RegisteredSubCommands) {
+  for (auto *Sub : RegisteredSubCommands) {
 if (SC == Sub)
   continue;
 addLiteralOption(Opt, Sub, Name);
@@ -243,7 +243,7 @@
 // If we're adding this to all sub-commands, add it to the ones that have
 // already been registered.
 if (SC == &*AllSubCommands) {
-  for (const auto &Sub : RegisteredSubCommands) {
+  for (auto *Sub : RegisteredSubCommands) {
 if (SC == Sub)
   continue;
 addOption(O, Sub);
@@ -318,7 +318,7 @@
   }
 
   bool hasOptions() const {
-for (const auto &S : RegisteredSubCommands) {
+for (const auto *S : RegisteredSubCommands) {
   if (hasOptions(*S))
 return true;
 }
@@ -2112,7 +2112,7 @@
 static void
 sortSubCommands(const SmallPtrSetImpl &SubMap,
 SmallVectorImpl> &Subs) {
-  for (const auto &S : SubMap) {
+  for (auto *S : SubMap) {
 if (S->getName().empty())
   continue;
 Subs.push_back(std::make_pair(S->getName().data(), S));
Index: llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
@@ -421,7 +421,7 @@
   for (const auto &LC : Lines.Blocks) {
 Result->createBlock(LC.FileName);
 if (Result->hasColumnInfo()) {
-  for (const auto &Item : zip(LC.Lines, LC.Columns)) {
+  for (auto Item : zip(LC.Lines, LC.Columns)) {
 auto &L = std:

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8117542ac57: Adds -Wrange-loop-analysis to -Wall (authored 
by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D68912?vs=226539&id=235783#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68912

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-range-loop-analysis.cpp


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -8,7 +8,6 @@
 CHECK-NEXT:-Wdelete-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-non-abstract-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-abstract-non-virtual-dtor
-CHECK-NEXT:-Wfor-loop-analysis
 CHECK-NEXT:-Wformat
 CHECK-NEXT:  -Wformat-extra-args
 CHECK-NEXT:  -Wformat-zero-length
@@ -21,6 +20,9 @@
 CHECK-NEXT:  -Wimplicit-int
 CHECK-NEXT:-Winfinite-recursion
 CHECK-NEXT:-Wint-in-bool-context
+CHECK-NEXT:-Wloop-analysis
+CHECK-NEXT:  -Wfor-loop-analysis
+CHECK-NEXT:  -Wrange-loop-analysis
 CHECK-NEXT:-Wmismatched-tags
 CHECK-NEXT:-Wmissing-braces
 CHECK-NEXT:-Wmove
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -857,11 +857,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -8,7 +8,6 @@
 CHECK-NEXT:-Wdelete-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-non-abstract-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-abstract-non-virtual-dtor
-CHECK-NEXT:-Wfor-loop-analysis
 CHECK-NEXT:-Wformat
 CHECK-NEXT:  -Wformat-extra-args
 CHECK-NEXT:  -Wformat-zero-length
@@ -21,6 +20,9 @@
 CHECK-NEXT:  -Wimplicit-int
 CHECK-NEXT:-Winfinite-recursion
 CHECK-NEXT:-Wint-in-bool-context
+CHECK-NEXT:-Wloop-analysis
+CHECK-NEXT:  -Wfor-loop-analysis
+CHECK-NEXT:  -Wrange-loop-analysis
 CHECK-NEXT:-Wmismatched-tags
 CHECK-NEXT:-Wmissing-braces
 CHECK-NEXT:-Wmove
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -857,11 +857,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

2020-01-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D72056#1800524 , @jrtc27 wrote:

> I am still of the view that we should support rewriting the instruction 
> stream in the linker when necessary like BFD does. We need to do this to be 
> able to link in GCC-compiled code, including libraries. It is a very simple 
> thing to do with a patch available that provides consistency between the GNU 
> world and the LLVM world.


https://github.com/riscv/riscv-elf-psabi-doc/issues/126#issuecomment-570076329


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

2020-01-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay marked 2 inline comments as done.
MaskRay added a subscriber: bsdjhb.
MaskRay added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:680
+  if (MF->getTarget().isPositionIndependent() ||
+  MF->getTarget().getCodeModel() != CodeModel::Small) {
 const auto &STI = MF->getSubtarget();

jrtc27 wrote:
> This is confusing and differs from when you invoke the assembler manually, 
> even via the Clang driver with the right code model specified. Any `la`s 
> there will be assembled as `AUIPC`/`ADDI`. I am of the view it was a mistake 
> to make `la`'s behaviour conditional on PICness and it should have always 
> used the GOT, but this is what we have.
> I am of the view it was a mistake to make la's behaviour conditional on 
> PICness and it should have always used the GOT, but this is what we have.

I agree with you. `la`'s behavior should not be conditional on PIC. RISC-V 
probably should have a GOT relaxation mechanism. See my comment at 
https://github.com/riscv/riscv-elf-psabi-doc/issues/126



Comment at: llvm/lib/Target/TargetMachine.cpp:192
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)

jrtc27 wrote:
> Are we sure we want to do this and take the performance hit over GCC due to 
> the extra level of indirection on every single extern global access? If this 
> is the solution to take for extern weak, I think we should limit it to just 
> that and have a separate discussion about non-extern-weak.
I'd like to know more about `-mcmodel=medany -fno-pic`'s use cases. I guess 
@bsdjhb's FreeBSD kernel use case can probably be met by `-mcmodel=medany 
-fpie`.

I think there is no precedent which differs extern-strong and extern-weak. If 
GOT relaxation works, we can make this unconditional.

  // RISC-V prefer avoiding copy relocations.
  if (TT.isRISCV()
return false;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 marked an inline comment as done.
njames93 added a comment.

In D71846#1800503 , @aaron.ballman 
wrote:

> The new option LGTM but one of the tests can be updated to have a less 
> complex RUN line.


that was just put in when i was initially testing the behavoiur, removed it now


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

https://reviews.llvm.org/D71846



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

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

2020-01-01 Thread James Clarke via Phabricator via cfe-commits 
jrtc27 added inline comments.



Comment at: llvm/lib/Target/TargetMachine.cpp:192
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)

MaskRay wrote:
> jrtc27 wrote:
> > Are we sure we want to do this and take the performance hit over GCC due to 
> > the extra level of indirection on every single extern global access? If 
> > this is the solution to take for extern weak, I think we should limit it to 
> > just that and have a separate discussion about non-extern-weak.
> I'd like to know more about `-mcmodel=medany -fno-pic`'s use cases. I guess 
> @bsdjhb's FreeBSD kernel use case can probably be met by `-mcmodel=medany 
> -fpie`.
> 
> I think there is no precedent which differs extern-strong and extern-weak. If 
> GOT relaxation works, we can make this unconditional.
> 
>   // RISC-V prefer avoiding copy relocations.
>   if (TT.isRISCV()
> return false;
It can, but that implies GOT accesses for anything extern, which will hurt 
kernel performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-01 Thread Tamás Zolnai via Phabricator via cfe-commits 
ztamas updated this revision to Diff 235790.
ztamas added a comment.

Update warning message in test files and rebase patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-funsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t
+
+///
+/// Test cases correctly caught by the check.
+
+int SimpleVarDeclaration() {
+  signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CStyleCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = (int)CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int StaticCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = static_cast(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:33: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int FunctionalCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = int(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int NegativeConstValue() {
+  const signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CharPointer(signed char *CCharacter) {
+  int NCharacter = *CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///
+/// Test cases correctly ignored by the check.
+
+int UnsignedCharCast() {
+  unsigned char CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+int PositiveConstValue() {
+  const signed char CCharacter = 5;
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of declaration expression.
+int DescendantCast() {
+  signed char CCharacter = 'a';
+  int NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of assignment expression.
+int DescendantCastAssignment() {
+  signed char CCharacter = 'a';
+  int NCharacter;
+  NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolVarDeclaration() {
+  signed char CCharacter = 'a';
+  bool BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolAssignment() {
+  signed char CCharacter = 'a';
+  bool BCharacter;
+  BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// char is an integer type in clang; make sure to ignore it.
+unsigned char CharToCharCast() {
+  signed char SCCharacter = 'a';
+  unsigned char USCharacter;
+  USCharacter = SCCharacter;
+
+  return USCharacter;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp
==

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 235788.
njames93 added a comment.

removed the unnecessary explicit setting of WarnOnUnfixable value for test case


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

https://reviews.llvm.org/D71846

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-warn.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1071,6 +1071,35 @@
  LanguageMode::Cxx17OrLater));
 }
 
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+  EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+  ifStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(matches(
+  "void baz(int i) { switch (int j = i; j) { default: break; } }",
+  switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+  EXPECT_TRUE(matches("void baz() {"
+  "int items[] = {};"
+  "for (auto &arr = items; auto &item : arr) {}"
+  "}",
+  cxxForRangeStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx2aOrLater));
+  EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto &item : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
 matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
+  REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+///  void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+///  }
+///  void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+///  }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+///   matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+///   matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+///   matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+  CXXForRangeStmt),
+  internal::Matcher, InnerMatcher) {
+  const Stmt *Init = Node.getInit();
+  return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder);
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
 
 
 
+MatcherExpr>nullPointerConstant
+Matches expressions that resolve to a null pointer constant, such as
+GNU's __null, C++11's nullp

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 235791.
njames93 added a comment.

small reformat


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

https://reviews.llvm.org/D71846

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-warn.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1071,6 +1071,35 @@
  LanguageMode::Cxx17OrLater));
 }
 
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+  EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+  ifStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(matches(
+  "void baz(int i) { switch (int j = i; j) { default: break; } }",
+  switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+  EXPECT_TRUE(matches("void baz() {"
+  "int items[] = {};"
+  "for (auto &arr = items; auto &item : arr) {}"
+  "}",
+  cxxForRangeStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx2aOrLater));
+  EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto &item : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
 matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
+  REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+///  void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+///  }
+///  void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+///  }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+///   matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+///   matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+///   matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+  CXXForRangeStmt),
+  internal::Matcher, InnerMatcher) {
+  const Stmt *Init = Node.getInit();
+  return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder);
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
 
 
 
+MatcherExpr>nullPointerConstant
+Matches expressions that resolve to a null pointer constant, such as
+GNU's __null, C++11's nullptr, or C's NULL macro.
+
+Given:
+  void *v1 = NULL;
+  void *v2

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

2020-01-01 Thread Whisperity via Phabricator via cfe-commits 
whisperity planned changes to this revision.
whisperity added a comment.

There are a few really minor bug fixes, test additions, documentation update, 
etc. coming along soon, but I've some more pressing matters. However, please 
feel free to review the patch as-is!


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

https://reviews.llvm.org/D69560



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

[PATCH] D72057: [docs] Update dead anchor in hacking page

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/www/hacking.html:301
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 

Should this instead point to 
`https://llvm.org/docs/GettingStarted.html#sending-patches` to more closely 
match the original target anchor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72057



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

As you mention, we're already inconsistent with how we designate where the repo 
lives on disk, so I'm fine with landing this as-is and making the root part of 
the path consistent in a follow-up. LGTM, with the "getting started" changes 
pulled out.




Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:66
 
-Once you are done, change to the ``llvm/tools/clang/tools/extra`` directory, 
and
+Once you are done, change to the ``llvm/clang-tools-extra`` directory, and
 let's start!

AlexanderLanin wrote:
> aaron.ballman wrote:
> > Jim wrote:
> > > I am no sure that "llvm/clang-tools-extra" should be replaced as 
> > > "llvm-project/clang-tools-extra".
> > > Maybe someone would confuse with "llvm", "llvm-project" and 
> > > "llvm-project/llvm"
> > Elsewhere we use `path/to/llvm/source`, which seems to be sufficiently 
> > clear.
> While this goes slightly beyond the scope of the original pull request I tend 
> to agree as `llvm` can easily be confused with `llvm-project/llvm` as Jim 
> wrote.
> However I'm not clear on the exact target: looking through other docs 
> probably most often you'll find `path/to/llvm/source` as Aaron mentioned, but 
> other times it's `/path/to/llvm-project/`, `llvm-project/`, `~/llvm/`, 
> `~/clang-llvm/`, `/path/to/llvm`, `/path/to/llvm/src`, 
> `/path/to/llvm/sources` or `/path/to/llvm/tree`.
> 
> While this is not that important, it's difficult enough to get started with 
> anything inside llvm as it is. This is low hanging fruit. I would create a 
> separate pull request afterwards to align those.
A separate pull request to make them consistent would be a great follow-up to 
this one.



Comment at: clang/www/hacking.html:301
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 

AlexanderLanin wrote:
> Jim wrote:
> > This change should be in a separate patch.
> Ok, separate patch: https://reviews.llvm.org/D72057
Please be sure to back this change out before landing this patch.


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

https://reviews.llvm.org/D71982



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.

Still LG -- do you need someone to land this on your behalf?


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

https://reviews.llvm.org/D71846



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

Yes please, I'm still very new to the llvm contributing system so I would have 
no idea what to do next.


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

https://reviews.llvm.org/D71846



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

[clang] 8ca79da - Revert "Adds -Wrange-loop-analysis to -Wall"

2020-01-01 Thread Mark de Wever via cfe-commits 

Author: Mark de Wever
Date: 2020-01-01T22:19:18+01:00
New Revision: 8ca79dac559219358b0c6bb00bded30935c7aa6a

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

LOG: Revert "Adds -Wrange-loop-analysis to -Wall"

The sanitizer-x86_64-linux buildbot failed to build lld with -Werror.

This reverts commit d8117542ac57f6051674ca70ea14c0e0d7d9b046.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/test/Misc/warning-wall.c
clang/test/SemaCXX/warn-range-loop-analysis.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index a15fb908c537..5b2183531a44 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -857,11 +857,11 @@ def Most : DiagGroup<"most", [
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
+ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
-LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a98054e75895..2b27b67eafa1 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -8,6 +8,7 @@ CHECK-NEXT:-Wcomment
 CHECK-NEXT:-Wdelete-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-non-abstract-non-virtual-dtor
 CHECK-NEXT:  -Wdelete-abstract-non-virtual-dtor
+CHECK-NEXT:-Wfor-loop-analysis
 CHECK-NEXT:-Wformat
 CHECK-NEXT:  -Wformat-extra-args
 CHECK-NEXT:  -Wformat-zero-length
@@ -20,9 +21,6 @@ CHECK-NEXT:  -Wimplicit-function-declaration
 CHECK-NEXT:  -Wimplicit-int
 CHECK-NEXT:-Winfinite-recursion
 CHECK-NEXT:-Wint-in-bool-context
-CHECK-NEXT:-Wloop-analysis
-CHECK-NEXT:  -Wfor-loop-analysis
-CHECK-NEXT:  -Wrange-loop-analysis
 CHECK-NEXT:-Wmismatched-tags
 CHECK-NEXT:-Wmissing-braces
 CHECK-NEXT:-Wmove

diff  --git a/clang/test/SemaCXX/warn-range-loop-analysis.cpp 
b/clang/test/SemaCXX/warn-range-loop-analysis.cpp
index c1bcdf6b0ed9..21c574799369 100644
--- a/clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ b/clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,4 +1,3 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14322
+  if (SrcForMask->getType()->isPointerTy()) {
+/// TODO: Use ptrmask instead of ptrtoint/inttoptr
+// Result = Builder.CreateIntrinsic(

Until we are there, can we still emit sane IR, without `inttoptr`?
https://godbolt.org/z/atBtSx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72057: [docs] Update dead anchor in hacking page

2020-01-01 Thread Alexander Lanin via Phabricator via cfe-commits 
AlexanderLanin marked an inline comment as done.
AlexanderLanin added inline comments.



Comment at: clang/www/hacking.html:301
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 

aaron.ballman wrote:
> Should this instead point to 
> `https://llvm.org/docs/GettingStarted.html#sending-patches` to more closely 
> match the original target anchor?
For a precise match yes, but I figured that one would need to start with git 
first of all as git is not mentioned anywhere else on the page.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72057



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 235798.
arichardson added a comment.

Address feedback: Avoid inttoptr by using ptrtoint + GEP instead.

Using two GEPs for align_up seems to generate better code than select + a 
single GEP.
See https://godbolt.org/z/UdPjZk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_is_aligned(&MemPtr::vf

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson marked an inline comment as done.
arichardson added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14322
+  if (SrcForMask->getType()->isPointerTy()) {
+/// TODO: Use ptrmask instead of ptrtoint/inttoptr
+// Result = Builder.CreateIntrinsic(

lebedev.ri wrote:
> Until we are there, can we still emit sane IR, without `inttoptr`?
> https://godbolt.org/z/atBtSx
It seems like avoiding the select might be better?
https://godbolt.org/z/UdPjZk

I've done that in the current version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 235799.
arichardson added a comment.

- Also update the other codegen test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_is_aligned(&MemPtr::vfunc, 64); // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+}
+
+void test_references(Foo &i) {

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61172 tests passed, 1 failed 
and 728 were skipped.

  failed: Clang.CodeGen/builtin-align-array.c

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61173 tests passed, 0 failed 
and 728 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

2020-01-01 Thread Alexander Lanin via Phabricator via cfe-commits 
AlexanderLanin updated this revision to Diff 235801.
AlexanderLanin marked an inline comment as done.
AlexanderLanin edited the summary of this revision.
AlexanderLanin added a comment.

Removed change in hacking page as discussed.
Can someone commit this as apparently I cannot do it myself (I'll create a PR 
with updated getting started documentation later...), thanks.


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

https://reviews.llvm.org/D71982

Files:
  clang-tools-extra/docs/clang-include-fixer.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/pp-trace.rst

Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -104,16 +104,16 @@
 
   ---
   - Callback: FileChanged
-Loc: "c:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "c:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
 (etc.)
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:5:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:5:1"
 Reason: ExitFile
 FileType: C_User
-PrevFID: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
+PrevFID: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/Input/Level1B.h"
   - Callback: EndOfMainFile
   ...
 
@@ -172,7 +172,7 @@
 Example:::
 
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
@@ -248,8 +248,8 @@
 FileName: "Input/Level1B.h"
 IsAngled: false
 FilenameRange: "Input/Level1B.h"
-File: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
-SearchPath: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace"
+File: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/Input/Level1B.h"
+SearchPath: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace"
 RelativePath: "Input/Level1B.h"
 Imported: (null)
 
@@ -271,8 +271,8 @@
 Example:::
 
   - Callback: moduleImport
-ImportLoc: "d:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:2"
-Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
+ImportLoc: "d:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:2"
+Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
 Imported: Level2B
 
 `EndOfMainFile `_ Callback
@@ -309,7 +309,7 @@
 Example:::
 
   - Callback: Ident
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-ident.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-ident.cpp:3:1"
 str: "$Id$"
 
 `PragmaDirective `_ Callback
@@ -329,7 +329,7 @@
 Example:::
 
   - Callback: PragmaDirective
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Introducer: PIK_HashPragma
 
 `PragmaComment `_ Callback
@@ -350,7 +350,7 @@
 Example:::
 
   - Callback: PragmaComment
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Kind: library
 Str: kernel32.lib
 
@@ -372,7 +372,7 @@
 Example:::
 
   - Callback: PragmaDetectMismatch
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Name: name
 Value: value
 
@@ -393,7 +393,7 @@
 Example:::
 
   - Callback: PragmaDebug
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 DebugType: warning
 
 `PragmaMessage `_ Callback
@@ -415,7 +415,7 @@
 Example:::
 
   - Callback: PragmaMessage
-Loc: "D:/Clang/llvm/tools/clang/t

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

I really like the new feature. Thanks for making the efforts!

However, I am afraid I don't like some of the fixes here. You can replace 
`const auto` with `const auto &` and call that a fix... IMHO if the type is not 
obvious, `const ConcreteType &` will be better.

In addition, I think there are some false positives, e.g. the diagnostic may 
ask me to replace `const std::pair` with `const std::pair &` when it is clear that the type is trivial and cheap to copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71857



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

[PATCH] D70764: build: reduce CMake handling for zlib

2020-01-01 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
compnerd closed this revision.
compnerd added a comment.

GIT 68a235d07f9 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.

2020-01-01 Thread Wang Tianqing via Phabricator via cfe-commits 
tianqing added a comment.

Could someone help commit this patch? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71884



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

2020-01-01 Thread Jim Lin via Phabricator via cfe-commits 
Jim added a comment.

@AlexanderLanin
git commit --amend --author="What-is-your-name " ?


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

https://reviews.llvm.org/D71982



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

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

2020-01-01 Thread Brian Gesiak via Phabricator via cfe-commits 
modocache planned changes to this revision.
modocache marked 2 inline comments as done and an inline comment as not done.
modocache added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1227
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroElidePass()));
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));

junparser wrote:
> modocache wrote:
> > junparser wrote:
> > > Since coro elision depends on other optimization pass(inline and so on)  
> > > implicitly,  how can we adjust the pipeline to achieve this.
> > One option would be to use the new pass manager's registration callbacks, 
> > like `PassBuilder::registerPipelineStartEPCallback` or 
> > `PassBuilder::registerOptimizerLastEPCallback`. These work similarly to the 
> > old pass manager's `PassManagerBuilder::addExtension`. That's something 
> > that I think would be good to improve in a follow-up patch, but let me know 
> > if you'd rather see it in this one.
> yes,  please. It should be done in this patch sets. 
Will do!



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1228
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroElidePass()));
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));

wenlei wrote:
> Manually scheduling the 2nd coro-split passes in the same CGSCC pipeline 
> would make the resume/suspend funclet ineligible for the bulk of CSGSS opts, 
> given the split point is relatively early. The implication would be 
> discouraging the use of coroutine in performance critical path. I would love 
> to see this being addressed before we claim coroutine is ready for new PM.
> 
> As commented in the 2nd patch, we may not need the devirt trick used with 
> legacy PM, instead for new PM, we could try to leverage `CGSCCUpdateResult` 
> without involving artificial indirect call and devirt (or follow how 
> outlining is handled by new PM).
> I would love to see this being addressed before we claim coroutine is ready 
> for new PM.

I apologize, since I didn't intend to make such a claim. In 
http://lists.llvm.org/pipermail/llvm-dev/2019-December/137835.html I explained 
that these 6 patches were focused on lowering coroutine intrinsics. My goal was 
to resolve https://bugs.llvm.org/show_bug.cgi?id=42867, so that C++ coroutines 
code didn't trigger a fatal error in ISel.

That being said, I'm happy to make changes here. I'll send updates for D71899 
and this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71903



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

[PATCH] D72066: [clangd] Assert that the testcases in LocateSymbol.All have no diagnostics

2020-01-01 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Also fix some bugs in the testcases which this exposed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72066

Files:
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -253,7 +253,7 @@
   )cpp",
 
   R"cpp(// Function definition via pointer
-int [[foo]](int) {}
+void [[foo]](int) {}
 int main() {
   auto *X = &^foo;
 }
@@ -270,7 +270,7 @@
 struct Foo { int [[x]]; };
 int main() {
   Foo bar;
-  bar.^x;
+  (void)bar.^x;
 }
   )cpp",
 
@@ -322,7 +322,7 @@
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
-struct Foo { static void bar(); }
+struct Foo { static void bar(); };
 } // namespace ns
 int main() { ^ns::Foo::bar(); }
   )cpp",
@@ -352,7 +352,7 @@
 
   R"cpp(// Symbol concatenated inside macro (not supported)
int *pi;
-   #define POINTER(X) p # X;
+   #define POINTER(X) p ## X;
int i = *POINTER(^i);
   )cpp",
 
@@ -433,10 +433,10 @@
   )cpp",
 
   R"cpp(// No implicit constructors
-class X {
+struct X {
   X(X&& x) = default;
 };
-X [[makeX]]() {}
+X $decl[[makeX]]();
 void foo() {
   auto x = m^akeX();
 }
@@ -444,7 +444,7 @@
 
   R"cpp(
 struct X {
-  X& [[operator]]++() {}
+  X& [[operator]]++() { return *this; }
 };
 void foo(X& x) {
   +^+x;
@@ -529,7 +529,20 @@
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 
+// Disable warnings which some testcases intentionally trigger,
+// so that we can assert the testcases have no diagnostics and
+// catch mistakes.
+TU.ExtraArgs.push_back("-Wno-gnu-designator");
+TU.ExtraArgs.push_back("-Wno-macro-redefined");
+
 auto AST = TU.build();
+// We don't expect there to be diagnostics, but if there are,
+// it facilitates debugging to print them.
+for (auto &Diag : AST.getDiagnostics()) {
+  llvm::errs() << Diag << "\n";
+}
+ASSERT_TRUE(AST.getDiagnostics().empty()) << Test;
+
 auto Results = locateSymbolAt(AST, T.point());
 
 if (!WantDecl) {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -253,7 +253,7 @@
   )cpp",
 
   R"cpp(// Function definition via pointer
-int [[foo]](int) {}
+void [[foo]](int) {}
 int main() {
   auto *X = &^foo;
 }
@@ -270,7 +270,7 @@
 struct Foo { int [[x]]; };
 int main() {
   Foo bar;
-  bar.^x;
+  (void)bar.^x;
 }
   )cpp",
 
@@ -322,7 +322,7 @@
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
-struct Foo { static void bar(); }
+struct Foo { static void bar(); };
 } // namespace ns
 int main() { ^ns::Foo::bar(); }
   )cpp",
@@ -352,7 +352,7 @@
 
   R"cpp(// Symbol concatenated inside macro (not supported)
int *pi;
-   #define POINTER(X) p # X;
+   #define POINTER(X) p ## X;
int i = *POINTER(^i);
   )cpp",
 
@@ -433,10 +433,10 @@
   )cpp",
 
   R"cpp(// No implicit constructors
-class X {
+struct X {
   X(X&& x) = default;
 };
-X [[makeX]]() {}
+X $decl[[makeX]]();
 void foo() {
   auto x = m^akeX();
 }
@@ -444,7 +444,7 @@
 
   R"cpp(
 struct X {
-  X& [[operator]]++() {}
+  X& [[operator]]++() { return *this; }
 };
 void foo(X& x) {
   +^+x;
@@ -529,7 +529,20 @@
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 
+// Disable warnings which some testcases intentionally trigger,
+// so that we can assert the testcases have no diagnostics and
+// catch mistakes.
+TU.ExtraArgs.push_back("-Wno-gnu-designator");
+TU.ExtraArgs.push_back("-Wno-macro-redefined");
+
 auto AST = TU.build();
+// We don't expect there to be diagnostics, but if there are,
+// it facilitates debugging to print them.
+for (auto &Diag : AST.getDiagnostics()) {
+  llvm::errs() << Diag << "\n";
+}
+ASSERT_TRUE(AST.getDiagnostics().empty()) << Test;
+
 auto Results = locateSymbolAt(AST, T.point());
 
 if (!WantDecl) {
___
cfe-commits mailing

[PATCH] D72066: [clangd] Assert that the testcases in LocateSymbol.All have no diagnostics

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61162 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72066



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

[clang] d2bb8c1 - [MC][TargetMachine] Delete MCTargetOptions::MCPIECopyRelocations

2020-01-01 Thread Fangrui Song via cfe-commits 

Author: Fangrui Song
Date: 2020-01-01T00:50:18-08:00
New Revision: d2bb8c16e711602481c8b33d0e2ccc9994eb6641

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

LOG: [MC][TargetMachine] Delete MCTargetOptions::MCPIECopyRelocations

clang/lib/CodeGen/CodeGenModule performs the -mpie-copy-relocations
check and sets dso_local on applicable global variables. We don't need
to duplicate the work in TargetMachine shouldAssumeDSOLocal.

Verified that -mpie-copy-relocations can still emit PC relative
relocations for external variable accesses.

clang -target x86_64 -fpie -mpie-copy-relocations -c => R_X86_64_PC32
clang -target aarch64 -fpie -mpie-copy-relocations -c => 
R_AARCH64_ADR_PREL_PG_HI21+R_AARCH64_LDST64_ABS_LO12_NC

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
llvm/lib/MC/MCTargetOptions.cpp
llvm/lib/Target/TargetMachine.cpp

Removed: 
llvm/test/CodeGen/X86/global-access-pie-copyrelocs.ll



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 9b1e91788346..645ef0165a53 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -485,7 +485,6 @@ static void initTargetOptions(llvm::TargetOptions &Options,
   Options.MCOptions.MCNoExecStack = CodeGenOpts.NoExecStack;
   Options.MCOptions.MCIncrementalLinkerCompatible =
   CodeGenOpts.IncrementalLinkerCompatible;
-  Options.MCOptions.MCPIECopyRelocations = CodeGenOpts.PIECopyRelocations;
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;

diff  --git a/llvm/include/llvm/MC/MCTargetOptions.h 
b/llvm/include/llvm/MC/MCTargetOptions.h
index f184620ff047..51a5fc9aa26a 100644
--- a/llvm/include/llvm/MC/MCTargetOptions.h
+++ b/llvm/include/llvm/MC/MCTargetOptions.h
@@ -46,7 +46,6 @@ class MCTargetOptions {
   bool MCSaveTempLabels : 1;
   bool MCUseDwarfDirectory : 1;
   bool MCIncrementalLinkerCompatible : 1;
-  bool MCPIECopyRelocations : 1;
   bool ShowMCEncoding : 1;
   bool ShowMCInst : 1;
   bool AsmVerbose : 1;

diff  --git a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc 
b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
index 9f1177f470b9..93e21b626eac 100644
--- a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
+++ b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
@@ -28,8 +28,6 @@ static cl::opt IncrementalLinkerCompatible(
 "When used with filetype=obj, "
 "emit an object file which can be used with an incremental linker"));
 
-static cl::opt PIECopyRelocations("pie-copy-relocations", cl::desc("PIE 
Copy Relocations"));
-
 static cl::opt DwarfVersion("dwarf-version", cl::desc("Dwarf version"),
   cl::init(0));
 
@@ -55,7 +53,6 @@ static MCTargetOptions InitMCTargetOptionsFromFlags() {
   MCTargetOptions Options;
   Options.MCRelaxAll = RelaxAll;
   Options.MCIncrementalLinkerCompatible = IncrementalLinkerCompatible;
-  Options.MCPIECopyRelocations = PIECopyRelocations;
   Options.DwarfVersion = DwarfVersion;
   Options.ShowMCInst = ShowMCInst;
   Options.ABIName = ABIName;

diff  --git a/llvm/lib/MC/MCTargetOptions.cpp b/llvm/lib/MC/MCTargetOptions.cpp
index 96bb094134fe..5848e3ecadbe 100644
--- a/llvm/lib/MC/MCTargetOptions.cpp
+++ b/llvm/lib/MC/MCTargetOptions.cpp
@@ -15,8 +15,8 @@ MCTargetOptions::MCTargetOptions()
 : MCRelaxAll(false), MCNoExecStack(false), MCFatalWarnings(false),
   MCNoWarn(false), MCNoDeprecatedWarn(false), MCSaveTempLabels(false),
   MCUseDwarfDirectory(false), MCIncrementalLinkerCompatible(false),
-  MCPIECopyRelocations(false), ShowMCEncoding(false), ShowMCInst(false),
-  AsmVerbose(false), PreserveAsmComments(true) {}
+  ShowMCEncoding(false), ShowMCInst(false), AsmVerbose(false),
+  PreserveAsmComments(true) {}
 
 StringRef MCTargetOptions::getABIName() const {
   return ABIName;

diff  --git a/llvm/lib/Target/TargetMachine.cpp 
b/llvm/lib/Target/TargetMachine.cpp
index 519918405066..97a1eb2f190a 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -184,15 +184,14 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
 const Function *F = dyn_cast_or_null(GV);
 if (F && F->hasFnAttribute(Attribute::NonLazyBind))
   return false;
-
-bool IsTLS = GV && GV->isThreadLocal();
-bool IsAccessViaCopyRelocs =
-GV && Options.MCOptions.MCPIECopyRelocations && 
isa(GV);
 Triple::ArchType Arch = TT.getArch();
-bool IsPPC =
-Arch == Triple::ppc || Arch == Triple::ppc64 || Arch == 
Triple::ppc64le;
-// Check if we can use copy

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 235752.
njames93 added a comment.

hopefully adhered to all conventions :)


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

https://reviews.llvm.org/D71846

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1071,6 +1071,35 @@
  LanguageMode::Cxx17OrLater));
 }
 
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+  EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+  ifStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(matches(
+  "void baz(int i) { switch (int j = i; j) { default: break; } }",
+  switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+  EXPECT_TRUE(matches("void baz() {"
+  "int items[] = {};"
+  "for (auto &arr = items; auto &item : arr) {}"
+  "}",
+  cxxForRangeStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx2aOrLater));
+  EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto &item : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
 matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
+  REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+///  void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+///  }
+///  void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+///  }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+///   matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+///   matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+///   matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+  CXXForRangeStmt),
+  internal::Matcher, InnerMatcher) {
+  const Stmt *Init = Node.getInit();
+  return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder);
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
 
 
 
+MatcherExpr>nullPointerConstant
+Matches expressions that resolve to a null pointer constant, such as
+GNU's __null, C++11's nullptr, or C's NULL macro.
+
+Given:
+  void *v1 = NULL;
+  void *v2 = nullptr;
+  void *v3 = __null; // GNU extension
+  char *cp = (char *)0;
+  int *ip = 0;
+  int i = 0;
+expr(nullPointerConstan

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 235755.

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

https://reviews.llvm.org/D71980

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s readability-braces-around-statements %t -- -- -std=c++17
+
+void handle(bool);
+
+template 
+void shouldFail() {
+  if constexpr (branch)
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: statement should be inside braces [readability-braces-around-statements]
+handle(true);
+  else
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: statement should be inside braces [readability-braces-around-statements]
+handle(false);
+}
+
+template 
+void shouldPass() {
+  if constexpr (branch) {
+handle(true);
+  } else {
+handle(false);
+  }
+}
+
+void shouldFailNonTemplate() {
+  constexpr bool branch = false;
+  if constexpr (branch)
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: statement should be inside braces [readability-braces-around-statements]
+handle(true);
+  else
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: statement should be inside braces [readability-braces-around-statements]
+handle(false);
+}
+
+void shouldPass() {
+  constexpr bool branch = false;
+  if constexpr (branch) {
+handle(true);
+  } else {
+handle(false);
+  }
+}
+
+void run() {
+shouldFail();
+shouldFail();
+shouldPass();
+shouldPass();
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- -- -std=c++17
+
+void handle(int);
+
+template 
+void shouldFail() {
+  if constexpr (Index == 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: repeated branch in conditional chain [bugprone-branch-clone]
+handle(0);
+  } else if constexpr (Index == 1) {
+handle(1);
+  } else {
+handle(0);
+  }
+}
+
+template 
+void shouldPass() {
+  if constexpr (Index == 0) {
+handle(0);
+  } else if constexpr (Index == 1) {
+handle(1);
+  } else {
+handle(2);
+  }
+}
+
+void shouldFailNonTemplate() {
+  constexpr unsigned Index = 1;
+  if constexpr (Index == 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: repeated branch in conditional chain [bugprone-branch-clone]
+handle(0);
+  } else if constexpr (Index == 1) {
+handle(1);
+  } else {
+handle(0);
+  }
+}
+
+void shouldPassNonTemplate() {
+  constexpr unsigned Index = 1;
+  if constexpr (Index == 0) {
+handle(0);
+  } else if constexpr (Index == 1) {
+handle(1);
+  } else {
+handle(2);
+  }
+}
+
+void run() {
+shouldFail<0>();
+shouldFail<1>();
+shouldFail<2>();
+shouldPass<0>();
+shouldPass<1>();
+shouldPass<2>();
+}
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,10 @@
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation(
+  .bind("if"),
+  this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,8 @@
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  ifStmt(stmt().bind("if"),
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+ stmt().bind("if"),
  hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")),
  hasElse(stmt().bind("else"))),
   this);
___
cfe-commits mailing list
cfe-commi

[PATCH] D72049: clang-tidy doc: Remove severities as they don't make consensus

2020-01-01 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru created this revision.
Herald added subscribers: cfe-commits, whisperity.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
sylvestre.ledru added a reviewer: aaron.ballman.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72049

Files:
  clang-tools-extra/docs/clang-tidy/checks/list.rst

Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -3,405 +3,401 @@
 Clang-Tidy Checks
 =
 
-.. Severities are coming from Codechecker:
-   https://github.com/Ericsson/codechecker/blob/master/config/checker_severity_map.json
-   If you change it here, please submit a PR on codechecker too
-
 .. csv-table::
-   :header: "Name", "Offers fixes", "Severity"
-   :widths: 50, 20, 10
+   :header: "Name", "Offers fixes"
+   :widths: 50, 20
 
-   `abseil-duration-addition `_, "Yes", ""
-   `abseil-duration-comparison `_, "Yes", ""
-   `abseil-duration-conversion-cast `_, "Yes", ""
-   `abseil-duration-division `_, "Yes", ""
-   `abseil-duration-factory-float `_, "Yes", ""
-   `abseil-duration-factory-scale `_, "Yes", ""
-   `abseil-duration-subtraction `_, "Yes", ""
-   `abseil-duration-unnecessary-conversion `_, "Yes", ""
-   `abseil-faster-strsplit-delimiter `_, "Yes", ""
-   `abseil-no-internal-dependencies `_, , ""
-   `abseil-no-namespace `_, , ""
-   `abseil-redundant-strcat-calls `_, "Yes", ""
-   `abseil-str-cat-append `_, "Yes", ""
-   `abseil-string-find-startswith `_, "Yes", "style"
-   `abseil-time-comparison `_, "Yes", ""
-   `abseil-time-subtraction `_, "Yes", ""
-   `abseil-upgrade-duration-conversions `_, "Yes", ""
-   `android-cloexec-accept `_, "Yes", ""
-   `android-cloexec-accept4 `_, , ""
-   `android-cloexec-creat `_, , "medium"
-   `android-cloexec-dup `_, , ""
-   `android-cloexec-epoll-create `_, , ""
-   `android-cloexec-epoll-create1 `_, , ""
-   `android-cloexec-fopen `_, , "medium"
-   `android-cloexec-inotify-init `_, , ""
-   `android-cloexec-inotify-init1 `_, , ""
-   `android-cloexec-memfd-create `_, , ""
-   `android-cloexec-open `_, , "medium"
-   `android-cloexec-pipe `_, , ""
-   `android-cloexec-pipe2 `_, , ""
-   `android-cloexec-socket `_, , "medium"
-   `android-comparison-in-temp-failure-retry `_, "Yes", ""
-   `boost-use-to-string `_, "Yes", "low"
-   `bugprone-argument-comment `_, "Yes", "low"
-   `bugprone-assert-side-effect `_, , "medium"
-   `bugprone-bad-signal-to-kill-thread `_, , ""
-   `bugprone-bool-pointer-implicit-conversion `_, "Yes", "low"
-   `bugprone-branch-clone `_, , "low"
-   `bugprone-copy-constructor-init `_, "Yes", "medium"
-   `bugprone-dangling-handle `_, , "high"
-   `bugprone-dynamic-static-initializers `_, , ""
-   `bugprone-exception-escape `_, , "medium"
-   `bugprone-fold-init-type `_, , "high"
-   `bugprone-forward-declaration-namespace `_, , "low"
-   `bugprone-forwarding-reference-overload `_, , "low"
-   `bugprone-inaccurate-erase `_, "Yes", "high"
-   `bugprone-incorrect-roundings `_, , "high"
-   `bugprone-infinite-loop `_, , "medium"
-   `bugprone-integer-division `_, , "medium"
-   `bugprone-lambda-function-name `_, , "low"
-   `bugprone-macro-parentheses `_, "Yes", "medium"
-   `bugprone-macro-repeated-side-effects `_, , "medium"
-   `bugprone-misplaced-operator-in-strlen-in-alloc `_, , "medium"
-   `bugprone-misplaced-widening-cast `_, "Yes", "high"
-   `bugprone-move-forwarding-reference `_, "Yes", "medium"
-   `bugprone-multiple-statement-macro `_, , "medium"
-   `bugprone-not-null-terminated-result `_, "Yes", "medium"
-   `bugprone-parent-virtual-call `_, "Yes", "medium"
-   `bugprone-posix-return `_, "Yes", ""
-   `bugprone-sizeof-container `_, , "high"
-   `bugprone-sizeof-expression `_, , "high"
-   `bugprone-string-constructor `_, "Yes", "high"
-   `bugprone-string-integer-assignment `_, "Yes", "low"
-   `bugprone-string-literal-with-embedded-nul `_, , "medium"
-   `bugprone-suspicious-enum-usage `_, , "high"
-   `bugprone-suspicious-memset-usage `_, "Yes", "high"
-   `bugprone-suspicious-missing-comma `_, , "high"
-   `bugprone-suspicious-semicolon `_, "Yes", "high"
-   `bugprone-suspicious-string-compare `_, "Yes", "medium"
-   `bugprone-swapped-arguments `_, "Yes", "high"
-   `bugprone-terminating-continue `_, "Yes", "medium"
-   `bugprone-throw-keyword-missing `_, , "medium"
-   `bugprone-too-small-loop-variable `_, , "medium"
-   `bugprone-undefined-memory-manipulation `_, , "medium"
-   `bugprone-undelegated-constructor `_, , "medium"
-   `bugprone-unhandled-self-assignment `_, , "medium"
-   `bugprone-unused-raii `_, "Yes", "high"
-   `bugprone-unused-return-value `_, , "medium"
-   `bugprone-use-after-move `_, , "high"
-   `bugprone-virtual-near-miss `_, "Yes", "medium"
-   `cert-dcl21-cpp `_, , "low"
-   `cert-dcl50-cpp `_, , "low"
-   `cert-dcl58-cpp `_, , "high"
-   `cert-env33-c `_, ,

[PATCH] D71963: clang-tidy doc: Add the severities description

2020-01-01 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru added a comment.

@aaron.ballman done in https://reviews.llvm.org/D72049

By the way, when you say:

> There are other models that exist and are maintained.



> Other models are also pretty good.

which lists do you have in mind?
thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

I'm in two minds about issuing a warning when scope restrictions prevent a fix. 
Do you think creating an option to enable or disable emitting warnings for 
cases where the scope prevents a fix would be a good idea?


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

https://reviews.llvm.org/D71846



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

Re: [PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-01 Thread Florin Iucha via cfe-commits 

Hi Jonas,


Don't know how to attach another patch to Phabricator, but I have added 
some tests that fail.



Happy New Year!

florin

On 12/31/19 10:11 AM, Jonas Toth via Phabricator wrote:

JonasToth updated this revision to Diff 235712.
JonasToth added a comment.

- fix error from macro-defined variables
- move test files in sub-directory
- fix false positive of news in type-dependent contextes
- add a true positive in typedependent context
- update to latest utility version


Repository:
   rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
   clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
   clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
   clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
   
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
   clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
   clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
   clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
   clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
   clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
   clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
   clang-tools-extra/clang-tidy/utils/LexerUtils.h
   clang-tools-extra/docs/ReleaseNotes.rst
   
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
   
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
   
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
   
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
   
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
   clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
   clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
   clang/lib/Analysis/ExprMutationAnalyzer.cpp
   clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

>From 4cfb85cf652996211d57176a2cb3a17e43102dcf Mon Sep 17 00:00:00 2001
From: Florin Iucha 
Date: Mon, 30 Dec 2019 17:20:35 -0500
Subject: [PATCH] Add extra tests

---
 ...oreguidelines-const-correctness-values.cpp | 165 ++
 1 file changed, 165 insertions(+)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
index 8f7692fb7fe..572b29fc24c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -574,3 +574,168 @@ void placement_new_in_unique_ptr() {
   int np_local0 = p_local0;
   new to_construct(np_local0);
 }
+
+// Test bit fields
+struct HardwareRegister {
+   unsigned field: 5;
+   unsigned : 7;
+   unsigned another: 20;
+};
+
+void TestRegisters () {
+   HardwareRegister np_reg0;
+   np_reg0.field = 3;
+
+   HardwareRegister p_reg1{ 3, 22 };
+   // CHECK-MESSAGES: [[@LINE-1]]:4: warning: variable 'p_reg1' of type 'HardwareRegister' can be declared 'const'
+   const unsigned p_val = p_reg1.another;
+}
+
+struct IntMaker
+{
+   operator bool() const;
+};
+
+IntMaker& operator>>(IntMaker&, int&);
+
+int TestExtractionOperator()
+{
+   int np_foo;
+   IntMaker np_maker;
+
+   if (np_maker >> np_foo) {
+  return np_foo + 2;
+   }
+
+   int np_bar;
+   np_maker >> np_bar;
+   return np_bar;
+}
+
+template 
+struct MyPair
+{
+   L left;
+   R right;
+
+   MyPair(const L& ll, const R& rr) : left{ll}, right{rr} {}
+};
+
+template 
+class MyDict
+{
+   public:
+  static constexpr int initial_size = 16;
+
+  ~MyDict() {
+ if (items) {
+delete[] items;
+ }
+  }
+
+  struct value_type
+  {
+ K kk;
+ V vv;
+  };
+
+  using iterator = value_type*;
+
+  iterator begin() { return items; }
+  iterator end() { return items + used; }
+
+  iterator cbegin() const { return items; }
+  iterator cend() const { return items + used; }
+
+  void emplace(K kk, V vv) {
+ if (! items) {
+count = initial_size;
+items = new value_type[count];
+ }
+ else {
+if (count == used) {
+   const auto old = items;
+   count += initial_size;
+   items = new value_type[count];
+   for (int ii = 0; ii < used; ++ ii) {
+  items[ii] = old[ii];
+   }
+}
+ }
+ items[used].kk = kk;
+ items[used].vv = vv;
+ ++ used;
+  }
+
+   private:
+
+  value_type* items = nullptr;
+  int count = 0;
+  int used = 0;
+};
+
+MyDict MakeDict()
+{
+   MyDict np_dict;
+
+   np_dict.emplace(

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits to be fixed.




Comment at: clang/lib/AST/ExprConstant.cpp:8156-8158
+  } else if (const Expr *E = Value.Base.dyn_cast()) {
+return GetAlignOfExpr(Info, E, UETT_AlignOf);
+  } else {

aaron.ballman wrote:
> No `else` after a `return`.
You can still drop this `else` (and use a regular `if`) because of the 
preceding `return`.



Comment at: clang/lib/AST/ExprConstant.cpp:10685
+if (Src.isLValue()) {
+  // If we evaluated a pointer, check the minimum known alignment
+  LValue Ptr;

aaron.ballman wrote:
> Comment missing a full stop at the end.
The comment is still missing its full stop at the end.



Comment at: clang/lib/Sema/SemaChecking.cpp:246-259
+if (AlignValue < 1) {
+  S.Diag(AlignOp->getExprLoc(), diag::err_alignment_too_small) << 1;
+  return true;
+} else if (llvm::APSInt::compareValues(AlignValue, MaxValue) > 0) {
+  S.Diag(AlignOp->getExprLoc(), diag::err_alignment_too_big)
+  << MaxValue.toString(10);
+  return true;

There are a bunch of `else` after `return` issues in this ladder. I would 
rearrange to put the error cases first and the warning case last.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71963: clang-tidy doc: Add the severities description

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D71963#1800102 , @sylvestre.ledru 
wrote:

> ok, thanks!
>  I will remove them tomorrow or the next day.
>
> Do you have any guidance about the next steps to add them back?


Yes, sorry about failing to talk about it! I think this is worth an RFC that 
CCs some of the main folks from clang-tidy and the clang static analyzer to see 
if there's an appetite for supporting the concept. The RFC can include 
information like what problem this is solving, why we should pay the 
maintenance and review burden to support it, and some concrete heuristics for 
picking a severity as consistently as possible (what you have above is an okay 
start, but often won't lead to consistently picking a severity because of the 
overlap in the descriptions). As part of the RFC, it would be helpful if you 
pointed out how some of the coding standards we support calculate severities 
(if you can find the information) and how related tools like codechecker (etc) 
calculate severity to see if we can find a heuristic that works for us. If you 
don't have all of the answers in the RFC, that's fine -- the hope is to get the 
discussion going in the right directions, not to start off with a perfect 
solution.

In D71963#1800343 , @sylvestre.ledru 
wrote:

> @aaron.ballman done in https://reviews.llvm.org/D72049
>
> By the way, when you say:
>
> > There are other models that exist and are maintained.
>
>
>
> > Other models are also pretty good.
>
> which lists do you have in mind?
> thanks


I know one reasonably well because I worked on the coding standard, which is 
CERT's way of calculating rule priorities based on several independent factors. 
More information can be found at: 
https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RiskAssessment
 but the heuristics are the same for the C++ coding standard as well. Also, it 
is common for static analysis tools to calculate severities for given check 
violations, so we may want to see if any of them document their heuristics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963



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

[PATCH] D72049: clang-tidy doc: Remove severities as they don't make consensus

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for backing this out while we discuss the best way forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72049



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D71846#1800344 , @njames93 wrote:

> I'm in two minds about issuing a warning when scope restrictions prevent a 
> fix. Do you think creating an option to enable or disable emitting warnings 
> for cases where the scope prevents a fix would be a good idea?


It's not uncommon for fixits to only be generated under specific circumstances, 
so I'm wondering what your concern is with warning when we can't provide a 
fixit? The cases I am thinking about all seem reasonable to diagnose (are true 
positives) without fixing, but maybe you have different circumstances in mind.


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

https://reviews.llvm.org/D71846



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

[PATCH] D70048: [LLD] Add NetBSD support as a new flavor of LLD (nb.lld)

2020-01-01 Thread Kamil Rytarowski via Phabricator via cfe-commits 
krytarowski added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70048



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

[PATCH] D72049: clang-tidy doc: Remove severities as they don't make consensus

2020-01-01 Thread Sylvestre Ledru via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG773667b8c20d: clang-tidy doc: Remove severities as they 
don't make consensus (authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72049

Files:
  clang-tools-extra/docs/clang-tidy/checks/list.rst

Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -3,405 +3,401 @@
 Clang-Tidy Checks
 =
 
-.. Severities are coming from Codechecker:
-   https://github.com/Ericsson/codechecker/blob/master/config/checker_severity_map.json
-   If you change it here, please submit a PR on codechecker too
-
 .. csv-table::
-   :header: "Name", "Offers fixes", "Severity"
-   :widths: 50, 20, 10
+   :header: "Name", "Offers fixes"
+   :widths: 50, 20
 
-   `abseil-duration-addition `_, "Yes", ""
-   `abseil-duration-comparison `_, "Yes", ""
-   `abseil-duration-conversion-cast `_, "Yes", ""
-   `abseil-duration-division `_, "Yes", ""
-   `abseil-duration-factory-float `_, "Yes", ""
-   `abseil-duration-factory-scale `_, "Yes", ""
-   `abseil-duration-subtraction `_, "Yes", ""
-   `abseil-duration-unnecessary-conversion `_, "Yes", ""
-   `abseil-faster-strsplit-delimiter `_, "Yes", ""
-   `abseil-no-internal-dependencies `_, , ""
-   `abseil-no-namespace `_, , ""
-   `abseil-redundant-strcat-calls `_, "Yes", ""
-   `abseil-str-cat-append `_, "Yes", ""
-   `abseil-string-find-startswith `_, "Yes", "style"
-   `abseil-time-comparison `_, "Yes", ""
-   `abseil-time-subtraction `_, "Yes", ""
-   `abseil-upgrade-duration-conversions `_, "Yes", ""
-   `android-cloexec-accept `_, "Yes", ""
-   `android-cloexec-accept4 `_, , ""
-   `android-cloexec-creat `_, , "medium"
-   `android-cloexec-dup `_, , ""
-   `android-cloexec-epoll-create `_, , ""
-   `android-cloexec-epoll-create1 `_, , ""
-   `android-cloexec-fopen `_, , "medium"
-   `android-cloexec-inotify-init `_, , ""
-   `android-cloexec-inotify-init1 `_, , ""
-   `android-cloexec-memfd-create `_, , ""
-   `android-cloexec-open `_, , "medium"
-   `android-cloexec-pipe `_, , ""
-   `android-cloexec-pipe2 `_, , ""
-   `android-cloexec-socket `_, , "medium"
-   `android-comparison-in-temp-failure-retry `_, "Yes", ""
-   `boost-use-to-string `_, "Yes", "low"
-   `bugprone-argument-comment `_, "Yes", "low"
-   `bugprone-assert-side-effect `_, , "medium"
-   `bugprone-bad-signal-to-kill-thread `_, , ""
-   `bugprone-bool-pointer-implicit-conversion `_, "Yes", "low"
-   `bugprone-branch-clone `_, , "low"
-   `bugprone-copy-constructor-init `_, "Yes", "medium"
-   `bugprone-dangling-handle `_, , "high"
-   `bugprone-dynamic-static-initializers `_, , ""
-   `bugprone-exception-escape `_, , "medium"
-   `bugprone-fold-init-type `_, , "high"
-   `bugprone-forward-declaration-namespace `_, , "low"
-   `bugprone-forwarding-reference-overload `_, , "low"
-   `bugprone-inaccurate-erase `_, "Yes", "high"
-   `bugprone-incorrect-roundings `_, , "high"
-   `bugprone-infinite-loop `_, , "medium"
-   `bugprone-integer-division `_, , "medium"
-   `bugprone-lambda-function-name `_, , "low"
-   `bugprone-macro-parentheses `_, "Yes", "medium"
-   `bugprone-macro-repeated-side-effects `_, , "medium"
-   `bugprone-misplaced-operator-in-strlen-in-alloc `_, , "medium"
-   `bugprone-misplaced-widening-cast `_, "Yes", "high"
-   `bugprone-move-forwarding-reference `_, "Yes", "medium"
-   `bugprone-multiple-statement-macro `_, , "medium"
-   `bugprone-not-null-terminated-result `_, "Yes", "medium"
-   `bugprone-parent-virtual-call `_, "Yes", "medium"
-   `bugprone-posix-return `_, "Yes", ""
-   `bugprone-sizeof-container `_, , "high"
-   `bugprone-sizeof-expression `_, , "high"
-   `bugprone-string-constructor `_, "Yes", "high"
-   `bugprone-string-integer-assignment `_, "Yes", "low"
-   `bugprone-string-literal-with-embedded-nul `_, , "medium"
-   `bugprone-suspicious-enum-usage `_, , "high"
-   `bugprone-suspicious-memset-usage `_, "Yes", "high"
-   `bugprone-suspicious-missing-comma `_, , "high"
-   `bugprone-suspicious-semicolon `_, "Yes", "high"
-   `bugprone-suspicious-string-compare `_, "Yes", "medium"
-   `bugprone-swapped-arguments `_, "Yes", "high"
-   `bugprone-terminating-continue `_, "Yes", "medium"
-   `bugprone-throw-keyword-missing `_, , "medium"
-   `bugprone-too-small-loop-variable `_, , "medium"
-   `bugprone-undefined-memory-manipulation `_, , "medium"
-   `bugprone-undelegated-constructor `_, , "medium"
-   `bugprone-unhandled-self-assignment `_, , "medium"
-   `bugprone-unused-raii `_, "Yes", "high"
-   `bugprone-unused-return-value `_, , "medium"
-   `bugprone-use-after-move `_, , "high"
-   `bugprone-virtual-near-miss `_, "Yes", "medium"
-   `cert-dcl21-cpp `_, , "low"
-   `cert-dcl50-cpp `_, , "

[clang-tools-extra] 773667b - clang-tidy doc: Remove severities as they don't make consensus

2020-01-01 Thread Sylvestre Ledru via cfe-commits 

Author: Sylvestre Ledru
Date: 2020-01-01T15:42:46+01:00
New Revision: 773667b8c20d35c18334f8c7663df8ceacfdd2e5

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

LOG: clang-tidy doc: Remove severities as they don't make consensus

Reviewers: jdoerfert, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: whisperity, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 7273f2f2c10a..39eca157244f 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -3,405 +3,401 @@
 Clang-Tidy Checks
 =
 
-.. Severities are coming from Codechecker:
-   
https://github.com/Ericsson/codechecker/blob/master/config/checker_severity_map.json
-   If you change it here, please submit a PR on codechecker too
-
 .. csv-table::
-   :header: "Name", "Offers fixes", "Severity"
-   :widths: 50, 20, 10
+   :header: "Name", "Offers fixes"
+   :widths: 50, 20
 
-   `abseil-duration-addition `_, "Yes", ""
-   `abseil-duration-comparison `_, "Yes", ""
-   `abseil-duration-conversion-cast `_, 
"Yes", ""
-   `abseil-duration-division `_, "Yes", ""
-   `abseil-duration-factory-float `_, 
"Yes", ""
-   `abseil-duration-factory-scale `_, 
"Yes", ""
-   `abseil-duration-subtraction `_, "Yes", ""
-   `abseil-duration-unnecessary-conversion 
`_, "Yes", ""
-   `abseil-faster-strsplit-delimiter 
`_, "Yes", ""
-   `abseil-no-internal-dependencies `_, 
, ""
-   `abseil-no-namespace `_, , ""
-   `abseil-redundant-strcat-calls `_, 
"Yes", ""
-   `abseil-str-cat-append `_, "Yes", ""
-   `abseil-string-find-startswith `_, 
"Yes", "style"
-   `abseil-time-comparison `_, "Yes", ""
-   `abseil-time-subtraction `_, "Yes", ""
-   `abseil-upgrade-duration-conversions 
`_, "Yes", ""
-   `android-cloexec-accept `_, "Yes", ""
-   `android-cloexec-accept4 `_, , ""
-   `android-cloexec-creat `_, , "medium"
-   `android-cloexec-dup `_, , ""
-   `android-cloexec-epoll-create `_, , ""
-   `android-cloexec-epoll-create1 `_, , ""
-   `android-cloexec-fopen `_, , "medium"
-   `android-cloexec-inotify-init `_, , ""
-   `android-cloexec-inotify-init1 `_, , ""
-   `android-cloexec-memfd-create `_, , ""
-   `android-cloexec-open `_, , "medium"
-   `android-cloexec-pipe `_, , ""
-   `android-cloexec-pipe2 `_, , ""
-   `android-cloexec-socket `_, , "medium"
-   `android-comparison-in-temp-failure-retry 
`_, "Yes", ""
-   `boost-use-to-string `_, "Yes", "low"
-   `bugprone-argument-comment `_, "Yes", "low"
-   `bugprone-assert-side-effect `_, , 
"medium"
-   `bugprone-bad-signal-to-kill-thread 
`_, , ""
-   `bugprone-bool-pointer-implicit-conversion 
`_, "Yes", "low"
-   `bugprone-branch-clone `_, , "low"
-   `bugprone-copy-constructor-init `_, 
"Yes", "medium"
-   `bugprone-dangling-handle `_, , "high"
-   `bugprone-dynamic-static-initializers 
`_, , ""
-   `bugprone-exception-escape `_, , "medium"
-   `bugprone-fold-init-type `_, , "high"
-   `bugprone-forward-declaration-namespace 
`_, , "low"
-   `bugprone-forwarding-reference-overload 
`_, , "low"
-   `bugprone-inaccurate-erase `_, "Yes", "high"
-   `bugprone-incorrect-roundings `_, , 
"high"
-   `bugprone-infinite-loop `_, , "medium"
-   `bugprone-integer-division `_, , "medium"
-   `bugprone-lambda-function-name `_, , 
"low"
-   `bugprone-macro-parentheses `_, "Yes", 
"medium"
-   `bugprone-macro-repeated-side-effects 
`_, , "medium"
-   `bugprone-misplaced-operator-in-strlen-in-alloc 
`_, , "medium"
-   `bugprone-misplaced-widening-cast 
`_, "Yes", "high"
-   `bugprone-move-forwarding-reference 
`_, "Yes", "medium"
-   `bugprone-multiple-statement-macro 
`_, , "medium"
-   `bugprone-not-null-terminated-result 
`_, "Yes", "medium"
-   `bugprone-parent-virtual-call `_, "Yes", 
"medium"
-   `bugprone-posix-return `_, "Yes", ""
-   `bugprone-sizeof-container `_, , "high"
-   `bugprone-sizeof-expression `_, , "high"
-   `bugprone-string-constructor `_, "Yes", 
"high"
-   `bugprone-string-integer-assignment 
`_, "Yes", "low"
-   `bugprone-string-literal-with-embedded-nul 
`_, , "medium"
-   `bugprone-suspicious-enum-usage `_, , 
"high"
-   `bugprone-suspicious-memset-usage 
`_, "Yes", "high"
-   `bugprone-suspicious-missing-comma 
`_, , "high"
-   `bugprone-suspicious-semicolon `_, 
"Yes", "high"
-   `bugprone-suspicious-string-compare 
`_, "Yes", "medium"
-   `bugprone-swapped-arguments `_, "Yes", 
"high"
-   `bugprone-terminating-continue `_, 
"Yes", "medium"
-   `bugprone-throw-keyword-missing `_, , 
"medium"
-   `bugprone-too-small-loop-variable 
`_, , "medium"
-   `bugprone-un

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 



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

https://reviews.llvm.org/D71966



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

[PATCH] D72052: [UserManual] Update the C++ standard support

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

This brings the state in sync with http://clang.llvm.org/cxx_status.html.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72052

Files:
  clang/docs/UsersManual.rst


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -2555,8 +2555,8 @@
 =
 
 clang fully implements all of standard C++98 except for exported
-templates (which were removed in C++11), and all of standard C++11
-and the current draft standard for C++1y.
+templates (which were removed in C++11), and all of standards C++11, C++14,
+and C++17. It has experimental support for the current draft standard C++2a.
 
 Controlling implementation limits
 -


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -2555,8 +2555,8 @@
 =
 
 clang fully implements all of standard C++98 except for exported
-templates (which were removed in C++11), and all of standard C++11
-and the current draft standard for C++1y.
+templates (which were removed in C++11), and all of standards C++11, C++14,
+and C++17. It has experimental support for the current draft standard C++2a.
 
 Controlling implementation limits
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante updated this revision to Diff 235763.
Mordante retitled this revision from "Fixes -Wrange-loop-analysis warnings" to 
"[NFC] Fixes -Wrange-loop-analysis warnings".
Mordante added a comment.

Reviewed the types and added a `*` for pointers and added a `const` when 
applicable.


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

https://reviews.llvm.org/D71857

Files:
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  llvm/include/llvm/Analysis/LoopInfo.h
  llvm/include/llvm/Analysis/LoopInfoImpl.h
  llvm/include/llvm/Support/GenericDomTree.h
  llvm/lib/Analysis/DomTreeUpdater.cpp
  llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/InterleavedLoadCombinePass.cpp
  llvm/lib/CodeGen/RegAllocFast.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
  llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
  llvm/lib/IR/TypeFinder.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/MC/XCOFFObjectWriter.cpp
  llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
  llvm/lib/MCA/Stages/InstructionTables.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
  llvm/lib/Support/CommandLine.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -132,7 +132,7 @@
 }
 
 AMDGPU::GPUKind llvm::AMDGPU::parseArchAMDGCN(StringRef CPU) {
-  for (const auto C : AMDGCNGPUs) {
+  for (const auto &C : AMDGCNGPUs) {
 if (CPU == C.Name)
   return C.Kind;
   }
@@ -141,7 +141,7 @@
 }
 
 AMDGPU::GPUKind llvm::AMDGPU::parseArchR600(StringRef CPU) {
-  for (const auto C : R600GPUs) {
+  for (const auto &C : R600GPUs) {
 if (CPU == C.Name)
   return C.Kind;
   }
@@ -163,12 +163,12 @@
 
 void AMDGPU::fillValidArchListAMDGCN(SmallVectorImpl &Values) {
   // XXX: Should this only report unique canonical names?
-  for (const auto C : AMDGCNGPUs)
+  for (const auto &C : AMDGCNGPUs)
 Values.push_back(C.Name);
 }
 
 void AMDGPU::fillValidArchListR600(SmallVectorImpl &Values) {
-  for (const auto C : R600GPUs)
+  for (const auto &C : R600GPUs)
 Values.push_back(C.Name);
 }
 
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -187,7 +187,7 @@
 // If we're adding this to all sub-commands, add it to the ones that have
 // already been registered.
 if (SC == &*AllSubCommands) {
-  for (const auto &Sub : RegisteredSubCommands) {
+  for (auto *Sub : RegisteredSubCommands) {
 if (SC == Sub)
   continue;
 addLiteralOption(Opt, Sub, Name);
@@ -243,7 +243,7 @@
 // If we're adding this to all sub-commands, add it to the ones that have
 // already been registered.
 if (SC == &*AllSubCommands) {
-  for (const auto &Sub : RegisteredSubCommands) {
+  for (auto *Sub : RegisteredSubCommands) {
 if (SC == Sub)
   continue;
 addOption(O, Sub);
@@ -318,7 +318,7 @@
   }
 
   bool hasOptions() const {
-for (const auto &S : RegisteredSubCommands) {
+for (const auto *S : RegisteredSubCommands) {
   if (hasOptions(*S))
 return true;
 }
@@ -2112,7 +2112,7 @@
 static void
 sortSubCommands(const SmallPtrSetImpl &SubMap,
 SmallVectorImpl> &Subs) {
-  for (const auto &S : SubMap) {
+  for (auto *S : SubMap) {
 if (S->getName().empty())
   continue;
 Subs.push_back(std::make_pair(S->getName().data(), S));
Index: llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
@@ -421,7 +421,7 @@
   for (const auto &LC : Lines.Blocks) {
 Result->createBlock(LC.FileName);
 if (Result->hasColumnInfo()) {
-  for (const auto &Item : zip(LC.Lines, LC.Columns)) {
+

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D71846#1800381 , @aaron.ballman 
wrote:

> In D71846#1800344 , @njames93 wrote:
>
> > I'm in two minds about issuing a warning when scope restrictions prevent a 
> > fix. Do you think creating an option to enable or disable emitting warnings 
> > for cases where the scope prevents a fix would be a good idea?
>
>
> It's not uncommon for fixits to only be generated under specific 
> circumstances, so I'm wondering what your concern is with warning when we 
> can't provide a fixit? The cases I am thinking about all seem reasonable to 
> diagnose (are true positives) without fixing, but maybe you have different 
> circumstances in mind.


Right now an issue is raised for every else after return flag, but not all else 
after return flags can be fixed due to declaration statements and scope issues. 
My suggestion is that you can choose to warn about those cases or not. For 
example a developer may want else after return for when they need to limit the 
scope and getting a warning for it may be undesirable.


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

https://reviews.llvm.org/D71846



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

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante marked 8 inline comments as done.
Mordante added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp:22
 
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Log.h"

xbolva00 wrote:
> NFC patch?
Changed to NFC.



Comment at: 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp:75
 return false;
-  for (const auto ¶m : call_inst->operand_values())
+  for (auto param : call_inst->operand_values())
 if (isRSAllocationPtrTy(param->getType()))

aaron.ballman wrote:
> `auto *`?
Changed to  `const auto *`


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

https://reviews.llvm.org/D71857



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 235764.
arichardson marked an inline comment as done.
arichardson added a comment.

- Address remaining comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_is_aligned(&MemPtr::vfunc, 64); // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}

[PATCH] D72052: [UserManual] Update the C++ standard support

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61168 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72052



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

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!


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

https://reviews.llvm.org/D71857



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D71846#1800401 , @njames93 wrote:

> In D71846#1800381 , @aaron.ballman 
> wrote:
>
> > In D71846#1800344 , @njames93 
> > wrote:
> >
> > > I'm in two minds about issuing a warning when scope restrictions prevent 
> > > a fix. Do you think creating an option to enable or disable emitting 
> > > warnings for cases where the scope prevents a fix would be a good idea?
> >
> >
> > It's not uncommon for fixits to only be generated under specific 
> > circumstances, so I'm wondering what your concern is with warning when we 
> > can't provide a fixit? The cases I am thinking about all seem reasonable to 
> > diagnose (are true positives) without fixing, but maybe you have different 
> > circumstances in mind.
>
>
> Right now an issue is raised for every else after return flag, but not all 
> else after return flags can be fixed due to declaration statements and scope 
> issues. My suggestion is that you can choose to warn about those cases or 
> not. For example a developer may want else after return for when they need to 
> limit the scope and getting a warning for it may be undesirable.


Okay, I can see the value in having an option for that -- especially given that 
silencing the diagnostic would add `// NOLINT` noise to the code. Do you think 
the option should default to diagnosing all cases, even ones without a fixit?


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

https://reviews.llvm.org/D71846



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61172 tests passed, 0 failed 
and 728 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-01 Thread Tamás Zolnai via Phabricator via cfe-commits 
ztamas updated this revision to Diff 235765.
ztamas marked an inline comment as done.
ztamas added a comment.

Update docs / warning message, according to reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-funsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t
+
+///
+/// Test cases correctly caught by the check.
+
+int SimpleVarDeclaration() {
+  signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CStyleCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = (int)CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int StaticCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = static_cast(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:33: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int FunctionalCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = int(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int NegativeConstValue() {
+  const signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CharPointer(signed char *CCharacter) {
+  int NCharacter = *CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///
+/// Test cases correctly ignored by the check.
+
+int UnsignedCharCast() {
+  unsigned char CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+int PositiveConstValue() {
+  const signed char CCharacter = 5;
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of declaration expression.
+int DescendantCast() {
+  signed char CCharacter = 'a';
+  int NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of assignment expression.
+int DescendantCastAssignment() {
+  signed char CCharacter = 'a';
+  int NCharacter;
+  NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolVarDeclaration() {
+  signed char CCharacter = 'a';
+  bool BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolAssignment() {
+  signed char CCharacter = 'a';
+  bool BCharacter;
+  BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// char is an integer type in clang; make sure to ignore it.
+unsigned char CharToCharCast() {
+  signed char SCCharacter = 'a';
+  unsigned char USCharacter;
+  USCharacter = SCCharacter;
+
+  return USCharacter;
+}
Index: clang-tools-ex

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante marked 2 inline comments as done.
Mordante added a comment.

Thanks for the review! I'll commit all the -Wrange-loop-analysis patches later 
today.


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

https://reviews.llvm.org/D71857



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

[PATCH] D72053: [RFC] Handling implementation limits

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante created this revision.
Mordante added a project: clang.
Herald added a subscriber: mgorny.

This is a proof-of-concept patch to be discussed on the dev ml.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72053

Files:
  clang/docs/ImplementationQuantities.rst
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/ImplementationQuantities.h
  clang/include/clang/Basic/ImplementationQuantities.inc
  clang/include/clang/Basic/ImplementationQuantities.td
  clang/lib/CodeGen/CGRecordLayout.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -81,8 +81,14 @@
  llvm::raw_ostream &OS);
 void EmitClangCommentCommandList(llvm::RecordKeeper &Records,
  llvm::raw_ostream &OS);
+
 void EmitClangOpcodes(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
+void EmitClangImplementationQuantitiesDocs(llvm::RecordKeeper &Records,
+   llvm::raw_ostream &OS);
+void EmitClangImplementationQuantities(llvm::RecordKeeper &Records,
+   llvm::raw_ostream &OS);
+
 void EmitNeon(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitFP16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeonSema(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -60,6 +60,8 @@
   GenClangCommentHTMLNamedCharacterReferences,
   GenClangCommentCommandInfo,
   GenClangCommentCommandList,
+  GenClangImplementationQuantitiesDocs,
+  GenClangImplementationQuantities,
   GenClangOpenCLBuiltins,
   GenArmNeon,
   GenArmFP16,
@@ -172,6 +174,14 @@
 clEnumValN(GenClangCommentCommandList, "gen-clang-comment-command-list",
"Generate list of commands that are used in "
"documentation comments"),
+clEnumValN(GenClangImplementationQuantitiesDocs,
+   "gen-clang-implementation-quantities-docs",
+   "Generate documentation of the implementation quantities "
+   "as defined in the C++ standard"),
+clEnumValN(GenClangImplementationQuantities,
+   "gen-clang-implementation-quantities",
+   "Generate of the implementation quantities "
+   "as defined in the C++ standard"),
 clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
"Generate OpenCL builtin declaration handlers"),
 clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"),
@@ -321,6 +331,12 @@
   case GenClangCommentCommandList:
 EmitClangCommentCommandList(Records, OS);
 break;
+  case GenClangImplementationQuantitiesDocs:
+EmitClangImplementationQuantitiesDocs(Records, OS);
+break;
+  case GenClangImplementationQuantities:
+EmitClangImplementationQuantities(Records, OS);
+break;
   case GenClangOpenCLBuiltins:
 EmitClangOpenCLBuiltins(Records, OS);
 break;
Index: clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
@@ -0,0 +1,105 @@
+//===--- ClangCommentCommandInfoEmitter.cpp - Generate command lists -//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+
+void EmitClangImplementationQuantitiesDocs(RecordKeeper &Records,
+   raw_ostream &OS) {
+  // Get the documentation introduction paragraph.
+  const Record *Documentation = Records.getDef("GlobalDocumentation");
+  if (!Documentation) {
+llvm::PrintFatalError("The Documentation top-level definition is missing, "
+  "no documentation will be generated.");
+return;
+  }
+
+  auto WriteValue = [&OS](Record *Tag) {
+if (Tag->getValueAsBit("isBitLimit")) {
+  int Bits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

I think this generally looks good, thank you for all the hard work on this! I 
just found some minor nits and testing requests. Assuming no surprises with the 
tests, LGTM.




Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:175-176
 const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
-  Inserter = std::make_unique(SM, getLangOpts(),
-   IncludeStyle);
+  Inserter =
+  std::make_unique(SM, getLangOpts(), 
IncludeStyle);
   PP->addPPCallbacks(Inserter->CreatePPCallbacks());

Unrelated change?



Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h:37
+enum class QualifierTarget {
+  Pointee, /// Transforming a pointer goes for the pointee and not the pointer
+   /// itself. For references and normal values this option has no

aaron.ballman wrote:
> goes for -> attaches to?
attaches for -> attaches to



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+

I don't think this is needed -- just add a newline between the literals and let 
string concat work its magic? (Same elsewhere)
```
StringRef S = "typedef int MyInt;"
  "MyInt target = 0;";
```



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:83-84
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),

This does what I would expect, but boy does it make me sad to see (because 
`target` is not actually a `const int *` but is instead an `int * const`).



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:535
+
+TEST(TagTypes, Struct) {
+  StringRef T = "struct Foo { int data; int method(); };\n";

Can you add a test for this scenario:
```
struct S {
  int i;
} x = { 0 };
```
where you want to apply the `const` to the type of `x`?



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006
+}
+
+} // namespace test

Can you also add some ObjC pointer tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-01 Thread Tamás Zolnai via Phabricator via cfe-commits 
ztamas marked 2 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97-98
+  diag(CastExpression->getBeginLoc(),
+   "singed char -> integer (%0) conversion; "
+   "consider to cast to unsigned char first.")
+  << *IntegerType;

aaron.ballman wrote:
> How about: `'signed char' to %0 conversion; consider casting to 'unsigned 
> char' first`?
> 
> Also, should we have a fix-it to automatically insert the cast to `unsigned 
> char` in the proper location for the user?
I don't think it's a good idea to add a cast automatically. I think this kind 
of issue typically needs a human programmer to check what can be done in the 
code.
For example, when the program uses a char variable, but it is used as an int 
intentionally (without adding an int alias). It's not a good practice of course 
but it can happen. In this case, the best would be to use an int variable 
instead. Adding a cast might break the code if it's called with negative values.
It also can happen that the code works with actual characters, but handles the 
negative values on its own. In this case, it might be not enough to add an 
unsigned char cast, but it might be needed to adjust the surrounding code too.
All in all, based on my experience with the findings in the LibreOffice code 
base, I would not use an automatic correction here.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:25
+See also:
+`STR34-C. Cast characters to unsigned char before converting to larger integer 
sizes
+`_

aaron.ballman wrote:
> ztamas wrote:
> > aaron.ballman wrote:
> > > Should this check also be registered in the CERT module?
> > This check does not cover the whole CERT description. I guess a cert check 
> > should catch all bugous code related to the CERT issue, right?
> So long as this covers a decent amount of the CERT rule, I think it is fine 
> to document the areas where we don't quite match the CERT rule.
My plan is to extend this check with other use cases in the next months. Can we 
get back to this question after that? Now, It's hard to tell how well it covers 
the CERT rule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97-98
+  diag(CastExpression->getBeginLoc(),
+   "singed char -> integer (%0) conversion; "
+   "consider to cast to unsigned char first.")
+  << *IntegerType;

ztamas wrote:
> aaron.ballman wrote:
> > How about: `'signed char' to %0 conversion; consider casting to 'unsigned 
> > char' first`?
> > 
> > Also, should we have a fix-it to automatically insert the cast to `unsigned 
> > char` in the proper location for the user?
> I don't think it's a good idea to add a cast automatically. I think this kind 
> of issue typically needs a human programmer to check what can be done in the 
> code.
> For example, when the program uses a char variable, but it is used as an int 
> intentionally (without adding an int alias). It's not a good practice of 
> course but it can happen. In this case, the best would be to use an int 
> variable instead. Adding a cast might break the code if it's called with 
> negative values.
> It also can happen that the code works with actual characters, but handles 
> the negative values on its own. In this case, it might be not enough to add 
> an unsigned char cast, but it might be needed to adjust the surrounding code 
> too.
> All in all, based on my experience with the findings in the LibreOffice code 
> base, I would not use an automatic correction here.
Okay, that makes sense, thank you!



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:25
+See also:
+`STR34-C. Cast characters to unsigned char before converting to larger integer 
sizes
+`_

ztamas wrote:
> aaron.ballman wrote:
> > ztamas wrote:
> > > aaron.ballman wrote:
> > > > Should this check also be registered in the CERT module?
> > > This check does not cover the whole CERT description. I guess a cert 
> > > check should catch all bugous code related to the CERT issue, right?
> > So long as this covers a decent amount of the CERT rule, I think it is fine 
> > to document the areas where we don't quite match the CERT rule.
> My plan is to extend this check with other use cases in the next months. Can 
> we get back to this question after that? Now, It's hard to tell how well it 
> covers the CERT rule.
I checked and I think it covers the rule pretty closely. However, if you're 
already planning changes to cover new use cases, I'm fine waiting to add the 
alias.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:73
`bugprone-posix-return `_, "Yes", ""
+   `bugprone-signed-char-misuse `_, , 
"medium"
`bugprone-sizeof-container `_, , "high"

Just an FYI, but the severity columns were recently removed, so you'll probably 
have to rebase your patch.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp:6
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') 
conversion; consider to cast to unsigned char first. 
[bugprone-signed-char-misuse]
+

I think you need to update all the diagnostic text in the tests as the 
diagnostic has changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D72053: [RFC] Handling implementation limits

2020-01-01 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61168 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72053



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D71846#1800411 , @aaron.ballman 
wrote:

> In D71846#1800401 , @njames93 wrote:
>
> > In D71846#1800381 , @aaron.ballman 
> > wrote:
> >
> > > In D71846#1800344 , @njames93 
> > > wrote:
> > >
> > > > I'm in two minds about issuing a warning when scope restrictions 
> > > > prevent a fix. Do you think creating an option to enable or disable 
> > > > emitting warnings for cases where the scope prevents a fix would be a 
> > > > good idea?
> > >
> > >
> > > It's not uncommon for fixits to only be generated under specific 
> > > circumstances, so I'm wondering what your concern is with warning when we 
> > > can't provide a fixit? The cases I am thinking about all seem reasonable 
> > > to diagnose (are true positives) without fixing, but maybe you have 
> > > different circumstances in mind.
> >
> >
> > Right now an issue is raised for every else after return flag, but not all 
> > else after return flags can be fixed due to declaration statements and 
> > scope issues. My suggestion is that you can choose to warn about those 
> > cases or not. For example a developer may want else after return for when 
> > they need to limit the scope and getting a warning for it may be 
> > undesirable.
>
>
> Okay, I can see the value in having an option for that -- especially given 
> that silencing the diagnostic would add `// NOLINT` noise to the code. Do you 
> think the option should default to diagnosing all cases, even ones without a 
> fixit?


Definitely default to diagnosing everything, that's how my current 
implementation looks right now. Also is there a way to do options as bool 
because I don't particularly like putting 1 for true in the config file


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

https://reviews.llvm.org/D71846



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D71846#1800480 , @njames93 wrote:

> In D71846#1800411 , @aaron.ballman 
> wrote:
>
> > In D71846#1800401 , @njames93 
> > wrote:
> >
> > > In D71846#1800381 , 
> > > @aaron.ballman wrote:
> > >
> > > > In D71846#1800344 , @njames93 
> > > > wrote:
> > > >
> > > > > I'm in two minds about issuing a warning when scope restrictions 
> > > > > prevent a fix. Do you think creating an option to enable or disable 
> > > > > emitting warnings for cases where the scope prevents a fix would be a 
> > > > > good idea?
> > > >
> > > >
> > > > It's not uncommon for fixits to only be generated under specific 
> > > > circumstances, so I'm wondering what your concern is with warning when 
> > > > we can't provide a fixit? The cases I am thinking about all seem 
> > > > reasonable to diagnose (are true positives) without fixing, but maybe 
> > > > you have different circumstances in mind.
> > >
> > >
> > > Right now an issue is raised for every else after return flag, but not 
> > > all else after return flags can be fixed due to declaration statements 
> > > and scope issues. My suggestion is that you can choose to warn about 
> > > those cases or not. For example a developer may want else after return 
> > > for when they need to limit the scope and getting a warning for it may be 
> > > undesirable.
> >
> >
> > Okay, I can see the value in having an option for that -- especially given 
> > that silencing the diagnostic would add `// NOLINT` noise to the code. Do 
> > you think the option should default to diagnosing all cases, even ones 
> > without a fixit?
>
>
> Definitely default to diagnosing everything, that's how my current 
> implementation looks right now.


SGTM

> Also is there a way to do options as bool because I don't particularly like 
> putting 1 for true in the config file

Not to my knowledge.


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

https://reviews.llvm.org/D71846



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D71846#1800481 , @aaron.ballman 
wrote:

> In D71846#1800480 , @njames93 wrote:
>
> > Definitely default to diagnosing everything, that's how my current 
> > implementation looks right now.
>
>
> SGTM
>
> > Also is there a way to do options as bool because I don't particularly like 
> > putting 1 for true in the config file
>
> Not to my knowledge.


I just did a quick grep and found that all checks that take bools for config 
options use a 1 for true, 0 for false, so I guess best stick to convention 
rather than hack at OptionsView to get and set bools


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

https://reviews.llvm.org/D71846



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 235775.
njames93 added a comment.

Added option to disable warning when an automatic fix can't be applied due to 
scope restrictions of variables, default option is to show all warnings


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

https://reviews.llvm.org/D71846

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-warn.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1071,6 +1071,35 @@
  LanguageMode::Cxx17OrLater));
 }
 
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+  EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+  ifStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+  EXPECT_TRUE(matches(
+  "void baz(int i) { switch (int j = i; j) { default: break; } }",
+  switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+  EXPECT_TRUE(matches("void baz() {"
+  "int items[] = {};"
+  "for (auto &arr = items; auto &item : arr) {}"
+  "}",
+  cxxForRangeStmt(hasInitStatement(anything())),
+  LanguageMode::Cxx2aOrLater));
+  EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto &item : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
 matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
+  REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+///  void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+///  }
+///  void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+///  }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+///   matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+///   matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+///   matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+  CXXForRangeStmt),
+  internal::Matcher, InnerMatcher) {
+  const Stmt *Init = Node.getInit();
+  return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder);
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
 
 
 
+MatcherExpr>nullPointerConstant
+Matches expressions that r

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2020-01-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.

The new option LGTM but one of the tests can be updated to have a less complex 
RUN line.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:2-3
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-else-after-return.WarnOnUnfixable, value: 
1}, \
+// RUN: ]}' -- -fexceptions -std=c++17

You don't need to explicitly set it to `1` here.


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

https://reviews.llvm.org/D71846



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

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

2020-01-01 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay created this revision.
Herald added subscribers: llvm-commits, cfe-commits, luismarques, apazos, 
sameer.abuasal, pzheng, s.egerton, lenary, Jim, benna, psnobl, jocewei, PkmX, 
rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, 
jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, 
asb, hiraditya.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72056

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/RISCV/pic-models.ll

Index: llvm/test/CodeGen/RISCV/pic-models.ll
===
--- llvm/test/CodeGen/RISCV/pic-models.ll
+++ llvm/test/CodeGen/RISCV/pic-models.ll
@@ -8,6 +8,10 @@
 ; RUN: llc -mtriple=riscv64 -relocation-model=pic < %s \
 ; RUN: | FileCheck -check-prefix=RV64-PIC %s
 
+;; Use PIC address sequence for medium code model, even if position dependent.
+; RUN: llc -mtriple=riscv64 -relocation-model=static -code-model=medium < %s \
+; RUN: | FileCheck -check-prefix=RV64-PIC %s
+
 ; Check basic lowering of PIC addressing.
 ; TODO: Check other relocation models?
 
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -189,6 +189,9 @@
 // PowerPC prefers avoiding copy relocations.
 if (Arch == Triple::ppc || TT.isPPC64())
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)
+  return false;
 
 // Check if we can use copy relocations.
 if (!(GV && GV->isThreadLocal()) && RM == Reloc::Static)
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -429,25 +429,11 @@
  bool IsLocal) const {
   SDLoc DL(N);
   EVT Ty = getPointerTy(DAG.getDataLayout());
-
-  if (isPositionIndependent()) {
-SDValue Addr = getTargetNode(N, DL, Ty, DAG, 0);
-if (IsLocal)
-  // Use PC-relative addressing to access the symbol. This generates the
-  // pattern (PseudoLLA sym), which expands to (addi (auipc %pcrel_hi(sym))
-  // %pcrel_lo(auipc)).
-  return SDValue(DAG.getMachineNode(RISCV::PseudoLLA, DL, Ty, Addr), 0);
-
-// Use PC-relative addressing to access the GOT for this symbol, then load
-// the address from the GOT. This generates the pattern (PseudoLA sym),
-// which expands to (ld (addi (auipc %got_pcrel_hi(sym)) %pcrel_lo(auipc))).
-return SDValue(DAG.getMachineNode(RISCV::PseudoLA, DL, Ty, Addr), 0);
-  }
-
-  switch (getTargetMachine().getCodeModel()) {
-  default:
+  CodeModel::Model M = getTargetMachine().getCodeModel();
+  if (M != CodeModel::Small && M != CodeModel::Medium)
 report_fatal_error("Unsupported code model for lowering");
-  case CodeModel::Small: {
+
+  if (!isPositionIndependent() && M == CodeModel::Small) {
 // Generate a sequence for accessing addresses within the first 2 GiB of
 // address space. This generates the pattern (addi (lui %hi(sym)) %lo(sym)).
 SDValue AddrHi = getTargetNode(N, DL, Ty, DAG, RISCVII::MO_HI);
@@ -455,14 +441,18 @@
 SDValue MNHi = SDValue(DAG.getMachineNode(RISCV::LUI, DL, Ty, AddrHi), 0);
 return SDValue(DAG.getMachineNode(RISCV::ADDI, DL, Ty, MNHi, AddrLo), 0);
   }
-  case CodeModel::Medium: {
-// Generate a sequence for accessing addresses within any 2GiB range within
-// the address space. This generates the pattern (PseudoLLA sym), which
-// expands to (addi (auipc %pcrel_hi(sym)) %pcrel_lo(auipc)).
-SDValue Addr = getTargetNode(N, DL, Ty, DAG, 0);
+
+  SDValue Addr = getTargetNode(N, DL, Ty, DAG, 0);
+  if (IsLocal)
+// Use PC-relative addressing to access the symbol. This generates the
+// pattern (PseudoLLA sym), which expands to (addi (auipc %pcrel_hi(sym))
+// %pcrel_lo(auipc)).
 return SDValue(DAG.getMachineNode(RISCV::PseudoLLA, DL, Ty, Addr), 0);
-  }
-  }
+
+  // Use PC-relative addressing to access the GOT for this symbol, then load
+  // the address from the GOT. This generates the pattern (PseudoLA sym),
+  // which expands to (ld (addi (auipc %got_pcrel_hi(sym)) %pcrel_lo(auipc))).
+  return SDValue(DAG.getMachineNode(RISCV::PseudoLA, DL, Ty, Addr), 0);
 }
 
 SDValue RISCVTargetLowering::lowerGlobalAddress(SDValue Op,
Index: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
===
--- llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
+++ llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
@@ -676,7 +676,8 @@
 
   unsigned SecondOpcode;
   unsigne

[PATCH] D72057: [docs] Update dead anchor in hacking page

2020-01-01 Thread Alexander Lanin via Phabricator via cfe-commits 
AlexanderLanin created this revision.
AlexanderLanin added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72057

Files:
  clang/www/hacking.html


Index: clang/www/hacking.html
===
--- clang/www/hacking.html
+++ clang/www/hacking.html
@@ -298,7 +298,7 @@
   Note that the paths embedded in the patch depend on where you run it,
   so changing directory to the llvm/tools/clang directory is recommended.
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 
   
   LLVM IR Generation


Index: clang/www/hacking.html
===
--- clang/www/hacking.html
+++ clang/www/hacking.html
@@ -298,7 +298,7 @@
   Note that the paths embedded in the patch depend on where you run it,
   so changing directory to the llvm/tools/clang directory is recommended.
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git to contribute to Clang.
 
   
   LLVM IR Generation
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71982: [docs] Update path to clang-tools-extra

2020-01-01 Thread Alexander Lanin via Phabricator via cfe-commits 
AlexanderLanin marked 3 inline comments as done.
AlexanderLanin added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:66
 
-Once you are done, change to the ``llvm/tools/clang/tools/extra`` directory, 
and
+Once you are done, change to the ``llvm/clang-tools-extra`` directory, and
 let's start!

aaron.ballman wrote:
> Jim wrote:
> > I am no sure that "llvm/clang-tools-extra" should be replaced as 
> > "llvm-project/clang-tools-extra".
> > Maybe someone would confuse with "llvm", "llvm-project" and 
> > "llvm-project/llvm"
> Elsewhere we use `path/to/llvm/source`, which seems to be sufficiently clear.
While this goes slightly beyond the scope of the original pull request I tend 
to agree as `llvm` can easily be confused with `llvm-project/llvm` as Jim wrote.
However I'm not clear on the exact target: looking through other docs probably 
most often you'll find `path/to/llvm/source` as Aaron mentioned, but other 
times it's `/path/to/llvm-project/`, `llvm-project/`, `~/llvm/`, 
`~/clang-llvm/`, `/path/to/llvm`, `/path/to/llvm/src`, `/path/to/llvm/sources` 
or `/path/to/llvm/tree`.

While this is not that important, it's difficult enough to get started with 
anything inside llvm as it is. This is low hanging fruit. I would create a 
separate pull request afterwards to align those.



Comment at: clang/www/hacking.html:301
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 

Jim wrote:
> This change should be in a separate patch.
Ok, separate patch: https://reviews.llvm.org/D72057


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

https://reviews.llvm.org/D71982



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

[clang] 8dc7b98 - [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-01 Thread Mark de Wever via cfe-commits 

Author: Mark de Wever
Date: 2020-01-01T20:01:37+01:00
New Revision: 8dc7b982b4556c243e0502e6e230bdd53ddd65ff

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

LOG: [NFC] Fixes -Wrange-loop-analysis warnings

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang-tools-extra/clang-doc/MDGenerator.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clangd/index/MemIndex.cpp
clang/lib/CodeGen/CodeGenPGO.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
clang/lib/Tooling/ASTDiff/ASTDiff.cpp
clang/tools/clang-refactor/TestSupport.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
lldb/source/Plugins/Platform/Android/AdbClient.cpp
lldb/source/Target/StackFrameRecognizer.cpp
llvm/include/llvm/Analysis/LoopInfo.h
llvm/include/llvm/Analysis/LoopInfoImpl.h
llvm/include/llvm/Support/GenericDomTree.h
llvm/lib/Analysis/DomTreeUpdater.cpp
llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/CodeGen/InlineSpiller.cpp
llvm/lib/CodeGen/InterleavedLoadCombinePass.cpp
llvm/lib/CodeGen/RegAllocFast.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
llvm/lib/IR/TypeFinder.cpp
llvm/lib/Linker/IRMover.cpp
llvm/lib/MC/XCOFFObjectWriter.cpp
llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
llvm/lib/MCA/Stages/InstructionTables.cpp
llvm/lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
llvm/lib/Support/CommandLine.cpp
llvm/lib/Support/TargetParser.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-doc/MDGenerator.cpp 
b/clang-tools-extra/clang-doc/MDGenerator.cpp
index 73fb3d4dc27e..ff99c9001349 100644
--- a/clang-tools-extra/clang-doc/MDGenerator.cpp
+++ b/clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -215,7 +215,7 @@ static void genMarkdown(const RecordInfo &I, 
llvm::raw_ostream &OS) {
 
   if (!I.Members.empty()) {
 writeHeader("Members", 2, OS);
-for (const auto Member : I.Members) {
+for (const auto &Member : I.Members) {
   std::string Access = getAccess(Member.Access);
   if (Access != "")
 writeLine(Access + " " + Member.Type.Name + " " + Member.Name, OS);

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
index 8ad475053384..62c4c768e840 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -75,7 +75,7 @@ void SlicingCheck::DiagnoseSlicedOverriddenMethods(
 const CXXRecordDecl &BaseDecl) {
   if (DerivedDecl.getCanonicalDecl() == BaseDecl.getCanonicalDecl())
 return;
-  for (const auto &Method : DerivedDecl.methods()) {
+  for (const auto *Method : DerivedDecl.methods()) {
 // Virtual destructors are OK. We're ignoring constructors since they are
 // tagged as overrides.
 if (isa(Method) || isa(Method))

diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index e046023106bf..bd736743ae1c 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -792,7 +792,7 @@ void IdentifierNamingCheck::check(const 
MatchFinder::MatchResult &Result) {
   }
 
   if (const auto *Decl = Result.Nodes.getNodeAs("using")) {
-for (const auto &Shadow : Decl->shadows()) {
+for (const auto *Shadow : Decl->shadows()) {
   addUsage(NamingCheckFailures, Shadow->getTargetDecl(),
Decl->getNameInfo().getSourceRange());
 }

diff  --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp 
b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 628b8506811e..53f9336b6fc7 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -23,7 +23,7 @@ using llvm::SmallPtrSet;
 namespace {
 
 template  bool isSetDifferenceEmpty(const S &S1, const S &S2) {
-  for (cons

[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

2020-01-01 Thread James Clarke via Phabricator via cfe-commits 
jrtc27 added a comment.

I am still of the view that we should support rewriting the instruction stream 
in the linker when necessary like BFD does. We need to do this to be able to 
link in GCC-compiled code, including libraries. It is a very simple thing to do 
with a patch available that provides consistency between the GNU world and the 
LLVM world.




Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:680
+  if (MF->getTarget().isPositionIndependent() ||
+  MF->getTarget().getCodeModel() != CodeModel::Small) {
 const auto &STI = MF->getSubtarget();

This is confusing and differs from when you invoke the assembler manually, even 
via the Clang driver with the right code model specified. Any `la`s there will 
be assembled as `AUIPC`/`ADDI`. I am of the view it was a mistake to make 
`la`'s behaviour conditional on PICness and it should have always used the GOT, 
but this is what we have.



Comment at: llvm/lib/Target/TargetMachine.cpp:192
   return false;
+// RISC-V non-small code models prefer avoiding copy relocations.
+if (TT.isRISCV() && getCodeModel() != CodeModel::Small)

Are we sure we want to do this and take the performance hit over GCC due to the 
extra level of indirection on every single extern global access? If this is the 
solution to take for extern weak, I think we should limit it to just that and 
have a separate discussion about non-extern-weak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056



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

[PATCH] D71806: Improve Wrange-loop-analyses for rvalue reference

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5ab1e49f958: Improve Wrange-loop-analyses for rvalue 
reference (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71806

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -84,34 +84,46 @@
 void test1() {
   Container A;
 
+  for (const int &&x : A) {}
+  // No warning, rvalue-reference to the temporary
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
+  for (int&& x : A) {}
+  // No warning, rvalue-reference to the temporary
   //for (int &x : A) {}
   // Binding error
   for (int x : A) {}
   // No warning, non-reference type indicates copy is made
 
+  for (const double &&x : A) {}
+  // No warning, rvalue-reference to the temporary
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
+  for (double &&x : A) {}
+  // No warning, rvalue-reference to the temporary
   //for (double &x : A) {}
   // Binding error
   for (double x : A) {}
   // No warning, non-reference type indicates copy is made
 
+  for (const Bar &&x : A) {}
+  // No warning, rvalue-reference to the temporary
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
+  for (Bar &&x : A) {}
+  // No warning, rvalue-reference to the temporary
   //for (Bar &x : A) {}
   // Binding error
   for (Bar x : A) {}
@@ -121,30 +133,50 @@
 void test2() {
   Container B;
 
+  //for (const int &&x : B) {}
+  // Binding error
   for (const int &x : B) {}
   // No warning, this reference is not a temporary
   for (const int x : B) {}
   // No warning on POD copy
+  //for (int &x : B) {}
+  // Binding error
   for (int &x : B) {}
   // No warning
   for (int x : B) {}
   // No warning
 
+  for (const double &&x : B) {}
+  // expected-warning@-1 {{resulting in a copy}}
+  // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:23}:""
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
+  for (double &&x : B) {}
+  // expected-warning@-1 {{resulting in a copy}}
+  // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:17}:""
   //for (double &x : B) {}
   // Binding error
   for (double x : B) {}
   // No warning
 
+  for (const Bar &&x : B) {}
+  // expected-warning@-1 {{resulting in a copy}}
+  // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
+  for (Bar &&x : B) {}
+  // expected-warning@-1 {{resulting in a copy}}
+  // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
   //for (Bar &x : B) {}
   // Binding error
   for (Bar x : B) {}
@@ -154,23 +186,31 @@
 void test3() {
   Container C;
 
+  for (const Bar &&x : C) {}
+  // No warning, rvalue-reference to the temporary
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
+  for (Bar &&x : C) {}
+  // No warning, rvalue-reference to the temporary
   //for (Bar &x : C) {}
   // Binding error
   for (Bar x : C) {}
   // No warning, non-reference type indicates copy is made
 
+  for (const int &&x : C) {}
+  // No warning, rvalue-reference to the temporary
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
+  for (int &&x : C) {}
+  // No warning, rvalue-reference to the temporary
   //for