[PATCH] D46140: [coroutines] Add std::experimental::task type

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker marked an inline comment as done.
lewissbaker added a comment.

@EricWF This implementation will currently not work with MSVC as MSVC does not 
yet support the symmetric-transfer capability added to the coroutines 
specification in P0913R0 .

Is MSVC a target that libc++ needs to support coroutines with?
Or can we just say the `task` class is unsupported for compilers that don't 
support P0913?

I have added the 'requires coroutines' line to the task modulemap entry which 
should hopefully fix the module build failures this diff was seeing.
Are there any other potential gotchas that I should make sure I test?




Comment at: test/std/experimental/task/task.basic/task_of_value.pass.cpp:25
+   auto p = std::make_unique(123);
+   co_return p; // Should be implicit std::move(p) here.
+ }

@EricWF This line may fail to compile on older versions of clang without the 
coroutines bugfix for https://bugs.llvm.org/show_bug.cgi?id=37265.

Should I just change this back to `std::move(p)` here, since the intention is 
to test the library, not the compiler?
Alternatively I can conditionally compile this line as either `co_return p;` or 
`co_return std::move(p);` depending on the compiler version.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46140



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

[PATCH] D59863: [HIP] Support gpu arch gfx906+sram-ecc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.
Herald added a subscriber: jholewinski.

Add a new gpu arch gfx906+sram-ecc for HIP.

It is similar to gfx906 but implies target feature sram-ecc enabled,
whereas gfx906 implies target sram-ecc disabled. Corresponding
option -mattr=[+|-]sram-ecc will be passed to llc based on the
gpu arch.


https://reviews.llvm.org/D59863

Files:
  include/clang/Basic/Cuda.h
  lib/Basic/Cuda.cpp
  lib/Basic/Targets/NVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-toolchain-sram-ecc.hip

Index: test/Driver/hip-toolchain-sram-ecc.hip
===
--- /dev/null
+++ test/Driver/hip-toolchain-sram-ecc.hip
@@ -0,0 +1,38 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx906 --cuda-gpu-arch=gfx906+sram-ecc \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx906"
+
+// CHECK: [[OPT:".*opt"]] {{".*-gfx906-linked.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx906"
+// CHECK-SAME: "-o" [[OPT_906_BC:".*-gfx906-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_906_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx906"
+// CHECK-SAME: "-mattr=-sram-ecc" "-o" {{".*-gfx906-.*o"}}
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx906"
+
+// CHECK: [[OPT]] {{".*-gfx906\+sram-ecc.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx906"
+// CHECK-SAME: "-o" [[OPT_906ECC_BC:".*-gfx906\+sram-ecc.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_906ECC_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx906"
+// CHECK-SAME: "-mattr=+sram-ecc" "-o" {{".*-gfx906\+sram-ecc.*o"}}
+
+// CHECK: {{".*clang-offload-bundler"}}
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa-gfx906,hip-amdgcn-amd-amdhsa-gfx906+sram-ecc"
Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -31,6 +31,14 @@
 
 namespace {
 
+void stripFeatureString(StringRef &Arch) {
+  Arch = Arch.take_until([](char c) { return c == '+'; });
+}
+
+StringRef getFeatureString(StringRef Arch) {
+  return Arch.drop_until([](char c) { return c == '+'; });
+}
+
 static void addBCLib(Compilation &C, const ArgList &Args,
  ArgStringList &CmdArgs, ArgStringList LibraryPaths,
  StringRef BCName) {
@@ -114,6 +122,7 @@
 Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
 const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
+  stripFeatureString(SubArchName);
   // Construct opt command.
   ArgStringList OptArgs;
   // The input to opt is the output from llvm-link.
@@ -162,11 +171,24 @@
 Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
 const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
+
+  // Only two gpu archs for gfx906 are accepted: gfx906 and gfx906+sram-ecc.
+  // gfx906 implies sram-ecc off, whereas gfx906+sram-ecc implies sram-ecc on.
+  StringRef Feature = getFeatureString(SubArchName);
+  stripFeatureString(SubArchName);
+  assert(Feature.empty() ||
+ (SubArchName == "gfx906" && Feature == "+sram-ecc"));
+  if (SubArchName == "gfx906" && Feature.empty())
+Feature = "-sram-ecc";
+
   // Construct llc command.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
 "-filetype=obj", "-mattr=-code-object-v3",
 Args.MakeArgString("-mcpu=" + SubArchName)};
 
+  if (!Feature.empty())
+LlcArgs.push_back(Args.MakeArgString("-mattr=" + Feature));
+
   // Extract all the -m options
   std::vector Features;
   handleTargetFeaturesGroup(
@@ -372,6 +394,7 @@
   }
 
   if (!BoundArch.empty()) {
+stripFeatureString(BoundArch);
 DAL->eraseArg(options::OPT_march_EQ);
 DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), BoundArch);
   }
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4951,6 +4951,7 @@
   case CudaArch::GFX902:
   case CudaArch::GFX904:
   case CudaArch::GFX906:
+  case CudaArch::GFX906_SRAM_ECC:
 

Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

[Rafael Auler via cfe-commits]

I’m familiar with this issue as I had to fix a memory bug in LLVM IRLinker that 
was exposed by this commit. That’s why I initially reverted it. However, after 
fixing it, I was able to do the full clang LTO self-hosting with lld on Linux. 
Is there any way I can repro this issue? It’s probably a bad interaction of 
attribute used and an optimization. See https://reviews.llvm.org/D59552


From: Michael Spencer 
Date: Tuesday, March 26, 2019 at 6:59 PM
To: Rafael Auler 
Cc: "cfe-commits@lists.llvm.org" 
Subject: Re: r356598 - Recommit "Support attribute used in member funcs of 
class templates"

On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: rafauler
Date: Wed Mar 20 12:22:24 2019
New Revision: 356598

URL: 
http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
Log:
Recommit "Support attribute used in member funcs of class templates"

This diff previously exposed a bug in LLVM's IRLinker, breaking
buildbots that tried to self-host LLVM with monolithic LTO.
The bug is now in LLVM by D59552

Original commit message:
As PR17480 describes, clang does not support the used attribute
for member functions of class templates. This means that if the member
function is not used, its definition is never instantiated. This patch
changes clang to emit the definition if it has the used attribute.

Test Plan: Added a testcase

Reviewed By: aaron.ballman

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

Added:

cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24 2019
@@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
 Owner->addDecl(Method);
   }

+  // PR17480: Honor the used attribute to instantiate member function
+  // definitions
+  if (Method->hasAttr()) {
+if (const auto *A = dyn_cast(Owner)) {
+  SourceLocation Loc;
+  if (const MemberSpecializationInfo *MSInfo =
+  A->getMemberSpecializationInfo())
+Loc = MSInfo->getPointOfInstantiation();
+  else if (const auto *Spec = dyn_cast(A))
+Loc = Spec->getPointOfInstantiation();
+  SemaRef.MarkFunctionReferenced(Loc, Method);
+}
+  }
+
   return Method;
 }


Added: 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
==
--- 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
(added)
+++ 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
Wed Mar 20 12:22:24 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
+// classes
+namespace InstantiateUsedMemberDefinition {
+template 
+struct S {
+  int __attribute__((used)) f() {
+return 0;
+  }
+};
+
+void test() {
+  // Check that InstantiateUsedMemberDefinition::S::f() is defined
+  // as a result of the S class template implicit instantiation
+  // CHECK: define linkonce_odr i32 
@_ZN31Inst

Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

[Michael Spencer via cfe-commits]

On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rafauler
> Date: Wed Mar 20 12:22:24 2019
> New Revision: 356598
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
> Log:
> Recommit "Support attribute used in member funcs of class templates"
>
> This diff previously exposed a bug in LLVM's IRLinker, breaking
> buildbots that tried to self-host LLVM with monolithic LTO.
> The bug is now in LLVM by D59552
>
> Original commit message:
> As PR17480 describes, clang does not support the used attribute
> for member functions of class templates. This means that if the member
> function is not used, its definition is never instantiated. This patch
> changes clang to emit the definition if it has the used attribute.
>
> Test Plan: Added a testcase
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D56928
>
> Added:
>
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24
> 2019
> @@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
>  Owner->addDecl(Method);
>}
>
> +  // PR17480: Honor the used attribute to instantiate member function
> +  // definitions
> +  if (Method->hasAttr()) {
> +if (const auto *A = dyn_cast(Owner)) {
> +  SourceLocation Loc;
> +  if (const MemberSpecializationInfo *MSInfo =
> +  A->getMemberSpecializationInfo())
> +Loc = MSInfo->getPointOfInstantiation();
> +  else if (const auto *Spec =
> dyn_cast(A))
> +Loc = Spec->getPointOfInstantiation();
> +  SemaRef.MarkFunctionReferenced(Loc, Method);
> +}
> +  }
> +
>return Method;
>  }
>
>
> Added:
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
>
> ==
> ---
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> (added)
> +++
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Wed Mar 20 12:22:24 2019
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s
> +
> +// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
> +// classes
> +namespace InstantiateUsedMemberDefinition {
> +template 
> +struct S {
> +  int __attribute__((used)) f() {
> +return 0;
> +  }
> +};
> +
> +void test() {
> +  // Check that InstantiateUsedMemberDefinition::S::f() is defined
> +  // as a result of the S class template implicit instantiation
> +  // CHECK: define linkonce_odr i32
> @_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
> +  S inst;
> +}
> +} // namespace InstantiateUsedMemberDefinition
>
>
I believe this commit broke our (Apple) internal LTO bots.  I'm working on
getting more information and narrowing it down.  The symptom is:


Undefined symbols for architecture x86_64:
  "llvm::cfg::Update::dump() const", referenced from:
  DominatorTreeBatchUpdates_LegalizeDomUpdates_Test::TestBody() in
DominatorTreeBatchUpdatesTest.cpp.o
  DominatorTreeBatchUpdates_LegalizePostDomUpdates_Test::TestBody()
in DominatorTreeBatchUpdatesTest.cpp.o


Has anyone else seen a similar issue with this commit?

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

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hi, i wanted to squeeze in D59861  somewhere 
in the middle of your work, would you mind?
I'll definitely have a look at your patches soon :)


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

https://reviews.llvm.org/D59516



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

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

(whoops sry nvm typo in reviewer list)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59861



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

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso, alexfh.
Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, 
a.sidorin, szepet, mgorny.
Herald added a project: clang.

Given how much effort @boga95 is spending on the taint checker, i decided to 
undig my effort to remove the whole taint API business from the `ProgramState` 
class and instead implement it as our usual, modern inter-checker communication 
API.

Modern as in "putting all such stuff into the `ProgramState` class clearly 
doesn't scale, but we still didn't come up with anything better, so let's just 
put it in the header and see if a better design suddenly emerges in the future 
after we do it a few hundred times". In other words, this was the plan that is 
part of the even-more-long-term plan of completely deprecating the 
`void*`-based Generic Data Map in favor of something that the analyzer core 
could introspect and help checkers keep track of, which would reduce 
boilerplate and make developing checkers easier.

The patch is rebased on top of the current master, but most likely conflicts 
with @boga95's latest patches. I'm sorry i didn't do this quickly enough, but 
better late than never, i suppose. I do not insist on this design, but i 
believe it looks much cleaner.


Repository:
  rC Clang

https://reviews.llvm.org/D59861

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintTag.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/lib/StaticAnalyzer/Core/TaintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/TaintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/TaintManager.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//== TaintManager.cpp -- -*- C++ -*--=//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
-
-using namespace clang;
-using namespace ento;
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index = 0;
-  return &index;
-}
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index;
-  return &index;
-}
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -16,7 +16,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -458,9 +457,6 @@
   // Print out the tracked dynamic types.
   printDynamicTypeInfo(this, Out, NL, Sep);
 
-  // Print out tainted symbols.
-  printTaint(Out, NL);
-
   // Print checker-specific data.
   Mgr.getOwningEngine().printState(Out, this, NL, Sep, LC);
 }
@@ -474,22 +470,6 @@
   print(llvm::errs());
 }
 
-void ProgramState::printTaint(raw_ostream &Out,
-  const char *NL) const {
-  TaintMapImpl TM = get();
-
-  if (!TM.isEmpty())
-Out <<"Tainted symbols:" << NL;
-
-  for (TaintMapImpl::iterator I = TM.begin(), E = TM.end(); I != E; ++I) {
-Out << I->first << " : " << I->second << NL;
-  }
-}
-
-void ProgramState::dumpTaint() const {
-  printTaint(llvm::errs());
-}
-
 AnalysisManager& ProgramState::getAnalysisManager() const {
   return stateMgr->getOwningEngine().getAnalysisManager();
 }
@@ -657,166 +637,3 @@
   }
   return true;
 }
-
-ProgramStateRef ProgramState::addTaint(const Stmt *S,
-   const LocationContext *LCtx,
-   TaintTagType Kind) const {
-  if (const Expr *E = dyn_cast_or_null(S))
-S = E->Ign

Buildbot numbers for the week of 03/17/2019 - 03/23/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 03/17/2019 -
03/23/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 clang-cmake-aarch64-global-isel | 60:56:28
 clang-cmake-aarch64-lld | 60:02:42
 clang-cmake-x86_64-sde-avx512-linux | 47:34:55
 aosp-O3-polly-before-vectorizer-unprofitable| 47:25:54
 clang-cmake-aarch64-quick   | 47:07:54
 llvm-clang-x86_64-expensive-checks-win  | 33:05:52
 clang-lld-x86_64-2stage | 31:22:24
 clang-cmake-aarch64-full| 29:05:00
 clang-with-lto-ubuntu   | 27:40:37
 clang-with-thin-lto-ubuntu  | 25:04:43
 sanitizer-x86_64-linux-fast | 24:13:18
 lld-x86_64-win7 | 24:00:09
 clang-x64-windows-msvc  | 23:50:37
 sanitizer-x86_64-linux-bootstrap| 21:16:31
 clang-cmake-armv7-selfhost  | 17:07:13
 clang-cmake-armv7-selfhost-neon | 17:06:37
 clang-cmake-thumbv7-full-sh | 14:57:54
 clang-cmake-armv7-global-isel   | 10:42:59
 clang-ppc64le-linux-multistage  | 09:05:35
 clang-cmake-armv8-global-isel   | 08:48:15
 clang-cmake-thumbv8-full-sh | 08:27:04
 clang-cmake-armv7-quick | 08:06:16
 clang-cmake-armv8-lld   | 08:06:15
 clang-ppc64be-linux-lnt | 07:43:02
 clang-cmake-armv8-full  | 07:39:23
 clang-ppc64be-linux | 07:30:42
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 07:22:46
 clang-cmake-armv8-quick | 07:06:42
 clang-ppc64le-linux-lnt | 06:36:38
 clang-ppc64be-linux-multistage  | 06:31:49
 clang-cmake-armv8-selfhost-neon | 06:17:44
 clang-s390x-linux-lnt   | 06:08:09
 clang-s390x-linux   | 06:06:58
 ppc64le-lld-multistage-test | 06:03:15
 clang-s390x-linux-multistage| 05:57:16
 clang-cmake-armv7-full  | 05:51:13
 clang-cmake-x86_64-avx2-linux   | 05:49:02
 sanitizer-x86_64-linux  | 05:25:25
 sanitizer-x86_64-linux-bootstrap-ubsan  | 05:21:49
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 05:21:38
 clang-ppc64le-linux | 05:16:03
 sanitizer-x86_64-linux-bootstrap-msan   | 05:00:31
 clang-hexagon-elf   | 04:57:31
 llvm-hexagon-elf| 04:52:01
 sanitizer-x86_64-linux-fuzzer   | 04:47:05
 clang-cuda-build| 04:36:56
 reverse-iteration   | 04:02:12
 sanitizer-windows   | 03:18:07
 lld-x86_64-freebsd  | 03:17:07
 sanitizer-ppc64le-linux | 03:15:02
 lld-x86_64-darwin13 | 03:06:05
 clang-cmake-armv8-lnt   | 02:10:01
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11  | 02:00:42
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17  | 01:59:29
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a  | 01:58:11
 libcxx-libcxxabi-libunwind-armv7-linux-noexceptions | 01:57:51
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14  | 01:55:05
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 01:52:26
 clang-x86_64-linux-abi-test | 01:52:08
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03  | 01:51:37
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions   | 01:50:51
 lldb-x64-windows-ninja  | 01:44:57
 libcxx-libcxxa

Buildbot numbers for the week of 03/10/2019 - 03/16/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 03/10/2019 - 03/16/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 lldb-sphinx-docs| 47:00:46
 clang-cmake-aarch64-lld | 34:37:10
 clang-cmake-aarch64-full| 33:23:25
 clang-x64-windows-msvc  | 21:03:21
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 20:53:27
 lld-x86_64-win7 | 20:20:12
 clang-cmake-armv7-selfhost-neon | 18:02:47
 lldb-x64-windows-ninja  | 16:20:42
 clang-cmake-armv7-selfhost  | 13:09:51
 clang-cmake-thumbv7-full-sh | 07:56:26
 clang-cmake-armv7-full  | 05:44:46
 clang-lld-x86_64-2stage | 05:25:27
 clang-ppc64le-linux-lnt | 05:25:04
 sanitizer-x86_64-linux-bootstrap-ubsan  | 05:24:11
 clang-ppc64le-linux | 05:00:26
 clang-ppc64be-linux | 04:46:05
 clang-cuda-build| 04:45:19
 clang-cmake-thumbv8-full-sh | 04:31:57
 polly-amd64-linux   | 04:22:53
 clang-ppc64be-linux-lnt | 04:18:49
 clang-with-lto-ubuntu   | 04:12:43
 clang-s390x-linux-multistage| 04:07:27
 clang-ppc64le-linux-multistage  | 04:05:24
 sanitizer-x86_64-linux-android  | 04:02:09
 clang-cmake-armv8-full  | 03:42:39
 ppc64le-lld-multistage-test | 03:40:35
 clang-s390x-linux   | 03:14:00
 sanitizer-ppc64le-linux | 03:12:13
 clang-ppc64be-linux-multistage  | 03:06:27
 sanitizer-ppc64be-linux | 02:48:56
 clang-cmake-armv7-global-isel   | 02:44:13
 llvm-clang-x86_64-expensive-checks-win  | 02:07:51
 clang-s390x-linux-lnt   | 02:05:16
 clang-with-thin-lto-ubuntu  | 02:05:06
 clang-cmake-armv8-global-isel   | 01:59:40
 clang-cmake-x86_64-avx2-linux   | 01:58:07
 sanitizer-x86_64-linux  | 01:54:25
 libcxx-libcxxabi-libunwind-armv8-linux  | 01:51:24
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 01:50:45
 libcxx-libcxxabi-libunwind-aarch64-linux| 01:48:55
 clang-cmake-aarch64-global-isel | 01:33:50
 sanitizer-x86_64-linux-bootstrap-msan   | 01:30:08
 clang-cmake-armv8-quick | 01:24:06
 sanitizer-x86_64-linux-bootstrap| 01:20:39
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 01:19:58
 libcxx-libcxxabi-libunwind-armv7-linux-noexceptions | 01:16:02
 clang-hexagon-elf   | 01:11:28
 libcxx-libcxxabi-libunwind-armv7-linux  | 01:07:23
 lld-perf-testsuite  | 01:06:59
 clang-cmake-armv7-quick | 01:05:56
 clang-armv7-linux-build-cache   | 01:04:16
 sanitizer-x86_64-linux-fast | 01:03:32
 llvm-hexagon-elf| 01:02:56
 clang-cmake-x86_64-avx2-linux-perf  | 01:01:53
 clang-x86_64-debian-fast| 01:00:59
 clang-aarch64-linux-build-cache | 01:00:57
 lld-x86_64-darwin13 | 00:58:23
 reverse-iteration   | 00:57:47
 clang-cmake-x86_64-sde-avx512-linux | 00:56:19
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 00:51:39
 polly-arm-linux | 00:47:29
 sanitizer-x86_64-linux-autoconf | 00:30:55
 lld-x86_64-freebsd 

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

[Patrick Nappa via Phabricator via cfe-commits]

pnappa created this revision.
pnappa added a reviewer: lebedev.ri.
pnappa added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.

Fix for https://bugs.llvm.org/show_bug.cgi?id=41199#c1

Previously, if a user implicitly cast to a bool (say, in a conditional 
statement), the fix would be to add an explicit comparison. So, for a floating 
point implicit case to bool, from `if (f)`, the synthesised code would be `if 
(f != 0.0f)`.

Even if the flag "readability-uppercase-literal-suffix" was enabled, the 
synthesised suffix would be lowercase. This commit changes that, such that if 
that flag is enabled when "readability-implicit-bool-conversion" is enabled, 
the synthesised suffix is uppercase.

A non-covered case is when "modernize-use-default-member-init" is enabled, 
lower-case suffixes may still be synthesised (I think, based off the code). Any 
RFC whether this should be made a different issue or whether or not this 
behaviour should be added in?

Intended as my first commit to the llvm-project.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59859

Files:
  clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
  clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
  clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
  
clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix,readability-implicit-bool-conversion %t
+
+template
+void functionTaking(T);
+
+
+// Implicit conversion to bool with enforced uppercase suffix.
+
+// TODO: should we add complex number suffix tests? I don't see any here. It may be an annoyance thing due to it's a C++17 test. I should ask in phabricator when I submit.
+// XXX: ensure that tabs are 2 wide
+// XXX: run clang tidy on this using the clangtidy config at the root of the repo
+void implicitConversionToBoolSimpleCases() {
+  unsigned int unsignedInteger = 10u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: integer literal has suffix 'u', which is not uppercase
+  // CHECK-MESSAGES-NEXT: unsigned int unsignedInteger = 10u;
+  // CHECK-MESSAGES-NEXT: ^ ~
+  // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+  // CHECK-FIXES: unsigned int unsignedInteger = 10U;
+
+  functionTaking(unsignedInteger);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(unsignedInteger != 0U);
+
+  unsigned long unsignedLong = 10ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: integer literal has suffix 'ul', which is not uppercase [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: unsigned long unsignedLong = 10ul;
+  // CHECK-MESSAGES-NEXT: ^ ~~
+  // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+  // CHECK-FIXES: unsigned long unsignedLong = 10UL;
+
+  functionTaking(unsignedLong);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(unsignedLong != 0U);
+
+  float floating = 10.0f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: float floating = 10.0f;
+  // CHECK-MESSAGES-NEXT: ^   ~
+  // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}}
+  // CHECK-FIXES: float floating = 10.0F;
+
+  functionTaking(floating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(floating != 0.0F);
+
+  double doubleFloating = 10.0;
+  functionTaking(doubleFloating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(doubleFloating != 0.0);
+}
+
+void implicitConversionToBoolInComplexExpressions() {
+  bool boolean = true;
+
+  unsigned int integer = 10U;
+  unsigned int anotherInteger = 20U;
+  bool boolComingFromUnsignedInteger = integer + anotherInteger;
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: implicit conversion 'unsigned int' -> bool
+  // CHECK-FIXES: bool boolComingFromUnsignedInteger = (integer + anotherInteger) != 0U;
+
+  float floating = 0.2F;
+  // combination of two errors on the same line
+  bool boolComingFromFloating = floating - 0.3f || boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'float' -> bool
+  // CHECK-MESSAGES: :[[@LINE-2]]:44: warning: floa

[PATCH] D59725: Additions to creduce script

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Only a few more nits on my side, and this LGTM. WDYT, arichardson?




Comment at: clang/utils/creduce-clang-crash.py:137
+
+# If no message was found, use the top five stack trace functions,
+# ignoring some common functions

akhuang wrote:
> george.burgess.iv wrote:
> > Please expand a bit on why 5 was chosen (is there some deep reason behind 
> > it, or does it just seem like a sensible number?)
> There is no deep reason - it was an arbitrary smallish number to hopefully 
> not only pick up common stack trace functions
Sorry -- should've been clearer. I meant "in the comment in the code, please 
expand a bit [...]" :)



Comment at: clang/utils/creduce-clang-crash.py:362
+  r = Reduce(crash_script, file_to_reduce)
+  r.simplify_clang_args()
+  r.write_interestingness_test()

arichardson wrote:
> george.burgess.iv wrote:
> > akhuang wrote:
> > > george.burgess.iv wrote:
> > > > I'm unclear on why we do a partial simplification of clang args here, 
> > > > then a full reduction after creduce. Is there a disadvantage to instead 
> > > > doing:
> > > > 
> > > > r.write_interestingness_test()
> > > > r.clang_preprocess()
> > > > r.reduce_clang_args()
> > > > r.run_creduce()
> > > > r.reduce_clang_args()
> > > > 
> > > > ?
> > > > 
> > > > The final `reduce_clang_args` being there to remove extra 
> > > > `-D`/`-I`/etc. args if preprocessing failed somehow, to remove 
> > > > `-std=foo` args if those aren't relevant after reduction, etc.
> > > > 
> > > > The advantage to this being that we no longer need to maintain a 
> > > > `simplify` function, and creduce runs with a relatively minimal set of 
> > > > args to start with.
> > > > 
> > > > In any case, can we please add comments explaining why we chose 
> > > > whatever order we end up going with, especially where subtleties make 
> > > > it counter to what someone might naively expect?
> > > Basically the disadvantage is that clang takes longer to run before the 
> > > reduction, so it takes unnecessary time to iterate through all the 
> > > arguments beforehand. 
> > > And yeah, the final `reduce_clang_args` is there to minimize the clang 
> > > arguments needed to reproduce the crash on the reduced source file. 
> > > 
> > > If that makes sense, I can add comments for this-
> > Eh, I don't have a strong opinion here. My inclination is to prefer a 
> > simpler reduction script if the difference is len(args) clang invocations, 
> > since I assume creduce is going to invoke clang way more than len(args) 
> > times. I see a docstring detailing that `simplify` is only meant to speed 
> > things up now, so I'm content with the way things are.
> I think it makes sense to remove some clang args before running creduce since 
> removal of some flags can massively speed up reduction later (e.g. not 
> emitting debug info or compiling at -O0, only doing -emit-llvm, etc) if they 
> avoid expensive optimizations that don't cause the crash.
Agreed. My question was more "why do we have special reduction code on both 
sides of this instead of just `reduce_clang_args`'ing on both sides of the 
`run_creduce`." It wasn't clear to me that `simplify_clang_args` was only 
intended to speed things up, but now it is. :)



Comment at: clang/utils/creduce-clang-crash.py:198
+# Instead of modifying the filename in the test file, just run the command
+fd, empty_file = tempfile.mkstemp()
+if self.check_expected_output(filename=empty_file):

Did we want to use `NamedTemporaryFile` here as rnk suggested?

(If not, you can lift the `os.close`s to immediately after this line.)



Comment at: clang/utils/creduce-clang-crash.py:206
+print("\nTrying to preprocess the source file...")
+fd, tmpfile = tempfile.mkstemp()
+

Similar question about `NamedTemporaryFile`.

Please note that you'll probably have to pass `delete=False`, since apparently 
`delete=True` sets O_TEMPORARY on Windows, which... might follow the file 
across renames? I'm unsure.


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

https://reviews.llvm.org/D59725



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 192394.
axzhang added a comment.

Add documentation about the added option for `modernize-make-unique`.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make-uniqu

[PATCH] D59857: [analyzer] PR37501: Disable the assertion for reverse-engineering logical op short circuits.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso, alexfh.
Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, 
a.sidorin, szepet.
Herald added a project: clang.

Replace the incorrect assertion with a lot of swearing in comments. This should 
take care of these rare crashes such as 
https://bugs.llvm.org/show_bug.cgi?id=37501 and the minimal fallback behavior i 
added should hopefully be better than just dropping the assertion (instead of 
picking a path for backtracking at random, we admit that we don't know the 
value). The idea behind the original implementation is elegant, but it doesn't 
really work.

I did try to replace the logic with trying to evaluate logical operators as if 
they were regular operators: take the LHS value, take the RHS value, apply 
math. It's not easy because it requires tweaking liveness analysis to make sure 
that all the necessary values are alive; i couldn't immediately fix all the 
failres on tests. Even if i did get it working, i suspect that this is also a 
dead end, because we have these "unknown value" things. Even if the values of 
the operands were too complicated to compute and evaluated to `UnknownVal`s, we 
can still obtain the correct value of the logical operator via backtracking 
("we're at this point in our execution path, therefore the LHS must have been 
assumed to be true"), but not via math ("whoops, the LHS is unknown").

We could combine both techniques (backtracking and math, use one when the other 
fails), but that's an overkill for such a rare problem, so i decided to just 
remove the assertion.

The truly reliable solution in our situation would be to set a flag in the 
program state whenever we're doing a short circuit operation, and consume it in 
`VisitLogicalExpr`. But that also requires making sure we don't mess up with 
all those CFG cornercases, so i decided to leave it as an exercise for the 
future generations of Static Analyzer developers.


Repository:
  rC Clang

https://reviews.llvm.org/D59857

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Analysis/logical-ops.c


Index: clang/test/Analysis/logical-ops.c
===
--- clang/test/Analysis/logical-ops.c
+++ clang/test/Analysis/logical-ops.c
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-pointer-bool-conversion 
-analyzer-checker=core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection\
+// RUN:-analyzer-config eagerly-assume=false -verify %s
 
 void clang_analyzer_eval(int);
 
@@ -33,7 +34,21 @@
   return x >= start && x < end;
 }
 
-int undef(void) {} // expected-warning{{control reaches end of non-void 
function}}
+int undef(void) {}
 void useUndef(void) { 0 || undef(); }
 
 void testPointer(void) { (void) (1 && testPointer && 0); }
+
+char *global_ap, *global_bp, *global_cp;
+void ambiguous_backtrack_1() {
+  for (;;) {
+(global_bp - global_ap ? global_cp[global_bp - global_ap] : 0) || 1;
+global_bp++;
+  }
+}
+
+int global_a, global_b;
+void ambiguous_backtrack_2(int x) {
+  global_a = x >= 2 ? 1 : x;
+  global_b == x && 9 || 2;
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -627,6 +627,21 @@
 
 void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
   ExplodedNodeSet &Dst) {
+  // This method acts upon CFG elements for logical operators && and ||
+  // and attaches the value (true or false) to them as expressions.
+  // It doesn't produce any state splits.
+  // If we made it that far, we're past the point when we modeled the short
+  // circuit. It means that we should have precise knowledge about whether
+  // we've short-circuited. If we did, we already know the value we need to
+  // bind. If we didn't, the value of the RHS (casted to the boolean type)
+  // is the answer.
+  // Currently this method tries to figure out whether we've short-circuited
+  // by looking at the ExplodedGraph. This method is imperfect because there
+  // could inevitably have been merges that would have resulted in multiple
+  // potential path traversal histories. We bail out when we fail.
+  // Due to this ambiguity, a more reliable solution would have been to
+  // track the short circuit operation history path-sensitively until
+  // we evaluate the respective logical operator.
   assert(B->getOpcode() == BO_LAnd ||
  B->getOpcode() == BO_LOr);
 
@@ -648,10 +663,20 @@
 ProgramPoint P = N->getLocation();
 assert(P.getAs()|| P.getAs());
 (void) P;
-assert(N->pred_size() == 1);
+if (N->pred_size() != 1) {
+  // We failed to 

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Ok, cool. In that case, I think this patch is ready to be committed, and would 
appreciate it if someone could commit it. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59407: [clangd] Add RelationSlab

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Submitting code as it becomes ready is the usual practice here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59838: gn build: Add build files for clang-include-fixer and find-all-symbols

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357042: gn build: Add build files for clang-include-fixer 
and find-all-symbols (authored by nico, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59838?vs=192334&id=192387#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59838

Files:
  llvm/trunk/utils/gn/secondary/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn

Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
@@ -0,0 +1,26 @@
+static_library("clang-include-fixer") {
+  output_name = "clangIncludeFixer"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Parse",
+"//clang/lib/Sema",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FuzzySymbolIndex.cpp",
+"InMemorySymbolIndex.cpp",
+"IncludeFixer.cpp",
+"IncludeFixerContext.cpp",
+"SymbolIndexManager.cpp",
+"YamlSymbolIndex.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
@@ -0,0 +1,18 @@
+executable("clang-include-fixer") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer",
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Rewrite",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"ClangIncludeFixer.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
@@ -0,0 +1,23 @@
+static_library("find-all-symbols") {
+  output_name = "findAllSymbols"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Tooling",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FindAllMacros.cpp",
+"FindAllSymbols.cpp",
+"FindAllSymbolsAction.cpp",
+"HeaderMapCollector.cpp",
+"PathConfig.cpp",
+"PragmaCommentHandler.cpp",
+"STLPostfixHeaderMap.cpp",
+"SymbolInfo.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
@@ -0,0 +1,17 @@
+executable("find-all-symbols") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"FindAllSymbolsMain.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/BUILD.gn
@@ -7,6 +7,8 @@
 "//clang-tools-extra/clang-apply-replacements/tool:clang-apply-replacements",
 "//clang-tools-extra/clang-change-namespace/tool:clang-change-namespace",
 "//clang-tools-extra/clang-doc/tool:clang-doc",
+"//clang-tools-extra/clang-in

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

As this is the first of a series of patches adding support for relations, and 
then building type hierarchy subtypes on top (as discussed here 
), how should we go about landing this 
-- should we land each patch in the series as soon as it's ready, or should we 
wait to land them all together?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington closed this revision.
erik.pilkington added a comment.

Landed in r357040. (I forgot to write Differential revision:!)


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

https://reviews.llvm.org/D59670



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

r357040 - [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Mar 26 16:21:19 2019
New Revision: 357040

URL: http://llvm.org/viewvc/llvm-project?rev=357040&view=rev
Log:
[Sema] Fix an assert when a block captures a constexpr local

MarkVarDeclODRUsed indirectly calls captureInBlock, which creates a copy
expression. The copy expression is insulated in it's own
ExpressionEvaluationContext, so it saves, mutates, and restores MaybeODRUseExprs
as CleanupVarDeclMarking is iterating through it, leading to a crash. Fix this
by iterating through a local copy of MaybeODRUseExprs.

rdar://47493525

https://reviews.llvm.org/D59670

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/blocks.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=357040&r1=357039&r2=357040&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Mar 26 16:21:19 2019
@@ -587,13 +587,13 @@ public:
   /// element type here is ExprWithCleanups::Object.
   SmallVector ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a reference to a variable (constant) that may or may not
-  ///  be odr-used in this Expr, and we won't know until all lvalue-to-rvalue
-  ///  and discarded value conversions have been applied to all subexpressions
-  ///  of the enclosing full expression.  This is cleared at the end of each
-  ///  full expression.
-  llvm::SmallPtrSet MaybeODRUseExprs;
+  /// Store a set of either DeclRefExprs or MemberExprs that contain a 
reference
+  /// to a variable (constant) that may or may not be odr-used in this Expr, 
and
+  /// we won't know until all lvalue-to-rvalue and discarded value conversions
+  /// have been applied to all subexpressions of the enclosing full expression.
+  /// This is cleared at the end of each full expression.
+  using MaybeODRUseExprSet = llvm::SmallPtrSet;
+  MaybeODRUseExprSet MaybeODRUseExprs;
 
   std::unique_ptr PreallocatedFunctionScope;
 
@@ -1029,7 +1029,7 @@ public:
 /// context (i.e. the number of TypoExprs created).
 unsigned NumTypos;
 
-llvm::SmallPtrSet SavedMaybeODRUseExprs;
+MaybeODRUseExprSet SavedMaybeODRUseExprs;
 
 /// The lambdas that are present within this context, if it
 /// is indeed an unevaluated context.

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=357040&r1=357039&r2=357040&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Mar 26 16:21:19 2019
@@ -15690,7 +15690,12 @@ ExprResult Sema::ActOnConstantExpression
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
 VarDecl *Var;
 SourceLocation Loc;
 if (DeclRefExpr *DRE = dyn_cast(E)) {
@@ -15707,10 +15712,10 @@ void Sema::CleanupVarDeclMarking() {
/*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
-  MaybeODRUseExprs.clear();
+  assert(MaybeODRUseExprs.empty() &&
+ "MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
 }
 
-
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
 VarDecl *Var, Expr *E) {
   assert((!E || isa(E) || isa(E)) &&

Modified: cfe/trunk/test/SemaCXX/blocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/blocks.cpp?rev=357040&r1=357039&r2=357040&view=diff
==
--- cfe/trunk/test/SemaCXX/blocks.cpp (original)
+++ cfe/trunk/test/SemaCXX/blocks.cpp Tue Mar 26 16:21:19 2019
@@ -145,3 +145,11 @@ namespace test6c {
 A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}


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

r357041 - Emit -Wfortify-source using DiagRuntimeBehaviour

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Mar 26 16:21:22 2019
New Revision: 357041

URL: http://llvm.org/viewvc/llvm-project?rev=357041&view=rev
Log:
Emit -Wfortify-source using DiagRuntimeBehaviour

This fixes a false positive on the following, where st is configured to have
different sizes based on some preprocessor logic:

  if (sizeof(buf) == sizeof(*st))
memcpy(&buf, st, sizeof(*st));

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/warn-fortify-source.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=357041&r1=357040&r2=357041&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Mar 26 16:21:22 2019
@@ -431,9 +431,10 @@ void Sema::checkFortifiedBuiltinMemoryFu
 FunctionName = FunctionName.drop_front(std::strlen("__builtin_"));
   }
 
-  Diag(TheCall->getBeginLoc(), DiagID)
-  << FunctionName << ObjectSize.toString(/*Radix=*/10)
-  << UsedSize.toString(/*Radix=*/10);
+  DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
+  PDiag(DiagID)
+  << FunctionName << ObjectSize.toString(/*Radix=*/10)
+  << UsedSize.toString(/*Radix=*/10));
 }
 
 static bool SemaBuiltinSEHScopeCheck(Sema &SemaRef, CallExpr *TheCall,

Modified: cfe/trunk/test/Sema/warn-fortify-source.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-fortify-source.c?rev=357041&r1=357040&r2=357041&view=diff
==
--- cfe/trunk/test/Sema/warn-fortify-source.c (original)
+++ cfe/trunk/test/Sema/warn-fortify-source.c Tue Mar 26 16:21:22 2019
@@ -20,6 +20,9 @@ void call_memcpy() {
   char dst[10];
   char src[20];
   memcpy(dst, src, 20); // expected-warning {{memcpy' will always overflow; 
destination buffer has size 10, but size argument is 20}}
+
+  if (sizeof(dst) == sizeof(src))
+memcpy(dst, src, 20); // no warning, unreachable
 }
 
 void call_memcpy_type() {


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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In D58797#1443411 , @efriedma wrote:

> For that particular case, I think we could suppress the false positive using 
> DiagRuntimeBehavior.  How many total false positives are we talking about, 
> and how many can the compiler statically prove are unreachable?


Sure, that seems like a perfect fit, I added it in r357041. There are only two 
false-positives IIUC, and they're both immediately guarded by an `if (sizeof(x) 
== sizeof(y))`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 added a comment.

I add a new taint type, which represents a lack of taintedness. That's why I 
changed the name of `addTaint()` to `setTaint()`. Of course, it's not an 
important change, I can move it to another patch.


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

https://reviews.llvm.org/D59516



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

[PATCH] D46140: [coroutines] Add std::experimental::task type

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker updated this revision to Diff 192384.
lewissbaker added a comment.

Added missing 'require coroutines' for experimental/task entry in 
module.modulemap
Added missing #include in experimental/task header that was failing module 
builds


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46140

Files:
  include/CMakeLists.txt
  include/experimental/__memory
  include/experimental/memory_resource
  include/experimental/task
  include/module.modulemap
  test/std/experimental/task/awaitable_traits.hpp
  test/std/experimental/task/counted.hpp
  test/std/experimental/task/lit.local.cfg
  test/std/experimental/task/manual_reset_event.hpp
  test/std/experimental/task/sync_wait.hpp
  test/std/experimental/task/task.basic/task_custom_allocator.pass.cpp
  test/std/experimental/task/task.basic/task_of_value.pass.cpp
  test/std/experimental/task/task.basic/task_of_void.pass.cpp
  test/std/experimental/task/task.lifetime/task_parameter_lifetime.pass.cpp
  test/std/experimental/task/task.lifetime/task_return_value_lifetime.pass.cpp

Index: test/std/experimental/task/task.lifetime/task_return_value_lifetime.pass.cpp
===
--- /dev/null
+++ test/std/experimental/task/task.lifetime/task_return_value_lifetime.pass.cpp
@@ -0,0 +1,86 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include 
+#include 
+#include 
+
+#include "../counted.hpp"
+#include "../sync_wait.hpp"
+
+DEFINE_COUNTED_VARIABLES();
+
+void test_return_value_lifetime()
+{
+  counted::reset();
+
+  auto f = [](bool x) -> std::experimental::task
+  {
+if (x) {
+  counted c;
+  co_return std::move(c);
+}
+co_return {};
+  };
+
+  {
+auto t = f(true);
+
+assert(counted::active_instance_count() == 0);
+assert(counted::copy_constructor_count() == 0);
+assert(counted::move_constructor_count() == 0);
+
+{
+  auto c = sync_wait(std::move(t));
+  assert(c.id() == 1);
+
+  assert(counted::active_instance_count() == 2);
+  assert(counted::copy_constructor_count() == 0);
+  assert(counted::move_constructor_count() > 0);
+  assert(counted::default_constructor_count() == 1);
+}
+
+// The result value in 't' is still alive until 't' destructs.
+assert(counted::active_instance_count() == 1);
+  }
+
+  assert(counted::active_instance_count() == 0);
+
+  counted::reset();
+
+  {
+auto t = f(false);
+
+assert(counted::active_instance_count() == 0);
+assert(counted::copy_constructor_count() == 0);
+assert(counted::move_constructor_count() == 0);
+
+{
+  auto c = sync_wait(std::move(t));
+  assert(c.id() == 1);
+
+  assert(counted::active_instance_count() == 2);
+  assert(counted::copy_constructor_count() == 0);
+  assert(counted::move_constructor_count() > 0);
+  assert(counted::default_constructor_count() == 1);
+}
+
+// The result value in 't' is still alive until 't' destructs.
+assert(counted::active_instance_count() == 1);
+  }
+}
+
+int main()
+{
+  test_return_value_lifetime();
+  return 0;
+}
Index: test/std/experimental/task/task.lifetime/task_parameter_lifetime.pass.cpp
===
--- /dev/null
+++ test/std/experimental/task/task.lifetime/task_parameter_lifetime.pass.cpp
@@ -0,0 +1,57 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include 
+#include 
+
+#include "../counted.hpp"
+#include "../sync_wait.hpp"
+
+DEFINE_COUNTED_VARIABLES();
+
+void test_parameter_lifetime()
+{
+  counted::reset();
+
+  auto f = [](counted c) -> std::experimental::task
+  {
+co_return c.id();
+  };
+
+  {
+auto t = f({});
+
+assert(counted::active_instance_count() == 1);
+assert(counted::copy_constructor_count() == 0);
+assert(counted::move_constructor_count() <= 2); // Ideally <= 1
+
+auto id = sync_wait(t);
+assert(id == 1);
+
+assert(counted::active_instance_count() == 1);
+assert(counted::copy_constructor_count() == 0);
+
+// We are relying on C++17 copy-elision when passing the temporary counter
+// into f(). Then f() must move the parameter into the coroutine fra

[PATCH] D46140: [coroutines] Add std::experimental::task type

[Lewis Baker via Phabricator via cfe-commits]

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

Reopening as the commit for this diff was reverted due to it breaking the 
buildbot module builds.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46140



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192383.
stephanemoore added a comment.

Update tests to match updated diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of ``-self`` on super instances in initializers of subclasses
+of ``NSObject`` and recommends calling a superclass initializer instead.
+
+Invoking ``-self`` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,12 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of ``-self`` on super instances in initializers of
+  subclasses of ``NSObject`` and recommends calling a superclass initializer
+  instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-super-self.html
+class Super

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

[Csaba Dabis via Phabricator via cfe-commits]

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

Cool! I have found this message-semantic idea useful in Unity where every 
`GameObject` could talk with each other in a very generic way 
(https://docs.unity3d.com/ScriptReference/GameObject.SendMessage.html).
I am looking forward for more!


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

https://reviews.llvm.org/D58367



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

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

slavapestov wrote:
> rjmccall wrote:
> > This isn't for an arbitrary class ref, it's for the global class list.  I'd 
> > say something like "the global class list sets the LSB to 1 on any class 
> > stubs".
> Can you explain what the difference is? My understanding is that the thing 
> you pass to objc_loadClassref() (or load from directly) is a "classref". This 
> is for classes you reference, not classes you define.
Oh, I see what you're saying, nevermind.


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

https://reviews.llvm.org/D59628



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

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 192378.
slavapestov marked 2 inline comments as done.
slavapestov added a comment.

Second revision adds the following:

- Validation of the attribute in Sema
- Sema tests for the above
- Correct behavior of super method calls of a class with the attribute
- More comprehensive CodeGen tests


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

https://reviews.llvm.org/D59628

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/CodeGenObjC/class-stubs.m
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/SemaObjC/class-stub-attr-unsupported.m
  test/SemaObjC/class-stub-attr.m

Index: test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_stub(123))) // expected-error {{'objc_class_stub' attribute takes no arguments}}
+@interface InvalidClassStubAttribute : NSObject
+@end
+
+__attribute__((objc_class_stub)) // expected-error {{'objc_class_stub' attribute only applies to Objective-C interfaces}}
+int cannotHaveObjCClassStubAttribute() {}
Index: test/SemaObjC/class-stub-attr-unsupported.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr-unsupported.m
@@ -0,0 +1,10 @@
+// RUN: %clang -target i386-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target i386-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface StubClass : NSObject // expected-error {{'objc_class_stub' attribute is not supported for this target}}
+@end
Index: test/Misc/pragma-attribute-supported-attributes-list.test
===
--- test/Misc/pragma-attribute-supported-attributes-list.test
+++ test/Misc/pragma-attribute-supported-attributes-list.test
@@ -95,6 +95,7 @@
 // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridgeRelated (SubjectMatchRule_record)
+// CHECK-NEXT: ObjCClassStub (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCCompleteDefinition (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCDesignatedInitializer (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface)
Index: test/CodeGenObjC/class-stubs.m
===
--- /dev/null
+++ test/CodeGenObjC/class-stubs.m
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
+// RUN: FileCheck %s
+
+// -- classref for the message send in main()
+//
+// The class is declared with objc_class_stub, so LSB of the class pointer
+// must be set to 1.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_REFERENCES_$_" = private global i8* getelementptr (i8, i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_Base" to i8*), i32 1), section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8
+
+// -- classref for the super message send in anotherClassMethod()
+//
+// Metaclasses do not use the "stub" mechanism and are referenced statically.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_SUP_REFS_$_" = private global %struct._class_t* @"OBJC_METACLASS_$_Derived", section "__DATA,__objc_superrefs,regular,no_dead_strip", align 8
+
+// -- classref for the super message send in anotherInstanceMethod()
+//
+// The class is declared with objc_class_stub, so LSB of the class pointer
+// must be set to 1.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_SUP_REFS_$_.1" = private global i8* getelementptr (i8, i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_Derived" to i8*), i32 1), section "__DATA,__objc_superrefs,regular,no_dead_strip", align 8
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface 

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+   "invoke a superclass initializer?")
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),

stephanemoore wrote:
> aaron.ballman wrote:
> > Is there a %0 missing from the diagnostic for this method declaration 
> > you're passing in?
> Good catch! That's egg on my face 😅🥚
> 
> I think I was internally conflicted over specifically mentioning -[NSObject 
> self] versus using the method declaration of the message expression and 
> somehow got stuck halfway in-between 😓 I think it's better to mention the 
> method declaration of the message. Let me know if you think that I should 
> instead mention -[NSObject self].
Whoops; I forgot to update the message in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov marked 11 inline comments as done.
slavapestov added a comment.

Thanks for the comments everyone. In the next version of the patch, I fixed 
most of the review feedback and also addressed an issue where super message 
sends were not calling `objc_loadClassRef()`.




Comment at: include/clang/Basic/Attr.td:1802
+  let Documentation = [ObjCClassStubDocs];
+}
+

aaron.ballman wrote:
> Does this attribute make sense outside of ObjC? e.g., should it require an 
> ObjC compiland? If it should only be used in ObjC, then it should have a 
> `LangOpts` field.
The other ObjC attributes, such as ObjCSubclassingRestricted, do not have a 
LangOpts. Do you want me to add a LangOpts field to those too? Or is it 
unnecessary since they're already restricted to InterfaceDecls?



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

aaron.ballman wrote:
> This seems like the wrong category -- the attribute doesn't apply to 
> functions.
Would DocCatType make more sense? Would you like me to change 
ObjCRuntimeVisible and a handful of other miscategorized attributes too?



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

rjmccall wrote:
> This isn't for an arbitrary class ref, it's for the global class list.  I'd 
> say something like "the global class list sets the LSB to 1 on any class 
> stubs".
Can you explain what the difference is? My understanding is that the thing you 
pass to objc_loadClassref() (or load from directly) is a "classref". This is 
for classes you reference, not classes you define.



Comment at: lib/CodeGen/CGObjCMac.cpp:7285
 
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
  false, llvm::GlobalValue::PrivateLinkage,

jordan_rose wrote:
> Is there a concern here in the non-stub case if GetClassGlobal no longer 
> produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check 
> [again].)
You raise a good point. I brought back the assert in the next iteration of the 
patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628



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

[PATCH] D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

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

dexonsmith closed this revision.
dexonsmith added a comment.

Committed r357038.


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

https://reviews.llvm.org/D59388



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

r357038 - Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

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

Author: dexonsmith
Date: Tue Mar 26 15:32:06 2019
New Revision: 357038

URL: http://llvm.org/viewvc/llvm-project?rev=357038&view=rev
Log:
Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

FileManager constructs a VFS in its constructor if it isn't passed one,
and there's no way to reset it.  Make that contract clear by returning a
reference from its accessor.

https://reviews.llvm.org/D59388

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPLexerChange.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=357038&r1=357037&r2=357038&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Tue Mar 26 15:32:06 2019
@@ -221,9 +221,7 @@ public:
   FileSystemOptions &getFileSystemOpts() { return FileSystemOpts; }
   const FileSystemOptions &getFileSystemOpts() const { return FileSystemOpts; }
 
-  IntrusiveRefCntPtr getVirtualFileSystem() const {
-return FS;
-  }
+  llvm::vfs::FileSystem &getVirtualFileSystem() const { return *FS; }
 
   /// Retrieve a file entry for a "virtual" file that acts as
   /// if there were a file with the given name on disk.

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=357038&r1=357037&r2=357038&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Tue Mar 26 15:32:06 2019
@@ -380,7 +380,7 @@ public:
   /// {
 
   llvm::vfs::FileSystem &getVirtualFileSystem() const {
-return *getFileManager().getVirtualFileSystem();
+return getFileManager().getVirtualFileSystem();
   }
 
   /// }

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue Mar 26 15:32:06 2019
@@ -1079,7 +1079,7 @@ bool ASTUnit::Parse(std::shared_ptrgetVirtualFileSystem() &&
+assert(VFS == &FileMgr->getVirtualFileSystem() &&
"VFS passed to Parse and VFS in FileMgr are different");
 
   auto CCInvocation = std::make_shared(*Invocation);
@@ -1097,7 +1097,7 @@ bool ASTUnit::Parse(std::shared_ptrgetVirtualFileSystem() == VFS)
+  if (VFS && FileMgr && &FileMgr->getVirtualFileSystem() == VFS)
 Clang->setFileManager(&*FileMgr);
   else
 FileMgr = Clang->createFileManager(std::move(VFS));
@@ -1690,7 +1690,7 @@ std::unique_ptr ASTUnit::LoadFr
 
   if (AST->LoadFromCompilerInvocation(std::move(PCHContainerOps),
   PrecompilePreambleAfterNParses,
-  AST->FileMgr->getVirtualFileSystem()))
+  &AST->FileMgr->getVirtualFileSystem()))
 return nullptr;
   return AST;
 }
@@ -1797,7 +1797,7 @@ bool ASTUnit::Reparse(std::shared_ptrgetVirtualFileSystem();
+VFS = &FileMgr->getVirtualFileSystem();
   }
 
   clearFileLevelDecls();
@@ -2211,18 +2211,18 @@ void ASTUnit::CodeComplete(
   if (Preamble) {
 std::string CompleteFilePath(File);
 
-auto VFS = FileMgr.getVirtualFileSystem();
-auto CompleteFileStatus = VFS->status(CompleteFilePath);
+auto &VFS = FileMgr.getVirtualFileSystem();
+auto CompleteFileStatus = VFS.status(CompleteFilePath);
 if (CompleteFileStatus) {
   llvm::sys::fs::UniqueID CompleteFileID = 
CompleteFileStatus->getUniqueID();
 
   std::string MainPath(OriginalSourceFile);
-  auto MainStatus = VFS->status(MainPath);
+  auto MainStatus = VFS.status(MainPath);
   if (MainStatus) {
 llvm::sys::fs::UniqueID MainID = MainStatus->getUniqueID();
 if (CompleteFileID == MainID && Line > 1)
   OverrideMainBuffer = getMainBufferWithPrecompiledPreamble(
-  PCHContainerOps, Inv, VFS, false, Line - 1);
+  PCHContainerOps, Inv, &VFS, false, Line - 1);
   }
 }
   }
@@ -2233,7 +2233,8 @@ void ASTUnit::CodeComplete(
 assert(Preamble &&
"No preamble was built, but OverrideMainBuffer is not null");
 
-auto VFS = FileMgr.getVirtualFileSystem();
+IntrusiveRefCntPtr VFS

[clang-tools-extra] r357038 - Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

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

Author: dexonsmith
Date: Tue Mar 26 15:32:06 2019
New Revision: 357038

URL: http://llvm.org/viewvc/llvm-project?rev=357038&view=rev
Log:
Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

FileManager constructs a VFS in its constructor if it isn't passed one,
and there's no way to reset it.  Make that contract clear by returning a
reference from its accessor.

https://reviews.llvm.org/D59388

Modified:
clang-tools-extra/trunk/clang-move/Move.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp

Modified: clang-tools-extra/trunk/clang-move/Move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/Move.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clang-move/Move.cpp (original)
+++ clang-tools-extra/trunk/clang-move/Move.cpp Tue Mar 26 15:32:06 2019
@@ -87,8 +87,7 @@ std::string MakeAbsolutePath(StringRef C
 std::string MakeAbsolutePath(const SourceManager &SM, StringRef Path) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
-  SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
-  AbsolutePath))
+  
SM.getFileManager().getVirtualFileSystem().makeAbsolute(AbsolutePath))
 llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
  << '\n';
   // Handle symbolic link path cases.

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Tue Mar 26 15:32:06 2019
@@ -362,7 +362,7 @@ ClangTidyASTConsumerFactory::CreateASTCo
   auto WorkingDir = Compiler.getSourceManager()
 .getFileManager()
 .getVirtualFileSystem()
-->getCurrentWorkingDirectory();
+.getCurrentWorkingDirectory();
   if (WorkingDir)
 Context.setCurrentBuildDirectory(WorkingDir.get());
 
@@ -555,7 +555,7 @@ void handleErrors(llvm::ArrayRef BaseFS) {
   ErrorReporter Reporter(Context, Fix, BaseFS);
   llvm::vfs::FileSystem &FileSystem =
-  *Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
+  Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
   auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
   if (!InitialWorkingDir)
 llvm::report_fatal_error("Cannot get current working path.");

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Mar 26 15:32:06 2019
@@ -265,7 +265,7 @@ llvm::Optional getCanonical
   llvm::SmallString<128> FilePath = F->getName();
   if (!llvm::sys::path::is_absolute(FilePath)) {
 if (auto EC =
-SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(
+SourceMgr.getFileManager().getVirtualFileSystem().makeAbsolute(
 FilePath)) {
   elog("Could not turn relative path '{0}' to absolute: {1}", FilePath,
EC.message());


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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 192372.
NoQ marked 6 inline comments as done.
NoQ added a comment.

Remove memoization for now. Address a few inline comments. I'm mostly done with 
this first patch, did i accidentally miss anything?

In D58367#1402796 , @Szelethus wrote:

> 1. It would be great to have unit tests for this.


Mm, i have problems now that checker registry is super locked down: i need a 
bug report object => i need a checker (or at least a `CheckName`) => i need the 
whole `AnalysisConsumer` => i can no longer override `ASTConsumer` methods for 
testing purposes (because `AnalysisConsumer` is a final sub-class of 
`ASTConsumer`). Do you have a plan B for that?

I also generally don't see many good unit tests we could write here, that would 
add much to the integration tests we already have, but this memoization problem 
could have made a nice unit test.

In D58367#1423497 , @Charusso wrote:

> In D58367#1423451 , @NoQ wrote:
>
> > Found a bug! The lambda has to manually make sure that the bug report 
> > actually does have something to do with the checker.
>
>
> I think message-semantic is better than storing some data in two places. 
> BugReport has `getCheckName()` and may if the `NoteTag` knows the name of its 
> author then it is comparable.
>  I am not sure but may you could hack the lifetime of that `StringRef` from 
> `CheckName` to not to report on something purged out (/not exists?).


Storing and comparing the checker pointer ought to be cheaper and more precise 
than storing and comparing its name as a string that can be easily extracted 
from it. Still, storing a string may be interesting when it comes to 
identifying non-checker note sources. I guess we'll have to think more about 
that.


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

https://reviews.llvm.org/D58367

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -91,6 +91,14 @@
  // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  vm_deallocate(port, address, size); // no-note
+  1 / 0; // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+  return KERN_SUCCESS;
+}
+
 // Make sure we find the bug when the object is destroyed within an
 // automatic destructor.
 MIG_SERVER_ROUTINE
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -201,7 +201,9 @@
   svalBuilder(StateMgr.getSValBuilder()),
   ObjCNoRet(mgr.getASTContext()),
   BR(mgr, *this),
-  VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
+  VisitedCallees(VisitedCalleesIn),
+  HowToInline(HowToInlineIn),
+  NoteTags(G.getAllocator()) {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
   if (TrimInterval != 0) {
 // Enable eager node reclamation when constructing the ExplodedGraph.
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2500,6 +2500,30 @@
   return nullptr;
 }
 
+int NoteTag::Kind = 0;
+
+void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
+  static int Tag = 0;
+  ID.AddPointer(&Tag);
+}
+
+std::shared_ptr
+TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+  BugReport &R) {
+  ProgramPoint PP = N->getLocation();
+  const NoteTag *T = dyn_cast_or_null(PP.getTag());
+  if (!T)
+return nullptr;
+
+  if (Optional Msg = T->generateMessage(BRC, R)) {
+PathDiagnosticLocation Loc =
+PathDiagnosticLocation::create(PP, BRC.getSourceManager());
+return std::make_shared(Loc, *Msg);
+  }
+
+  return nullptr;
+}
+
 void FalsePositiveRefutationBRVisitor::Profile(
 llv

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.

Eugene.Zelenko wrote:
> Please use double backticks for language constructs. Same in other places.
Sorry I misread your earlier comment. I have been editing too much Markdown 
recently 😅 I have updated to use double backticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192373.
stephanemoore added a comment.

Use double backticks rather than single backticks for symbols in documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of ``-self`` on super instances in initializers of subclasses
+of ``NSObject`` and recommends calling a superclass initializer instead.
+
+Invoking ``-self`` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,12 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of ``-self`` on super instances in initializers of
+  subclasses of ``NSObject`` and recommends calling a superclass initializer
+  instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org

[PATCH] D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

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

dexonsmith closed this revision.
dexonsmith added a comment.

Committed r357037.


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

https://reviews.llvm.org/D59377



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

[clang-tools-extra] r357037 - Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

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

Author: dexonsmith
Date: Tue Mar 26 15:18:52 2019
New Revision: 357037

URL: http://llvm.org/viewvc/llvm-project?rev=357037&view=rev
Log:
Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager.  CompilerInstance and its clients already went to some
trouble to make these match.  Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager, to avoid repeating construction
logic in clients that just wanted to customize the VFS.

https://reviews.llvm.org/D59377

Modified:
clang-tools-extra/trunk/clangd/Compiler.cpp

Modified: clang-tools-extra/trunk/clangd/Compiler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- clang-tools-extra/trunk/clangd/Compiler.cpp (original)
+++ clang-tools-extra/trunk/clangd/Compiler.cpp Tue Mar 26 15:18:52 2019
@@ -95,7 +95,7 @@ prepareCompilerInstance(std::unique_ptr<
   if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
   Clang->getInvocation(), Clang->getDiagnostics(), VFS))
 VFS = VFSWithRemapping;
-  Clang->setVirtualFileSystem(VFS);
+  Clang->createFileManager(VFS);
 
   Clang->setTarget(TargetInfo::CreateTargetInfo(
   Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));


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

r357037 - Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

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

Author: dexonsmith
Date: Tue Mar 26 15:18:52 2019
New Revision: 357037

URL: http://llvm.org/viewvc/llvm-project?rev=357037&view=rev
Log:
Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager.  CompilerInstance and its clients already went to some
trouble to make these match.  Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager, to avoid repeating construction
logic in clients that just wanted to customize the VFS.

https://reviews.llvm.org/D59377

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Tue Mar 26 15:18:52 2019
@@ -175,6 +175,10 @@ class FileManager : public RefCountedBas
   void fillRealPathName(FileEntry *UFE, llvm::StringRef FileName);
 
 public:
+  /// Construct a file manager, optionally with a custom VFS.
+  ///
+  /// \param FS if non-null, the VFS to use.  Otherwise uses
+  /// llvm::vfs::getRealFileSystem().
   FileManager(const FileSystemOptions &FileSystemOpts,
   IntrusiveRefCntPtr FS = nullptr);
   ~FileManager();

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Tue Mar 26 15:18:52 2019
@@ -82,9 +82,6 @@ class CompilerInstance : public ModuleLo
   /// Auxiliary Target info.
   IntrusiveRefCntPtr AuxTarget;
 
-  /// The virtual file system.
-  IntrusiveRefCntPtr VirtualFileSystem;
-
   /// The file manager.
   IntrusiveRefCntPtr FileMgr;
 
@@ -382,20 +379,8 @@ public:
   /// @name Virtual File System
   /// {
 
-  bool hasVirtualFileSystem() const { return VirtualFileSystem != nullptr; }
-
   llvm::vfs::FileSystem &getVirtualFileSystem() const {
-assert(hasVirtualFileSystem() &&
-   "Compiler instance has no virtual file system");
-return *VirtualFileSystem;
-  }
-
-  /// Replace the current virtual file system.
-  ///
-  /// \note Most clients should use setFileManager, which will implicitly reset
-  /// the virtual file system to the one contained in the file manager.
-  void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
-VirtualFileSystem = std::move(FS);
+return *getFileManager().getVirtualFileSystem();
   }
 
   /// }
@@ -645,7 +630,8 @@ public:
   /// Create the file manager and replace any existing one with it.
   ///
   /// \return The new file manager on success, or null on failure.
-  FileManager *createFileManager();
+  FileManager *
+  createFileManager(IntrusiveRefCntPtr VFS = nullptr);
 
   /// Create the source manager and replace any existing one with it.
   void createSourceManager(FileManager &FileMgr);

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue Mar 26 15:18:52 2019
@@ -1078,28 +1078,29 @@ bool ASTUnit::Parse(std::shared_ptrgetVirtualFileSystem() &&
+   "VFS passed to Parse and VFS in FileMgr are different");
+
   auto CCInvocation = std::make_shared(*Invocation);
   if (OverrideMainBuffer) {
 assert(Preamble &&
"No preamble was built, but OverrideMainBuffer is not null");
-IntrusiveRefCntPtr OldVFS = VFS;
 Preamble->AddImplicitPreamble(*CCInvocation, VFS, 
OverrideMainBuffer.get());
-if (OldVFS != VFS && FileMgr) {
-  assert(OldVFS == FileMgr->getVirtualFileSystem() &&
- "VFS passed to Parse and VFS in FileMgr are different");
-  FileMgr = new FileManager(FileMgr->getFileSystemOpts(), VFS);
-}
+// VFS may have changed...
   }
 
   // Create the compiler instance to use for building the AST.
   std::unique_ptr Clang(
   new CompilerInstance(std::move(PCHContainerOps)));
-  if (FileMgr && VFS) {
-assert(VFS == FileMgr->getVirtualFileSystem() &&
-   "VFS passed to Parse and VFS in FileMgr are different");
-  } else if (VFS) {
-Clang

r357036 - [cmake] Reset variable before using it

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Tue Mar 26 15:16:53 2019
New Revision: 357036

URL: http://llvm.org/viewvc/llvm-project?rev=357036&view=rev
Log:
[cmake] Reset variable before using it

A bunch of macros use the same variable name, and since CMake macros
don't get their own scope, the value persists across macro invocations,
and we can end up exporting targets which shouldn't be exported. Clear
the variable before each use to avoid this.

Converting these macros to functions would also help, since it would
avoid the variable leaking into its parent scope, and that's something I
plan to follow up with. It won't fully address the problem, however,
since functions still inherit variables from their parent scopes, so if
someone in the parent scope just happened to use the same variable name
we'd still have the same issue.

Modified:
cfe/trunk/cmake/modules/AddClang.cmake

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=357036&r1=357035&r2=357036&view=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Tue Mar 26 15:16:53 2019
@@ -89,7 +89,7 @@ macro(add_clang_library name)
 target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang")
-
+  set(export_to_clangtargets)
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
@@ -137,6 +137,7 @@ macro(add_clang_tool name)
   add_dependencies(${name} clang-resource-headers)
 
   if (CLANG_BUILD_TOOLS)
+set(export_to_clangtargets)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
 NOT LLVM_DISTRIBUTION_COMPONENTS)
   set(export_to_clangtargets EXPORT ClangTargets)


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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192370.
hintonda added a comment.

- Address additional comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of ``cast<>`` used as the condition of a conditional
+statements which will assert on failure in Debug builds. The use of
+``cast<>`` implies that the operation cannot fail, and should not be
+used as the condition of a conditional statement.
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
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
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of ``cast<>`` used as condition in conditional statements
+  which will assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #includ

[PATCH] D58236: Make address space conversions a bit stricter.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D58236#1443556 , @ebevhan wrote:

> In D58236#1443082 , @rjmccall wrote:
>
> > I think C probably requires us to allow this under an explicit cast, but we 
> > can at least warn about it.  It does not require us to allow this as an 
> > implicit conversion, and that should be an error.
>
>
> Are you referring to casts to and from `void*`? I think the standard says 
> those have to be allowed, and I don't see anything about whether or not the 
> conversion has to be explicit.


No, I mean casts between pointer types that change the pointee address space.


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

https://reviews.llvm.org/D58236



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

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7285
 
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
  false, llvm::GlobalValue::PrivateLinkage,

Is there a concern here in the non-stub case if GetClassGlobal no longer 
produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check 
[again].)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1951
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+

```
if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum)) 
  if (auto *ToOriginEnum = dyn_cast(ToOrigin))
ToEnum = ToOriginEnum;
```


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

https://reviews.llvm.org/D59845



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

[PATCH] D59838: gn build: Add build files for clang-include-fixer and find-all-symbols

[Mirko Bonadei via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D59838



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.

Okay if we change the flag then I believe the tests under 
llvm/tests/lib/DebugInfo/CodeView must be updated. If we use NonTrivial all the 
existing tests work as expected.

LLVM change is here (you are all reviewers).
https://reviews.llvm.org/D59348


Repository:
  rC Clang

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

https://reviews.llvm.org/D59347



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

LLDB regression test that goes with this fix: https://reviews.llvm.org/D59847


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

https://reviews.llvm.org/D59845



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

r357031 - Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

[Jessica Paquette via cfe-commits]

Author: paquette
Date: Tue Mar 26 14:22:42 2019
New Revision: 357031

URL: http://llvm.org/viewvc/llvm-project?rev=357031&view=rev
Log:
Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

Since AArch64 has default outlining behaviour, we need to make sure that
-mno-outline is actually passed along to the linker in this case. Otherwise,
it will run by default on minsize functions even when -mno-outline is specified.

Also fix the darwin-ld test for this, which wasn't actually doing anything.

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Tue Mar 26 14:22:42 2019
@@ -494,14 +494,23 @@ void darwin::Linker::ConstructJob(Compil
   }
 
   // Propagate the -moutline flag to the linker in LTO.
-  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
-if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-machine-outliner");
+  if (Arg *A =
+  Args.getLastArg(options::OPT_moutline, options::OPT_mno_outline)) {
+if (A->getOption().matches(options::OPT_moutline)) {
+  if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-machine-outliner");
 
-  // Outline from linkonceodr functions by default in LTO.
+// Outline from linkonceodr functions by default in LTO.
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-linkonceodr-outlining");
+  }
+} else {
+  // Disable all outlining behaviour if we have mno-outline. We need to do
+  // this explicitly, because targets which support default outlining will
+  // try to do work if we don't.
   CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-linkonceodr-outlining");
+  CmdArgs.push_back("-enable-machine-outliner=never");
 }
   }
 

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Tue Mar 26 14:22:42 2019
@@ -372,5 +372,11 @@
 // Check that we can pass the outliner down to the linker.
 // RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
 // RUN:   %clang -target arm64-apple-darwin -moutline -### %t.o 2> %t.log
-// MOUTLINE: ld
+// RUN: FileCheck -check-prefix=MOUTLINE %s < %t.log
+// MOUTLINE: {{ld(.exe)?"}}
 // MOUTLINE-SAME: "-mllvm" "-enable-machine-outliner" "-mllvm" 
"-enable-linkonceodr-outlining"
+// RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
+// RUN:   %clang -target arm64-apple-darwin -mno-outline -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=MNO_OUTLINE %s < %t.log
+// MNO_OUTLINE: {{ld(.exe)?"}}
+// MNO_OUTLINE-SAME: "-mllvm" "-enable-machine-outliner=never"


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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: martong, teemperor, friss, a_sidorin.
Herald added subscribers: jdoerfert, rnkovacs.

We may try and re-import an EnumDecl while trying to complete it in 
`IsStructuralMatch(...)` specialization for `EnumDecl`. This change mirrors a 
similar fix for the specialization for `RecordDecl`.


https://reviews.llvm.org/D59845

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,17 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+
+  if (ToOrigin) {
+auto *ToOriginEnum = dyn_cast(ToOrigin);
+
+if (ToOriginEnum)
+ToEnum = ToOriginEnum;
+  }
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,17 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+
+  if (ToOrigin) {
+auto *ToOriginEnum = dyn_cast(ToOrigin);
+
+if (ToOriginEnum)
+ToEnum = ToOriginEnum;
+  }
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59841: [Gnu Driver] If -pie and -static-pie are both passed, let -static-pie win.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:311
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;

It's not clear to me that the command line -static-pie -no-pie should result in 
static-pie, given the way the rest of that function is coded.

I might make it a ternary enum: "nothing" "pie" "static-pie" with the last one 
winning. That method seems more consistent with current behavior.

This would allow anyone checking the state of pie to use a single function and 
just check the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59841



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

[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Looks like this may cause some unexpected failures. See 
https://bugs.llvm.org/show_bug.cgi?id=41247 for more details.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59603



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

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 192350.
akhuang marked 2 inline comments as done.
akhuang added a comment.
Herald added a subscriber: jdoerfert.

added to error message regexes and command line flags


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

https://reviews.llvm.org/D59725

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -1,8 +1,14 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+
+Output files:
+  *.reduced.sh -- crash reproducer with minimal arguments
+  *.reduced.cpp -- the reduced file
+  *.test.sh -- interestingness test for C-Reduce
 """
 
-from argparse import ArgumentParser
+from __future__ import print_function
+from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
 import stat
@@ -15,10 +21,14 @@
 from distutils.spawn import find_executable
 
 verbose = False
-llvm_bin = None
 creduce_cmd = None
+clang_cmd = None
 not_cmd = None
 
+def verbose_print(*args, **kwargs):
+  if verbose:
+print(*args, **kwargs)
+
 def check_file(fname):
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
@@ -33,166 +43,334 @@
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
-sys.exit("ERROR: executable %s not found" % (cmd_path))
+sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
   cmd = find_executable(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
-  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
 
-def quote_cmd(cmd):
-  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
-  for arg in cmd)
-
-def get_crash_cmd(crash_script):
-  with open(crash_script) as f:
-# Assume clang call is on the last line of the script
-line = f.readlines()[-1]
-cmd = shlex.split(line)
-
-# Overwrite the script's clang with the user's clang path
-new_clang = check_cmd('clang', llvm_bin)
-cmd[0] = pipes.quote(new_clang)
-return cmd
+  if not cmd_dir:
+cmd_dir = "$PATH"
+  sys.exit("ERROR: `%s` not found in %s" % (cmd_name, cmd_dir))
 
-def has_expected_output(crash_cmd, expected_output):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-  return all(msg in crash_output for msg in expected_output)
-
-def get_expected_output(crash_cmd):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-
-  # If there is an assertion failure, use that;
-  # otherwise use the last five stack trace functions
-  assertion_re = r'Assertion `([^\']+)\' failed'
-  assertion_match = re.search(assertion_re, crash_output)
-  if assertion_match:
-return [assertion_match.group(1)]
-  else:
-stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
-matches = re.findall(stacktrace_re, crash_output)
-return matches[-5:]
-
-def write_interestingness_test(testfile, crash_cmd, expected_output,
-   file_to_reduce):
-  filename = os.path.basename(file_to_reduce)
-  if filename not in crash_cmd:
-sys.exit("ERROR: expected %s to be in the crash command" % filename)
-
-  # Replace all instances of file_to_reduce with a command line variable
-  output = ['#!/bin/bash',
-'if [ -z "$1" ] ; then',
-'  f=%s' % (pipes.quote(filename)),
-'else',
-'  f="$1"',
-'fi']
-  cmd = ['$f' if s == filename else s for s in crash_cmd]
-
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
-  quote_cmd(cmd)))
-
-  for msg in expected_output:
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
-
-  with open(testfile, 'w') as f:
-f.write('\n'.join(output))
-  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
-
-def check_interestingness(testfile, file_to_reduce):
-  testfile = os.path.abspath(testfile)
-
-  # Check that the test considers the original file interesting
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call(testfile, stdout=devnull)
-  if returncode:
-sys.exit("The interestingness test does not pass for the original file.")
-
-  # Check that an empty file is not interesting
-  _, empty_file = tempfile.mkstemp()
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call([testfile, empty_file], stdout=devnull)
-  os.remove(empty_file)
-  if not returncode:
-sys.exit("The interestingness test passes for an empty file.")
-
-def clang_preprocess(file_to_reduce, crash_cmd, expected_output):
-  _, tmpfile = tempfile.mkstemp()
-  shutil.copy(file_to_reduce, tmpfile)

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

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



Comment at: clang/utils/creduce-clang-crash.py:145
+  matches = re.findall(stacktrace_re, crash_output)
+  result = filter(lambda x: x and x.strip() not in filters, matches)[:5]
+  for msg in result:

arichardson wrote:
> george.burgess.iv wrote:
> > nit: please prefer `[x for x in matches if x and x.strip() not in 
> > filters][:5]`. py3's filter returns a generator, whereas py2's returns a 
> > list.
> Stack traces also look different on macOS and it would be nice to handle that 
> too.
> 
> Here's a sample I got from adding a llvm_unreachable at a random location:
> ```
> My unreachable message...
> UNREACHABLE executed at 
> /Users/alex/cheri/llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4468!
> Stack dump:
> 0.Program arguments: 
> /Users/alex/cheri/llvm-project/cmake-build-debug/bin/opt 
> -mtriple=cheri-unknown-freebsd -mcpu=cheri128 -mattr=+cheri128 -target-abi 
> purecap -relocation-model pic -S -instcombine -simplifycfg 
> /Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll
>  -o - 
> 1.Running pass 'Function Pass Manager' on module 
> '/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll'.
> 2.Running pass 'Combine redundant instructions' on function 
> '@cannot_fold_tag_unknown'
> 0  libLLVMSupport.dylib 0x000114515a9d 
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 45
> 1  libLLVMSupport.dylib 0x0001145153f1 llvm::sys::RunSignalHandlers() 
> + 65
> 2  libLLVMSupport.dylib 0x000114515fbf SignalHandler(int) + 111
> 3  libsystem_platform.dylib 0x7fff5b637b3d _sigtramp + 29
> 4  libsystem_platform.dylib 0x7ffee20d0cf0 _sigtramp + 2259259856
> 5  libsystem_c.dylib0x7fff5b4f51c9 abort + 127
> 6  libLLVMSupport.dylib 0x00011446bb12 
> llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 162
> 7  libLLVMInstCombine.dylib 0x000112c345c8 
> llvm::InstCombiner::foldICmpUsingKnownBits(llvm::ICmpInst&) + 4136
> 8  libLLVMInstCombine.dylib 0x000112c34d19 
> llvm::InstCombiner::visitICmpInst(llvm::ICmpInst&) + 569
> 9  libLLVMInstCombine.dylib 0x000112bb9cf2 llvm::InstCombiner::run() + 
> 1522
> 10 libLLVMInstCombine.dylib 0x000112bbb310 
> combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, 
> llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, 
> llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, 
> llvm::LoopInfo*) + 624
> 11 libLLVMInstCombine.dylib 0x000112bbb6d6 
> llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) + 214
> 12 libLLVMCore.dylib0x000111c0bb4d 
> llvm::FPPassManager::runOnFunction(llvm::Function&) + 317
> 13 libLLVMCore.dylib0x000111c0be83 
> llvm::FPPassManager::runOnModule(llvm::Module&) + 99
> 14 libLLVMCore.dylib0x000111c0c2c4 (anonymous 
> namespace)::MPPassManager::runOnModule(llvm::Module&) + 420
> 15 libLLVMCore.dylib0x000111c0c036 
> llvm::legacy::PassManagerImpl::run(llvm::Module&) + 182
> 16 opt  0x00010db6657b main + 7163
> 17 libdyld.dylib0x7fff5b44ced9 start + 1
> ```
I changed the regex to ignore the # at the beginning of the line - I think that 
should cover the mac os stack trace 



Comment at: clang/utils/creduce-clang-crash.py:303
+opts_startswith=["-O"])
+self.clang_args = new_args
+verbose_print("Simplified command:", quote_cmd(self.get_crash_cmd()))

arichardson wrote:
> george.burgess.iv wrote:
> > FWIW, opportunistically trying to add `-fsyntax-only` may help here: if the 
> > crash is in the frontend, it means that non-repros will stop before 
> > codegen, rather than trying to generate object files, or whatever they were 
> > trying to generate in the first place.
> Yes that sounds like a good idea! I just do -emit-llvm to avoid assembly 
> output but for parser/sema crashes -fsyntax-only would save some time.
> 
> Another one I found useful was `-Werror=implicit-int` to get more readable 
> test cases from creduce: 
> https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/utils/creduce_crash_testcase.py#L851
> 
> Without that flag lots of test cases look really weird due to the implicit 
> int and various inferred semicolons.
> 
Sounds good-- I added `-fsyntax-only`, `-emit-llvm` and `-Werror=implicit-int`



Comment at: clang/utils/creduce-clang-crash.py:64
+
+class Reduce:
+  def __init__(self, crash_script, file_to_reduce):

arichardson wrote:
> Does this need to be `Reduce(object):` for python2? 
I think it still works, but adding `object` makes sense. 



Comment at: clang/utils/creduce-clang-crash.py:123
+# Look for specific error messages
+regexes = [r"Asserti

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443527 , @aaron.ballman 
wrote:

> Would it make sense to transform `if (dyn_cast_or_null(Obj))` into `if 
> (Obj && isa(Obj))`  or are there bad transformations from that?


Sure, that sounds reasonable.  I only see about 12 cases across all repos, so 
it isn't that common.  Whereas the idiom you present is used quite often.

I haven't looked yet, but wouldn't the use of `isa_or_null<>` be more efficient 
in cases like this?

./clang/lib/Sema/AnalysisBasedWarnings.cpp:401:  if (B->getTerminator() 
&& isa(B->getTerminator()))
./clang/lib/AST/Expr.cpp:2734:if (MCE->getMethodDecl() && 
isa(MCE->getMethodDecl()))

Granted, there aren't a lot of these.

> 
> 
>> Btw, I also found the same pattern used for `while()`, so I'll add that too. 
>>  Here's a sample of the patterns I'm seeing:
>> 
>> ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
>> while (dyn_cast(last_stmt)) {
> 
> Hah, good catch!
> 
>> ./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
>> (dyn_cast_or_null(D)) . // <--- this one's okay
> 
> I think this could be expressed as `if (D && isa(D))`, no?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

(Forgot to refresh before pressing 'Submit', so maybe efriedma's comment 
answers all of the questions in mine ;) )


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

We have warnings like -Wdivision-by-zero that take reachability into account: 
https://godbolt.org/z/3PD-eF. I don't immediately know how it's all done (e.g. 
in batch because CFG construction is expensive? ...), but looking there for 
inspiration may be useful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59841: [Gnu Driver] Is -pie and -static-pie are both passed, let -static-pie win.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: saugustine.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A static-pie binary is a PIE after all, so letting -static-pie win makes
the resulting binary statically linked as well as being a PIE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59841

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58236: Make address space conversions a bit stricter.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D58236#1443082 , @rjmccall wrote:

> I think C probably requires us to allow this under an explicit cast, but we 
> can at least warn about it.  It does not require us to allow this as an 
> implicit conversion, and that should be an error.


Are you referring to casts to and from `void*`? I think the standard says those 
have to be allowed, and I don't see anything about whether or not the 
conversion has to be explicit.


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

https://reviews.llvm.org/D58236



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357027: [clang-format] Add style option 
AllowShortLambdasOnASingleLine (authored by rdwampler, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57687?vs=191872&id=192337#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57687

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -306,6 +306,39 @@
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+/// Never merge lambdas into a single line.
+SLS_None,
+/// Only merge empty lambdas.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) {
+///   return a;
+///   };
+/// \endcode
+SLS_Empty,
+/// Merge lambda into a single line if argument of a function.
+/// \code
+///   auto lambda = [](int a) {
+///   return a;
+///   };
+///   sort(a.begin(), a.end(), ()[] { return x < y; })
+/// \endcode
+SLS_Inline,
+/// Merge all lambdas fitting on a single line.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) { return a; };
+/// \endcode
+SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
@@ -1805,6 +1838,7 @@
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -65,6 +65,7 @@
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \
   TYPE(LambdaArrow)\
+  TYPE(LambdaLBrace)   \
   TYPE(LambdaLSquare)  \
   TYPE(LeadingJavaAnnotation)  \
   TYPE(LineComment)\
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -119,6 +119,17 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO, FormatStyle::ShortLambdaStyle &Value) {
+IO.enumCase(Value, "None", FormatStyle::SLS_None);
+IO.enumCase(Value, "false", FormatStyle::SLS_None);
+IO.enumCase(Value, "Empty", FormatStyle::SLS_Empty);
+IO.enumCase(Value, "Inline", FormatStyle::SLS_Inline);
+IO.enumCase(Value, "All", FormatStyle::SLS_All);
+IO.enumCase(Value, "true", FormatStyle::SLS_All);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
 IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
@@ -347,6 +358,8 @@
Style.AllowShortCaseLabelsOnASingleLine);
 IO.mapOptional("AllowShortFunctionsOnASingleLine",
Style.AllowShortFunctionsOnASingleLine);
+IO.mapOptional("AllowShortLambdasOnASingleLine",
+   S

r357027 - [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via cfe-commits]

Author: rdwampler
Date: Tue Mar 26 13:18:14 2019
New Revision: 357027

URL: http://llvm.org/viewvc/llvm-project?rev=357027&view=rev
Log:
[clang-format] Add style option AllowShortLambdasOnASingleLine

Summary:
This option `AllowShortLambdasOnASingleLine` similar to the other `AllowShort*` 
options, but applied to C++ lambdas.

Reviewers: djasper, klimek

Reviewed By: klimek

Subscribers: MyDeveloperDay, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Mar 26 13:18:14 2019
@@ -449,6 +449,45 @@ the configuration (without a prefix: ``A
   return;
}
 
+**AllowShortLambdasOnASingleLine** (``ShortLambdaStyle``)
+  Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  single line.
+
+  Possible values:
+
+  * ``SLS_None`` (in configuration: ``None``)
+Never merge lambdas into a single line.
+
+  * ``SLS_Empty`` (in configuration: ``Empty``)
+Only merge empty lambdas.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {}
+  auto lambda2 = [](int a) {
+  return a;
+  };
+
+  * ``SLS_Inline`` (in configuration: ``Inline``)
+Merge lambda into a single line if argument of a function.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {
+  return a;
+  };
+  sort(a.begin(), a.end(), ()[] { return x < y; })
+
+  * ``SLS_All`` (in configuration: ``All``)
+Merge all lambdas fitting on a single line.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {}
+  auto lambda2 = [](int a) { return a; };
+
+
+
 **AllowShortLoopsOnASingleLine** (``bool``)
   If ``true``, ``while (true) continue;`` can be put on a single
   line.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Tue Mar 26 13:18:14 2019
@@ -306,6 +306,39 @@ struct FormatStyle {
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+/// Never merge lambdas into a single line.
+SLS_None,
+/// Only merge empty lambdas.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) {
+///   return a;
+///   };
+/// \endcode
+SLS_Empty,
+/// Merge lambda into a single line if argument of a function.
+/// \code
+///   auto lambda = [](int a) {
+///   return a;
+///   };
+///   sort(a.begin(), a.end(), ()[] { return x < y; })
+/// \endcode
+SLS_Inline,
+/// Merge all lambdas fitting on a single line.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) { return a; };
+/// \endcode
+SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on 
a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
@@ -1805,6 +1838,7 @@ struct FormatStyle {
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine 
&&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Mar 26 13:18:14 2019
@@ -119,6 +119,17 @@ template <> struct ScalarEnumerationTrai
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO 

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59802#1443515 , @hintonda wrote:

> In D59802#1443451 , @hintonda wrote:
>
> > In D59802#1443340 , @aaron.ballman 
> > wrote:
> >
> > > Should this check also try to find this pattern:
> > >
> > >   if (dyn_cast(Bobble))
> > > ...
> > >
> > >
> > > in order to recommend the proper replacement:
> > >
> > >   if (isa(Bobble))
> > > ...
> > >
> > >
> > > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it 
> > > would also cover this situation and I run into it during code reviews 
> > > with some frequency (more often than I run into `cast<>` being used in a 
> > > conditional).
> >
> >
> > Yes, I can add that, and provide a fix-it too.  Thanks...
>
>
> I did a quick grep and found a few of these, but also found the `_or_null<>` 
> variety.  Guess they'll need to stay the same since `isa<>` can't handle 
> nulls.


Would it make sense to transform `if (dyn_cast_or_null(Obj))` into `if (Obj 
&& isa(Obj))`  or are there bad transformations from that?

> Btw, I also found the same pattern used for `while()`, so I'll add that too.  
> Here's a sample of the patterns I'm seeing:
> 
> ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
> while (dyn_cast(last_stmt)) {

Hah, good catch!

> ./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
> (dyn_cast_or_null(D)) . // <--- this one's okay

I think this could be expressed as `if (D && isa(D))`, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443451 , @hintonda wrote:

> In D59802#1443340 , @aaron.ballman 
> wrote:
>
> > Should this check also try to find this pattern:
> >
> >   if (dyn_cast(Bobble))
> > ...
> >
> >
> > in order to recommend the proper replacement:
> >
> >   if (isa(Bobble))
> > ...
> >
> >
> > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it 
> > would also cover this situation and I run into it during code reviews with 
> > some frequency (more often than I run into `cast<>` being used in a 
> > conditional).
>
>
> Yes, I can add that, and provide a fix-it too.  Thanks...


I did a quick grep and found a few of these, but also found the `_or_null<>` 
variety.  Guess they'll need to stay the same since `isa<>` can't handle nulls.

Btw, I also found the same pattern used for `while()`, so I'll add that too.  
Here's a sample of the patterns I'm seeing:

./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
while (dyn_cast(last_stmt)) {
./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
(dyn_cast_or_null(D)) . // <--- this one's okay
./clang/lib/CodeGen/CodeGenModule.cpp:3950:if 
(dyn_cast(callSite)) {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.

Please use double backticks for language constructs. Same in other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59838: gn build: Add build files for clang-include-fixer and find-all-symbols

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: mbonadei.
Herald added a project: LLVM.

(Not adding a build file for clang-include-fixer/plugin yet.)


https://reviews.llvm.org/D59838

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
  
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
  
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
@@ -0,0 +1,18 @@
+executable("clang-include-fixer") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer",
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Rewrite",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"ClangIncludeFixer.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
@@ -0,0 +1,17 @@
+executable("find-all-symbols") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"FindAllSymbolsMain.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
@@ -0,0 +1,23 @@
+static_library("find-all-symbols") {
+  output_name = "findAllSymbols"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Tooling",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FindAllMacros.cpp",
+"FindAllSymbols.cpp",
+"FindAllSymbolsAction.cpp",
+"HeaderMapCollector.cpp",
+"PathConfig.cpp",
+"PragmaCommentHandler.cpp",
+"STLPostfixHeaderMap.cpp",
+"SymbolInfo.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
@@ -0,0 +1,26 @@
+static_library("clang-include-fixer") {
+  output_name = "clangIncludeFixer"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Parse",
+"//clang/lib/Sema",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FuzzySymbolIndex.cpp",
+"InMemorySymbolIndex.cpp",
+"IncludeFixer.cpp",
+"IncludeFixerContext.cpp",
+"SymbolIndexManager.cpp",
+"YamlSymbolIndex.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/BUILD.gn
===
--- llvm/utils/gn/secondary/BUILD.gn
+++ llvm/utils/gn/secondary/BUILD.gn
@@ -7,6 +7,8 @@
 "//clang-tools-extra/clang-apply-replacements/tool:clang-apply-replacements",
 "//clang-tools-extra/clang-change-namespace/tool:clang-change-namespace",
 "//clang-tools-extra/clang-doc/tool:clang-doc",
+"//clang-tools-extra/clang-include-fixer/find-all-symbols/tool:find-all-symbols",
+"//clang-tools-extra/clang-include-fixer/tool:clang-include-fixer",
 "//clang-tools-extra/clang-move/tool:clang-move",
 "//clang-tools-extra/clang-query/tool:clang-query",
 "//clang-tools-extra/clang-reorder-fields/tool:clang-reorder-fields",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443340 , @aaron.ballman 
wrote:

> Should this check also try to find this pattern:
>
>   if (dyn_cast(Bobble))
> ...
>
>
> in order to recommend the proper replacement:
>
>   if (isa(Bobble))
> ...
>
>
> I ask because the name `llvm-avoid-cast-in-conditional` sounds like it would 
> also cover this situation and I run into it during code reviews with some 
> frequency (more often than I run into `cast<>` being used in a conditional).


Yes, I can add that, and provide a fix-it too.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Anastasia wrote:
> bader wrote:
> > Shouldn't we invalidate Var declaration?
> This early exit is to prevent adding default initializer implicitly to the 
> declarations that are valid.
"In OpenCL, we can't initialize objects in the __local address space, even 
implicitly, so don't synthesize an implicit initializer."

I think it's important to add the underscores on `__local` to make it obvious 
that we're talking about OpenCL's "local" address space, not the address space 
that local variables appear in.


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

https://reviews.llvm.org/D59646



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+  ast_matchers::internal::Matcher,

aaron.ballman wrote:
> This matcher seems like it would be generally useful -- would you mind adding 
> it to the AST matcher interface rather than local to this check? It doesn't 
> have to be done as part of this patch (we can leave the matcher here for the 
> moment).
Definitely agreed. I will send out a followup for a new AST matcher.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+   "invoke a superclass initializer?")
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),

aaron.ballman wrote:
> Is there a %0 missing from the diagnostic for this method declaration you're 
> passing in?
Good catch! That's egg on my face 😅🥚

I think I was internally conflicted over specifically mentioning -[NSObject 
self] versus using the method declaration of the message expression and somehow 
got stuck halfway in-between 😓 I think it's better to mention the method 
declaration of the message. Let me know if you think that I should instead 
mention -[NSObject self].



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:110
+  Finds invocations of -self on super instances in initializers of subclasses
+  of NSObject and recommends calling a superclass initializer instead.
+

Eugene.Zelenko wrote:
> Please enclose NSObject in ``. Probably same for -self if this is language 
> construct.
Good catch. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192317.
stephanemoore added a comment.

Enclose -self in backticks in check documentation (overlooked in previous 
attempt).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of `-self` on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking `-self` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/c

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192316.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Enclose -self in backticks in release notes and check documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of `-self` on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+///

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192315.
stephanemoore marked 5 inline comments as done.
stephanemoore added a comment.

Fix diagnostic format string to actually use the message's method declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of -self on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of -self on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

For that particular case, I think we could suppress the false positive using 
DiagRuntimeBehavior.  How many total false positives are we talking about, and 
how many can the compiler statically prove are unreachable?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192312.
stephanemoore added a comment.

Removed usage of `auto` with a nonobvious type in isSubclassOf matcher.
Added backticks around `NSObject` in docs.
Synchronized check description in release notes and check documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of -self on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of -self on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initial

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+  ast_matchers::internal::Matcher,

This matcher seems like it would be generally useful -- would you mind adding 
it to the AST matcher interface rather than local to this check? It doesn't 
have to be done as part of this patch (we can leave the matcher here for the 
moment).



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+   "invoke a superclass initializer?")
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),

Is there a %0 missing from the diagnostic for this method declaration you're 
passing in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst:8
+statements which will assert on failure in Debug builds. The use of
+cast<> implies that the operation cannot fail, and should not be used
+as the condition of a conditional statement.

This cast<> still needs to be enclosed in ``.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Should this check also try to find this pattern:

  if (dyn_cast(Bobble))
...

in order to recommend the proper replacement:

  if (isa(Bobble))
...

I ask because the name `llvm-avoid-cast-in-conditional` sounds like it would 
also cover this situation and I run into it during code reviews with some 
frequency (more often than I run into `cast<>` being used in a conditional).




Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:20
+void AvoidCastInConditionalCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  ifStmt(anyOf(

No need to register this matcher if C++ isn't enabled.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:35-36
+diag(MatchedDecl->getBeginLoc(),
+ "Found cast<> in conditional statement; consider using dyn_cast<> if "
+ "it can fail, or removal of conditional if it can't.");
+;

clang-tidy diagnostics should not be complete sentences; you should make this 
ungrammatical. How about: `cast<> in conditional will assert rather than return 
a null pointer`?



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:37
+ "it can fail, or removal of conditional if it can't.");
+;
+  }

Spurious semicolon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58797#1439888 , @erik.pilkington 
wrote:

> Ah, I didn't consider that case. Presumably `st` is configured to have 
> different sizes based on the target? I agree that this is a false-positive, 
> but it seems like a pretty narrow edge case, and there is a pretty obvious 
> source workaround that doesn't affect readability: `memcpy(&buf, st, 
> sizeof(buf))`. @aaron.ballman/@rsmith Any thoughts here? IMO keeping this 
> diagnostic is worth it.


Yes, I think we should keep this diagnostic. However, if we can find a way to 
silence it for this particular false-positive pattern, that would be great!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/Sema/SemaType.cpp:261
+/// necessary.
+QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement) {
+  QualType T = sema.ReplaceAutoType(TypeWithAuto, Replacement);

vsapsai wrote:
> aaron.ballman wrote:
> > I think this work should be done within `SubstituteDeducedTypeTransform` 
> > rather than on the side. Any caller to `Sema::ReplaceAutoType()` should get 
> > this same behavior.
> Doing this work in `SubstituteDeducedTypeTransform` involves teaching 
> `SubstituteDeducedTypeTransform` about `TypeProcessingState` and probably 
> adding `TypeProcessingState` to `Sema::ReplaceAutoType()`. As for me, it 
> exposes `TypeProcessingState` in more places than necessary. And it feels 
> somewhat awkward that `SubstituteDeducedTypeTransform` is used in multiple 
> places but `TypeProcessingState` is required only here.
> 
> I've modelled my approach after `TypeProcessingState::getAttributedType` 
> where it forwards the call to Sema and keeps `AttrsForTypes` up to date. Do 
> you still think `SubstituteDeducedTypeTransform` would be a better place for 
> this code?
Hmm, yeah, it doesn't seem like it would make sense there. This is keeping the 
`TypeProcessingState` up to date which is really only of interest during some 
stages of processing.


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

https://reviews.llvm.org/D58659



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[John McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

erik.pilkington wrote:
> rjmccall wrote:
> > It looks like `SmallPtrSet`'s move constructor does actually guarantee to 
> > leave the source empty; you could probably just assert that.  But I don't 
> > think there's an algorithmic cost, so this is fine, too.
> > 
> > Please leave an assertion at the bottom of this function that the set is 
> > empty.
> If you have to actually check the constructor to verify the client code is 
> doing the right thing then it makes more sense to just be explicit, IMO. New 
> patch adds the assert though.
If it were perf-significant I might disagree, but it shouldn't be, so this is 
fine.  LGTM.


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

https://reviews.llvm.org/D59670



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

rjmccall wrote:
> It looks like `SmallPtrSet`'s move constructor does actually guarantee to 
> leave the source empty; you could probably just assert that.  But I don't 
> think there's an algorithmic cost, so this is fine, too.
> 
> Please leave an assertion at the bottom of this function that the set is 
> empty.
If you have to actually check the constructor to verify the client code is 
doing the right thing then it makes more sense to just be explicit, IMO. New 
patch adds the assert though.


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

https://reviews.llvm.org/D59670



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 192300.
erik.pilkington marked an inline comment as done.
erik.pilkington added a comment.

Add an assert.


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

https://reviews.llvm.org/D59670

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/blocks.cpp


Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -145,3 +145,11 @@
 A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15690,7 +15690,12 @@
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
 VarDecl *Var;
 SourceLocation Loc;
 if (DeclRefExpr *DRE = dyn_cast(E)) {
@@ -15707,10 +15712,10 @@
/*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
-  MaybeODRUseExprs.clear();
+  assert(MaybeODRUseExprs.empty() &&
+ "MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
 }
 
-
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
 VarDecl *Var, Expr *E) {
   assert((!E || isa(E) || isa(E)) &&
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@
   /// element type here is ExprWithCleanups::Object.
   SmallVector ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a reference to a variable (constant) that may or may not
-  ///  be odr-used in this Expr, and we won't know until all lvalue-to-rvalue
-  ///  and discarded value conversions have been applied to all subexpressions
-  ///  of the enclosing full expression.  This is cleared at the end of each
-  ///  full expression.
-  llvm::SmallPtrSet MaybeODRUseExprs;
+  /// Store a set of either DeclRefExprs or MemberExprs that contain a 
reference
+  /// to a variable (constant) that may or may not be odr-used in this Expr, 
and
+  /// we won't know until all lvalue-to-rvalue and discarded value conversions
+  /// have been applied to all subexpressions of the enclosing full expression.
+  /// This is cleared at the end of each full expression.
+  using MaybeODRUseExprSet = llvm::SmallPtrSet;
+  MaybeODRUseExprSet MaybeODRUseExprs;
 
   std::unique_ptr PreallocatedFunctionScope;
 
@@ -1029,7 +1029,7 @@
 /// context (i.e. the number of TypoExprs created).
 unsigned NumTypos;
 
-llvm::SmallPtrSet SavedMaybeODRUseExprs;
+MaybeODRUseExprSet SavedMaybeODRUseExprs;
 
 /// The lambdas that are present within this context, if it
 /// is indeed an unevaluated context.


Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -145,3 +145,11 @@
 A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15690,7 +15690,12 @@
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
 VarDecl *Var;
 SourceLocation Loc;
 if (DeclRefExpr *DRE = dyn_cast(E)) {
@@ -15707,10 +15712,10 @@
/*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
-  MaybeODRUseExprs.clear();
+  assert(MaybeODRUseExprs.empty() &&
+ "MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
 }
 
-
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
 VarDecl *Var, Expr *E) {
   assert((!E || isa(E) || isa(E)) &&
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@
   /// element type here is ExprWithCleanups::Object.
   SmallVector ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a refer

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r356987.


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

https://reviews.llvm.org/D59492



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:274
+
+  if (F->getLocation().isInvalid())
+return;

bernhardmgruber wrote:
> aaron.ballman wrote:
> > bernhardmgruber wrote:
> > > aaron.ballman wrote:
> > > > Should this also bail out if the function is `main()`?
> > > How strange does
> > > 
> > > ```
> > > auto main(int argc, const char* argv[]) -> int {
> > > return 0;
> > > }
> > > ```
> > > look to you?
> > > 
> > > I think the transformation is valid here. But I can understand that 
> > > people feel uneasy after typing `int main ...` for decades. Should we 
> > > also create an option here to turn it off for main? Or just not transform 
> > > it? I do not mind. If I want `main` to start with `auto` I could also do 
> > > this transformation manually.
> > This comment was marked as done, but I don't see any changes or mention of 
> > what should happen. I suppose the more general question is: should there be 
> > a list of functions that should not have this transformation applied, like 
> > program entrypoints? Or do you want to see this check diagnose those 
> > functions as well?
> I am sorry for marking it as done. I do not know how people work here exactly 
> and how phabricator behaves. I thought the check boxes are handled for 
> everyone individually. Also, if I add a new comment, it is checked by default.
> 
> How are you/most people going to use clang-tidy? Do you run it regularly and 
> automatic? Do you expect it to find zero issues once you applied the check?
> In other words, if you do not want to transform some functions, do you need 
> an option to disable the check for these, so it runs clean on the full source 
> code?
> 
> Personally, I do not need this behavior, as I run clang-tidy manually once in 
> a while and revert transformations I do not want before checking in the 
> changes.
> I am sorry for marking it as done. I do not know how people work here exactly 
> and how phabricator behaves. I thought the check boxes are handled for 
> everyone individually. Also, if I add a new comment, it is checked by default.

No worries -- that new comments are automatically marked done by default is 
certainly a surprise to me!

> How are you/most people going to use clang-tidy? Do you run it regularly and 
> automatic? Do you expect it to find zero issues once you applied the check? 
> In other words, if you do not want to transform some functions, do you need 
> an option to disable the check for these, so it runs clean on the full source 
> code?

I think it's hard to gauge how most people do anything, really. However, I 
think there are people who enable certain clang-tidy checks and have them run 
automatically as part of CI, etc. Those kind of folks may need the ability to 
silence the diagnostics. We could do this in a few ways (options to control 
methods not to diagnose, NOLINT comments, etc).

I kind of think we don't need an option for the user to list functions not to 
transform. They can use NOLINT to cover those situations as a one-off. The only 
situation where I wonder if everyone is going to want to write NOLINT is for 
`main()`. It might make sense to have an option to not check program 
entrypoints, but there is technically nothing stopping someone from using a 
trailing return type with a program entrypoint so maybe this option isn't 
needed at all.

How about we not add any options and if someone files a bug report, we can 
address it then?



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:180-184
+  if (Info.hasMacroDefinition()) {
+// The CV qualifiers of the return type are inside macros.
+diag(F.getLocation(), Message);
+return {};
+  }

bernhardmgruber wrote:
> aaron.ballman wrote:
> > Perhaps I'm a bit dense on a Monday morning, but why should this be a 
> > diagnostic? I am worried this will diagnose situations like (untested 
> > guesses):
> > ```
> > #define const const
> > const int f(void); // Why no fixit?
> > 
> > #define attr __attribute__((frobble))
> > attr void g(void); // Why diagnosed as needing a trailing return type?
> > ```
> Because I would also like to rewrite functions which contain macros in the 
> return type. However, I cannot provide a fixit in all cases. Clang can give 
> me a `SourceRange` without CV qualifiers which seems to work in all my tests 
> so far. But to include CV qualifiers I have to do some manual lexing left and 
> right of the return type `SourceRange`. If I encounter macros along this way, 
> I bail out because handling these cases becomes compilated very easily.
> 
> Your second case does not give a diagnostic, as it is not matched by the 
> check, because it returns `void`.
> Because I would also like to rewrite functions which contain macros in the 
> return type. However, I cannot provide a fixit in all cases. Clang can give 
> me a SourceRange 

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Zola Bridges via Phabricator via cfe-commits]

zbrid updated this revision to Diff 192297.
zbrid added a comment.
Herald added a subscriber: jsji.

update whitespace in wasm file to match surrounding


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59827

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-speculation-safe-value.c
  clang/test/Preprocessor/init.c
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Target/TargetSelectionDAG.td
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll

Index: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
@@ -0,0 +1,71 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i32, align 4
+  %b_safe = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  store i32 %mul, i32* %b, align 4
+  %1 = load i32, i32* %b, align 4
+  %2 = call i32 @llvm.speculationsafevalue.i32(i32 %1)
+; X64: movl -12(%rbp), %eax
+; X64: lfence
+; X64: movl %eax, -8(%rbp)
+  store i32 %2, i32* %b_safe, align 4
+  %3 = load i32, i32* %b_safe, align 4
+  %add = add nsw i32 %3, 100
+  store i32 %add, i32* %c, align 4
+  %4 = load i32, i32* %c, align 4
+  ret i32 %4
+}
+
+; Function Attrs: nounwind
+declare i32 @llvm.speculationsafevalue.i32(i32) #1
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo64i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i64, align 8
+  %b_safe = alloca i64, align 8
+  %c = alloca i64, align 8
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  %conv = sext i32 %mul to i64
+  store i64 %conv, i64* %b, align 8
+  %1 = load i64, i64* %b, align 8
+  %2 = call i64 @llvm.speculationsafevalue.i64(i64 %1)
+; X64: movq -32(%rbp), %rax
+; X64: lfence
+; X64: movq %rax, -24(%rbp)
+  store i64 %2, i64* %b_safe, align 8
+  %3 = load i64, i64* %b_safe, align 8
+  %add = add nsw i64 %3, 100
+  store i64 %add, i64* %c, align 8
+  %4 = load i64, i64* %c, align 8
+  %conv1 = trunc i64 %4 to i32
+  ret i32 %conv1
+}
+
+; Function Attrs: nounwind
+declare i64 @llvm.speculationsafevalue.i64(i64) #1
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 6fd90b5505fe7cddd0fd798fe9608ea0e0325302)"}
Index: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
===
--- llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
+++ llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
@@ -212,6 +212,7 @@
   void hardenIndirectCallOrJumpInstr(
   MachineInstr &MI,
   SmallDenseMap &AddrRegToHardenedReg);
+  bool lowerIntrinsic(MachineFunction &MF);
 };
 
 } // end anonymous namespace
@@ -402,16 +403,19 @@
   LLVM_DEBUG(dbgs() << "** " << getPassName() << " : " << MF.getName()
 << " **\n");
 
-  // Only run if this pass is forced enabled or we detect the relevant function
-  // attribute requesting SLH.
-  if (!EnableSpeculativeLoadHardening &&
-  !MF.getFunction().hasFnAt

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: clang/test/Preprocessor/init.c:9678
 // WEBASSEMBLY-NEXT:#define __GXX_ABI_VERSION 1002
+// WEBASSEMBLY-NEXT: #define __HAVE_SPECULATION_SAFE_VALUE 1
 // WEBASSEMBLY32-NEXT:#define __ILP32__ 1

Nit: Remove the whitespace to be consistent with adjacent lines? (I think 
having a whitespace is better in general though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59827



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

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Zola Bridges via Phabricator via cfe-commits]

zbrid created this revision.
zbrid added reviewers: chandlerc, kristof.beyls, aaron.ballman, 
devinj.jeanpierre.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, jfb, aheejin, 
hiraditya, javed.absar, dschuff.
Herald added projects: clang, LLVM.

This is similar to the work Kristof did for ARM here: 
https://reviews.llvm.org/D49072

For now, I have only implemented the version that lowers the intrinsic using an 
LFENCE. I'm workign on a version that can be lowered as an LFENCE or lowered 
using the control flow speculation available, so users have the option just as 
they do in the ARM patch.

This is intended to add to the discussion rather than be a definitive patch 
relating to the way we will handle spot mitigations as far as the final 
API/implementation in LLVM goes. Any comments about the API, the way 
implemented this, or anything else are welcome.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59827

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-speculation-safe-value.c
  clang/test/Preprocessor/init.c
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Target/TargetSelectionDAG.td
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll

Index: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
@@ -0,0 +1,71 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i32, align 4
+  %b_safe = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  store i32 %mul, i32* %b, align 4
+  %1 = load i32, i32* %b, align 4
+  %2 = call i32 @llvm.speculationsafevalue.i32(i32 %1)
+; X64: movl -12(%rbp), %eax
+; X64: lfence
+; X64: movl %eax, -8(%rbp)
+  store i32 %2, i32* %b_safe, align 4
+  %3 = load i32, i32* %b_safe, align 4
+  %add = add nsw i32 %3, 100
+  store i32 %add, i32* %c, align 4
+  %4 = load i32, i32* %c, align 4
+  ret i32 %4
+}
+
+; Function Attrs: nounwind
+declare i32 @llvm.speculationsafevalue.i32(i32) #1
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo64i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i64, align 8
+  %b_safe = alloca i64, align 8
+  %c = alloca i64, align 8
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  %conv = sext i32 %mul to i64
+  store i64 %conv, i64* %b, align 8
+  %1 = load i64, i64* %b, align 8
+  %2 = call i64 @llvm.speculationsafevalue.i64(i64 %1)
+; X64: movq -32(%rbp), %rax
+; X64: lfence
+; X64: movq %rax, -24(%rbp)
+  store i64 %2, i64* %b_safe, align 8
+  %3 = load i64, i64* %b_safe, align 8
+  %add = add nsw i64 %3, 100
+  store i64 %add, i64* %c, align 8
+  %4 = load i64, i64* %c, align 8
+  %conv1 = trunc i64 %4 to i32
+  ret i32 %conv1
+}
+
+; Function Attrs: nounwind
+declare i64 @llvm.speculationsafevalue.i64(i64) #1
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 6fd90b5505fe7cddd0fd798fe9608ea0e0325302)"}
Index: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
==

[PATCH] D46140: [coroutines] Add std::experimental::task type

[Brian Gesiak via Phabricator via cfe-commits]

modocache closed this revision.
modocache added a comment.

Committed in rL357010 . Apologies, I forgot 
to include the differential revision in the commit message so this diff wasn't 
closed automatically as a result. I'll comment on rL357010 
 with the missing information.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46140



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

[PATCH] D58091: Customize warnings for missing built-in type

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In D58091#1397586 , @jyknight wrote:

> I think this warning (-Wbuiltin-requires-header) doesn't really make sense as 
> its own warning.
>
> We already have two related (on-by-default) warnings.


...

In D58091#1414250 , @bcain wrote:

>


...

> FWIW, I'm satisfied with the fix as proposed here for now and I wouldn't be 
> opposed to following up with an improvement over other warnings in lieu of 
> this warning.
> 
> @jyknight -- James, (or others) care to weigh in on this proposal?

@jdoerfert and @jyknight -- let's please un-stall this review.

James: is this change acceptable or unacceptable as-is?  Could we follow up 
with a change that removed this warning?

Johannes: if it's acceptable to James, let's please submit this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik marked an inline comment as done.
shafik added a comment.

In D59665#1442826 , @martong wrote:

> In D59665#1442328 , @shafik wrote:
>
> > @martong your idea does not work b/c default construction 
> > `DeclarationName()` treats it the same as being empty. So `if (!Name)` is 
> > still `true`.
>
>
> And did you try moving the `push_back` to the other scope? I'd expect the the 
> ConflictingDecls to be empty then.


Yes, I did and I think it looks good but I have to run a regression.




Comment at: lib/AST/ASTImporter.cpp:2463
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),

martong wrote:
> shafik wrote:
> > a_sidorin wrote:
> > > If I understand correctly, this will replace Name with SearchName causing 
> > > an anonymous enum to be imported as a named a few lines below. It doesn't 
> > > look like a correct behaviour to me.
> > This is correct because either `SearchName` is `Name` or it is the name of 
> > the typedef for the anonymous enum set via `ImportInto(SearchName, 
> > D->getTypedefNameForAnonDecl()->getDeclName())`
> Okay, this is indeed correct. But then we should make a similar change in 
> VisitRecordDecl too (there we do exactly the same with typedefs).
This is fixing a specific bug, so I would want to keep this change specifically 
related to that and I can add a second review for `VisitRecordDecl`


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

https://reviews.llvm.org/D59665



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

@lebedev.ri where are tests for AST serialization are located ? i didn't find 
them.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:61
+  // Check if any of the superclasses of the class match.
+  for (const auto *SuperClass = Node.getClassInterface()->getSuperClass();
+   SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:110
+  Finds invocations of -self on super instances in initializers of subclasses
+  of NSObject and recommends calling a superclass initializer instead.
+

Please enclose NSObject in ``. Probably same for -self if this is language 
construct.



Comment at: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst:6
+
+Finds invocations of -self on super instances (`[super self]`) in initializers
+of subclasses of NSObject and recommends invoking a superclass initializer

Please synchronize with Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D58236: Make address space conversions a bit stricter.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think C probably requires us to allow this under an explicit cast, but we can 
at least warn about it.  It does not require us to allow this as an implicit 
conversion, and that should be an error.


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

https://reviews.llvm.org/D58236



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

r357001 - [CodeGen] Delete never used LValueAlign

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Mar 26 08:39:45 2019
New Revision: 357001

URL: http://llvm.org/viewvc/llvm-project?rev=357001&view=rev
Log:
[CodeGen] Delete never used LValueAlign

It was added by rC176658 but never used since then.

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=357001&r1=357000&r2=357001&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Tue Mar 26 08:39:45 2019
@@ -35,7 +35,6 @@ namespace {
 uint64_t ValueSizeInBits;
 CharUnits AtomicAlign;
 CharUnits ValueAlign;
-CharUnits LValueAlign;
 TypeEvaluationKind EvaluationKind;
 bool UseLibcall;
 LValue LVal;
@@ -132,7 +131,6 @@ namespace {
 QualType getAtomicType() const { return AtomicTy; }
 QualType getValueType() const { return ValueTy; }
 CharUnits getAtomicAlignment() const { return AtomicAlign; }
-CharUnits getValueAlignment() const { return ValueAlign; }
 uint64_t getAtomicSizeInBits() const { return AtomicSizeInBits; }
 uint64_t getValueSizeInBits() const { return ValueSizeInBits; }
 TypeEvaluationKind getEvaluationKind() const { return EvaluationKind; }


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

[PATCH] D54978: Move the SMT API to LLVM

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks!

The problem with these methods is that LLVM_DUMP_METHOD forces the function to 
be emitted even if it is not used and Clang modules without local submodule 
visibility will implicitly include all header files in a module, which will 
then cause the missing symbol error.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

[PATCH] D59426: [PR41010][OpenCL] Allow OpenCL C style vector initialization in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: rsmith.
rjmccall added inline comments.



Comment at: lib/Sema/SemaInit.cpp:1295
+? InitializedEntity::InitializeTemporary(ElemType)
+: Entity;
+

There should at least be a comment here explaining this, but mostly it's been 
so long since I've really understood this code that I'd like Richard to look at 
it.


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

https://reviews.llvm.org/D59426



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

It looks like `SmallPtrSet`'s move constructor does actually guarantee to leave 
the source empty; you could probably just assert that.  But I don't think 
there's an algorithmic cost, so this is fine, too.

Please leave an assertion at the bottom of this function that the set is empty.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59670



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1352
+
+  llvm::TimeTraceScope TimeScope("Backend", StringRef(""));
+

We don't need explicit StringRef constructor call here. Just passing "" is 
enough.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

Remove StringRef?



Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:131
   NumIdentifierLookupHits() {
+  llvm::TimeTraceScope TimeScope("Module LoadIndex", StringRef(""));
   // Read the global index.

Remove StringRef?



Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:745
   using namespace llvm;
+  llvm::TimeTraceScope TimeScope("Module WriteIndex", StringRef(""));
 

Remove StringRef?



Comment at: clang/tools/driver/cc1_main.cpp:224
+  {
+llvm::TimeTraceScope TimeScope("ExecuteCompiler", StringRef(""));
+Success = ExecuteCompilerInvocation(Clang.get());

Remove StringRef?



Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+  if (isPrint(C)) {
+OS += C;

include StringExtras.h for this?


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

https://reviews.llvm.org/D58675



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

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59008#1442962 , @probinson wrote:

> In D59008#1442515 , @dblaikie wrote:
>
> > In D59008#1442256 , @t-tye wrote:
> >
> > > In D59008#1442014 , @dblaikie 
> > > wrote:
> > >
> > > > In D59008#1441903 , @t-tye 
> > > > wrote:
> > > >
> > > > > LGTM
> > > > >
> > > > > Do we know the state of split DWARF and DWARF compression for DWARF 5 
> > > > > (compared to DWARF 2)?
> > > >
> > > >
> > > > State of them in what sense? Compression is pretty orthogonal to any 
> > > > DWARF version - it's more about the container (ELF, etc) you use. Split 
> > > > DWARF is non-standardly supported in pre-v5, and I think it's 
> > > > functioning in the standards conformant v5 mode too.
> > >
> > >
> > > I had heard mention that at least split DWARF may not be fully supported 
> > > yet for DWARF 5 in LLVM. But maybe that is old news:-)
> >
> >
> > Might be - nothing I know of right now. (type units aren't fully supported 
> > in DWARFv5 - but that's the only big thing on my list)
>
>
> Anything having to do with section format and placement ought to be correct 
> at this point.  Certainly I see v5 type units coming out in the .debug_info 
> section per spec, and the .dwo files look right.  If there's something 
> missing please file a bug and CC me.
>
> There was certainly a release where split-dwarf and type units didn't work 
> yet, but that's all done now.


Ah, thanks for the correction - I thought that was the case, and then I thought 
I'd tested it a couple of weeks ago and seen it wasn't (when I'd expected it 
was)... must've messed up my test, maybe used an old compiler build I had lying 
around.

Good stuff!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59008



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@asmith: Where's the LLVM-side change/review that goes with this, btw?

In D59347#1442970 , @probinson wrote:

> As a rule I would prefer flags with positive names, as it's slightly easier 
> to read `!isTrivial` than `!isNonTrivial`. And trivially shorter. :-)


Fair enough - I was mostly coming at this from the "the patch that was 
committed should be reverted" & then we could haggle over other things, but 
fair point.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59347



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

[PATCH] D54978: Move the SMT API to LLVM

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added a comment.

In D54978#1442493 , @aprantl wrote:

> I'm afraid this broke some bots that build with `LLVM_ENABLE_MODULES=1`.
>
> For example:
>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22411/consoleFull#710926295dd1929ea-7054-4089-b7ef-4624c3781fa4
>
>   Undefined symbols for architecture x86_64:
> "llvm::errs()", referenced from:
> llvm::SMTExpr::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTSolver::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTSort::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTExpr::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
> llvm::SMTSolver::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
> llvm::SMTSort::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
>
>
> Long story short: You can't have an LLVM_DUMP_METHOD defined inline inside of 
> a module.
>
> One way to fix this would be to move the function body of the various
>
>   LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); }
>
>
> functions into .cpp files.


Unfortunately, I was not able to reproduce the bug locally (when I enable 
modules, clang complains about some `std::shared_ptr`), 
however, I just pushed r356994 and I'll keep an eye on the bot.

Thanks for the report.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added a comment.

@dcoughlin I don't necessarily agree with you.
Let me explain why we think this feature is important.

We should give the users the possibility to list all possibly configurable 
checker options and their meaning.

Many of these options should be possible to be set by the end user to be able 
to fine tune the checker behaviour to match the analyzed code style.
Such examples are:
alpha.clone.CloneChecker:MinimumCloneComplexity
alpha.cplusplus.UninitializedObject:CheckPointeeInitialization
alpha.clone.CloneChecker:ReportNormalClones
alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField
etc.
Actually except for the debug checker options and  
unix.DynamicMemoryModeling:Optimistic all these options are meaningful end-user 
options.

Of course there are the debug checker options which are not interesting for the 
end user, but debug checkers should anyway be ignored by the end-users.

We always wanted to add checker option listing to CodeChecker, however did not 
want to duplicate the option list and option documentation in the CodeChecker 
codebase.
It belongs to the analyzer and actually to the checker implementation.
So from CodeChecker we would like to invoke the "clang -cc1 
-analyzer-checker-option-help" to be able to list these options to the end 
users.

The same feature is available already in clang-tidy: clang-tidy -dump-config

I think it is the responsibility of the end-user to decide what option he may 
want to configure.

I understand you would like to differentiate between "developer" and "end-user" 
options.
What we could do maybe is to indicate in the option explanation that the given 
option is "analyzer-internal", "experimental" or "developer only".

What do you think about that?

In D57858#1432714 , @dcoughlin wrote:

> I'm pretty worried about exposing this flag to end users.
>
> - Almost none of the options you've listed are user facing. Many represent 
> options intended for use by static analyzer developers: debugging options, 
> feature flags, and checkers that were never finished. Others represent 
> mechanisms for build systems to control the behavior of the analyzer. Even 
> these are not meant for end users to interact with but rather for 
> implementers of build systems and IDEs. I don't think end users should have 
> to understand these options to use the analyzer.
> - The help text refers to analyzer implementation details (such as 
> "SymRegion") that users won't have the context or knowledge to understand.
> - The help text also recommends invoking -cc1 directly or through the driver 
> with -Xclang. Neither of these are supported end-user interfaces to the 
> analyzer. Instead, users should use scan-build or another tool (such as 
> CodeChecker) that was designed to be used by humans.





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

https://reviews.llvm.org/D57858



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