[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

In D69435#1725795 , @AntonBikineev 
wrote:

> Does anybody have suggestions on this?


Can you be more specific what the question is?




Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:42
+
+  Finder->addMatcher(
+  cxxDestructorDecl(

Thanks, this looks good.


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

https://reviews.llvm.org/D69435



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 227216.
yaxunl retitled this revision from "[HIP] Disable emitting llvm.linker.options 
in device compilation" to "[CUDA][HIP] Disable emitting llvm.linker.options in 
device compilation".
yaxunl edited the summary of this revision.
yaxunl added a comment.

use -fno-autolink to disable linker option for device compilation.


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

https://reviews.llvm.org/D57829

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/linker-options.cu
  clang/test/Driver/hip-autolink.hip


Index: clang/test/Driver/hip-autolink.hip
===
--- /dev/null
+++ clang/test/Driver/hip-autolink.hip
@@ -0,0 +1,14 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-device-only -x hip %s -### 2>&1 | FileCheck 
--check-prefix=DEV %s
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-host-only -x hip %s -### 2>&1 | FileCheck --check-prefix=HOST 
%s
+
+// DEV: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEV-SAME: "-fno-autolink"
+
+// HOST: "-cc1" "-triple" "i386-pc-windows-msvc{{.*}}"
+// HOST-NOT: "-fno-autolink"
Index: clang/test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x hip %s \
+// RUN:   -fno-autolink | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions -x hip %s \
+// RUN:   | FileCheck -check-prefix=HOST %s
+
+// DEV-NOT: llvm.linker.options
+// DEV-NOT: llvm.dependent-libraries
+// HOST: llvm.linker.options
+// HOST: llvm.dependent-libraries
+#pragma comment(lib, "libcpmt")
+#pragma detect_mismatch("myLib_version", "9")
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -378,13 +378,18 @@
 CmdArgs.push_back("-fexceptions");
 }
 
-static bool ShouldDisableAutolink(const ArgList &Args, const ToolChain &TC) {
+static bool ShouldDisableAutolink(const ArgList &Args, const ToolChain &TC,
+  const JobAction &JA) {
   bool Default = true;
   if (TC.getTriple().isOSDarwin()) {
 // The native darwin assembler doesn't support the linker_option 
directives,
 // so we disable them if we think the .s file will be passed to it.
 Default = TC.useIntegratedAs();
   }
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;
   return !Args.hasFlag(options::OPT_fautolink, options::OPT_fno_autolink,
Default);
 }
@@ -4362,7 +4367,7 @@
   if (ShouldDisableDwarfDirectory(Args, TC))
 CmdArgs.push_back("-fno-dwarf-directory-asm");
 
-  if (ShouldDisableAutolink(Args, TC))
+  if (ShouldDisableAutolink(Args, TC, JA))
 CmdArgs.push_back("-fno-autolink");
 
   // Add in -fdebug-compilation-dir if necessary.


Index: clang/test/Driver/hip-autolink.hip
===
--- /dev/null
+++ clang/test/Driver/hip-autolink.hip
@@ -0,0 +1,14 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-device-only -x hip %s -### 2>&1 | FileCheck --check-prefix=DEV %s
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-host-only -x hip %s -### 2>&1 | FileCheck --check-prefix=HOST %s
+
+// DEV: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEV-SAME: "-fno-autolink"
+
+// HOST: "-cc1" "-triple" "i386-pc-windows-msvc{{.*}}"
+// HOST-NOT: "-fno-autolink"
Index: clang/test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x hip %s \
+// RUN:   -fno-autolink | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions -x hip %s \
+// RUN:   | FileCheck -check-prefix=HOST %s
+
+// DEV-NOT: llvm.linker.options
+// DEV-NOT: llvm.dependent-libraries
+// HOST: llvm.linker.options
+// HOST: llvm.dependent-libraries
+#pragma comment(lib, "libcpmt")
+#pragma detect_mismatch("myLib_version", "9")
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl reclaimed this revision.
yaxunl added a comment.

we still need this


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

https://reviews.llvm.org/D57829



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Michael Liao via Phabricator via cfe-commits]

hliao added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:395-400
 /// The number used to indicate this lambda expression for name
 /// mangling in the Itanium C++ ABI.
 unsigned ManglingNumber : 31;
 
+/// The device side mangling number.
+unsigned DeviceManglingNumber = 0;

rnk wrote:
> It seems a shame to grow LambdaDefinitionData by a pointer for all users of 
> C++ that do not use CUDA. Optimizing bitfields may be worth the time, but 
> I'll leave it to @rjmccall or @rsmith to give guidance on whether that's 
> worth it.
> 
> An alternative would be to store the device numbers in the mangling context 
> and look them up when needed, since they are so rarely needed.
I like the alternative way by storing all numbering into the mangle/numbering 
context instead of AST itself. But, it needs additional numbering 
post-processing after AST importing. Sound to me a major refactoring work 
likely to be addressed later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 227214.
hliao marked an inline comment as done.
hliao added a comment.

simplify again following suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/Mangle.h
  clang/include/clang/AST/MangleNumberingContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/CXXABI.h
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- clang/test/CodeGenCUDA/unnamed-types.cu
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -1,12 +1,17 @@
 // RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -aux-triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST
+// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-pc-windows-msvc -aux-triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s --check-prefix=MSVC
 // RUN: %clang_cc1 -std=c++11 -x hip -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm %s -o - | FileCheck %s --check-prefix=DEVICE
 
 #include "Inputs/cuda.h"
 
 // HOST: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1
+// HOST: @1 = private unnamed_addr constant [60 x i8] c"_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_\00", align 1
+// Check that, on MSVC, the same device kernel mangling name is generated.
+// MSVC: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1
+// MSVC: @1 = private unnamed_addr constant [60 x i8] c"_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_\00", align 1
 
 __device__ float d0(float x) {
-  return [](float x) { return x + 2.f; }(x);
+  return [](float x) { return x + 1.f; }(x);
 }
 
 __device__ float d1(float x) {
@@ -14,11 +19,21 @@
 }
 
 // DEVICE: amdgpu_kernel void @_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_(
+// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf(
 template 
 __global__ void k0(float *p, F f) {
   p[0] = f(p[0]) + d0(p[1]) + d1(p[2]);
 }
 
+// DEVICE: amdgpu_kernel void @_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_(
+// DEVICE: define internal float @_ZZ2f1PfENKUlfE_clEf(
+// DEVICE: define internal float @_ZZ2f1PfENKUlffE_clEff(
+// DEVICE: define internal float @_ZZ2f1PfENKUlfE0_clEf(
+template 
+__global__ void k1(float *p, F0 f0, F1 f1, F2 f2) {
+  p[0] = f0(p[0]) + f1(p[1], p[2]) + f2(p[3]);
+}
+
 void f0(float *p) {
   [](float *p) {
 *p = 1.f;
@@ -29,11 +44,17 @@
 // linkages are still required to keep the original `internal` linkage.
 
 // HOST: define internal void @_ZZ2f1PfENKUlS_E_clES_(
-// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf(
 void f1(float *p) {
   [](float *p) {
-k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; });
+k0<<<1,1>>>(p, [] __device__ (float x) { return x + 3.f; });
   }(p);
+  k1<<<1,1>>>(p,
+  [] __device__ (float x) { return x + 4.f; },
+  [] __device__ (float x, float y) { return x * y; },
+  [] __device__ (float x) { return x + 5.f; });
 }
 // HOST: @__hip_register_globals
 // HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
+// HOST: __hipRegisterFunction{{.*}}@_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_{{.*}}@1
+// MSVC: __hipRegisterFunction{{.*}}@"??$k0@V@?0???R1?0??f1@@YAXPEAM@Z@QEBA@0@Z@@@YAXPEAMV@?0???R0?0??f1@@YAX0@Z@QEBA@0@Z@@Z{{.*}}@0
+// MSVC: __hipRegisterFunction{{.*}}@"??$k1@V@?0??f1@@YAXPEAM@Z@V@?0??2@YAX0@Z@V@?0??2@YAX0@Z@@@YAXPEAMV@?0??f1@@YAX0@Z@V@?0??1@YAX0@Z@V@?0??1@YAX0@Z@@Z{{.*}}@1
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6226,6 +6226,7 @@
 Record->push_back(Lambda.NumExplicitCaptures);
 Record->push_back(Lambda.HasKnownInternalLinkage);
 Record->push_back(Lambda.ManglingNumber);
+Record->push_back(Lambda.DeviceManglingNumber);
 AddDeclRef(D->getLambdaContextDecl());
 AddTypeSourceInfo(Lambda.MethodTyInfo);
 for (unsigned I = 0, N = Lambda.NumCaptures; I != N; ++I) {
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1692,6 +1692,7 @@
 Lambda.NumExplicitCaptures = Record.readInt();
 Lambda.HasKnownInternalLinkage = Record.r

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Eric Astor via Phabricator via cfe-commits]

epastor added a comment.

In D69626#1728113 , @thakis wrote:

> Is this needed to parse system headers?


Interesting question. This change isn't - but I presume the (pre-existing) 
introduction of IgnoredComma was.

> In general, we try to emulate only warts we must emulate, and if feasible we 
> try to emit some -Wmicrosoft warning when the compat path is taken.

Sure - this change actually brings the preprocessor closer to compliance. I 
think the prior use of IgnoredComma had overshot MSVC by a bit. For an 
illustration, see
https://gcc.godbolt.org/z/XakAYf (same code as above, but in MSVC, clang, and 
clang -fms-compatibility)
MSVC shows it's non-compliant on TEST, and -fms-compatibility matches it there. 
However, MSVC matches clang's behavior for WRAPPED_TEST, as adding another 
layer of macro expansion is a standard trick for forcing MSVC's preprocessor to 
act compliant... but clang -fms-compatibility still gives the non-compliant 
result, since that trick doesn't work to remove IgnoredCommas.

If we wanted to detect use of the compatibility feature, we might need to apply 
the preprocessor both ways and see if the results were different; I'm not sure 
all IgnoredCommas will actually make a difference in the parsing. As is, we'd 
need to emit a -Wmicrosoft warning any time we saw an isolated comma as a macro 
parameter... which I think might get confusing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69626



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

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:333
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in constexpr context">,
+  InGroup>;

"constexpr context" doesn't mean anything, and I'm not sure it has the right 
implications (eg, is the body of a constexpr function a constexpr context?).

The technically-correct thing would be "in a manifestly constant-evaluated 
expression".





Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:334
+  "'%0' will always evaluate to 'true' in constexpr context">,
+  InGroup>;
+

Looks like you decided not to make this a subgroup of tautological-compare? 
(I'm OK with that, I just wanted to make sure this was intentional given the 
prior discussion.)



Comment at: clang/lib/AST/ExprConstant.cpp:10595
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||

xbolva00 wrote:
> !Info.CheckingPotentialConstantExpression
> 
> Basically just to silence test case:
> namespace std {
> constexpr bool is_constant_evaluated() noexcept {
>   return __builtin_is_constant_evaluated();
> }
> }
> 
> In real world code, this will not warn, since call's loc is in system header. 
> Leave it?
The check seems right to me.



Comment at: clang/lib/AST/ExprConstant.cpp:10597
+(Info.CallStackDepth == 1 ||
+ (Info.CallStackDepth == 2 && E->getNumArgs() == 0 &&
+  Callee->isInStdNamespace() && Callee->getIdentifier() &&

The number of arguments check is redundant; we know the builtin has no 
parameters.



Comment at: clang/lib/AST/ExprConstant.cpp:10600
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()

xbolva00 wrote:
> Without this condition, it warns 3 times for
> 
> if constexpr (std::is_constant_evaluated() == false) 
Please add a "// FIXME: Find a better way to avoid duplicated diagnostics." or 
similar. This is relying somewhat on the implementation details of callers of 
the constant evaluator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518



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

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Is this needed to parse system headers?

In general, we try to emulate only warts we must emulate, and if feasible we 
try to emit some -Wmicrosoft warning when the compat path is taken.

(MSVC added /experimental:preprocessor like 2 years ago, but it hasn't made it 
to a /Zc: flag yet so I suppose standard headers might keep needing these 
tweaks.)

(See 
https://docs.google.com/presentation/d/1oxNHaVjA9Gn_rTzX6HIpJHP7nXRua_0URXxxJ3oYRq0/edit#slide=id.g71ecd450e_2_843
 slide 23-27)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69626



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

Let's make it 3! Thank you so much for sticking by, I guess one of the reasons 
why a patch so obviously great and desired took so long is that we still didn't 
figure out how we imagine the `CallDescriptionMap` to be integrated into larger 
checkers :) In any case, this is a major step in the right direction, 
`MallocChecker` and some of the others should take notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM, Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69643



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

[PATCH] D69414: [clang-tidy] Regenerate clang-tidy check list 📋

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9baf4958cda4: [clang-tidy] Regenerate clang-tidy check list 
📋 (authored by stephanemoore).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69414

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
@@ -4,7 +4,6 @@
 =
 
 .. toctree::
-
abseil-duration-addition
abseil-duration-comparison
abseil-duration-conversion-cast
@@ -106,87 +105,87 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
-   clang-analyzer-core.CallAndMessage
-   clang-analyzer-core.DivideZero
+   clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DynamicTypePropagation
-   clang-analyzer-core.NonNullParamChecker
-   clang-analyzer-core.NullDereference
-   clang-analyzer-core.StackAddressEscape
-   clang-analyzer-core.UndefinedBinaryOperatorResult
-   clang-analyzer-core.VLASize
-   clang-analyzer-core.uninitialized.ArraySubscript
-   clang-analyzer-core.uninitialized.Assign
-   clang-analyzer-core.uninitialized.Branch
+   clang-analyzer-core.NonNullParamChecker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.NullDereference (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.StackAddressEscape (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.UndefinedBinaryOperatorResult (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.VLASize (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.ArraySubscript (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Assign (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Branch (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.uninitialized.CapturedBlockVariable
-   clang-analyzer-core.uninitialized.UndefReturn
+   clang-analyzer-core.uninitialized.UndefReturn (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-cplusplus.InnerPointer
-   clang-analyzer-cplusplus.Move
-   clang-analyzer-cplusplus.NewDelete
-   clang-analyzer-cplusplus.NewDeleteLeaks
-   clang-analyzer-deadcode.DeadStores
-   clang-analyzer-nullability.NullPassedToNonnull
-   clang-analyzer-nullability.NullReturnedFromNonnull
-   clang-analyzer-nullability.NullableDereferenced
-   clang-analyzer-nullability.NullablePassedToNonnull
+   clang-analyzer-cplusplus.Move (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-cplusplus.NewDelete (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-cplusplus.NewDeleteLeaks (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-deadcode.DeadStores (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-nullability.NullPassedToNonnull (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-nullability.NullReturnedFromNonnull (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-nullability.NullableDereferenced (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-nullability.NullablePassedToNonnull (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-nullability.NullableReturnedFromNonnull
-   clang-analyzer-optin.cplusplus.UninitializedObject
-   clang-analyzer-optin.cplusplus.VirtualCall
-   clang-analyzer-optin.mpi.MPI-Checker
+   clang-analyzer-optin.cplusplus.UninitializedObject (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-optin.cplusplus.VirtualCall (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-optin.mpi.MPI-Checker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-optin.osx.OSObjectCStyleCast
-   clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker
-   clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker
+   clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang

[clang] c1d2927 - Run clang-format on lib/CodeGen/CGCall.h and fix indentation

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2019-10-30T18:06:12-07:00
New Revision: c1d2927cc64731520e60ecd1fc0e0eeabdfbac40

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

LOG: Run clang-format on lib/CodeGen/CGCall.h and fix indentation

Added: 


Modified: 
clang/lib/CodeGen/CGCall.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h
index cc11ded704ab..34558be5adb1 100644
--- a/clang/lib/CodeGen/CGCall.h
+++ b/clang/lib/CodeGen/CGCall.h
@@ -29,14 +29,14 @@ class AttributeList;
 class Function;
 class Type;
 class Value;
-}
+} // namespace llvm
 
 namespace clang {
-  class ASTContext;
-  class Decl;
-  class FunctionDecl;
-  class ObjCMethodDecl;
-  class VarDecl;
+class ASTContext;
+class Decl;
+class FunctionDecl;
+class ObjCMethodDecl;
+class VarDecl;
 
 namespace CodeGen {
 
@@ -60,331 +60,327 @@ class CGCalleeInfo {
 return CalleeProtoTy;
   }
   const GlobalDecl getCalleeDecl() const { return CalleeDecl; }
+};
+
+/// All available information about a concrete callee.
+class CGCallee {
+  enum class SpecialKind : uintptr_t {
+Invalid,
+Builtin,
+PseudoDestructor,
+Virtual,
+
+Last = Virtual
   };
 
-  /// All available information about a concrete callee.
-  class CGCallee {
-enum class SpecialKind : uintptr_t {
-  Invalid,
-  Builtin,
-  PseudoDestructor,
-  Virtual,
-
-  Last = Virtual
-};
-
-struct BuiltinInfoStorage {
-  const FunctionDecl *Decl;
-  unsigned ID;
-};
-struct PseudoDestructorInfoStorage {
-  const CXXPseudoDestructorExpr *Expr;
-};
-struct VirtualInfoStorage {
-  const CallExpr *CE;
-  GlobalDecl MD;
-  Address Addr;
-  llvm::FunctionType *FTy;
-};
-
-SpecialKind KindOrFunctionPointer;
-union {
-  CGCalleeInfo AbstractInfo;
-  BuiltinInfoStorage BuiltinInfo;
-  PseudoDestructorInfoStorage PseudoDestructorInfo;
-  VirtualInfoStorage VirtualInfo;
-};
-
-explicit CGCallee(SpecialKind kind) : KindOrFunctionPointer(kind) {}
-
-CGCallee(const FunctionDecl *builtinDecl, unsigned builtinID)
-: KindOrFunctionPointer(SpecialKind::Builtin) {
-  BuiltinInfo.Decl = builtinDecl;
-  BuiltinInfo.ID = builtinID;
-}
-
-  public:
-CGCallee() : KindOrFunctionPointer(SpecialKind::Invalid) {}
-
-/// Construct a callee.  Call this constructor directly when this
-/// isn't a direct call.
-CGCallee(const CGCalleeInfo &abstractInfo, llvm::Value *functionPtr)
-: KindOrFunctionPointer(SpecialKind(uintptr_t(functionPtr))) {
-  AbstractInfo = abstractInfo;
-  assert(functionPtr && "configuring callee without function pointer");
-  assert(functionPtr->getType()->isPointerTy());
-  assert(functionPtr->getType()->getPointerElementType()->isFunctionTy());
-}
-
-static CGCallee forBuiltin(unsigned builtinID,
-   const FunctionDecl *builtinDecl) {
-  CGCallee result(SpecialKind::Builtin);
-  result.BuiltinInfo.Decl = builtinDecl;
-  result.BuiltinInfo.ID = builtinID;
-  return result;
-}
-
-static CGCallee forPseudoDestructor(const CXXPseudoDestructorExpr *E) {
-  CGCallee result(SpecialKind::PseudoDestructor);
-  result.PseudoDestructorInfo.Expr = E;
-  return result;
-}
-
-static CGCallee forDirect(llvm::Constant *functionPtr,
-const CGCalleeInfo &abstractInfo = CGCalleeInfo()) {
-  return CGCallee(abstractInfo, functionPtr);
-}
-
-static CGCallee
-forDirect(llvm::FunctionCallee functionPtr,
-  const CGCalleeInfo &abstractInfo = CGCalleeInfo()) {
-  return CGCallee(abstractInfo, functionPtr.getCallee());
-}
-
-static CGCallee forVirtual(const CallExpr *CE, GlobalDecl MD, Address Addr,
-   llvm::FunctionType *FTy) {
-  CGCallee result(SpecialKind::Virtual);
-  result.VirtualInfo.CE = CE;
-  result.VirtualInfo.MD = MD;
-  result.VirtualInfo.Addr = Addr;
-  result.VirtualInfo.FTy = FTy;
-  return result;
-}
-
-bool isBuiltin() const {
-  return KindOrFunctionPointer == SpecialKind::Builtin;
-}
-const FunctionDecl *getBuiltinDecl() const {
-  assert(isBuiltin());
-  return BuiltinInfo.Decl;
-}
-unsigned getBuiltinID() const {
-  assert(isBuiltin());
-  return BuiltinInfo.ID;
-}
-
-bool isPseudoDestructor() const {
-  return KindOrFunctionPointer == SpecialKind::PseudoDestructor;
-}
-const CXXPseudoDestructorExpr *getPseudoDestructorExpr() const {
-  assert(isPseudoDestructor());
-  return PseudoDestructorInfo.Expr;
-}
-
-bool isOrdinary() const {
-  return uintptr_t(KindOrFunctionPointer) > uintp

[PATCH] D69651: [CMake] Add cross ARM toolchain CMake cache file.

[Vlad Vereschaka via Phabricator via cfe-commits]

vvereschaka created this revision.
vvereschaka added reviewers: andreil99, gkistanova.
Herald added subscribers: cfe-commits, kristof.beyls, mgorny.
Herald added a project: clang.

This cache file can be used to build CrossARM toolchain on Windows platform. 
Also it is going to be used by the buildbot.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69651

Files:
  clang/cmake/caches/CrossARM.cmake

Index: clang/cmake/caches/CrossARM.cmake
===
--- /dev/null
+++ clang/cmake/caches/CrossARM.cmake
@@ -0,0 +1,104 @@
+# Set up a CMakeCache for an ARM cross toolchain build on Windows platform.
+#
+# This cache file can be used to build CrossARM toolchain on Windows platform. 
+# Also it is going to be used by the buildbot.
+#
+# Configure: 
+#  cmake -G Ninja ^
+#   -DCMAKE_INSTALL_PREFIX=../install ^
+#   -DDEFAULT_SYSROOT= ^
+#   -DLLVM_AR=/bin/llvm-ar[.exe] ^
+#   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -C/llvm-project/clang/caches/CrossARM.cmake ^
+#   /llvm-project/llvm
+# Build:
+#  cmake --build . --target install
+# Test:
+#  cmake --build . --target check-llvm
+#  cmake --build . --target check-clang
+#  cmake --build . --target check-lld
+
+if (NOT DEFINED DEFAULT_SYSROOT)
+  message(WARNING "DEFAULT_SYSROOT must be specified for the ARM cross toolchain build.")
+endif()
+
+if (DEFINED LLVM_AR)
+  set(CMAKE_AR "${LLVM_AR}" CACHE STRING "")
+endif()
+
+if (NOT DEFINED LLVM_TARGETS_TO_BUILD)
+  set(LLVM_TARGETS_TO_BUILD "ARM" CACHE STRING "")
+endif()
+
+if (NOT DEFINED CMAKE_C_COMPILER_TARGET)
+  # Required if COMPILER_RT_DEFAULT_TARGET_ONLY is ON
+  set(CMAKE_C_COMPILER_TARGET "armv7-linux-gnueabihf" CACHE STRING "")
+endif() 
+		
+if (NOT DEFINED CMAKE_BUILD_TYPE)   
+  set(CMAKE_BUILD_TYPE "Release" CACHE STRING "")
+endif() 
+		
+set(CMAKE_CROSSCOMPILINGON CACHE BOOL "")
+set(CMAKE_CL_SHOWINCLUDES_PREFIX"Note: including file: " CACHE STRING "")
+		
+set(LLVM_ENABLE_ASSERTIONS  ON CACHE BOOL "")
+set(LLVM_ENABLE_PROJECTS"clang;clang-tools-extra;lld" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES"compiler-rt;libunwind;libcxxabi;libcxx" CACHE STRING "")
+set(LLVM_DEFAULT_TARGET_TRIPLE  "${CMAKE_C_COMPILER_TARGET}" CACHE STRING "")
+set(LLVM_TARGET_ARCH"${CMAKE_C_COMPILER_TARGET}" CACHE STRING "")
+set(LLVM_LIT_ARGS   "-vv ${LLVM_LIT_ARGS}" CACHE STRING "" FORCE)
+		
+set(CLANG_DEFAULT_LINKER"lld" CACHE STRING "")
+		
+set(COMPILER_RT_BUILD_BUILTINS  ON CACHE BOOL "")
+set(COMPILER_RT_BUILD_SANITIZERSOFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_XRAY  OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_LIBFUZZER OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_PROFILE   OFF CACHE BOOL "")
+set(COMPILER_RT_DEFAULT_TARGET_ONLY ON CACHE BOOL "")
+		
+set(LIBUNWIND_USE_COMPILER_RT   ON CACHE BOOL "")
+set(LIBUNWIND_TARGET_TRIPLE "${CMAKE_C_COMPILER_TARGET}" CACHE STRING "")
+set(LIBUNWIND_SYSROOT   "${DEFAULT_SYSROOT}" CACHE STRING "")
+set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+		
+set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+set(LIBCXXABI_ENABLE_STATIC_UNWINDERON CACHE BOOL "")
+set(LIBCXXABI_USE_COMPILER_RT   ON CACHE BOOL "")
+set(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
+set(LIBCXXABI_TARGET_TRIPLE "${CMAKE_C_COMPILER_TARGET}" CACHE STRING "")
+set(LIBCXXABI_SYSROOT   "${DEFAULT_SYSROOT}" CACHE STRING "")
+		
+set(LIBCXX_USE_COMPILER_RT  ON CACHE BOOL "")
+set(LIBCXX_TARGET_TRIPLE"${CMAKE_C_COMPILER_TARGET}" CACHE STRING "")
+set(LIBCXX_SYSROOT  "${DEFAULT_SYSROOT}" CACHE STRING "")
+		
+set(BUILTINS_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
+set(RUNTIMES_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY 			ON CACHE BOOL "")
+set(LLVM_TOOLCHAIN_TOOLS
+  llvm-ar
+  llvm-cov
+  llvm-cxxfilt
+  llvm-dwarfdump
+  llvm-lib
+  llvm-nm
+  llvm-objdump
+  llvm-profdata
+  llvm-ranlib
+  llvm-readobj
+  llvm-size
+  llvm-symbolizer
+  CACHE STRING "")
+
+set(LLVM_DISTRIBUTION_COMPONENTS
+  clang
+  lld
+  LTO
+  clang-format
+  builtins
+  runtimes
+  ${LLVM_TOOLCHAIN_TOOLS}
+  CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 004ed2b - Revert "[CodeView] Add option to disable inline line tables."

[Amy Huang via cfe-commits]

Author: Amy Huang
Date: 2019-10-30T17:31:12-07:00
New Revision: 004ed2b0d1b86d424643ffc88fce20ad8bab6804

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

LOG: Revert "[CodeView] Add option to disable inline line tables."
because it breaks compiler-rt tests.

This reverts commit 6d03890384517919a3ba7fe4c35535425f278f89.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/docs/LangRef.rst
llvm/include/llvm/IR/Attributes.td
llvm/lib/Transforms/Utils/InlineFunction.cpp

Removed: 
clang/test/CodeGen/debug-info-no-inline-line-tables.c
llvm/test/Transforms/Inline/no-inline-line-tables.ll



diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index ebdcb3c1f48b..f8d94e352f28 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -136,8 +136,6 @@ CODEGENOPT(FatalWarnings , 1, 0) ///< Set when 
-Wa,--fatal-warnings is
  ///< enabled.
 CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
-CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
- ///< inline line tables.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 1bdeed69602f..2401a31ceb9a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1984,9 +1984,6 @@ def gcodeview_ghash : Flag<["-"], "gcodeview-ghash">,
   HelpText<"Emit type record hashes in a .debug$H section">,
   Flags<[CC1Option, CoreOption]>;
 def gno_codeview_ghash : Flag<["-"], "gno-codeview-ghash">, 
Flags<[CoreOption]>;
-def ginline_line_tables : Flag<["-"], "ginline-line-tables">, 
Flags<[CoreOption]>;
-def gno_inline_line_tables : Flag<["-"], "gno-inline-line-tables">,
-  Flags<[CC1Option, CoreOption]>, HelpText<"Don't emit inline line tables">;
 
 // Equivalent to our default dwarf version. Forces usual dwarf emission when
 // CodeView is enabled.

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 1b1d391a63df..3f9a52ab7638 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -764,10 +764,6 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
   Fn->addFnAttr("no-jump-tables",
 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
 
-  // Add no-inline-line-tables value.
-  if (CGM.getCodeGenOpts().NoInlineLineTables)
-Fn->addFnAttr("no-inline-line-tables");
-
   // Add profile-sample-accurate value.
   if (CGM.getCodeGenOpts().ProfileSampleAccurate)
 Fn->addFnAttr("profile-sample-accurate");

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f5abdf15fd44..c60dc76ae1bf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3296,12 +3296,6 @@ static void RenderDebugOptions(const ToolChain &TC, 
const Driver &D,
 }
   }
 
-  // Omit inline line tables if requested.
-  if (!Args.hasFlag(options::OPT_ginline_line_tables,
-options::OPT_gno_inline_line_tables, false)) {
-CmdArgs.push_back("-gno-inline-line-tables");
-  }
-
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 29736d2dee18..f197a67e7a38 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -806,7 +806,6 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList 
&Args, InputKind IK,
   Opts.RecordCommandLine = Args.getLastArgValue(OPT_record_command_line);
   Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);
   Opts.NoCommon = Args.hasArg(OPT_fno_common);
-  Opts.NoInlineLineTables = Args.hasArg(OPT_gno_inline_line_tables);
   Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);
   Opts.OptimizeSize = getOptimizationLevelSize(Args);
   Opts.SimplifyLibCalls = !(Args.hasArg(OPT_fno_builtin) ||

diff  --git a/clang/test/CodeGen/debug-info-no-inline-line-tables.c 
b/clang/tes

[PATCH] D69498: IR: Invert convergent attribute handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

As far as optimization inhibition is concerned, noconvergent will be inferred 
for all functions that don't call convergent intrinsics (i.e. the state of the 
world for all functions on all CPU targets). The frontend needing to do 
something for optimization comes up in relation to calling external 
declarations which won't be inferred in non-LTO contexts (plus inline asm and 
indirect calls, the other opaque call types). IMO the mild inconvenience of 
needing to add another attribute to optimize external call declarations is not 
unreasonable. This is already the situation for nounwind with languages that 
don't have exceptions. I don't see what real benefit there is to inventing some 
new inconsistent, target-specific IR construct to avoid frontends needing to 
add an attribute that behaves nearly identically to nounwind


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

https://reviews.llvm.org/D69498



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69498#1728039 , @tra wrote:

> Perhaps we can deal with that by providing a way to specify per-module 
> default for the assumed convergence of the functions and then checking in the 
> back-end (only those that do care about convergence) that the default 
> convergence is explicitly set (and, perhaps, set to something specific?) via 
> function/module attributes or CLI.
>
> This way the unintentional use of vanilla IR w/o attributes with NVPTX 
> back-end will produce an error complaining that default convergence is not 
> set and we don't know if the IR is still sound. If necessary, the user can 
> set appropriate convergence wholesale via CLI or module attribute. The burden 
> on platforms that don't care about convergence will be limited to setting the 
> default and applying attributes on entities that do not match the default 
> assumption (there may be none of those).


Convergent can be correctly stripped in many cases (and noconvergent inferred 
in the same cases), so erroring if a function isn't convergent won't really work


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

https://reviews.llvm.org/D69498



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jorg Brown via Phabricator via cfe-commits]

jorgbrown added a comment.

+1

Thanks also for fixing the return statement of testConversionSpan() in 
span.pass.cpp


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

https://reviews.llvm.org/D69520



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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM Pacific time today.

Thanks

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

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

It looks like this patch contains a few other changes, besides the changes to 
the prototypes.  In particular, the change to CGBuiltin.cpp, and there are a 
few new lines in the .td files that don't correspond to anything in the old 
versions.  Is that accidental, or is it part of cleaning up the prototypes, 
somehow?


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

https://reviews.llvm.org/D69618



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I had some time to take a look. D69636  makes 
things work on mac.


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

https://reviews.llvm.org/D63607



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

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: docs/ReleaseNotes.rst:84
+  In a future release of Clang, we intend to change the default to
+  ``-fno-lax-vector-conversions``.
+

efriedma wrote:
> kristof.beyls wrote:
> > efriedma wrote:
> > > rsmith wrote:
> > > > efriedma wrote:
> > > > > And if you want to allow your code to build with both clang-9 and 
> > > > > clang-10, you have to do version detection in your build scripts?
> > > > I guess you'd detect whether the compiler supports 
> > > > `-flax-vector-conversions=all`, and pass that if so, and otherwise pass 
> > > > `-flax-vector-conversions`. Well, either that or you fix your code to 
> > > > not rely on lax vector conversions between int and float vectors. If 
> > > > your code builds with GCC, you did that already (they never supported 
> > > > lax conversions between float and int vectors, as far as I can tell).
> > > > 
> > > > Do you have a preferred alternative?
> > > All the alternatives are terrible in their own way:
> > > 
> > > 1. This status quo, which breaks compatibility with previous versions of 
> > > clang
> > > 2. We could make -flax-vector-conversions mean the same thing as earlier 
> > > versions of clang.  So the flag wouldn't have the same meaning as gcc's 
> > > flag.
> > > 3. Some mix of the previous options: we could wait until there are one or 
> > > two released versions that support -flax-vector-conversions=all , then 
> > > change the meaning of -flax-vector-conversions.  But I have no idea how 
> > > we would decide on a timeline.
> > > 
> > > I ran into this issue recently updating our compiler.  That said, the 
> > > code in question was only using the implicit conversion in a couple 
> > > places by accident, so it was easy enough to just fix the source.
> > Maybe adding an entry in the release notes about this change could help 
> > with making option 1 slightly more palatable?
> > My guess is that option 1 is the right one for the long term (compatibility 
> > between gcc and clang so code is more portable between both compilers).
> It probably makes sense to call out the behavior change to 
> -flax-vector-conversions in the release notes, yes.
@kristof.beyls Are you looking for more changes to the release notes in 
addition to what's already in this change? If so, what would you like to see?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67678



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

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: docs/ReleaseNotes.rst:84
+  In a future release of Clang, we intend to change the default to
+  ``-fno-lax-vector-conversions``.
+

kristof.beyls wrote:
> efriedma wrote:
> > rsmith wrote:
> > > efriedma wrote:
> > > > And if you want to allow your code to build with both clang-9 and 
> > > > clang-10, you have to do version detection in your build scripts?
> > > I guess you'd detect whether the compiler supports 
> > > `-flax-vector-conversions=all`, and pass that if so, and otherwise pass 
> > > `-flax-vector-conversions`. Well, either that or you fix your code to not 
> > > rely on lax vector conversions between int and float vectors. If your 
> > > code builds with GCC, you did that already (they never supported lax 
> > > conversions between float and int vectors, as far as I can tell).
> > > 
> > > Do you have a preferred alternative?
> > All the alternatives are terrible in their own way:
> > 
> > 1. This status quo, which breaks compatibility with previous versions of 
> > clang
> > 2. We could make -flax-vector-conversions mean the same thing as earlier 
> > versions of clang.  So the flag wouldn't have the same meaning as gcc's 
> > flag.
> > 3. Some mix of the previous options: we could wait until there are one or 
> > two released versions that support -flax-vector-conversions=all , then 
> > change the meaning of -flax-vector-conversions.  But I have no idea how we 
> > would decide on a timeline.
> > 
> > I ran into this issue recently updating our compiler.  That said, the code 
> > in question was only using the implicit conversion in a couple places by 
> > accident, so it was easy enough to just fix the source.
> Maybe adding an entry in the release notes about this change could help with 
> making option 1 slightly more palatable?
> My guess is that option 1 is the right one for the long term (compatibility 
> between gcc and clang so code is more portable between both compilers).
It probably makes sense to call out the behavior change to 
-flax-vector-conversions in the release notes, yes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67678



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@sylvestre.ledru : quick backport for llvm9: 
https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

Seems like this is good to be committed then. And it sounds like implementing 
more thresholds would be useful to do in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723



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

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

[Amy Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d0389038451: [CodeView] Add option to disable inline line 
tables. (authored by akhuang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-no-inline-line-tables.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/no-inline-line-tables.ll

Index: llvm/test/Transforms/Inline/no-inline-line-tables.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/no-inline-line-tables.ll
@@ -0,0 +1,99 @@
+; RUN: opt < %s -inline -S | FileCheck %s
+
+; This tests that functions with the attribute `no-inline-line-tables` have the
+; correct debug information when they are inlined.
+
+; ModuleID = 't.c'
+source_filename = "t.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: alwaysinline nounwind
+define dso_local i32 @f(i32 %x) #0 !dbg !7 {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !12, metadata !DIExpression()), !dbg !13
+  %0 = load i32, i32* %x.addr, align 4, !dbg !14
+  ret i32 %0, !dbg !14
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: alwaysinline nounwind
+define i32 @g(i32 %x) #0 !dbg !15 {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !16, metadata !DIExpression()), !dbg !17
+  br label %L, !dbg !17
+
+L:; preds = %entry
+  call void @llvm.dbg.label(metadata !18), !dbg !19
+  store i32 42, i32* %x.addr, align 4, !dbg !20
+  %0 = load i32, i32* %x.addr, align 4, !dbg !21
+  ret i32 %0, !dbg !21
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.label(metadata) #1
+
+; Check that debug info for inlined code uses the call location and that debug
+; intrinsics are removed.
+; Function Attrs: noinline nounwind optnone
+define i32 @main() #2 !dbg !22 {
+entry:
+; CHECK-LABEL: @main()
+; CHECK-NOT: @f
+; CHECK-NOT: @llvm.dbg.declare
+; CHECK: %{{[0-9]+}} = load i32, i32* %x.addr.i, align 4, !dbg ![[VAR1:[0-9]+]]
+  %call = call i32 @f(i32 3), !dbg !25
+
+; Another test for inlining debug intrinsics where the intrinsic appears at the
+; start of the basic block.
+; CHECK-NOT: @g
+; CHECK-NOT: @llvm.dbg.label
+; CHECK: %{{[0-9]+}} = load i32, i32* %x.addr.i1, align 4, !dbg ![[VAR2:[0-9]+]]
+  %call1 = call i32 @g(i32 340), !dbg !26
+  ret i32 0, !dbg !27
+}
+
+; CHECK: ![[VAR1]] = !DILocation(line: 10, scope: ![[SCOPE:[0-9]+]])
+; CHECK: ![[VAR2]] = !DILocation(line: 11, scope: ![[SCOPE]])
+
+attributes #0 = { alwaysinline nounwind "no-inline-line-tables" }
+attributes #2 = { noinline nounwind optnone "no-inline-line-tables"}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0 (https://github.com/llvm/llvm-project.git 03ec3a12a94bbbaa11999b6da3a43221a5aa54a5)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "/usr/local/google/home/akhuang/testing/inline-line-tables", checksumkind: CSK_MD5, checksum: "38a4785b48742d3ea655b8f3461436a4")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 (https://github.com/llvm/llvm-project.git 03ec3a12a94bbbaa11999b6da3a43221a5aa54a5)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "t.c", directory: "/usr/local/google/home/akhuang/testing/inline-line-tables", checksumkind: CSK_MD5, checksum: "38a4785b48742d3ea655b8f3461436a4")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "x", arg: 1, scope: !7, file: !8, line: 1, type: !11)
+!13 = !DILocation(line: 1, scope: !7)
+!14 = !DILocation(line: 2, scope: !7)
+!15 = distinct !DISubprogram(name: "g", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit

[clang] 6d03890 - [CodeView] Add option to disable inline line tables.

[Amy Huang via cfe-commits]

Author: Amy Huang
Date: 2019-10-30T16:52:39-07:00
New Revision: 6d03890384517919a3ba7fe4c35535425f278f89

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

LOG: [CodeView] Add option to disable inline line tables.

Summary:
This adds a clang option to disable inline line tables. When it is used,
the inliner uses the call site as the location of the inlined function instead 
of
marking it as an inline location with the function location.

See https://bugs.llvm.org/show_bug.cgi?id=42344

Reviewers: rnk

Subscribers: hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 
clang/test/CodeGen/debug-info-no-inline-line-tables.c
llvm/test/Transforms/Inline/no-inline-line-tables.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/docs/LangRef.rst
llvm/include/llvm/IR/Attributes.td
llvm/lib/Transforms/Utils/InlineFunction.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index f8d94e352f28..ebdcb3c1f48b 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -136,6 +136,8 @@ CODEGENOPT(FatalWarnings , 1, 0) ///< Set when 
-Wa,--fatal-warnings is
  ///< enabled.
 CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
+CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
+ ///< inline line tables.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 2401a31ceb9a..1bdeed69602f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1984,6 +1984,9 @@ def gcodeview_ghash : Flag<["-"], "gcodeview-ghash">,
   HelpText<"Emit type record hashes in a .debug$H section">,
   Flags<[CC1Option, CoreOption]>;
 def gno_codeview_ghash : Flag<["-"], "gno-codeview-ghash">, 
Flags<[CoreOption]>;
+def ginline_line_tables : Flag<["-"], "ginline-line-tables">, 
Flags<[CoreOption]>;
+def gno_inline_line_tables : Flag<["-"], "gno-inline-line-tables">,
+  Flags<[CC1Option, CoreOption]>, HelpText<"Don't emit inline line tables">;
 
 // Equivalent to our default dwarf version. Forces usual dwarf emission when
 // CodeView is enabled.

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 3f9a52ab7638..1b1d391a63df 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -764,6 +764,10 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
   Fn->addFnAttr("no-jump-tables",
 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
 
+  // Add no-inline-line-tables value.
+  if (CGM.getCodeGenOpts().NoInlineLineTables)
+Fn->addFnAttr("no-inline-line-tables");
+
   // Add profile-sample-accurate value.
   if (CGM.getCodeGenOpts().ProfileSampleAccurate)
 Fn->addFnAttr("profile-sample-accurate");

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c60dc76ae1bf..f5abdf15fd44 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3296,6 +3296,12 @@ static void RenderDebugOptions(const ToolChain &TC, 
const Driver &D,
 }
   }
 
+  // Omit inline line tables if requested.
+  if (!Args.hasFlag(options::OPT_ginline_line_tables,
+options::OPT_gno_inline_line_tables, false)) {
+CmdArgs.push_back("-gno-inline-line-tables");
+  }
+
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f197a67e7a38..29736d2dee18 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -806,6 +806,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList 
&Args, InputKind IK,
   Opts.RecordCommandLine = Args.getLastArgValue(OPT_record_command_line);
   Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);
   Opts.NoCommon = Args.hasArg(OPT_fno_common);
+  Opts.NoInlineLi

[PATCH] D69498: IR: Invert convergent attribute handling

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Would I be too far off the mark to summarize the situation this way?

- current default for unattributed functions is unsound for GPU back-ends, but 
is fine for most of the other back-ends.
- it's easy to unintentionally end up using/mis-optimizing unattributed 
functions for back-ends where that care about convergence.
- making functions convergent by default everywhere is correct, but is overly 
pessimistic for back-ends that do not need it. Such back-ends will need to add 
explicit attributes in order to maintain current level of optimizations. In a 
way it's the other side of the current situation -- default is not good for 
some back-ends and we have to add attributes everywhere to make things work. 
Flipping the default improves correctness-by-default, but places logistical 
burden on back- and front-ends that may not care about convergence otherwise.

Perhaps we can deal with that by providing a way to specify per-module default 
for the assumed convergence of the functions and then checking in the back-end 
(only those that do care about convergence) that the default convergence is 
explicitly set (and, perhaps, set to something specific?) via function/module 
attributes or CLI.

This way the unintentional use of vanilla IR w/o attributes with NVPTX back-end 
will produce an error complaining that default convergence is not set and we 
don't know if the IR is still sound. If necessary, the user can set appropriate 
convergence wholesale via CLI or module attribute. The burden on platforms that 
don't care about convergence will be limited to setting the default and 
applying attributes on entities that do not match the default assumption (there 
may be none of those).

One pitfall of this approach is that we may run optimizations based on wrong 
assumptions and end up miscompiling things before we know what those 
assumptions will be (e.g. opt vs llc). Perhaps we can document the default 
assumption to be nonconvergent and always set a module attribute indicating the 
assumption used. This assumption will percolate through the optimization 
pipeline. If its the wrong one we would be able to reject / warn about it in 
the passes for the back-ends that have different preference.

Would something like that be a reasonable compromise?


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

https://reviews.llvm.org/D69498



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso marked an inline comment as done.
Charusso added a comment.
This revision is now accepted and ready to land.

My concern was the too heavy `Optional` and `bool` usage. Cool patch!




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:61
+  using FnCheck = bool (StreamChecker::*)(const CallEvent &,
+  CheckerContext &) const;
+

balazske wrote:
> Szelethus wrote:
> > Charusso wrote:
> > > I prefer `std::function`, because it is modern.
> > > ```
> > >   using StreamCheck = 
> > >   std::function > >  CheckerContext &)>;
> > > ```
> > > I think it is fine with pointers, but at some point we need to modernize 
> > > this.
> > But its also a lot more expensive. 
> > https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/
> > 
> > `std::function` is able to wrap lambdas with different captures and all 
> > sorts of things like that, which comes at a cost.
> Now `std::function` and `std::bind` is used. Probably more expensive but it 
> is called once in a `evalCall`, that should be no problem?
There is no real overhead, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rnk what's your take on that version?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: MyDeveloperDay, klimek, sammccall.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes an edge case in the `SpacesInSquareBrackets` option where an 
initial `&ref` lambda parameter is not padded with an initial space.

`int foo = [&bar ]() {}` is fixed to give `int foo = [ &bar ]() {}`


Repository:
  rC Clang

https://reviews.llvm.org/D69649

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 227194.
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.

Moved to a simpler, easier to review, less error prone implementation. 
Validates just fines on llvm bootstrap an cpython code base. @sylvestre.ledru 
that's the patch you can play with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/Cod

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Let's land this patch, given that it grew way too big for a refactoring pass, 
and let @balazske further clean things up later in his follow-up patches. This 
is already a large improvement and i'm very grateful!

For the `Optional` vs. `bool` debate: let's see. For now we have roughly the 
following:

  bool checkNullStream(..., ProgramStateRef *State) {
if (!NonBuggyState && BuggyState) {
  C.emitReport(..., C.generateErrorNode(BuggyState));
  return false;
}
  
if (NonBuggyState) {
  *State = NonBuggyState;
  return true;
}
  
return false;
  }
  
  void checkFseekWhence(..., ProgramStateRef *State) const {
if (!isBuggy(State))
  return;
  
C.emitReport(..., C.generateNonFatalErrorNode(State));
  }
  
  void evalFseek(...) {
ProgramStateRef State = C.getState();
bool StateChanged = checkNullStream(..., &State);
if (C.isDifferent())
  return;
  
checkFseekWhence(..., &State);
if (!C.isDifferent() && StateChanged)
  C.addTransition(State);
  }

In this code functions `checkNullStream` and `checkFseekWhence` are very 
similar. Both of them do some checking and potentially emit a report. In 
high-level terms, all you're trying to do is conduct two checks at the same 
program point.

How did these two functions end up being so completely different? What will you 
do if you have not 2 but, say, 10 different checks that you need to conduct? 
That's not a random question; checkers do tend to grow this way, so i'm pretty 
sure you'll eventually need to answer this question.

In `ExprEngine`, the class that's responsible for modeling all statements that 
have effect on the program state, we have a very common idiom for chaining 
pieces of work together. It looks roughly like this:

  void modelSomethingPart1(ExplodedNode *Pred, set *Succs);
  void modelSomethingPart2(ExplodedNode *Pred, set *Succs);
  void modelSomethingPart3(ExplodedNode *Pred, set *Succs);
  ...
  void modelSomethingPartK(ExplodedNode *Pred, set *Succs);
  
  
  void modelSomething(ExplodedNode *Pred) {
set Set0 = { Pred };
  
set Set1;
for (P : Set0)
  modelSomethingPart1(P, &Set1);

set Set2;
for (P : Set1)
  modelSomethingPart2(P, &Set2);

set Set3;
for (P : Set2)
  modelSomethingPart3(P, &Set3);
  
...
  
set SetK;
for (P : SetKMinus1)
  modelSomethingPartK(P, &SetK);
  
for (N : SetK)
  addTransition(N);
  }

As you see, it scales nicely for any number of sub-tasks and you don't need to 
juggle boolean flags or optionals when you add another sub-task. Moreover, you 
can further split each of your sub-tasks into even smaller sub-sub-tasks in a 
similar manner.

In `ExprEngine` this idiom is completely essential due to how arbitrary most of 
the effects-of-statements tend to be. In checkers, however, this level of 
complexity has never been necessary so far because most sub-methods either 
produce exactly one new state, or sink completely. For checkers like this one, 
which do multiple checks, i recommend passing a single node around:

  ExplodedNode *checkNullStream(..., ExplodedNode *Pred) {
if (!NonBuggyState && BuggyState)
  C.emitReport(..., C.generateErrorNode(BuggyState, Pred));
  
return C.addTransition(NonBuggyState, Pred);
  }
  
  ExplodedNode *checkFseekWhence(..., ExplodedNode *Pred) const {
State = Pred->getState();
if (!isBuggy(State))
  return Pred;
ExplodedNode *N = C.generateNonFatalErrorNode(State);
if (N)
  C.emitReport(..., N);
return N;
  }
  
  void evalFseek(...) {
ExplodedNode *N = C.getPredecessor();
  
N = checkNullStream(..., N);
if (!N)
  return;
  
N = checkFseekWhence(..., N);
if (!N)
  return;
  
N = checkSomethingElse1(..., N);
if (!N)
  return;
  
N = checkSomethingElse2(..., N);
if (!N)
  return;
  
...
  
checkSomethingElseK(..., N);
  }

This wastes exploded nodes a little bit but that's fine. It's much less scary 
than accidentally adding unnecessary state splits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

Re: [clang] 33a745e - [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via cfe-commits]

On Wed, Oct 30, 2019 at 3:54 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I XFAILED the test on Windows in 52194350cfe. The [/\\] regexes might need
> to match two slashes, and the PREFIX variable ends up having inconsistent
> slash direction.
>

Hmm, fixing the slashes should be easy, but I'm not sure there's a good way
to get the PREFIX right. I feel like FileCheck needs an ignore slash
direction mode.

- Michael Spencer


>
> On Wed, Oct 30, 2019 at 3:27 PM Michael Spencer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Michael Spencer
>> Date: 2019-10-30T15:27:27-07:00
>> New Revision: 33a745e6fe7e81d3793f7831d2832aa0785ef327
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327.diff
>>
>> LOG: [clang][clang-scan-deps] Add support for extracting full module
>> dependencies.
>>
>> This is a recommit of d8a4ef0e685c with the nondeterminism fixed.
>>
>> This adds experimental support for extracting a Clang module dependency
>> graph
>> from a compilation database. The output format is experimental and will
>> change.
>> It is currently a concatenation of JSON outputs for each compilation.
>> Future
>> patches will change this to deduplicate modules between compilations.
>>
>> Differential Revision: https://reviews.llvm.org/D69420
>>
>> Added:
>> clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
>> clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
>> clang/test/ClangScanDeps/modules-full.cpp
>>
>> Modified:
>>
>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>
>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>>
>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
>> clang/lib/Tooling/DependencyScanning/CMakeLists.txt
>> clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
>> clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
>> clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
>> clang/tools/clang-scan-deps/ClangScanDeps.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git
>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>> index fd8ed80b143c..76edf150dbee 100644
>> ---
>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>> +++
>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>> @@ -30,15 +30,30 @@ enum class ScanningMode {
>>MinimizedSourcePreprocessing
>>  };
>>
>> +/// The format that is output by the dependency scanner.
>> +enum class ScanningOutputFormat {
>> +  /// This is the Makefile compatible dep format. This will include all
>> of the
>> +  /// deps necessary for an implicit modules build, but won't include any
>> +  /// intermodule dependency information.
>> +  Make,
>> +
>> +  /// This outputs the full module dependency graph suitable for use for
>> +  /// explicitly building modules.
>> +  Full,
>> +};
>> +
>>  /// The dependency scanning service contains the shared state that is
>> used by
>>  /// the invidual dependency scanning workers.
>>  class DependencyScanningService {
>>  public:
>> -  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager =
>> true,
>> +  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat
>> Format,
>> +bool ReuseFileManager = true,
>>  bool SkipExcludedPPRanges = true);
>>
>>ScanningMode getMode() const { return Mode; }
>>
>> +  ScanningOutputFormat getFormat() const { return Format; }
>> +
>>bool canReuseFileManager() const { return ReuseFileManager; }
>>
>>bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
>> @@ -49,6 +64,7 @@ class DependencyScanningService {
>>
>>  private:
>>const ScanningMode Mode;
>> +  const ScanningOutputFormat Format;
>>const bool ReuseFileManager;
>>/// Set to true to use the preprocessor optimization that skips
>> excluded PP
>>/// ranges by bumping the buffer pointer in the lexer instead of
>> lexing the
>>
>> diff  --git
>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>> index c950cbe167cd..78b49e4fa0c5 100644
>> ---
>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>> +++
>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>> @@ -40,6 +40,7 @@ class DependencyScanningTool {
>>  StringRef CWD);
>>
>>  private:
>> +  const ScanningOutputFormat Format;
>>DependencyScanningWorker Worker;
>

Zorg migration to GitHub/monorepo - Step 2

[Galina Kistanova via cfe-commits]

Hello everyone,

As all of you know, the migration to github monorepo is done.

The production master is currently working in a transition mode, when
it supports both, bots already working with github, and bots still
working with SVN.

Today we will turn "github only" mode for the production master.

Those builders which depend on ported build factories and were staged are good.

The builders which depend on ported build factories and have not been
staged might just work after this change. Those which turn red will be
temporarily removed from the production master. If you are an owner of
such builder, please feel free to stage it, make it green in staging,
and then you could bring it to the production build bot.

The builders which still require SVN to build will be removed form the
production master. If you are an owner of such builder, please feel
free to stage it, port it to work with the github monorepo, make it
green, and then you could bring it to the production build bot.

Here is the list of not yet staged builders of the last 2 kinds in the
alphabetical order:

aosp-O3-polly-before-vectorizer-unprofitable
clang-aarch64-linux-build-cache
clang-armv7-linux-build-cache
clang-atom-d525-fedora-rel
clang-cmake-x86_64-avx2-linux
clang-cmake-x86_64-avx2-linux-perf
clang-cmake-x86_64-sde-avx512-linux
clang-cuda-build
clang-hexagon-elf
clang-s390x-linux
clang-s390x-linux-lnt
clang-s390x-linux-multistage
clang-x64-ninja-win7
libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu
libcxx-libcxxabi-x86_64-linux-ubuntu-32bit
libcxx-libcxxabi-x86_64-linux-ubuntu-asan
libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03
libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11
libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14
libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17
libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a
libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11
libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std
libcxx-libcxxabi-x86_64-linux-ubuntu-msan
libcxx-libcxxabi-x86_64-linux-ubuntu-tsan
libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan
llvm-hexagon-elf
polly-amd64-linux
polly-arm-linux
reverse-iteration

Please let me know if you have any questions.

Thank you for your patience and understanding.

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

[clang] 17b64e3 - [clang][ScanDeps] Fix shadowing warning.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-30T15:53:38-07:00
New Revision: 17b64e36e90ad05a32a99b9f5784b55310cc10e8

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

LOG: [clang][ScanDeps] Fix shadowing warning.

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index bffd7c338124..4b10f24167a8 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -85,7 +85,7 @@ DependencyScanningTool::getDependencyFile(const std::string 
&Input,
 }
 
 void handleModuleDependency(ModuleDeps MD) override {
-  ModuleDeps[MD.ContextHash + MD.ModuleName] = std::move(MD);
+  ClangModuleDeps[MD.ContextHash + MD.ModuleName] = std::move(MD);
 }
 
 void handleContextHash(std::string Hash) override {
@@ -95,7 +95,7 @@ DependencyScanningTool::getDependencyFile(const std::string 
&Input,
 void printDependencies(std::string &S, StringRef MainFile) {
   // Sort the modules by name to get a deterministic order.
   std::vector Modules;
-  for (auto &&Dep : ModuleDeps)
+  for (auto &&Dep : ClangModuleDeps)
 Modules.push_back(Dep.first);
   std::sort(Modules.begin(), Modules.end());
 
@@ -105,14 +105,14 @@ DependencyScanningTool::getDependencyFile(const 
std::string &Input,
 
   Array Imports;
   for (auto &&ModName : Modules) {
-auto &MD = ModuleDeps[ModName];
+auto &MD = ClangModuleDeps[ModName];
 if (MD.ImportedByMainFile)
   Imports.push_back(MD.ModuleName);
   }
 
   Array Mods;
   for (auto &&ModName : Modules) {
-auto &MD = ModuleDeps[ModName];
+auto &MD = ClangModuleDeps[ModName];
 Object Mod{
 {"name", MD.ModuleName},
 {"file-deps", toJSONSorted(MD.FileDeps)},
@@ -136,7 +136,7 @@ DependencyScanningTool::getDependencyFile(const std::string 
&Input,
 
   private:
 std::vector Dependencies;
-std::unordered_map ModuleDeps;
+std::unordered_map ClangModuleDeps;
 std::string ContextHash;
   };
 



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

Re: [clang] 33a745e - [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Reid Kleckner via cfe-commits]

I XFAILED the test on Windows in 52194350cfe. The [/\\] regexes might need
to match two slashes, and the PREFIX variable ends up having inconsistent
slash direction.

On Wed, Oct 30, 2019 at 3:27 PM Michael Spencer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Michael Spencer
> Date: 2019-10-30T15:27:27-07:00
> New Revision: 33a745e6fe7e81d3793f7831d2832aa0785ef327
>
> URL:
> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327
> DIFF:
> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327.diff
>
> LOG: [clang][clang-scan-deps] Add support for extracting full module
> dependencies.
>
> This is a recommit of d8a4ef0e685c with the nondeterminism fixed.
>
> This adds experimental support for extracting a Clang module dependency
> graph
> from a compilation database. The output format is experimental and will
> change.
> It is currently a concatenation of JSON outputs for each compilation.
> Future
> patches will change this to deduplicate modules between compilations.
>
> Differential Revision: https://reviews.llvm.org/D69420
>
> Added:
> clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
> clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
> clang/test/ClangScanDeps/modules-full.cpp
>
> Modified:
>
> clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
> clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>
> clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
> clang/lib/Tooling/DependencyScanning/CMakeLists.txt
> clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
> clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
> clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
> clang/tools/clang-scan-deps/ClangScanDeps.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
> index fd8ed80b143c..76edf150dbee 100644
> ---
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
> +++
> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
> @@ -30,15 +30,30 @@ enum class ScanningMode {
>MinimizedSourcePreprocessing
>  };
>
> +/// The format that is output by the dependency scanner.
> +enum class ScanningOutputFormat {
> +  /// This is the Makefile compatible dep format. This will include all
> of the
> +  /// deps necessary for an implicit modules build, but won't include any
> +  /// intermodule dependency information.
> +  Make,
> +
> +  /// This outputs the full module dependency graph suitable for use for
> +  /// explicitly building modules.
> +  Full,
> +};
> +
>  /// The dependency scanning service contains the shared state that is
> used by
>  /// the invidual dependency scanning workers.
>  class DependencyScanningService {
>  public:
> -  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager =
> true,
> +  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat
> Format,
> +bool ReuseFileManager = true,
>  bool SkipExcludedPPRanges = true);
>
>ScanningMode getMode() const { return Mode; }
>
> +  ScanningOutputFormat getFormat() const { return Format; }
> +
>bool canReuseFileManager() const { return ReuseFileManager; }
>
>bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
> @@ -49,6 +64,7 @@ class DependencyScanningService {
>
>  private:
>const ScanningMode Mode;
> +  const ScanningOutputFormat Format;
>const bool ReuseFileManager;
>/// Set to true to use the preprocessor optimization that skips
> excluded PP
>/// ranges by bumping the buffer pointer in the lexer instead of lexing
> the
>
> diff  --git
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
> index c950cbe167cd..78b49e4fa0c5 100644
> ---
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
> +++
> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
> @@ -40,6 +40,7 @@ class DependencyScanningTool {
>  StringRef CWD);
>
>  private:
> +  const ScanningOutputFormat Format;
>DependencyScanningWorker Worker;
>const tooling::CompilationDatabase &Compilations;
>  };
>
> diff  --git
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
> index 45c9fb4f029d..689119330c41 100644
> ---
> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
> +++
> b/clang/include/clang/Tooling/DependencyScannin

[clang] 5219435 - Disable new clang-scan-deps test on Windows due to backslash path matching problems

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-10-30T15:51:34-07:00
New Revision: 52194350cfe834f27e41771c315fe0656067fdc4

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

LOG: Disable new clang-scan-deps test on Windows due to backslash path matching 
problems

Added: 


Modified: 
clang/test/ClangScanDeps/modules-full.cpp

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-full.cpp 
b/clang/test/ClangScanDeps/modules-full.cpp
index f8bff06d8f74..693dffeecbf7 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -15,6 +15,9 @@
 // RUN:   -mode preprocess-minimized-sources -format experimental-full >> 
%t.result
 // RUN: cat %t.result | FileCheck --check-prefixes=CHECK %s
 
+// FIXME: Backslash issues.
+// XFAIL: system-windows
+
 #include "header.h"
 
 // CHECK: [[PREFIX:(.*[/\\])+[a-zA-Z0-9.-]+]]



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

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

I split off the backend changes into a separate patch -> 
https://reviews.llvm.org/D69639
The issue with passing 0 for a pointer should be fixed there.

@DarkShadow44 
Some of the differences come from the fact that we're implementing the mixed 
pointer sizes with three address spaces, which get printed out as 
`__ptr32_sptr`, `__ptr32_uptr`, and `__ptr64`. So they are intended but maybe 
also confusing.

> __ptr32_sptr doesn't exist and seems wrong

In this case it picks the address space __ptr32_sptr because it does sign 
extension by default.

> The __ptr32 part should be right of the asterisk

I think this happens because address spaces are attached to the pointee type 
and not the pointer.

> Why does PINT64 have PINT64 as type, but PINT32 not PINT32?

I'm not sure why this happens, but I can look into it later. It makes sense 
that PINT64 would work as it did before because on a 64-bit system adding 
__ptr64 doesn't do anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66827



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

[clang] 33a745e - [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-30T15:27:27-07:00
New Revision: 33a745e6fe7e81d3793f7831d2832aa0785ef327

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

LOG: [clang][clang-scan-deps] Add support for extracting full module 
dependencies.

This is a recommit of d8a4ef0e685c with the nondeterminism fixed.

This adds experimental support for extracting a Clang module dependency graph
from a compilation database. The output format is experimental and will change.
It is currently a concatenation of JSON outputs for each compilation. Future
patches will change this to deduplicate modules between compilations.

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

Added: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp

Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/lib/Tooling/DependencyScanning/CMakeLists.txt
clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index fd8ed80b143c..76edf150dbee 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -30,15 +30,30 @@ enum class ScanningMode {
   MinimizedSourcePreprocessing
 };
 
+/// The format that is output by the dependency scanner.
+enum class ScanningOutputFormat {
+  /// This is the Makefile compatible dep format. This will include all of the
+  /// deps necessary for an implicit modules build, but won't include any
+  /// intermodule dependency information.
+  Make,
+
+  /// This outputs the full module dependency graph suitable for use for
+  /// explicitly building modules.
+  Full,
+};
+
 /// The dependency scanning service contains the shared state that is used by
 /// the invidual dependency scanning workers.
 class DependencyScanningService {
 public:
-  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager = true,
+  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
+bool ReuseFileManager = true,
 bool SkipExcludedPPRanges = true);
 
   ScanningMode getMode() const { return Mode; }
 
+  ScanningOutputFormat getFormat() const { return Format; }
+
   bool canReuseFileManager() const { return ReuseFileManager; }
 
   bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
@@ -49,6 +64,7 @@ class DependencyScanningService {
 
 private:
   const ScanningMode Mode;
+  const ScanningOutputFormat Format;
   const bool ReuseFileManager;
   /// Set to true to use the preprocessor optimization that skips excluded PP
   /// ranges by bumping the buffer pointer in the lexer instead of lexing the

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index c950cbe167cd..78b49e4fa0c5 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -40,6 +40,7 @@ class DependencyScanningTool {
 StringRef CWD);
 
 private:
+  const ScanningOutputFormat Format;
   DependencyScanningWorker Worker;
   const tooling::CompilationDatabase &Compilations;
 };

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 45c9fb4f029d..689119330c41 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -15,6 +15,8 @@
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include 
@@ -26,7 +28,6 @@ class Depende

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked 2 inline comments as done.
xbolva00 added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:10595
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||

!Info.CheckingPotentialConstantExpression

Basically just to silence test case:
namespace std {
constexpr bool is_constant_evaluated() noexcept {
  return __builtin_is_constant_evaluated();
}
}

In real world code, this will not warn, since call's loc is in system header. 
Leave it?



Comment at: clang/lib/AST/ExprConstant.cpp:10600
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()

Without this condition, it warns 3 times for

if constexpr (std::is_constant_evaluated() == false) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518



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

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 227191.
xbolva00 added a comment.

- Merge branch 'master' of https://github.com/llvm/llvm-project


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp

Index: clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+namespace std {
+constexpr bool is_constant_evaluated() noexcept {
+  return __builtin_is_constant_evaluated();
+}
+}
+
+constexpr int fn1() {
+  if constexpr (std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn2() {
+  if constexpr (!std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn3() {
+  if constexpr (std::is_constant_evaluated() == false) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn4() {
+  if constexpr (__builtin_is_constant_evaluated() == true) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+
+constexpr int fn5() {
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn1() {
+  if (std::is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn2() {
+  if (!__builtin_is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10590,8 +10590,23 @@
 return false;
   }
 
-  case Builtin::BI__builtin_is_constant_evaluated:
+  case Builtin::BI__builtin_is_constant_evaluated: {
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||
+ (Info.CallStackDepth == 2 && E->getNumArgs() == 0 &&
+  Callee->isInStdNamespace() && Callee->getIdentifier() &&
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()
+   : Info.CurrentCall->CallLoc,
+diag::warn_is_constant_evaluated_always_true_constexpr)
+<< (Info.CallStackDepth == 1 ? "__builtin_is_constant_evaluated"
+ : "std::is_constant_evaluated");
+}
+
 return Success(Info.InConstantContext, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -329,6 +329,10 @@
 def note_unimplemented_constexpr_lambda_feature_ast : Note<
 "unimplemented constexpr lambda feature: %0 (coming soon!)">;
 
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in constexpr context">,
+  InGroup>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -2777,6 +2777,13 @@
FD->getIdentifier() && FD->getIdentifier()->isStr("move");
   }
 
+  bool isCallToStdIsConstantEvaluated() const {
+const FunctionDecl *FD = getDirectCallee();
+return getNumArgs() == 0 && FD && FD->isInStdNamespace() &&
+   FD->getIdentifier() &&
+   FD->getIdentifier()->isStr("is_constant_evaluated");
+  }
+
   static bool classof(const Stmt *T) {
 return T->getStmtClass() >= firstCallExprConstant &&
T->getStmtClass() <= lastCallExprConstant;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 227192.
xbolva00 added a comment.

- Merge branch 'master' of https://github.com/llvm/llvm-project
===

Removed old unused change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp

Index: clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+namespace std {
+constexpr bool is_constant_evaluated() noexcept {
+  return __builtin_is_constant_evaluated();
+}
+}
+
+constexpr int fn1() {
+  if constexpr (std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn2() {
+  if constexpr (!std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn3() {
+  if constexpr (std::is_constant_evaluated() == false) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn4() {
+  if constexpr (__builtin_is_constant_evaluated() == true) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+
+constexpr int fn5() {
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in constexpr context}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn1() {
+  if (std::is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn2() {
+  if (!__builtin_is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10590,8 +10590,23 @@
 return false;
   }
 
-  case Builtin::BI__builtin_is_constant_evaluated:
+  case Builtin::BI__builtin_is_constant_evaluated: {
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||
+ (Info.CallStackDepth == 2 && E->getNumArgs() == 0 &&
+  Callee->isInStdNamespace() && Callee->getIdentifier() &&
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()
+   : Info.CurrentCall->CallLoc,
+diag::warn_is_constant_evaluated_always_true_constexpr)
+<< (Info.CallStackDepth == 1 ? "__builtin_is_constant_evaluated"
+ : "std::is_constant_evaluated");
+}
+
 return Success(Info.InConstantContext, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -329,6 +329,10 @@
 def note_unimplemented_constexpr_lambda_feature_ast : Note<
 "unimplemented constexpr lambda feature: %0 (coming soon!)">;
 
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in constexpr context">,
+  InGroup>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1c88d66 - Revert "[clang][clang-scan-deps] Add support for extracting full module dependencies."

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-30T15:16:38-07:00
New Revision: 1c88d662230e79bc2dd2283d753d950c1ad0bed6

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

LOG: Revert "[clang][clang-scan-deps] Add support for extracting full module 
dependencies."

This reverts commit d8a4ef0e685cec1fc73d4953b48220b649d05b40.

This commit broke some of the bots. I believe it's due to nondeterminism. Will 
fix and recommit.

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/lib/Tooling/DependencyScanning/CMakeLists.txt
clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp



diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 76edf150dbee..fd8ed80b143c 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -30,30 +30,15 @@ enum class ScanningMode {
   MinimizedSourcePreprocessing
 };
 
-/// The format that is output by the dependency scanner.
-enum class ScanningOutputFormat {
-  /// This is the Makefile compatible dep format. This will include all of the
-  /// deps necessary for an implicit modules build, but won't include any
-  /// intermodule dependency information.
-  Make,
-
-  /// This outputs the full module dependency graph suitable for use for
-  /// explicitly building modules.
-  Full,
-};
-
 /// The dependency scanning service contains the shared state that is used by
 /// the invidual dependency scanning workers.
 class DependencyScanningService {
 public:
-  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
-bool ReuseFileManager = true,
+  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager = true,
 bool SkipExcludedPPRanges = true);
 
   ScanningMode getMode() const { return Mode; }
 
-  ScanningOutputFormat getFormat() const { return Format; }
-
   bool canReuseFileManager() const { return ReuseFileManager; }
 
   bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
@@ -64,7 +49,6 @@ class DependencyScanningService {
 
 private:
   const ScanningMode Mode;
-  const ScanningOutputFormat Format;
   const bool ReuseFileManager;
   /// Set to true to use the preprocessor optimization that skips excluded PP
   /// ranges by bumping the buffer pointer in the lexer instead of lexing the

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 78b49e4fa0c5..c950cbe167cd 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -40,7 +40,6 @@ class DependencyScanningTool {
 StringRef CWD);
 
 private:
-  const ScanningOutputFormat Format;
   DependencyScanningWorker Worker;
   const tooling::CompilationDatabase &Compilations;
 };

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 689119330c41..45c9fb4f029d 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -15,8 +15,6 @@
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
-#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include 
@@ -28,6 +26,7 @@ class DependencyOutputOptions;
 namespace tooling {
 namespace dependencies {
 
+class DependencyScanningService;
 class DependencyScanningWorkerFilesystem;
 
 class DependencyConsumer {
@@ -37,9 +36,7 @@ class DependencyConsumer {
   virtual void handleFileDependency(co

[clang] 812bdb3 - [MS] Don't reference deleted copy ctors from catchable types

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-10-30T15:06:26-07:00
New Revision: 812bdb3c13210759341e8a1b08b864a539ce9dc7

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

LOG: [MS] Don't reference deleted copy ctors from catchable types

When throwing objects with deleted copy constructors, the copy ctor
field of the catchable type should remain null and the mangle name
changes. This already worked in simple cases, but in cases involving
non-trivial subobjects, sometimes LookupCopyingConstructor could return
a non-null but deleted constructor decl. Skip those and don't reference
them.

Fixes PR43680

Added: 


Modified: 
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenCXX/microsoft-abi-throw.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4fdd15bf466f..67492a2cd463 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -921,7 +921,7 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc,
   // cannot be a simple walk of the class's decls.  Instead, we must 
perform
   // lookup and overload resolution.
   CXXConstructorDecl *CD = LookupCopyingConstructor(Subobject, 0);
-  if (!CD)
+  if (!CD || CD->isDeleted())
 continue;
 
   // Mark the constructor referenced as it is used by this throw 
expression.

diff  --git a/clang/test/CodeGenCXX/microsoft-abi-throw.cpp 
b/clang/test/CodeGenCXX/microsoft-abi-throw.cpp
index f55b94acf1d7..937f2daed705 100644
--- a/clang/test/CodeGenCXX/microsoft-abi-throw.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-throw.cpp
@@ -14,6 +14,8 @@
 // CHECK-DAG: @"_CTA5?AUY@@" = linkonce_odr unnamed_addr constant 
%eh.CatchableTypeArray.5 { i32 5, [5 x %eh.CatchableType*] [%eh.CatchableType* 
@"_CT??_R0?AUY@@@8??0Y@@QAE@ABU0@@Z8", %eh.CatchableType* @"_CT??_R0?AUZ@@@81", 
%eh.CatchableType* @"_CT??_R0?AUW@@@8??0W@@QAE@ABU0@@Z44", %eh.CatchableType* 
@"_CT??_R0?AUM@@@818", %eh.CatchableType* @"_CT??_R0?AUV@@@81044"] }, section 
".xdata", comdat
 // CHECK-DAG: @"_TI5?AUY@@" = linkonce_odr unnamed_addr constant %eh.ThrowInfo 
{ i32 0, i8* bitcast (void (%struct.Y*)* @"??_DY@@QAEXXZ" to i8*), i8* null, 
i8* bitcast (%eh.CatchableTypeArray.5* @"_CTA5?AUY@@" to i8*) }, section 
".xdata", comdat
 // CHECK-DAG: @"_CT??_R0?AUDefault@@@8??_ODefault@@QAEXAAU0@@Z1" = 
linkonce_odr unnamed_addr constant %eh.CatchableType { i32 0, i8* bitcast 
(%rtti.TypeDescriptor13* @"??_R0?AUDefault@@@8" to i8*), i32 0, i32 -1, i32 0, 
i32 1, i8* bitcast (void (%struct.Default*, %struct.Default*)* 
@"??_ODefault@@QAEXAAU0@@Z" to i8*) }, section ".xdata", comdat
+// CHECK-DAG: @"_CT??_R0?AUDeletedCopy@@@81" = linkonce_odr unnamed_addr 
constant %eh.CatchableType { i32 0, i8* bitcast (%rtti.TypeDescriptor17* 
@"??_R0?AUDeletedCopy@@@8" to i8*), i32 0, i32 -1, i32 0, i32 1, i8* null }, 
section ".xdata", comdat
+// CHECk-DAG: @"_CT??_R0?AUMoveOnly@@@84" = linkonce_odr unnamed_addr constant 
%eh.CatchableType { i32 0, i8* bitcast (%rtti.TypeDescriptor14* 
@"??_R0?AUMoveOnly@@@8" to i8*), i32 0, i321-1, i32 0, i32 4, i8* null }, 
section ".xdata", comda
 // CHECK-DAG: @"_CT??_R0?AUVariadic@@@8??_OVariadic@@QAEXAAU0@@Z1" = 
linkonce_odr unnamed_addr constant %eh.CatchableType { i32 0, i8* bitcast 
(%rtti.TypeDescriptor14* @"??_R0?AUVariadic@@@8" to i8*), i32 0, i32 -1, i32 0, 
i32 1, i8* bitcast (void (%struct.Variadic*, %struct.Variadic*)* 
@"??_OVariadic@@QAEXAAU0@@Z" to i8*) }, section ".xdata", comdat
 // CHECK-DAG: 
@"_CT??_R0?AUTemplateWithDefault@@@8??$?_OH@TemplateWithDefault@@QAEXAAU0@@Z1" 
= linkonce_odr unnamed_addr constant %eh.CatchableType { i32 0, i8* bitcast 
(%rtti.TypeDescriptor25* @"??_R0?AUTemplateWithDefault@@@8" to i8*), i32 0, i32 
-1, i32 0, i32 1, i8* bitcast (void (%struct.TemplateWithDefault*, 
%struct.TemplateWithDefault*)* @"??$?_OH@TemplateWithDefault@@QAEXAAU0@@Z" to 
i8*) }, section ".xdata", comdat
 // CHECK-DAG: @"_CTA2$$T" = linkonce_odr unnamed_addr constant 
%eh.CatchableTypeArray.2 { i32 2, [2 x %eh.CatchableType*] [%eh.CatchableType* 
@"_CT??_R0$$T@84", %eh.CatchableType* @"_CT??_R0PAX@84"] }, section ".xdata", 
comdat
@@ -71,6 +73,31 @@ void h(Default &d) {
   throw d;
 }
 
+struct DeletedCopy {
+  DeletedCopy();
+  DeletedCopy(DeletedCopy &&);
+  DeletedCopy(const DeletedCopy &) = delete;
+};
+void throwDeletedCopy() { throw DeletedCopy(); }
+
+
+struct MoveOnly {
+  MoveOnly();
+  MoveOnly(MoveOnly &&);
+  ~MoveOnly();
+  MoveOnly(const MoveOnly &) = delete;
+
+  // For some reason this subobject was important for reproducing PR43680
+  struct HasCopy {
+HasCopy();
+HasCopy(const HasCopy &o);
+~HasCopy();
+int x;
+  } sub;
+};
+
+void throwMoveOnly() { throw MoveOnly(); }
+
 struct Variadic {
   Variadic(Variadic &, ...);

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8a4ef0e685c: [clang][clang-scan-deps] Add support for 
extracting full module dependencies. (authored by Bigcheese).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69420

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -59,6 +59,17 @@
 llvm::cl::init(ScanningMode::MinimizedSourcePreprocessing),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt Format(
+"format", llvm::cl::desc("The output format for the dependencies"),
+llvm::cl::values(clEnumValN(ScanningOutputFormat::Make, "make",
+"Makefile compatible dep file"),
+ clEnumValN(ScanningOutputFormat::Full, "experimental-full",
+"Full dependency graph suitable"
+" for explicitly building modules. This format "
+"is experimental and will change.")),
+llvm::cl::init(ScanningOutputFormat::Make),
+llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -200,7 +211,7 @@
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, ReuseFileManager,
+  DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
 SkipExcludedPPRanges);
 #if LLVM_ENABLE_THREADS
   unsigned NumWorkers =
Index: clang/test/ClangScanDeps/modules-full.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-full.cpp
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: rm -rf %t.module-cache
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: cp %s %t.dir/modules_cdb_input2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb.json > %t.cdb
+//
+// RUN: echo %t.dir > %t.result
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 \
+// RUN:   -mode preprocess-minimized-sources -format experimental-full >> %t.result
+// RUN: cat %t.result | FileCheck --check-prefixes=CHECK %s
+
+#include "header.h"
+
+// CHECK: [[PREFIX:(.*[/\\])+[a-zA-Z0-9.-]+]]
+// CHECK-NEXT: {
+// CHECK-NEXT:  "clang-context-hash": "[[CONTEXT_HASH:[A-Z0-9]+]]",
+// CHECK-NEXT:  "clang-module-deps": [
+// CHECK-NEXT:"header1"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "clang-modules": [
+// CHECK-NEXT:{
+// CHECK-NEXT:  "clang-module-deps": [
+// CHECK-NEXT:"header2"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "clang-modulemap-file": "[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}header.h"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "name": "header1"
+// CHECK-NEXT:},
+// CHECK-NEXT:{
+// CHECK-NEXT:  "clang-module-deps": [],
+// CHECK-NEXT:  "clang-modulemap-file": "[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}header2.h",
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "name": "header2"
+// CHECK-NEXT:}
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}modules_cdb_input2.cpp"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "input-file": "[[PREFIX]]{{[/\\]}}modules_cdb_input2.cpp"
+// CHECK-NEXT:},
+// CHECK-NEXT:{
+// CHECK-NOT:   "clang-context-hash": "[[CONTEXT_HASH]]",
+// CHECK

[clang] d8a4ef0 - [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-30T14:46:17-07:00
New Revision: d8a4ef0e685cec1fc73d4953b48220b649d05b40

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

LOG: [clang][clang-scan-deps] Add support for extracting full module 
dependencies.

This adds experimental support for extracting a Clang module dependency graph
from a compilation database. The output format is experimental and will change.
It is currently a concatenation of JSON outputs for each compilation. Future
patches will change this to deduplicate modules between compilations.

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

Added: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp

Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/lib/Tooling/DependencyScanning/CMakeLists.txt
clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index fd8ed80b143c..76edf150dbee 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -30,15 +30,30 @@ enum class ScanningMode {
   MinimizedSourcePreprocessing
 };
 
+/// The format that is output by the dependency scanner.
+enum class ScanningOutputFormat {
+  /// This is the Makefile compatible dep format. This will include all of the
+  /// deps necessary for an implicit modules build, but won't include any
+  /// intermodule dependency information.
+  Make,
+
+  /// This outputs the full module dependency graph suitable for use for
+  /// explicitly building modules.
+  Full,
+};
+
 /// The dependency scanning service contains the shared state that is used by
 /// the invidual dependency scanning workers.
 class DependencyScanningService {
 public:
-  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager = true,
+  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
+bool ReuseFileManager = true,
 bool SkipExcludedPPRanges = true);
 
   ScanningMode getMode() const { return Mode; }
 
+  ScanningOutputFormat getFormat() const { return Format; }
+
   bool canReuseFileManager() const { return ReuseFileManager; }
 
   bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
@@ -49,6 +64,7 @@ class DependencyScanningService {
 
 private:
   const ScanningMode Mode;
+  const ScanningOutputFormat Format;
   const bool ReuseFileManager;
   /// Set to true to use the preprocessor optimization that skips excluded PP
   /// ranges by bumping the buffer pointer in the lexer instead of lexing the

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index c950cbe167cd..78b49e4fa0c5 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -40,6 +40,7 @@ class DependencyScanningTool {
 StringRef CWD);
 
 private:
+  const ScanningOutputFormat Format;
   DependencyScanningWorker Worker;
   const tooling::CompilationDatabase &Compilations;
 };

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 45c9fb4f029d..689119330c41 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -15,6 +15,8 @@
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include 
@@ -26,7 +28,6 @@ class DependencyOutputOptions;
 namespace tooling {
 namespace dependencies {
 

[PATCH] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

@NoQ, I hope this is what you have suggested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69642



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

[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Bigcheese added reviewers: arphaman, klimek.
Bigcheese added a project: clang.
Herald added a subscriber: dexonsmith.

Previously, given a CompilationDatabase with two commands for the same
source file we would report that file twice with the union of the
dependencies for each command both times.

This was due to the way `ClangTool` runs actions given an input source
file (see the comment in `DependencyScanningTool.cpp`). This commit adds
a `SingleCommandCompilationDatabase` that is created with each
`CompileCommand` in the original CDB, which is then used for each
`ClangTool` invocation. This gives us a single run of
`DependencyScanningAction` per `CompileCommand`.

I looked at using `AllTUsToolExecutor` which is a parallel tool
executor, but I'm not sure it's suitable for `clang-scan-deps` as it
does a lot more sharing of state than `AllTUsToolExecutor` expects.

I've added Manuel as a reviewer to see if there's an obviously better way to do 
this with libTooling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69643

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -97,6 +97,24 @@
   return ObjFileName.str();
 }
 
+class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  virtual std::vector
+  getCompileCommands(StringRef FilePath) const {
+return {Command};
+  }
+
+  virtual std::vector getAllCompileCommands() const {
+return {Command};
+  }
+
+private:
+  tooling::CompileCommand Command;
+};
+
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///
@@ -136,11 +154,6 @@
 
   llvm::cl::PrintOptionValues();
 
-  // By default the tool runs on all inputs in the CDB.
-  std::vector> Inputs;
-  for (const auto &Command : Compilations->getAllCompileCommands())
-Inputs.emplace_back(Command.Filename, Command.Directory);
-
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
   std::make_unique(
@@ -210,8 +223,12 @@
 #endif
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
-WorkerTools.push_back(std::make_unique(
-Service, *AdjustingCompilations));
+WorkerTools.push_back(std::make_unique(Service));
+
+  std::vector Inputs;
+  for (tooling::CompileCommand Cmd :
+   AdjustingCompilations->getAllCompileCommands())
+Inputs.emplace_back(Cmd);
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
@@ -226,20 +243,22 @@
 auto Worker = [I, &Lock, &Index, &Inputs, &HadErrors, &WorkerTools,
&DependencyOS, &Errs]() {
   while (true) {
-std::string Input;
-StringRef CWD;
+const SingleCommandCompilationDatabase *Input;
+std::string Filename;
+std::string CWD;
 // Take the next input.
 {
   std::unique_lock LockGuard(Lock);
   if (Index >= Inputs.size())
 return;
-  const auto &Compilation = Inputs[Index++];
-  Input = Compilation.first;
-  CWD = Compilation.second;
+  Input = &Inputs[Index++];
+  tooling::CompileCommand Cmd = Input->getAllCompileCommands()[0];
+  Filename = std::move(Cmd.Filename);
+  CWD = std::move(Cmd.Directory);
 }
 // Run the tool on it.
-auto MaybeFile = WorkerTools[I]->getDependencyFile(Input, CWD);
-if (handleDependencyToolResult(Input, MaybeFile, DependencyOS, Errs))
+auto MaybeFile = WorkerTools[I]->getDependencyFile(*Input, CWD);
+if (handleDependencyToolResult(Filename, MaybeFile, DependencyOS, Errs))
   HadErrors = true;
   }
 };
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -9,11 +9,11 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   Fil

[PATCH] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Charusso added a parent revision: D69599: [analyzer] DynamicSize: Remove 
'getSizeInElements()' from store.
Charusso added a comment.

@NoQ, I hope this is what you have suggested.


-


Repository:
  rC Clang

https://reviews.llvm.org/D69642

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
  clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicSize.cpp

Index: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
@@ -18,11 +18,17 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicSizeMap, const clang::ento::MemRegion *,
+   clang::ento::DefinedOrUnknownSVal)
+
 namespace clang {
 namespace ento {
 
 DefinedOrUnknownSVal getDynamicSize(ProgramStateRef State, const MemRegion *MR,
 SValBuilder &SVB) {
+  if (const DefinedOrUnknownSVal *Size = State->get(MR))
+return *Size;
+
   return MR->getMemRegionManager().getStaticSize(MR, SVB);
 }
 
@@ -47,5 +53,30 @@
   return SVB.makeIntVal(RegionSize / ElementSize, SVB.getArrayIndexType());
 }
 
+ProgramStateRef setDynamicSizeAssumption(ProgramStateRef State,
+ const MemRegion *MR, SValBuilder &SVB,
+ DefinedOrUnknownSVal Size) {
+  DefinedOrUnknownSVal DynSize = getDynamicSize(State, MR, SVB);
+
+  if ((State = State->assume(SVB.evalEQ(State, DynSize, Size), true)))
+State = State->set(MR, Size);
+
+  return State;
+}
+
+std::pair
+setDynamicSizeDualAssumption(ProgramStateRef State, const MemRegion *MR,
+ SValBuilder &SVB, DefinedOrUnknownSVal Size) {
+  DefinedOrUnknownSVal DynSize = getDynamicSize(State, MR, SVB);
+
+  ProgramStateRef StateEQ, StateNE;
+  std::tie(StateEQ, StateNE) = State->assume(SVB.evalEQ(State, DynSize, Size));
+
+  if (StateEQ)
+StateEQ = StateEQ->set(MR, Size);
+
+  return {StateEQ, StateNE};
+}
+
 } // namespace ento
 } // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -168,13 +168,8 @@
 
   // Finally, assume that the array's size matches the given size.
   const LocationContext *LC = C.getLocationContext();
-  DefinedOrUnknownSVal DynSize =
-  getDynamicSize(state, state->getRegion(VD, LC), svalBuilder);
-
-  DefinedOrUnknownSVal ArraySize = ArraySizeVal.castAs();
-  DefinedOrUnknownSVal sizeIsKnown =
-  svalBuilder.evalEQ(state, DynSize, ArraySize);
-  state = state->assume(sizeIsKnown, true);
+  state = setDynamicSizeAssumption(state, state->getRegion(VD, LC), svalBuilder,
+   ArraySizeVal.castAs());
 
   // Assume should not fail at this point.
   assert(state);
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1403,16 +1403,14 @@
   CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
 
   if (ElementCount.getAs()) {
-DefinedOrUnknownSVal DynSize = getDynamicSize(State, Region, svalBuilder);
-
 // size in Bytes = ElementCount*TypeSize
 SVal SizeInBytes = svalBuilder.evalBinOpNN(
 State, BO_Mul, ElementCount.castAs(),
 svalBuilder.makeArrayIndex(TypeSize.getQuantity()),
 svalBuilder.getArrayIndexType());
-DefinedOrUnknownSVal DynSizeMatchesSize = svalBuilder.evalEQ(
-State, DynSize, SizeInBytes.castAs());
-State = State->assume(DynSizeMatchesSize, true);
+
+State = setDynamicSizeAssumption(
+State, Region, svalBuilder, SizeInBytes.castAs());
   }
   return State;
 }
@@ -1541,13 +1539,7 @@
 return nullptr;
   if (Optional DefinedSize =
   Size.getAs()) {
-DefinedOrUnknownSVal DynSize = getDynamicSize(State, R, svalBuilder);
-
-DefinedOrUnknownSVal DynSizeMatchesSize =
-svalBuilder.evalEQ(State, DynSize, *DefinedSize);
-
-State = State->assume(DynSizeMatchesSize, true);
-assert(State);
+State = setDynamicSizeAssumption(State, R, svalBuilder, *DefinedSize);
   }
 

[PATCH] D69564: Include the mangled name in -ast-dump=json

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> I am planning to use this feature to make update_cc_checks.py less fragile by 
> obtaining the mangled names directly from -ast-dump=json.

`update_cc_checks.py` -> `update_cc_test_checks.py`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69564



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

[clang-tools-extra] 3a0c86a - Add a test file that was missed in 4de6b1586807285e20a5db6596519c2336a64568

[Aaron Ballman via cfe-commits]

Author: Vladimir Plyashkun
Date: 2019-10-30T16:55:43-04:00
New Revision: 3a0c86a179efcd5c108521afade3aab6083376f7

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

LOG: Add a test file that was missed in 4de6b1586807285e20a5db6596519c2336a64568

Added: 

clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp

Modified: 


Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp
new file mode 100644
index ..643c36af0016
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand 
with a binary bitwise operator
+
+  unsigned URes2 = URes << 1; //Ok
+
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  test1 = 1 << 12, //Ok
+  test2 = one << two,
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of a signed integer operand 
with a binary bitwise operator
+  test3 = 1u << 12, //Ok
+};



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

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 227174.
cchen added a comment.

Rebase and fix add OpenMP 50 guard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_update_ast_print.cpp

Index: clang/test/OpenMP/target_update_ast_print.cpp
===
--- clang/test/OpenMP/target_update_ast_print.cpp
+++ clang/test/OpenMP/target_update_ast_print.cpp
@@ -20,6 +20,40 @@
 #pragma omp target update to(a) if(l>5) device(l) nowait depend(inout:l)
 
 #pragma omp target update from(b) if(l<5) device(l-1) nowait depend(inout:l)
+
+  U marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+
+#pragma omp target update from(marr[2][0:2][0:2])
+
+#pragma omp target update to(marr[:][0:2][0:2])
+
+#pragma omp target update from(marr[:][0:2][0:2])
+
+#pragma omp target update to(marr[:][:l][l:])
+
+#pragma omp target update from(marr[:][:l][l:])
+
+#pragma omp target update to(marr[:2][:1][:])
+
+#pragma omp target update from(marr[:2][:1][:])
+
+#pragma omp target update to(marr[:2][:][:1])
+
+#pragma omp target update from(marr[:2][:][:1])
+
+#pragma omp target update to(marr[:2][:][1:])
+
+#pragma omp target update from(marr[:2][:][1:])
+
+#pragma omp target update to(marr[:1][3:2][:2])
+
+#pragma omp target update from(marr[:1][3:2][:2])
+
+#pragma omp target update to(marr[:1][:2][0])
+
+#pragma omp target update from(marr[:1][:2][0])
+
   return a + targ + (T)b;
 }
 // CHECK:  static T a;
@@ -37,6 +71,23 @@
 // CHECK-NEXT: int l;
 // CHECK-NEXT: #pragma omp target update to(a) if(l > 5) device(l) nowait depend(inout : l)
 // CHECK-NEXT: #pragma omp target update from(b) if(l < 5) device(l - 1) nowait depend(inout : l)
+// CHECK-NEXT: marr[10][10][10];
+// CHECK-NEXT: #pragma omp target update to(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][:l][l:])
+// CHECK-NEXT: #pragma omp target update from(marr[:][:l][l:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][:2][0])
 
 int main(int argc, char **argv) {
   static int a;
@@ -50,6 +101,37 @@
 // CHECK-NEXT: #pragma omp target update to(a) if(f > 0.) device(n) nowait depend(in : n)
 #pragma omp target update from(f) if(f<0.0) device(n+1) nowait depend(in:n)
 // CHECK-NEXT: #pragma omp target update from(f) if(f < 0.) device(n + 1) nowait depend(in : n)
+
+  float marr[10][10][10];
+// CHECK-NEXT: marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[2][0:2][0:2])
+#pragma omp target update from(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[2][0:2][0:2])
+#pragma omp target update to(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][0:2][0:2])
+#pragma omp target update from(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:][0:2][0:2])
+#pragma omp target update to(marr[:][:n][n:])
+// CHECK-NEXT: #pragma omp target update to(marr[:][:n][n:])
+#pragma omp target update from(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:1][:])
+#pragma omp target update to(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][:1])
+#pragma omp target update from(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][:1])
+#pragma omp target update to(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][1:])
+#pragma omp target update from(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][1:])
+#pragma omp target update to(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][3:2][:2])
+#pragma omp target update from(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][3:2][:2])
+#pragma omp target update to(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][:2][0])
+#pragma omp target update from(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][:2][0])
   return foo(argc, f) + foo(argv[0][0], f) + a;
 }
 
Index: clang/lib/Sema/SemaOpenMP.cpp

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D68539#1727759 , @Daniel599 wrote:

> In D68539#1727503 , @aaron.ballman 
> wrote:
>
> > Thank you for the patch! I've commit on your behalf in 
> > e477988309dbde214a6d16ec690a416882714aac 
> > 
>
>
> Thank you for the commit,
>  just a little question. shouldn't the test file 
> `clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp`
>  be added?


It absolutely should have been, but I don't see why it didn't get added. I've 
fixed it in 7b6174bb147957b4695023ae57c95ca07af7b917 
, thank 
you for bringing it to my attention!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539



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

[clang-tools-extra] 7b6174b - Add a test file that was missed in e477988309dbde214a6d16ec690a416882714aac

[Aaron Ballman via cfe-commits]

Author: Daniel
Date: 2019-10-30T16:52:19-04:00
New Revision: 7b6174bb147957b4695023ae57c95ca07af7b917

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

LOG: Add a test file that was missed in e477988309dbde214a6d16ec690a416882714aac

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp

Modified: 


Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
new file mode 100644
index ..12ddcf55c119
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: 
lower_case} \
+// RUN:   ]}'
+
+int func(int Break) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for 
parameter 'Break'; cannot be fixed because 'break' would conflict with a keyword
+  // CHECK-FIXES: {{^}}int func(int Break) {{{$}}
+  if (Break == 1) {
+// CHECK-FIXES: {{^}}  if (Break == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}
+
+#define foo 3
+int func2(int Foo) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for 
parameter 'Foo'; cannot be fixed because 'foo' would conflict with a macro 
definition
+  // CHECK-FIXES: {{^}}int func2(int Foo) {{{$}}
+  if (Foo == 1) {
+// CHECK-FIXES: {{^}}  if (Foo == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}



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

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:951
 
   Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
   return Res;

Another interesting question is whether this `E` should include the xvalue 
cast, if we did it. I have no idea what would be expected here. I'm tending 
towards replacing `E` by `Arg`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68845



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1354
+  static bool isDeductionGuide(FormatToken &Current) {
+// Look for a deduction guide A()...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&

Parentheses not matching is comment.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1365
+if (TemplateCloser->is(tok::greater))
+  NestingLevel--;
+if (TemplateCloser->is(tok::kw_decltype))

Does this work?  What about `A() -> A<(3 < 2)>;`?



Comment at: clang/lib/Format/TokenAnnotator.cpp:1366
+  NestingLevel--;
+if (TemplateCloser->is(tok::kw_decltype))
+  return false;

What's this for?  What about `A() -> Afoo<1>>);`

I guess we don't have to look for the end of template, because a class X can't 
refer to its member called X with `->` because that member is constructor.  The 
approach to match the identifiers may not work in the future but may work fine 
for now.



Comment at: clang/unittests/Format/FormatTest.cpp:4992
+  verifyFormat("x = p->foo<3>();");
+  verifyFormat("A()->Afoo<3>())>;");
+}

This looks like a deduction guide.  It should be formatted as `A() -> 
Afoo<3>())>;` I assume?  If you cannot cover this case in this 
patch, this test case needs a comment.


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

https://reviews.llvm.org/D69577



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

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[Daniel via Phabricator via cfe-commits]

Daniel599 added a comment.

In D68539#1727503 , @aaron.ballman 
wrote:

> Thank you for the patch! I've commit on your behalf in 
> e477988309dbde214a6d16ec690a416882714aac 
> 


Thank you for the commit,
just a little question. shouldn't the test file 
`clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp`
 be added?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539



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

[clang] bb06149 - Fix __attribute__((enable_if)) to treat arguments with side-effects as

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-10-30T13:39:29-07:00
New Revision: bb061491316bbd516a7551fe36128ead0935010d

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

LOG: Fix __attribute__((enable_if)) to treat arguments with side-effects as
non-constant.

We previously failed the entire condition evaluation if an unmodeled
side-effect was encountered in an argument, even if that argument was
unused in the attribute's condition.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/enable_if.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 7ed082185670..ecd9eda76488 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1039,10 +1039,13 @@ namespace {
 /// cleanups would have had a side-effect, note that as an unmodeled
 /// side-effect and return false. Otherwise, return true.
 bool discardCleanups() {
-  for (Cleanup &C : CleanupStack)
-if (C.hasSideEffect())
-  if (!noteSideEffect())
-return false;
+  for (Cleanup &C : CleanupStack) {
+if (C.hasSideEffect() && !noteSideEffect()) {
+  CleanupStack.clear();
+  return false;
+}
+  }
+  CleanupStack.clear();
   return true;
 }
 
@@ -14340,27 +14343,41 @@ bool Expr::EvaluateWithSubstitution(APValue &Value, 
ASTContext &Ctx,
 assert(MD && "Don't provide `this` for non-methods.");
 assert(!MD->isStatic() && "Don't provide `this` for static methods.");
 #endif
-if (EvaluateObjectArgument(Info, This, ThisVal))
+if (!This->isValueDependent() &&
+EvaluateObjectArgument(Info, This, ThisVal) &&
+!Info.EvalStatus.HasSideEffects)
   ThisPtr = &ThisVal;
-if (Info.EvalStatus.HasSideEffects)
-  return false;
+
+// Ignore any side-effects from a failed evaluation. This is safe because
+// they can't interfere with any other argument evaluation.
+Info.EvalStatus.HasSideEffects = false;
   }
 
   ArgVector ArgValues(Args.size());
   for (ArrayRef::iterator I = Args.begin(), E = Args.end();
I != E; ++I) {
 if ((*I)->isValueDependent() ||
-!Evaluate(ArgValues[I - Args.begin()], Info, *I))
+!Evaluate(ArgValues[I - Args.begin()], Info, *I) ||
+Info.EvalStatus.HasSideEffects)
   // If evaluation fails, throw away the argument entirely.
   ArgValues[I - Args.begin()] = APValue();
-if (Info.EvalStatus.HasSideEffects)
-  return false;
+
+// Ignore any side-effects from a failed evaluation. This is safe because
+// they can't interfere with any other argument evaluation.
+Info.EvalStatus.HasSideEffects = false;
   }
 
+  // Parameter cleanups happen in the caller and are not part of this
+  // evaluation.
+  Info.discardCleanups();
+  Info.EvalStatus.HasSideEffects = false;
+
   // Build fake call to Callee.
   CallStackFrame Frame(Info, Callee->getLocation(), Callee, ThisPtr,
ArgValues.data());
-  return Evaluate(Value, Info, this) && Info.discardCleanups() &&
+  // FIXME: Missing ExprWithCleanups in enable_if conditions?
+  FullExpressionRAII Scope(Info);
+  return Evaluate(Value, Info, this) && Scope.destroy() &&
  !Info.EvalStatus.HasSideEffects;
 }
 

diff  --git a/clang/test/SemaCXX/enable_if.cpp 
b/clang/test/SemaCXX/enable_if.cpp
index fd1375136a2e..37664276e470 100644
--- a/clang/test/SemaCXX/enable_if.cpp
+++ b/clang/test/SemaCXX/enable_if.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
-
+// RUN: %clang_cc1 -std=c++2a -verify %s
 typedef int (*fp)(int);
 int surrogate(int);
 struct Incomplete;  // expected-note{{forward declaration of 'Incomplete'}} \
@@ -533,3 +533,31 @@ namespace StringLiteralDetector {
   }
 }
 
+namespace IgnoreUnusedArgSideEffects {
+  struct A { ~A(); };
+  void f(A a, bool b) __attribute__((enable_if(b, ""))); // expected-note 
2-3{{disabled}}
+  void test() {
+f(A(), true);
+f(A(), false); // expected-error {{no matching function}}
+int n;
+f((n = 1, A()), true);
+f(A(), (n = 1, true)); // expected-error {{no matching function}}
+f(A(), (A(), true));
+  }
+
+#if __cplusplus > 201702L
+  struct B { constexpr ~B() {} bool b; };
+  void g(B b) __attribute__((enable_if(b.b, ""))); // expected-note 
{{disabled}}
+  void test2() {
+g(B{true});
+g(B{false}); // expected-error {{no matching function}}
+f(A(), B{true}.b);
+f(A(), B{false}.b); // expected-error {{no matching function}}
+  }
+
+  // First condition is non-constant due to non-constexpr destructor of A.
+  int &h() __attribute__((enable_if((A(), true), "")));
+  float &h() __attribute__((enable_if((B(), true), "")));
+  float &x = h();
+#endif
+}



__

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 227169.
cchen added a comment.

Remove the restriction rule for array section to another patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_update_ast_print.cpp

Index: clang/test/OpenMP/target_update_ast_print.cpp
===
--- clang/test/OpenMP/target_update_ast_print.cpp
+++ clang/test/OpenMP/target_update_ast_print.cpp
@@ -20,6 +20,40 @@
 #pragma omp target update to(a) if(l>5) device(l) nowait depend(inout:l)
 
 #pragma omp target update from(b) if(l<5) device(l-1) nowait depend(inout:l)
+
+  U marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+
+#pragma omp target update from(marr[2][0:2][0:2])
+
+#pragma omp target update to(marr[:][0:2][0:2])
+
+#pragma omp target update from(marr[:][0:2][0:2])
+
+#pragma omp target update to(marr[:][:l][l:])
+
+#pragma omp target update from(marr[:][:l][l:])
+
+#pragma omp target update to(marr[:2][:1][:])
+
+#pragma omp target update from(marr[:2][:1][:])
+
+#pragma omp target update to(marr[:2][:][:1])
+
+#pragma omp target update from(marr[:2][:][:1])
+
+#pragma omp target update to(marr[:2][:][1:])
+
+#pragma omp target update from(marr[:2][:][1:])
+
+#pragma omp target update to(marr[:1][3:2][:2])
+
+#pragma omp target update from(marr[:1][3:2][:2])
+
+#pragma omp target update to(marr[:1][:2][0])
+
+#pragma omp target update from(marr[:1][:2][0])
+
   return a + targ + (T)b;
 }
 // CHECK:  static T a;
@@ -37,6 +71,23 @@
 // CHECK-NEXT: int l;
 // CHECK-NEXT: #pragma omp target update to(a) if(l > 5) device(l) nowait depend(inout : l)
 // CHECK-NEXT: #pragma omp target update from(b) if(l < 5) device(l - 1) nowait depend(inout : l)
+// CHECK-NEXT: marr[10][10][10];
+// CHECK-NEXT: #pragma omp target update to(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][:l][l:])
+// CHECK-NEXT: #pragma omp target update from(marr[:][:l][l:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][:2][0])
 
 int main(int argc, char **argv) {
   static int a;
@@ -50,6 +101,37 @@
 // CHECK-NEXT: #pragma omp target update to(a) if(f > 0.) device(n) nowait depend(in : n)
 #pragma omp target update from(f) if(f<0.0) device(n+1) nowait depend(in:n)
 // CHECK-NEXT: #pragma omp target update from(f) if(f < 0.) device(n + 1) nowait depend(in : n)
+
+  float marr[10][10][10];
+// CHECK-NEXT: marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[2][0:2][0:2])
+#pragma omp target update from(marr[2][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[2][0:2][0:2])
+#pragma omp target update to(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:][0:2][0:2])
+#pragma omp target update from(marr[:][0:2][0:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:][0:2][0:2])
+#pragma omp target update to(marr[:][:n][n:])
+// CHECK-NEXT: #pragma omp target update to(marr[:][:n][n:])
+#pragma omp target update from(marr[:2][:1][:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:1][:])
+#pragma omp target update to(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][:1])
+#pragma omp target update from(marr[:2][:][:1])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][:1])
+#pragma omp target update to(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update to(marr[:2][:][1:])
+#pragma omp target update from(marr[:2][:][1:])
+// CHECK-NEXT: #pragma omp target update from(marr[:2][:][1:])
+#pragma omp target update to(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][3:2][:2])
+#pragma omp target update from(marr[:1][3:2][:2])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][3:2][:2])
+#pragma omp target update to(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update to(marr[:1][:2][0])
+#pragma omp target update from(marr[:1][:2][0])
+// CHECK-NEXT: #pragma omp target update from(marr[:1][:2][0])
   return foo(argc, f) + foo(argv[0][0], f) + a;
 }
 
Index: clang/lib/Sema/SemaOpe

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

Revised direction in https://reviews.llvm.org/D69638


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[PATCH] D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable()

[David Tenty via Phabricator via cfe-commits]

daltenty created this revision.
Herald added subscribers: llvm-commits, cfe-commits, mgorny.
Herald added projects: clang, LLVM.
daltenty edited the summary of this revision.
daltenty added reviewers: hubert.reinterpretcast, beanz, lhames.
daltenty added a reverted change: D69356: [NFC] Rename LLVM_NO_DEAD_STRIP.

this allows us to move logic about when it is appropriate set
LLVM_NO_DEAD_STRIP out of each tool and into add_llvm_executable,
which will enable future platform specific handling.

This is a follow on to the reverted D69356 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69638

Files:
  clang/tools/driver/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/llc/CMakeLists.txt
  llvm/tools/opt/CMakeLists.txt

Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -24,9 +24,6 @@
   Passes
   )
 
-# Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
-
 add_llvm_tool(opt
   AnalysisWrappers.cpp
   BreakpointPrinter.cpp
@@ -39,6 +36,7 @@
 
   DEPENDS
   intrinsics_gen
+  SUPPORT_PLUGINS
   )
 export_executable_symbols(opt)
 
Index: llvm/tools/llc/CMakeLists.txt
===
--- llvm/tools/llc/CMakeLists.txt
+++ llvm/tools/llc/CMakeLists.txt
@@ -19,13 +19,11 @@
   Vectorize
   )
 
-# Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
-
 add_llvm_tool(llc
   llc.cpp
 
   DEPENDS
   intrinsics_gen
+  SUPPORT_PLUGINS
   )
 export_executable_symbols(llc)
Index: llvm/tools/bugpoint/CMakeLists.txt
===
--- llvm/tools/bugpoint/CMakeLists.txt
+++ llvm/tools/bugpoint/CMakeLists.txt
@@ -21,9 +21,6 @@
   Vectorize
   )
 
-# Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
-
 add_llvm_tool(bugpoint
   BugDriver.cpp
   CrashDebugger.cpp
@@ -37,6 +34,7 @@
 
   DEPENDS
   intrinsics_gen
+  SUPPORT_PLUGINS
   )
 export_executable_symbols(bugpoint)
 
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -732,7 +732,7 @@
 
 macro(add_llvm_executable name)
   cmake_parse_arguments(ARG
-"DISABLE_LLVM_LINK_LLVM_DYLIB;IGNORE_EXTERNALIZE_DEBUGINFO;NO_INSTALL_RPATH"
+"DISABLE_LLVM_LINK_LLVM_DYLIB;IGNORE_EXTERNALIZE_DEBUGINFO;NO_INSTALL_RPATH;SUPPORT_PLUGINS"
 "ENTITLEMENTS;BUNDLE_PATH"
 "DEPENDS"
 ${ARGN})
@@ -782,6 +782,11 @@
   if(NOT LLVM_ENABLE_OBJLIB)
 llvm_update_compile_flags(${name})
   endif()
+
+  if (ARG_SUPPORT_PLUGINS)
+set(LLVM_NO_DEAD_STRIP 1)
+  endif()
+
   add_link_opts( ${name} )
 
   # Do not add -Dname_EXPORTS to the command-line when building files in this
Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -19,12 +19,6 @@
 
 option(CLANG_PLUGIN_SUPPORT "Build clang with plugin support" ON)
 
-# Support plugins. This must be before add_clang_executable as it reads
-# LLVM_NO_DEAD_STRIP.
-if(CLANG_PLUGIN_SUPPORT)
-  set(LLVM_NO_DEAD_STRIP 1)
-endif()
-
 if(NOT CLANG_BUILT_STANDALONE)
   set(tablegen_deps intrinsics_gen)
 endif()
@@ -37,6 +31,7 @@
 
   DEPENDS
   ${tablegen_deps}
+  SUPPORT_PLUGINS
   )
 
 clang_target_link_libraries(clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Some of the previous comments are not fixed.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9100
+def note_omp_defaultmap_attr_none : Note<
+  "explicit data sharing attribute, data mapping attribute, or is_devise_ptr 
clause requested here">;
 def err_omp_wrong_dsa : Error<

is_device_ptr, not is_devise_ptr




Comment at: clang/lib/Sema/SemaOpenMP.cpp:1969-1972
+  (DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_alloc ||
+   DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_to ||

cchen wrote:
> ABataev wrote:
> > I think, alloc and to scalars also must be captured by value. Moreover, 
> > seems to me, alloc scalars should not be captured at all since their 
> > behavior is very similar to the behavior of private variables.
> Shouldn't defaultmap(alloc/to:scalar) have the same behavior (copy by ref) as 
> map(alloc/to) for scalar? Also, I don't think defaultmap(alloc:scalar) should 
> be the same as private (not copy at all) since you still need to register the 
> scalar address on the device.
Ah, yes, I forgot that we actually allocate variables on the host. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D69498#1727626 , @mehdi_amini wrote:

> In D69498#1727546 , @jdoerfert wrote:
>
> > In D69498#1727419 , @mehdi_amini 
> > wrote:
> >
> > > In D69498#1727080 , @jdoerfert 
> > > wrote:
> > >
> > > > Let me quote @arsenm here because this is so important: "Basically no 
> > > > frontend has gotten this right, including clang and non-clang 
> > > > frontends." For me, that statement alone is reason to change the status 
> > > > quo.
> > >
> > >
> > > I am not convinced by this: this seems overly simplistic. The fact that 
> > > convergent wasn't rolled out properly in frontend can also been as an 
> > > artifact of the past.
> >
> >
> > Evidence suggest this problem will resurface over and over again, calling 
> > it an artifact of the past is dangerous and not helpful.
>
>
> Can you provide the "evidence"? So far the only thing I have seen is 
> historical and closely tied to how convergence was rolled out AFAICT.


Recently, as mentioned a few times before, OpenMP target offloading gets/got it 
wrong long after convergence was rolled out.
r238264 introduced convergent in 2015, r315094 fixed it in part for OpenCL in 
2017 (with the idea to remove noduplicate which was also broken!) a year after 
OpenCL support was started.

>>> There is a logical gap between this and concluding that the only solution 
>>> is to change the default.
>> 
>> There is no gap.
> 
> Bluntly contradicting what I wrote without apparently trying to understand my 
> point or trying to explain your point with respect to what I'm trying to 
> explain is not gonna get me to agree with you magically, I'm not sure what 
> you expect here other than shutting down the discussion and leaving us in a 
> frustrating disagreement and lack of understanding with this?

You removed my explanation that directly follows my statement. Please do not 
use my quotes out of context like this to blame me.

To reiterate what actually followed my above quote 
: If we have a "by default 
unsound behavior" we will by design have cases we miss, e.g., anything new not 
added with this special case in mind. So changing the default to the "by 
construction sound" case, is in my opinion the only way to prevent that. That 
being said, I already agreed that this might not be necessary for every target, 
e.g., based on the data layout. However, I do try to bring forward reasons why 
a "split in semantics" on this level will hurt us as a community and 
maintainers of the IR continuously.


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

https://reviews.llvm.org/D69498



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

[PATCH] D69498: IR: Invert convergent attribute handling

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

As I've said before, I absolutely view this polarity flip as good for GPU 
compilers, and I don't mind Clang needing to take some patches to update how we 
operate when targeting the GPU and to update some GPU-specific tests.  I do not 
think we should view GPU targets as "marginal".  But I do think that GPUs have 
different base semantics from CPUs (under normal, non-vector-transformed rules) 
and that it's not appropriate to unconditionally make those weaker GPU 
semantics the base semantics of LLVM IR.


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

https://reviews.llvm.org/D69498



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69498#1727626 , @mehdi_amini wrote:

> In D69498#1727546 , @jdoerfert wrote:
>
> > In D69498#1727419 , @mehdi_amini 
> > wrote:
> >
> > > In D69498#1727080 , @jdoerfert 
> > > wrote:
> > >
> > > > Let me quote @arsenm here because this is so important: "Basically no 
> > > > frontend has gotten this right, including clang and non-clang 
> > > > frontends." For me, that statement alone is reason to change the status 
> > > > quo.
> > >
> > >
> > > I am not convinced by this: this seems overly simplistic. The fact that 
> > > convergent wasn't rolled out properly in frontend can also been as an 
> > > artifact of the past.
> >
> >
> > Evidence suggest this problem will resurface over and over again, calling 
> > it an artifact of the past is dangerous and not helpful.
>
>
> Can you provide the "evidence"? So far the only thing I have seen is 
> historical and closely tied to how convergence was rolled out AFAICT.


I'm not sure what other evidence could exist for how something is handled in 
practice besides historical record. The current design requires a higher 
awareness at all times to avoid subtlety breaking code, which I think is 
fundamentally more error prone.


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

https://reviews.llvm.org/D69498



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

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Style guide suggesting enforcing a space after template keyword

https://mesos.readthedocs.io/en/1.1.0/c++-style-guide/


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

https://reviews.llvm.org/D14927



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 227166.
MyDeveloperDay added a comment.

move detection of deduction guides into a function, add additional negative 
tests


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

https://reviews.llvm.org/D69577

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,21 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+
+  // Ensure not deduction guides.
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");
+  verifyFormat("A()->Afoo<3>())>;");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,6 +1350,38 @@
 }
   }
 
+  static bool isDeductionGuide(FormatToken &Current) {
+// Look for a deduction guide A()...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.
+  FormatToken *TemplateCloser = Current.Next->Next;
+  int NestingLevel = 1;
+  while (TemplateCloser && NestingLevel > 0) {
+TemplateCloser = TemplateCloser->Next;
+if (TemplateCloser->is(tok::less))
+  NestingLevel++;
+if (TemplateCloser->is(tok::greater))
+  NestingLevel--;
+if (TemplateCloser->is(tok::kw_decltype))
+  return false;
+  }
+  // Assuming we have found the end of the template ensure its followed
+  // with a ;
+  if (TemplateCloser && TemplateCloser->Next &&
+  TemplateCloser->Next->is(tok::semi) &&
+  Current.Previous->MatchingParen) {
+// Determine if the identifier `A` prior to the A<..>; is the same as
+// prior to the A(..)
+FormatToken *LeadingIdentifier =
+Current.Previous->MatchingParen->Previous;
+return (LeadingIdentifier &&
+LeadingIdentifier->TokenText == Current.Next->TokenText);
+  }
+}
+return false;
+  }
+
   void determineTokenType(FormatToken &Current) {
 if (!Current.is(TT_Unknown))
   // The token type is already known.
@@ -1397,6 +1429,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+
+} else if (isDeductionGuide(Current)) {
+  // Deduction guides trailing arrow "...) -> A;".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,21 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+
+  // Ensure not deduction guides.
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");
+  verifyFormat("A()->Afoo<3>())>;");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,6 +1350,38 @@
 }
   }
 
+  static bool isDeductionGuide(FormatToken &Current) {
+// Look for a deduction guide A()...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.
+  FormatToken *TemplateCloser = Current.Next->Next;
+  int NestingLevel =

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;

lichray wrote:
> MyDeveloperDay wrote:
> > lichray wrote:
> > > klimek wrote:
> > > > Why doesn't this trigger on function templates:
> > > >   c()->f();
> > > > 
> > > Comparing to the `else if` branch above, several questions can arise:
> > > 
> > > 1. Has deduction-guide be considered a declaration (it is, of course, in 
> > > standard)?  If yes, without `MustBeDeclaration`, how `x = p->foo<3>();` 
> > > being formatted?
> > > 2. Without restrictions on `NestingLevel`, how `A() -> 
> > > Afoo<3>())>;` being formatted?
> > > 3. How `x()->foo<1>;` being formatted?  What's the difference between 
> > > this and a deduction-guide?  A deduction-guide has to follow 
> > > `TheSameType(...) -> TheSameType<>;` and appears only at namespace 
> > > level, do these help?
> > > 
> > > Oh no, `auto x = p -> foo<1>();` this is a bug (I will look for bug 
> > > reports, don't mind).
> > This case I agree is wrong  (but that comes from the existing rule no?)
> > ```
> > auto x = p -> foo<1>();
> > ```
> > 
> > The other examples are currently as follows
> > ```
> > c()->f();
> > x()->foo<1>;
> > x = p->foo<3>();
> > A()->Afoo<3>())>;
> > ```
> > 
> > This is how they look from the last build @hans  made on the 17th October
> > 
> > ```
> > auto x = p -> foo<1>();
> > c()->f();
> > x()->foo<1>;
> > x = p->foo<3>();
> > A()->Afoo<3>())>;
> > ```
> > 
> > Debug info
> > 
> > ```
> > 
> > AnnotatedTokens(L=0):
> >  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=auto L=4 PPK=2 FakeLParens=2/ 
> > FakeRParens=0 II=0x2c52129f718 Text='auto'
> >  M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=220 Name=identifier L=6 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x2c5212a2da8 Text='x'
> >  M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=22 Name=equal L=8 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='='
> >  M=0 C=1 T=Unknown S=1 B=0 BK=0 P=22 Name=identifier L=10 PPK=2 
> > FakeLParens=0/ FakeRParens=0 II=0x2c5212a2dd8 Text='p'
> >  M=0 C=1 T=TrailingReturnArrow S=1 B=0 BK=0 P=23 Name=arrow L=13 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='->'
> >  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=23 Name=identifier L=17 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x2c5212a2d78 Text='foo'
> >  M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=18 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='<'
> >  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=numeric_constant L=19 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='1'
> >  M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=20 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='>'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=21 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text='('
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=59 Name=r_paren L=22 PPK=2 FakeLParens= 
> > FakeRParens=2 II=0x0 Text=')'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=23 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text=';'
> > 
> > AnnotatedTokens(L=0):
> >  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 
> > FakeLParens=0/ FakeRParens=0 II=0x2c5212a2e08 Text='c'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text='('
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text=')'
> >  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=170 Name=arrow L=5 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text='->'
> >  M=0 C=0 T=TrailingAnnotation S=0 B=0 BK=0 P=190 Name=identifier L=6 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x2c5212a2d48 Text='f'
> >  M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=7 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='<'
> >  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=int L=10 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x2c52129fa50 Text='int'
> >  M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=11 PPK=2 
> > FakeLParens= FakeRParens=0 II=0x0 Text='>'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=12 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text='('
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=13 PPK=2 FakeLParens= 
> > FakeRParens=1 II=0x0 Text=')'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=14 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text=';'
> > 
> > AnnotatedTokens(L=0):
> >  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 
> > FakeLParens=0/ FakeRParens=0 II=0x2c5212a2da8 Text='x'
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
> > FakeRParens=0 II=0x0 Text='('
> >  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
> > F

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Not answering inline because comments are getting longer.

> I still think that it makes sense to do implicit-move on any control-flow 
> construct that "exits" the current function. Right now that's `return` and 
> `throw`, and they both do implicit-move (albeit inconsistently with plenty of 
> implementation divergence documented in P1155 ). 
> C++2a adds `co_return` as another way to "exit." I think it would be 
> un-graceful and inconsistent to permit `return p` but require `co_return 
> std::move(p)`.

Even with `return` it's not obvious where to draw the line: basically we still 
require that nothing non-trivial happens between the `ReturnStmt` and the 
`DeclRefExpr`, so for example `return f(x)` won't be turned into `return 
f(std::move(x))`, and `return a + b` won't be turned into `return std::move(a) 
+ std::move(b)`. It's clear why this can't always happen: if an lvalue is 
referenced more than once, we might move it more than once. But one could argue 
that we should still move all local variables that only occur once. I work in a 
code base with a lot of code like

  SomeBuilder b;
  b.doStuff();
  return std::move(b).getResult();

calling a `getResult() &&` method at the end. It would be nice if I could omit 
the move. But I see why that's hard.

So basically we have the cases where implicit move is more or less necessary 
(because explicit moving would prevent NRVO), and we have the cases where it 
would be possible. And the difficulty is to find the right cutoff. If I 
understand this correctly, the current rules basically say that if we directly 
return a `DeclRefExpr`, possibly with implicit conversions, that will be 
implicitly moved, if possible. (Although it's a bit weird that we silently fall 
back in the case of ambiguity.)

With `co_return` the implicit move is never “necessary”, because NRVO can't 
happen, so we could just never do it. You're right that could be considered an 
inconsistency, but it wouldn't be hard to remember: "for `co_return`, always 
move."

> You've noticed that to codegen `t.return_value(x)` requires not just overload 
> resolution, but actually figuring out whether `task::return_value` is a 
> function at all —

Function templates definitely make sense to me, I also added test cases for 
that.

> it might be a static data member of function-pointer type, or something even 
> wilder 
> .

It could also be an object of class type with an `operator()`. I'm actually not 
sure if the standard intentionally allows all these variants.

(Unrelated, but in that blog post you write “for backward compatibility, C++17 
had to make `noexcept` function pointers implicitly convertible to 
non-`noexcept` function pointers.” I don't see this as a matter of backward 
compatibility: just like you can implicitly cast a reference/pointer to 
`const`, you should be able to cast `noexcept` away from pointers/references.)

> But eliminating implicit-move won't make that complexity go away, will it? 
> Doing implicit-move just means doing the required overload resolution twice 
> instead of once. That should be easy, compared to the rest of the mess.

Initially I thought so, and I wrote some very elegant (yet unfortunately wrong) 
code. I called `buildPromiseCall`, which returns an `ExprResult`, and if that 
was invalid I tried again without the xvalue conversion. Unfortunately, if that 
overload resolution fails, I get an error message //even if I discard the 
resulting expression//, and compilation fails. (I would also get warnings, if 
there are any.) So I looked into the call stack if I could disentangle the 
overload resolution from the diagnostics, but that wasn't so easy. That's why 
this is so much code.

At first I thought this was weird and there should be functionality for this in 
`Sema`, but I think there isn't. I believe that's because there is basically no 
precedent to “do overload resolution, and if that fails, silently do something 
else instead”. Normally, if overload resolution fails, that's just an immediate 
error, no need to wait for anything else. So basically I need to simulate 
`Sema::BuildCallExpr`, but without throwing errors. Fortunately many code paths 
aren't needed, so it's not terribly complicated, but I'm still missing at least 
two cases (function pointers and function objects).

For `return` this is much simpler, because we just do an initialization, and 
there are ways to “try” initialization without immediately throwing an error. 
For `co_return` I need to try (parameter) initialization **and** try to resolve 
the callee.

> That said, I would be happy to start a thread among you, me, David Stone, and 
> the EWG mailing list to consider removing the words "or `co_return`" from 
> class.copy.elision/3 . Say the 
> word.

I'm somewhat undecided. Things would perhaps also be s

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1969-1972
+  (DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_alloc ||
+   DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_to ||

ABataev wrote:
> I think, alloc and to scalars also must be captured by value. Moreover, seems 
> to me, alloc scalars should not be captured at all since their behavior is 
> very similar to the behavior of private variables.
Shouldn't defaultmap(alloc/to:scalar) have the same behavior (copy by ref) as 
map(alloc/to) for scalar? Also, I don't think defaultmap(alloc:scalar) should 
be the same as private (not copy at all) since you still need to register the 
scalar address on the device.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69498: IR: Invert convergent attribute handling

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

dexonsmith added a comment.

In D69498#1723606 , @rjmccall wrote:

> Perhaps there should be a global metadata, or something in the 
> increasingly-misnamed "data layout" string, which says that convergence is 
> meaningful, and we should only add the attribute in appropriately-annotated 
> modules?


Just wanted to resurface these alternatives from John.  Given that some targets 
want a fundamentally different default from what most frontends expect, I think 
we ought to find some way to encode the difference.


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

https://reviews.llvm.org/D69498



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D69498#1727546 , @jdoerfert wrote:

> In D69498#1727419 , @mehdi_amini 
> wrote:
>
> > In D69498#1727080 , @jdoerfert 
> > wrote:
> >
> > > Let me quote @arsenm here because this is so important: "Basically no 
> > > frontend has gotten this right, including clang and non-clang frontends." 
> > > For me, that statement alone is reason to change the status quo.
> >
> >
> > I am not convinced by this: this seems overly simplistic. The fact that 
> > convergent wasn't rolled out properly in frontend can also been as an 
> > artifact of the past.
>
>
> Evidence suggest this problem will resurface over and over again, calling it 
> an artifact of the past is dangerous and not helpful.


Can you provide the "evidence"? So far the only thing I have seen is historical 
and closely tied to how convergence was rolled out AFAICT.

>> There is a logical gap between this and concluding that the only solution is 
>> to change the default.
> 
> There is no gap.

Bluntly contradicting what I wrote without apparently trying to understand my 
point or trying to explain your point with respect to what I'm trying to 
explain is not gonna get me to agree with you magically, I'm not sure what you 
expect here other than shutting down the discussion and leaving us in a 
frustrating disagreement and lack of understanding with this?


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

https://reviews.llvm.org/D69498



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

[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Another interesting problem that we forgot to mention on the open projects page 
is the modeling of C++17 bindings and decompositions: 
https://bugs.llvm.org/show_bug.cgi?id=43042

Also, in my opinion, out of all construction context problems mentioned on the 
open projects page, the NRVO problem is probably the easiest. It might as well 
be the least rewarding of them, but i think it is the friendliest possible 
problem to start with, as it doesn't force you to invent large new facilities.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69308



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

[PATCH] D69632: [libTooling] Add Stencil constructor.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.
ymandel added a child revision: D69613: [libTooling] Simplify type structure of 
`Stencil`s..

Adds a constructor that takes a vector with which to initialize the `Parts`
field and a corresponding free function that forwards to the constructor. These
definitions are needed to assist in transitioning away from `Stencil` as a class
to defining it as a type alias.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69632

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h


Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -90,6 +90,7 @@
 class Stencil {
 public:
   Stencil() = default;
+  Stencil(std::vector Parts) : Parts(std::move(Parts)) {}
 
   /// Composes a stencil from a series of parts.
   template  static Stencil cat(Ts &&... Parts) {
@@ -140,6 +141,12 @@
 template  Stencil cat(Ts &&... Parts) {
   return Stencil::cat(std::forward(Parts)...);
 }
+/// Convenience wrapper for Stencil constructor of the same type. Declaration
+/// outside of the class supports transition of `Stencil` type to an alias
+/// rather than a class.
+inline Stencil cat(std::vector Parts) {
+  return Stencil(std::move(Parts));
+}
 
 /// \returns exactly the text provided.
 StencilPart text(llvm::StringRef Text);


Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -90,6 +90,7 @@
 class Stencil {
 public:
   Stencil() = default;
+  Stencil(std::vector Parts) : Parts(std::move(Parts)) {}
 
   /// Composes a stencil from a series of parts.
   template  static Stencil cat(Ts &&... Parts) {
@@ -140,6 +141,12 @@
 template  Stencil cat(Ts &&... Parts) {
   return Stencil::cat(std::forward(Parts)...);
 }
+/// Convenience wrapper for Stencil constructor of the same type. Declaration
+/// outside of the class supports transition of `Stencil` type to an alias
+/// rather than a class.
+inline Stencil cat(std::vector Parts) {
+  return Stencil(std::move(Parts));
+}
 
 /// \returns exactly the text provided.
 StencilPart text(llvm::StringRef Text);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 227159.
ymandel added a comment.

rebasing onto parent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -96,7 +96,7 @@
  .bind("expr")))
 .bind("stmt"));
 ASSERT_TRUE(StmtMatch);
-if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+if (auto ResultOrErr = Stencil->eval(StmtMatch->Result)) {
   ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
 } else {
   auto Err = llvm::handleErrors(ResultOrErr.takeError(),
@@ -131,11 +131,11 @@
   // Invert the if-then-else.
   auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ",
  statement(Then));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
-TEST_F(StencilTest, SingleStatementCallOperator) {
+TEST_F(StencilTest, StencilFactoryFunction) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
 if (true)
@@ -148,9 +148,9 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ",
-  statement(Then));
-  EXPECT_THAT_EXPECTED(S(StmtMatch->Result),
+  auto Consumer = stencil("if (!", node(Condition), ") ", statement(Else),
+  " else ", statement(Then));
+  EXPECT_THAT_EXPECTED(Consumer(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
@@ -165,7 +165,7 @@
  hasThen(stmt().bind("a2";
   ASSERT_TRUE(StmtMatch);
   auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
-  auto ResultOrErr = Stencil.eval(StmtMatch->Result);
+  auto ResultOrErr = Stencil->eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
   << "Expected unbound node, got " << *ResultOrErr;
 }
@@ -176,14 +176,14 @@
   StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue(Expected));
 }
 
 void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
  testing::Matcher MessageMatcher) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
Failed(testing::Property(
&StringError::getMessage, MessageMatcher)));
 }
@@ -195,28 +195,28 @@
 
 TEST_F(StencilTest, IfBoundOpBound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5");
+  testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
 }
 
 TEST_F(StencilTest, IfBoundOpUnbound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
+  testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
 }
 
 TEST_F(StencilTest, ExpressionOpNoParens) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(expression(Id)), "3");
+  testExpr(Id, "3;", expression(Id), "3");
 }
 
 // Don't parenthesize a parens expression.
 TEST_F(StencilTest, ExpressionOpNoParensParens) {
   StringRef Id = "id";
-  testExpr(Id, "(3);", cat(expression(Id)), "(3)");
+  testExpr(Id, "(3);", expression(Id), "(3)");
 }
 
 TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
   StringRef Id = "id";
-  testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)");
+  testExpr(Id, "3+4;", expression(Id), "(3+4)");
 }
 
 // `expression` shares code with other ops, so we get sufficient coverage of the
@@ -224,33 +224,33 @@
 // tests should be added.
 TEST_F(StencilTest, ExpressionOpUnbound) {
   StringRef Id = "id";
-  testFailure(Id, "3;", cat(expression("ACACA")),
+  testFailure(Id, "3;", expression("ACACA"),
   AllOf(HasSubstr("ACACA"), HasSubstr("not bound")));
 }
 
 TEST_F(StencilTest, DerefPointer) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x;", cat(deref(Id)), "*x");
+  testExpr(Id, "int *x; x;", deref(Id), "*x");
 }
 
 TEST_F(StencilTest, DerefBinOp) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x + 1;", cat(deref(Id)), "*(x + 1)");
+  testExpr(Id, "int 

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 227156.
ymandel added a comment.

added simple overload of `cat` -- needed to support client transition to new 
type structure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -96,7 +96,7 @@
  .bind("expr")))
 .bind("stmt"));
 ASSERT_TRUE(StmtMatch);
-if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+if (auto ResultOrErr = Stencil->eval(StmtMatch->Result)) {
   ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
 } else {
   auto Err = llvm::handleErrors(ResultOrErr.takeError(),
@@ -131,11 +131,11 @@
   // Invert the if-then-else.
   auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ",
  statement(Then));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
-TEST_F(StencilTest, SingleStatementCallOperator) {
+TEST_F(StencilTest, StencilFactoryFunction) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
 if (true)
@@ -148,9 +148,9 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ",
-  statement(Then));
-  EXPECT_THAT_EXPECTED(S(StmtMatch->Result),
+  auto Consumer = stencil("if (!", node(Condition), ") ", statement(Else),
+  " else ", statement(Then));
+  EXPECT_THAT_EXPECTED(Consumer(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
@@ -165,7 +165,7 @@
  hasThen(stmt().bind("a2";
   ASSERT_TRUE(StmtMatch);
   auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
-  auto ResultOrErr = Stencil.eval(StmtMatch->Result);
+  auto ResultOrErr = Stencil->eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
   << "Expected unbound node, got " << *ResultOrErr;
 }
@@ -176,14 +176,14 @@
   StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue(Expected));
 }
 
 void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
  testing::Matcher MessageMatcher) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
Failed(testing::Property(
&StringError::getMessage, MessageMatcher)));
 }
@@ -195,28 +195,28 @@
 
 TEST_F(StencilTest, IfBoundOpBound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5");
+  testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
 }
 
 TEST_F(StencilTest, IfBoundOpUnbound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
+  testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
 }
 
 TEST_F(StencilTest, ExpressionOpNoParens) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(expression(Id)), "3");
+  testExpr(Id, "3;", expression(Id), "3");
 }
 
 // Don't parenthesize a parens expression.
 TEST_F(StencilTest, ExpressionOpNoParensParens) {
   StringRef Id = "id";
-  testExpr(Id, "(3);", cat(expression(Id)), "(3)");
+  testExpr(Id, "(3);", expression(Id), "(3)");
 }
 
 TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
   StringRef Id = "id";
-  testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)");
+  testExpr(Id, "3+4;", expression(Id), "(3+4)");
 }
 
 // `expression` shares code with other ops, so we get sufficient coverage of the
@@ -224,33 +224,33 @@
 // tests should be added.
 TEST_F(StencilTest, ExpressionOpUnbound) {
   StringRef Id = "id";
-  testFailure(Id, "3;", cat(expression("ACACA")),
+  testFailure(Id, "3;", expression("ACACA"),
   AllOf(HasSubstr("ACACA"), HasSubstr("not bound")));
 }
 
 TEST_F(StencilTest, DerefPointer) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x;", cat(deref(Id)), "*x");
+  testExpr(Id, "int *x; x;", deref(Id), "*x");
 }
 
 TEST_F(StencilTest, DerefBinOp) {
   StringRef Id = "id";
-  testExpr

[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D69308#1727108 , @Szelethus wrote:

> Would love to see this comment in its entirety on the open projects page :^)


I'd rather have a mention that @dkrupp is already working on this project, so 
that if somebody wanted to help out they could cooperate nicely.




Comment at: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp:3-6
+// REQUIRES: non-existing-system
+
+// These test cases demonstrate lack of Static Analyzer features.
+// Remove REQUIRES line, when the feature gets implemented.

NoQ wrote:
> I prefer our FIXME-tests to keep running while documenting incorrect results, 
> so that people were informed when they fix something. Eg.:
> 
> ```lang=c++
> // FIXME: Should be TRUE.
> clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
> ```
> 
> I doubt that the whole file of tests will be fixed by a single commit, so 
> they'll have to change this anyway.
> 
> It's also nice to inform people that they accidentally changed something they 
> didn't expect to change.
Also let's not put it to "unsupported_features" so that we didn't have to move 
it (and lose history / or forget to move) later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69308



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

In D69356#1727581 , 
@hubert.reinterpretcast wrote:

> In D69356#1727484 , @beanz wrote:
>
> > We should not be adding more variables that are passed around by CMake's 
> > scope inheritance. Instead if we need to change this we should do it 
> > correctly.
>
>
> Just to clarify, the actions to take are to restore `LLVM_NO_DEAD_STRIP` in 
> order to support other uses and then to replace its use as an internal 
> variable? Given that direction, I agree the first course of action is to 
> revert this patch and then to pursue a patch to do the replacement separately.


Based on the discussion, I will revert this change and open a new review 
introducing SUPPORT_PLUGINS as an option to the add_llvm_tool macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[clang] 23df0c7 - Revert "[NFC] Rename LLVM_NO_DEAD_STRIP"

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2019-10-30T14:56:20-04:00
New Revision: 23df0c783c7053ddcdf665ebc8ddda350abd5bf2

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

LOG: Revert "[NFC] Rename LLVM_NO_DEAD_STRIP"

This reverts commit 11c2a85db8849db1a5907e80d9966592248ef825.

Added: 


Modified: 
clang/tools/driver/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/tools/bugpoint/CMakeLists.txt
llvm/tools/llc/CMakeLists.txt
llvm/tools/opt/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 5a8f57eb4668..590d708d837c 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -20,9 +20,9 @@ set( LLVM_LINK_COMPONENTS
 option(CLANG_PLUGIN_SUPPORT "Build clang with plugin support" ON)
 
 # Support plugins. This must be before add_clang_executable as it reads
-# LLVM_SUPPORT_PLUGINS.
+# LLVM_NO_DEAD_STRIP.
 if(CLANG_PLUGIN_SUPPORT)
-  set(LLVM_SUPPORT_PLUGINS 1)
+  set(LLVM_NO_DEAD_STRIP 1)
 endif()
 
 if(NOT CLANG_BUILT_STANDALONE)

diff  --git a/llvm/cmake/modules/AddLLVM.cmake 
b/llvm/cmake/modules/AddLLVM.cmake
index 41e72c7e9bcc..b5f612469ff9 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -228,7 +228,7 @@ function(add_link_opts target_name)
   # to enable. See https://sourceware.org/bugzilla/show_bug.cgi?id=17704.
 endif()
 
-if(NOT LLVM_SUPPORT_PLUGINS)
+if(NOT LLVM_NO_DEAD_STRIP)
   if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 # ld64's implementation of -dead_strip breaks tools that use plugins.
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
@@ -245,7 +245,7 @@ function(add_link_opts target_name)
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,--gc-sections")
   endif()
-else() #LLVM_SUPPORT_PLUGINS
+else() #LLVM_NO_DEAD_STRIP
   if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,-bnogc")

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 5aa72568c656..03acea487977 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -744,7 +744,7 @@ endif()
 # Add flags for add_dead_strip().
 # FIXME: With MSVS, consider compiling with /Gy and linking with /OPT:REF?
 # But MinSizeRel seems to add that automatically, so maybe disable these
-# flags instead if LLVM_SUPPORT_PLUGINS is set.
+# flags instead if LLVM_NO_DEAD_STRIP is set.
 if(NOT CYGWIN AND NOT WIN32)
   if(NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin" AND
  NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")

diff  --git a/llvm/tools/bugpoint/CMakeLists.txt 
b/llvm/tools/bugpoint/CMakeLists.txt
index 8b468691fa6b..031f51480cce 100644
--- a/llvm/tools/bugpoint/CMakeLists.txt
+++ b/llvm/tools/bugpoint/CMakeLists.txt
@@ -22,7 +22,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_SUPPORT_PLUGINS 1)
+set(LLVM_NO_DEAD_STRIP 1)
 
 add_llvm_tool(bugpoint
   BugDriver.cpp

diff  --git a/llvm/tools/llc/CMakeLists.txt b/llvm/tools/llc/CMakeLists.txt
index 8435b9de6fb4..300efdab10d0 100644
--- a/llvm/tools/llc/CMakeLists.txt
+++ b/llvm/tools/llc/CMakeLists.txt
@@ -20,7 +20,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_SUPPORT_PLUGINS 1)
+set(LLVM_NO_DEAD_STRIP 1)
 
 add_llvm_tool(llc
   llc.cpp

diff  --git a/llvm/tools/opt/CMakeLists.txt b/llvm/tools/opt/CMakeLists.txt
index 4f74a0571203..cb4ba5cfbced 100644
--- a/llvm/tools/opt/CMakeLists.txt
+++ b/llvm/tools/opt/CMakeLists.txt
@@ -25,7 +25,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_SUPPORT_PLUGINS 1)
+set(LLVM_NO_DEAD_STRIP 1)
 
 add_llvm_tool(opt
   AnalysisWrappers.cpp



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D69356#1727484 , @beanz wrote:

> We should not be adding more variables that are passed around by CMake's 
> scope inheritance. Instead if we need to change this we should do it 
> correctly.


Just to clarify, the actions to take are to restore `LLVM_NO_DEAD_STRIP` in 
order to support other uses and then to replace its use as an internal 
variable? Given that direction, I agree the first course of action is to revert 
this patch and then to pursue a patch to do the replacement separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:395-400
 /// The number used to indicate this lambda expression for name
 /// mangling in the Itanium C++ ABI.
 unsigned ManglingNumber : 31;
 
+/// The device side mangling number.
+unsigned DeviceManglingNumber = 0;

It seems a shame to grow LambdaDefinitionData by a pointer for all users of C++ 
that do not use CUDA. Optimizing bitfields may be worth the time, but I'll 
leave it to @rjmccall or @rsmith to give guidance on whether that's worth it.

An alternative would be to store the device numbers in the mangling context and 
look them up when needed, since they are so rarely needed.



Comment at: clang/include/clang/AST/MangleNumberingContext.h:57-58
+
+  /// Has device mangle numbering context.
+  virtual bool hasDeviceMangleNumberingContext() const { return false; }
+

hliao wrote:
> rnk wrote:
> > It would be nicer if there were a single entry point that does the right 
> > thing for all mangling contexts.
> could you elaborate in more details?
I'll add comments at the call site.



Comment at: clang/lib/Sema/SemaLambda.cpp:479-481
+if (MCtx->hasDeviceMangleNumberingContext()) {
+  Class->setDeviceLambdaManglingNumber(
+  MCtx->getDeviceManglingNumber(Method));

Rather than having a virtual method that returns bool, call a virtual method 
that does the entire thing you are trying to do. For example, MSHIP* could be 
responsible for setting the mangling number on the class. Although, maybe it 
should be storing the number in some side table now that I think about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D69356#1727381 , @beanz wrote:

> (1) dead stripping support is useful independent of plugins, so it is 
> valuable to have that option be separate


Okay.

>> Just because the current implementation of plug-in support is by disabling 
>> dead strip does not make it a great reason for "enabling plug-in support" to 
>> be called `LLVM_NO_DEAD_STRIP`.
> 
> `LLVM_NO_DEAD_STRIP` isn't the implementation of enabling plugin support.

Sure, it can be claimed that it merely has high affinity with the 
implementation of enabling plugin support in terms of linking in its use prior 
to the subject patch. Adding `LLVM_NO_DEAD_STRIP` back is fine if it is used 
for something else, although it would help people developing patches understand 
its use cases better if there was more comments and better documentation. The 
comments about `LLVM_NO_DEAD_STRIP` that can be seen in the patch context talk 
a lot about plug-in support, and mentions of other use cases are not really 
standing out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Kian Moniri via Phabricator via cfe-commits]

kianm added a comment.

Hi,

There is a problem with this commit because it asserts after errors have been 
recognized. I have attached a reduced test case. The assertion looks to occur 
here:

  clang: 
/home/kianm/llvm/dev/llvm-project/clang/lib/Parse/ParseExprCXX.cpp:585: 
clang::ExprResult clang::Parser::tryParseCXXIdExpression(clang::CXXScopeSpec &, 
bool, clang::Token &): Assertion `SS.isEmpty() && "undeclared non-type 
annotation should be unqualified"' failed.

The reproducible command is:

  clang -cc1 -x c++ reduced.c

F10590725: reduced.c 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68896



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

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp:29
+  operator=(incr_on_copy_for_simple_usage &&o) = delete;
+  ~incr_on_copy_for_simple_usage() = default;
+};

NoQ wrote:
> That's the reason why your tests seem to work. If you change it to 
> `~incr_on_copy_for_simple_usage() {}`, we will no longer evaluate the capture 
> correctly.
> 
> The difference here is that we allow ourselves inline the constructor without 
> a construction context when the destructor is trivial. But when the 
> destructor is non-trivial, our inability to keep track of the object under 
> construction until destruction will have terrible consequences.
*allow ourselves to inline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69602



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a subscriber: hfinkel.
jdoerfert added a comment.

In D69498#1727419 , @mehdi_amini wrote:

> In D69498#1727080 , @jdoerfert wrote:
>
> > Let me quote @arsenm here because this is so important: "Basically no 
> > frontend has gotten this right, including clang and non-clang frontends." 
> > For me, that statement alone is reason to change the status quo.
>
>
> I am not convinced by this: this seems overly simplistic. The fact that 
> convergent wasn't rolled out properly in frontend can also been as an 
> artifact of the past.


Evidence suggest this problem will resurface over and over again, calling it an 
artifact of the past is dangerous and not helpful.

> There is a logical gap between this and concluding that the only solution is 
> to change the default.

There is no gap. Making the default restrictive but sound will prevent various 
kinds of errors we have seen in the past. 
Assuming we could somehow "not repeat the errors in the future" is in my 
opinion the unrealistic view.

> For instance someone mentioned a pass that could be inserted as the very 
> beginning of the pipeline by any GPU compiler and add the convergent 
> attribute everywhere.

I talked to @hfinkel earlier about this and his idea was to use a pass (or 
IRBuilder mode, or sth.) to do the opposite: opt-into a "subset" of LLVM-IR 
behaviors.
The idea is that if you want just a low-level C for CPUs, you run this pass or 
enable this mode and you get the appropriate LLVM-IR. That would for this patch 
mean
you get `noconvergent` everywhere but in the future you could get other 
attributes/changes as well.

>>> As of the frontend, it seems to me that this is just about structuring the 
>>> code-path for function emission to define the right set of default 
>>> attribute. It isn't clear to me why a refactoring of the frontend isn't a 
>>> better course of actions here.
>> 
>> Whatever we do, there will be consequences for current and future 
>> front-ends. At the end of the day, the underlying question we need to answer 
>> here is: Do we want to have "optimization with opt-in soundness" or 
>> "soundness with opt-in optimizations", and I would choose the latter any 
>> time.
> 
> This view seems appropriate to me only if you consider a GPU (or SIMT) 
> compiler alone. Another view (which seems to be what @rjmccall has) is that 
> SIMT/GPU is a marginal use-case and "soundness" is already existing for most 
> LLVM use-cases.

I'm strongly opposing the idea to marginalize GPU, or any accelerator, targets. 
This is not only splitting the community by making it hostile towards some who 
have to work around whatever is deemed "the main use-case", it is also arguably 
a position of the past for many of us. Convenience for people that do not care 
about accelerators is a goal we should have in mind for sure, but it should not 
oppose a reasonable and sound support of accelerators.


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

https://reviews.llvm.org/D69498



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

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I recommend starting with CFG tests for this feature. I.e., do 
`-analyzer-checker debug.DumpCFG` and make sure that every `CFGStmt` that 
corresponds to a constructor is a `CFGConstructor` (i.e., has a 
`ConstructionContext` explained in the dump). Cf. 
`test/Analysis/cfg-rich-constructors.cpp`.




Comment at: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp:11
+void reached_function_from_simple_copy() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}

This test doesn't test whether the function is inlined. If the function is not 
inlined, the code will still be reachable when analyzed as top frame. See also 
`clang_analyzer_checkInlined()`.



Comment at: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp:29
+  operator=(incr_on_copy_for_simple_usage &&o) = delete;
+  ~incr_on_copy_for_simple_usage() = default;
+};

That's the reason why your tests seem to work. If you change it to 
`~incr_on_copy_for_simple_usage() {}`, we will no longer evaluate the capture 
correctly.

The difference here is that we allow ourselves inline the constructor without a 
construction context when the destructor is trivial. But when the destructor is 
non-trivial, our inability to keep track of the object under construction until 
destruction will have terrible consequences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69602



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

[PATCH] D69582: Let clang driver support parallel jobs

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Driver/Compilation.cpp:303
+}
+std::thread Th(Work);
+Th.detach();

Maybe a select() / fork() loop is a better approach than spawning one thread 
per subprocess? This is doing thread-level parallelism in addition to 
process-level parallelism :)

If llvm doesn't have a subprocess pool abstraction yet, ninja's is pretty 
small, self-contained, battle-tested and open-source, maybe you could copy that 
over (and remove bits you don't need)?

https://github.com/ninja-build/ninja/blob/master/src/subprocess.h
https://github.com/ninja-build/ninja/blob/master/src/subprocess-win32.cc
https://github.com/ninja-build/ninja/blob/master/src/subprocess-posix.cc


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

https://reviews.llvm.org/D69582



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

[PATCH] D69582: Let clang driver support parallel jobs

[Reid Kleckner via Phabricator via cfe-commits]

rnk added subscribers: aganea, amccarth, rnk.
rnk added a comment.

+@aganea @amccarth 
Users have been asking for /MP support in clang-cl for a while, which is 
basically this.

Is there anything in JobScheduler that could reasonably be moved down to 
llvm/lib/Support? I would also like to be able to use it to implement 
multi-process ThinLTO instead of multi-threaded ThinLTO.


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

https://reviews.llvm.org/D69582



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, many thanks!




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:293
+  // Now try to generate edits for all the refs.
+  for (auto &Entry : RefLocs) {
+const auto *OldDecl = Entry.first;

NIT: Maybe move declaration of `Replacements` here? To make it closer to the 
use-site.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've commit on your behalf in 
e477988309dbde214a6d16ec690a416882714aac 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539



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

Re: [clang-tools-extra] 0de262d - Move this release note to its appropriate location; NFC.

[Aaron Ballman via cfe-commits]

Unintentionally off-list; I thought I hit reply-all. :-D

Would you like me to remove the entry for include-fixer?

~Aaron

On Wed, Oct 30, 2019 at 2:17 PM Nico Weber  wrote:
>
> (intentionally off-list?)
>
> On Wed, Oct 30, 2019 at 2:14 PM Aaron Ballman  wrote:
>>
>> On Wed, Oct 30, 2019 at 1:57 PM Nico Weber via cfe-commits
>>  wrote:
>> >
>> > Aren't include-fixer and clang-include-fixer the same thing? Why do we 
>> > have to entries for it?
>>
>> I wasn't certain if they were or were not the same thing, so I left
>> the other entry. If you think one or both of them should be removed, I
>> have no real opinion on it.
>>
>> ~Aaron
>>
>> >
>> > On Wed, Oct 30, 2019 at 1:48 PM Aaron Ballman via cfe-commits 
>> >  wrote:
>> >>
>> >>
>> >> Author: Aaron Ballman
>> >> Date: 2019-10-30T13:48:26-04:00
>> >> New Revision: 0de262d7189c68897e8328d891d3daaf3aab3156
>> >>
>> >> URL: 
>> >> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156
>> >> DIFF: 
>> >> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156.diff
>> >>
>> >> LOG: Move this release note to its appropriate location; NFC.
>> >>
>> >> Added:
>> >>
>> >>
>> >> Modified:
>> >> clang-tools-extra/docs/ReleaseNotes.rst
>> >>
>> >> Removed:
>> >>
>> >>
>> >>
>> >> 
>> >> diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
>> >> b/clang-tools-extra/docs/ReleaseNotes.rst
>> >> index d2819258f17e..e6ef3cd5af0d 100644
>> >> --- a/clang-tools-extra/docs/ReleaseNotes.rst
>> >> +++ b/clang-tools-extra/docs/ReleaseNotes.rst
>> >> @@ -130,17 +130,17 @@ Improvements to clang-tidy
>> >>  - The 'objc-avoid-spinlock' check was renamed to 
>> >> :doc:`darwin-avoid-spinlock
>> >>`
>> >>
>> >> -Improvements to include-fixer
>> >> --
>> >> -
>> >> -The improvements are...
>> >> -
>> >>  - New :doc:`readability-redundant-access-specifiers
>> >>` check.
>> >>
>> >>Finds classes, structs, and unions that contain redundant member
>> >>access specifiers.
>> >>
>> >> +Improvements to include-fixer
>> >> +-
>> >> +
>> >> +The improvements are...
>> >> +
>> >>  Improvements to clang-include-fixer
>> >>  ---
>> >>
>> >>
>> >>
>> >>
>> >> ___
>> >> cfe-commits mailing list
>> >> cfe-commits@lists.llvm.org
>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] e477988 - Fix readability-identifier-naming to prevent variables becoming keywords.

[Aaron Ballman via cfe-commits]

Author: Daniel
Date: 2019-10-30T14:18:40-04:00
New Revision: e477988309dbde214a6d16ec690a416882714aac

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

LOG: Fix readability-identifier-naming to prevent variables becoming keywords.

Do not provide a fix-it when clang-tidy encounters a name that would become
a keyword.

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang/include/clang/Basic/IdentifierTable.h

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 322894041325..5b78155cd546 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -691,10 +691,11 @@ static void 
addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
 return;
 
-  if (!Failure.ShouldFix)
+  if (!Failure.ShouldFix())
 return;
 
-  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+  if (!utils::rangeCanBeFixed(Range, SourceMgr))
+Failure.FixStatus = IdentifierNamingCheck::ShouldFixStatus::InsideMacro;
 }
 
 /// Convenience method when the usage to be added is a NamedDecl
@@ -873,6 +874,16 @@ void IdentifierNamingCheck::check(const 
MatchFinder::MatchResult &Result) {
   DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation())
   .getSourceRange();
 
+  const IdentifierTable &Idents = Decl->getASTContext().Idents;
+  auto CheckNewIdentifier = Idents.find(Fixup);
+  if (CheckNewIdentifier != Idents.end()) {
+const IdentifierInfo *Ident = CheckNewIdentifier->second;
+if (Ident->isKeyword(getLangOpts()))
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;
+else if (Ident->hasMacroDefinition())
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition;
+  }
+
   Failure.Fixup = std::move(Fixup);
   Failure.KindName = std::move(KindName);
   addUsage(NamingCheckFailures, Decl, Range);
@@ -935,24 +946,35 @@ void IdentifierNamingCheck::onEndOfTranslationUnit() {
 if (Failure.KindName.empty())
   continue;
 
-if (Failure.ShouldFix) {
-  auto Diag = diag(Decl.first, "invalid case style for %0 '%1'")
-  << Failure.KindName << Decl.second;
-
-  for (const auto &Loc : Failure.RawUsageLocs) {
-// We assume that the identifier name is made of one token only. This 
is
-// always the case as we ignore usages in macros that could build
-// identifier names by combining multiple tokens.
-//
-// For destructors, we alread take care of it by remembering the
-// location of the start of the identifier and not the start of the
-// tilde.
-//
-// Other multi-token identifiers, such as operators are not checked at
-// all.
-Diag << FixItHint::CreateReplacement(
-SourceRange(SourceLocation::getFromRawEncoding(Loc)),
-Failure.Fixup);
+if (Failure.ShouldNotify()) {
+  auto Diag =
+  diag(Decl.first,
+   "invalid case style for %0 '%1'%select{|" // Case 0 is empty on
+ // purpose, because we
+ // intent to provide a
+ // fix
+   "; cannot be fixed because '%3' would conflict with a keyword|"
+   "; cannot be fixed because '%3' would conflict with a macro "
+   "definition}2")
+  << Failure.KindName << Decl.second
+  << static_cast(Failure.FixStatus) << Failure.Fixup;
+
+  if (Failure.ShouldFix()) {
+for (const auto &Loc : Failure.RawUsageLocs) {
+  // We assume that the identifier name is made of one token only. This
+  // is always the case as we ignore usages in macros that could build
+  // identifier names by combining multiple tokens.
+  //
+  // For destructors, we already take care of it by remembering the
+  // location of the start of the identifier and not the start of the
+  // tilde.
+  //
+  // Other multi-token identifiers, such as operators are not checked 
at
+  // all.
+  Diag << FixItHint::CreateReplacement(
+  SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+  Failure.Fixup);
+}
   }
 }
   }

diff  --git a/clang-tools-extra/clang-tidy/r

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D69356#1727451 , @daltenty wrote:

> The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools 
> which may have plugins and need to be built in a specific way (such as 
> avoiding deadstriping) if plugins are enabled.


Non-user facing options shouldn't be exposed this way. `LLVM_NO_DEAD_STRIP` has 
been around a long time and predates many of the modern CMake patterns. We 
should use arguments passed in explicitly to the calling functions rather than 
setting variables that are passed down.

> We could keep LLVM_NO_DEAD_STRIP as is and have it set by 
> LLVM_SUPPORT_PLUGINS when appropriate. That should accommodate both uses.

We should not be adding more variables that are passed around by CMake's scope 
inheritance. Instead if we need to change this we should do it correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks!

Looks like it's just the `-E` test that's causing problems; the test passes 
with that disabled:

  $ git diff
  diff --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
  index 97e4847f843..4cbc2cd8754 100644
  --- a/clang/test/Driver/flang/flang.f90
  +++ b/clang/test/Driver/flang/flang.f90
  @@ -1,6 +1,5 @@
   ! D63607 made mac builders unhappy by failing this test, and it isn't
   ! yet obvious why. Mark as unsupported as a temporary measure.
  -! UNSUPPORTED: darwin
   
   ! Check that flang -fc1 is invoked when in --driver-mode=flang.
   
  @@ -21,10 +20,9 @@
   
   ! Check that f90 files are not treated as "previously preprocessed"
   ! ... in --driver-mode=flang.
  -! RUN: %clang --driver-mode=flang -### -E  %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-E %s
  +! RUN: %clang --driver-mode=flang -###   %s 2>&1 | FileCheck 
--check-prefixes=ALL,CHECK-E %s
   ! CHECK-E-NOT: previously preprocessed input
  -! CHECK-E-DAG: "-E"
  -! CHECK-E-DAG: "-o" "-"
  +! CHECK-E-DAG: "-o"
   
   ! RUN: %clang --driver-mode=flang -### -emit-ast   %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-EMIT-AST %s
   ! CHECK-EMIT-AST-DAG: "-triple"

I'll debug a bit more.




Comment at: clang/lib/Driver/Driver.cpp:4883
+  // And say "no" if this is not a kind of action flang understands.
+  if (!isa(JA) && !isa(JA) && 
!isa(JA))
+return false;

(please run clang-format on your new code -- it might also answer why some 
enums have a comma after the last entry ;) )


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

https://reviews.llvm.org/D63607



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

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've committed on your behalf in 
4de6b1586807285e20a5db6596519c2336a64568 
.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



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

[clang-tools-extra] 4de6b15 - Add an option to hicpp-signed-bitwise for positive integer literals.

[Aaron Ballman via cfe-commits]

Author: Vladimir Plyashkun
Date: 2019-10-30T14:11:29-04:00
New Revision: 4de6b1586807285e20a5db6596519c2336a64568

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

LOG: Add an option to hicpp-signed-bitwise for positive integer literals.

This gives developers a way to deviate from the coding standard to reduce the
chattiness of the check.

Added: 


Modified: 
clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp 
b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
index 781a4430545e..b9c60772415a 100644
--- a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -17,9 +17,24 @@ namespace clang {
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+   : expr(ignoringImpCasts(hasType(isSignedInteger()
+  .bind("signed-operand");
 
   // The standard [bitmask.types] allows some integral types to be implemented
   // as signed types. Exclude these types from diagnosing for bitwise or(|) and

diff  --git a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h 
b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
index 34d5f096df6e..74b3f0eb235d 100644
--- a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@ namespace hicpp {
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index e6ef3cd5af0d..c706ae13c785 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,11 @@ Improvements to clang-tidy
   Finds uses of deprecated Googletest APIs with names containing ``case`` and
   replaces them with equivalent APIs with ``suite``.
 
+- Improved :doc:`hicpp-signed-bitwise
+  ` check.
+
+  The check now supports the ``IgnorePositiveIntegerLiterals`` option.
+
 - New :doc:`linuxkernel-must-use-errs
   ` check.
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst 
b/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
index 59b19b6f3044..4c6bc005a8ec 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
@@ -7,3 +7,11 @@ Finds uses of bitwise operations on signed integer types, 
which may lead to
 undefined or implementation defined behaviour.
 
 The according rule is defined in the `High Integrity C++ Standard, Section 
5.6.1 `_.
+
+Options
+---
+
+.. option:: IgnorePositiveIntegerLiterals
+
+   If this option is set to `true`, the check will not warn on bitwise 
operations with positive integer literals, e.g. `~0`, `2 << 1`, etc.
+   Default value is `false`.



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

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur requested changes to this revision.
Meinersbur added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/CodeGen/CGLoopInfo.cpp:272-277
+  if (Attrs.VectorizeWidth > 1 &&
+  Attrs.VectorizeEnable == LoopAttributes::Unspecified)
+Args.push_back(
+MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+  ConstantAsMetadata::get(ConstantInt::get(
+  llvm::Type::getInt1Ty(Ctx), 1))}));

[serious] Please handle the `llvm.loop.vectorize.enable` metadata in one place, 
i.e. where the other `llvm.loop.vectorize.enable` is handled. This introduces 
yet another mechanism when to add `llvm.loop.vectorize.enable` besides the one 
for `IsVectorPredicateEnabled`. Btw, with `vectorize_predicate(enable) 
vectorize_width(2)` this emits **two** `llvm.loop.vectorize.enable`.

Also, the changing relative order of `llvm.loop.vectorize.enable` to other 
metadata makes D69092 difficult.



Comment at: clang/test/CodeGenCXX/pragma-loop-predicate.cpp:61
 
-
 // CHECK:  ![[LOOP0]] = distinct !{![[LOOP0]], !3}

unintentional change?



Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:161-177
+void vec_width_1(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}vec_width_1{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;

As you might have noticed, updating FileCheck for metadata in multiple test 
cases is a lot of work since they influence each other. Why not adding the new 
test in separate files?



Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:239
+
+// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[WIDTH_1]]}

`-NEXT` should be unnecessary here. I'd even go towards `CHECK-DAG` since the 
order of the metadata is unimportant.


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

https://reviews.llvm.org/D69628



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

[PATCH] D69498: IR: Invert convergent attribute handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69498#1727419 , @mehdi_amini wrote:

> In D69498#1727080 , @jdoerfert wrote:
>
> > Let me quote @arsenm here because this is so important: "Basically no 
> > frontend has gotten this right, including clang and non-clang frontends." 
> > For me, that statement alone is reason to change the status quo.
>
>
> I am not convinced by this: this seems overly simplistic. The fact that 
> convergent wasn't rolled out properly in frontend can also been as an 
> artifact of the past. There is a logical gap between this and concluding that 
> the only solution is to change the default. For instance someone mentioned a 
> pass that could be inserted as the very beginning of the pipeline by any GPU 
> compiler and add the convergent attribute everywhere.


Avoiding human error is fundamental to good design. If you have to have an 
additional IR pass, then there's already a phase where the IR is broken and 
something could legally break the IR.

> 
> 
>>> As of the frontend, it seems to me that this is just about structuring the 
>>> code-path for function emission to define the right set of default 
>>> attribute. It isn't clear to me why a refactoring of the frontend isn't a 
>>> better course of actions here.
>> 
>> Whatever we do, there will be consequences for current and future 
>> front-ends. At the end of the day, the underlying question we need to answer 
>> here is: Do we want to have "optimization with opt-in soundness" or 
>> "soundness with opt-in optimizations", and I would choose the latter any 
>> time.
> 
> This view seems appropriate to me only if you consider a GPU (or SIMT) 
> compiler alone. Another view (which seems to be what @rjmccall has) is that 
> SIMT/GPU is a marginal use-case and "soundness" is already existing for most 
> LLVM use-cases.

I think this is a concerning argument. Declaring that GPUs are a "marginal" use 
case and LLVM only follows its design principles in cases where it benefits C++ 
x86/ARM users is an issue. In that case LLVM is no longer trying to be the 
compiler infrastructure for everyone that it tries to be. Soundness for most 
doesn't sound like a good design. I've proposed attributes in the past that 
were shot down for not following a correct by-default design and to me it seems 
like a problem if this principle isn't going to be universally followed. It's 
additionally concerning since most GPUs uses LLVM in some fashion if they're 
going to be declared a second class use case.


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

https://reviews.llvm.org/D69498



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

> I'm actually opposed to that happening, on two fronts.
>  (2) we already have `LLVM_ENABLE_PLUGINS` why do we also need 
> `LLVM_SUPPORT_PLUGINS` seems like duplication and bad design.

As I understand it LLVM_ENABLE_PLUGINS is a user-facing option which indicates 
whether the user wants to build with plugin support.

The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools 
which may have plugins and need to be built in a specific way (such as avoiding 
deadstriping) if plugins are enabled.

> (1) dead stripping support is useful independent of plugins, so it is 
> valuable to have that option be separate

We could keep LLVM_NO_DEAD_STRIP as is and have it set by LLVM_SUPPORT_PLUGINS 
when appropriate. That should accommodate both uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

Re: [clang-tools-extra] 0de262d - Move this release note to its appropriate location; NFC.

[Nico Weber via cfe-commits]

Aren't include-fixer and clang-include-fixer the same thing? Why do we have
to entries for it?

On Wed, Oct 30, 2019 at 1:48 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Aaron Ballman
> Date: 2019-10-30T13:48:26-04:00
> New Revision: 0de262d7189c68897e8328d891d3daaf3aab3156
>
> URL:
> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156
> DIFF:
> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156.diff
>
> LOG: Move this release note to its appropriate location; NFC.
>
> Added:
>
>
> Modified:
> clang-tools-extra/docs/ReleaseNotes.rst
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst
> b/clang-tools-extra/docs/ReleaseNotes.rst
> index d2819258f17e..e6ef3cd5af0d 100644
> --- a/clang-tools-extra/docs/ReleaseNotes.rst
> +++ b/clang-tools-extra/docs/ReleaseNotes.rst
> @@ -130,17 +130,17 @@ Improvements to clang-tidy
>  - The 'objc-avoid-spinlock' check was renamed to
> :doc:`darwin-avoid-spinlock
>`
>
> -Improvements to include-fixer
> --
> -
> -The improvements are...
> -
>  - New :doc:`readability-redundant-access-specifiers
>` check.
>
>Finds classes, structs, and unions that contain redundant member
>access specifiers.
>
> +Improvements to include-fixer
> +-
> +
> +The improvements are...
> +
>  Improvements to clang-include-fixer
>  ---
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits