[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Clement Courbet via Phabricator via cfe-commits
courbet reclaimed this revision.
courbet added a comment.

Actually, thinking more about this, adding this to the existing warning might 
not be a very consensual change. In the case of the warning:

  S f() {
T&& t = ...;
...
return t;
  }

there is no argument against doing the std::move(): the code is just as clear 
and has better performance:

  S f() {
T&& t = ...;
...
return std::move(t);
  }

In the case of a const variable, some people might value the safety from the 
const above the performance:

  S f() {
const T t = ...;
...  // here be dragons
return t;
  }

And they would be unhappy if the -Wreturn-std-move started warning about this.

So I think there are two options here:

- Add a new warning, or
- Add it as a clang-tidy as in this change.

What do you think ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70306: clang: Exherbo multiarch ajustments

2019-11-19 Thread Marc-Antoine Perennou via Phabricator via cfe-commits
Keruspe planned changes to this revision.
Keruspe marked 3 inline comments as done.
Keruspe added a comment.

Will give llvm::sys::path::stem a look and try to drop the x86_64 part from the 
include dir selection




Comment at: clang/lib/Driver/ToolChains/Linux.cpp:360
+  addPathIfExists(D,
+  LibPath + "/../../" + GCCTriple.str() + "/lib/../" +
+  OSLibDir + SelectedMultilib.osSuffix(),

compnerd wrote:
> Could you use `llvm::sys::path::stem` please?
This was just a copy of the code that as already there, with a different path, 
but I sure can give that a look



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:889
+  break;
+}
+  }

compnerd wrote:
> Why not just always construct the path?  `"/usr/" + getTriple.str() + 
> "/include"` is always going to be the path that we have for exherbo
That's what we had at first, but then e.g. `clang -m32` didn't work as it was 
searching inside of i383 and not i686, and wrt x86_64 it was using unknown at 
some point instead of pc.
The x86_64 part might have been because it was in another environment but the 
x86 part definitely is still relevant



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:984
+  if (Distro == Distro::Exherbo &&
+  addLibStdCXXIncludePaths(
+  LibDir.str() + "/../../" + TripleStr + "/include",

compnerd wrote:
> Why is this part of the `if`?  This should be part of the code executed 
> conditionally.
It just matches the if from below, with the distro check added. Not familiar 
with the llvm codebase so I'm trying to just stick to the similar code that's 
just next to it


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

https://reviews.llvm.org/D70306



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment.

Adding @Quuxplusone (warning author) for opinions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D50147: clang-format: support external styles

2019-11-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

In D50147#1648786 , @sammccall wrote:

> > First and forehand, I have a problem to solve in my organization : we have 
> > many projects, and maintaining the config file in some many repositories is 
> > not practical.
> >  In some case (like, LLVM, google or mozilla), this is not an issue because 
> > the formatting rules are hard-coded in clang-format.
>
> I think this option is available to anyone with a public style guide that 
> covers a reasonably large body of open-source code, and which can be 
> reasonably well supported by clang-format.
>  I'd expect QT, KDE etc should be able to use this mechanism.


unfortunately this is not open-source code, so we cannot modify clang-format :-(
hence the will to have the exact same behavior, but without modifying 
clang-format itself

>> In our case, we actually have more than one "standard" style, a combination 
>> of various OS (linux, windows, macos), and not a very strong control on user 
>> computers. So we cannot rely on a specific file or variable being setup by 
>> an administrator.
> 
> In this case my best advice would be in the short term to use .clang-format 
> files. Longer term, some combination of using well-known styles, publicising 
> and teaching clang-format about the styles you use, and gaining the ability 
> to set environment variables would reduce duplication.

what do you mean about "gaining the ability to set environment variables" ?
in our case, we have around 500 repositories, so it really becomes a problem 
maintaining hundreds of copies of each style and verifying projects do not have 
a "variation" on one of the official style.

>> I think many orgs have the same issue, but some of them have found a 
>> solution by hard-coding their style in clang-format...
> 
> I'd like to see evidence that this is a widespread problem.

indeed I have no number at all.
But I think in many case, people would either:

- switch to a built-in style (which is not bad in itself, but can be 
problematic if there is lot of code already),
- not to use clang-format at all (which does not help with ensuring consistent 
formatting),
- or "fork" clang-format (which would we are currently forced to do, but is 
really not efficient)

>>> With that in mind, I'd be very happy to approve the build time config 
>>> and/or an env variable, as long as they're off by default. It's easy to 
>>> turn them on later, but not easy to turn them off.
>>>  If they're going to be on by default, I think we need a strong reason.
>> 
>> I they are going to be off by default, it means we would still need to patch 
>> clang-format to use it, correct ?
> 
> Sorry, by "off by default" I mean that if the environment variable is not 
> set, there would be no default search directory. Relative paths would be an 
> error.
>  So you could install styles centrally on each machine, and they would work 
> if CLANG_FORMAT_STYLE_PATH was set, otherwise you'd get the fallback style. 
> Would that be workable?



- Build option is implemented. This allows turn the feature off if needed, at 
build time (by specifying empty search path). I would prefer to keep thos
- Overriding an environment varialbe to change the search path is fine by me. 
But I would still prefer to have a "working" default, so that it can be used 
out-of-the-box, with no extra env variable to set

>> In D50147#157 , @sammccall 
>> wrote:
>> 
>>> >> - understanding how distro packaging is going to work
>>>
>>> There's a mechanism, but how is it to be used? Will/should projects with a 
>>> style guide provide style packages for distros? Or should these be part of 
>>> the "official" clang-format package? 
>>>  If separate packages exist, how much is it going to confuse users that 
>>> clang-format will silently format the same project with a `.clang-format` 
>>> file different ways depending on what's installed?

The exact same thing happens today with clang-format's integrated styles.

But indeed this is an issue, and handling it would be my next step : once the 
concept of 'external style' is present, I'd like to allow referencing a style 
stored in remote git server or other URL, with clang-format handling the 
retrieval/update/cache.

>> The goal is to actually separate the styles from clang-format : so I don't 
>> see the point to make them part of the official clang-format package.
>>  Usage may be different: the styles may be setup through different packages 
>> (e.g. Qt style in qt-core package), installed manually by user, 
>>  This is surely not perfect, since different packages may indeed provide the 
>> same style : technically this is not an issue (packages must just be marked 
>> as conflicting), but it is indeed the organisation's responsibility to use 
>> different names for the styles...
>>  (to some extent, this may be improved by passing URLs for external styles, 
>> e.g. from git ; but this may 

[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision.
arsenm added reviewers: yaxunl, scott.linder, t-tye.
Herald added subscribers: tpr, dstuttard, wdng, kzhuravl.

Enabling optimization should allow frame pointer elimination.


https://reviews.llvm.org/D70424

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.cl


Index: clang/test/Driver/frame-pointer-elim.cl
===
--- /dev/null
+++ clang/test/Driver/frame-pointer-elim.cl
@@ -0,0 +1,8 @@
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKNONE %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 -fno-omit-frame-pointer 
%s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O0 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -cl-opt-disable %s -o %t.s 
2>&1 | FileCheck -check-prefix=CHECKALL %s
+
+// CHECKNONE: -mframe-pointer=none
+// CHECKALL: -mframe-pointer=all
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -538,6 +538,8 @@
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
 return !areOptimizationsEnabled(Args);
   default:
 break;


Index: clang/test/Driver/frame-pointer-elim.cl
===
--- /dev/null
+++ clang/test/Driver/frame-pointer-elim.cl
@@ -0,0 +1,8 @@
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKNONE %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 -fno-omit-frame-pointer %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O0 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -cl-opt-disable %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+
+// CHECKNONE: -mframe-pointer=none
+// CHECKALL: -mframe-pointer=all
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -538,6 +538,8 @@
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
 return !areOptimizationsEnabled(Args);
   default:
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-19 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe51484abd402: [clangd] Fix hover local scope to 
include class template params (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D70325?vs=229910=230004#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -910,13 +910,13 @@
}},
   // Constructor of partially-specialized class template
   {R"cpp(
-  template struct X;
+  template struct X;
   template struct X{ [[^X]](); };
   )cpp",
[](HoverInfo ) {
  HI.NamespaceScope = "";
  HI.Name = "X";
- HI.LocalScope = "X::";// FIXME: Should be X::
+ HI.LocalScope = "X::"; // FIXME: X::
  HI.Kind = SymbolKind::Constructor;
  HI.ReturnType = "X";
  HI.Definition = "X()";
@@ -1029,8 +1029,8 @@
  HI.Type = "enum Color";
  HI.Value = "1";
}},
-  // FIXME: We should use the Decl referenced, even if it comes from an
-  // implicit instantiation.
+  // FIXME: We should use the Decl referenced, even if from an implicit
+  // instantiation. Then the scope would be Add<1, 2> and the value 3.
   {R"cpp(
 template struct Add {
   static constexpr int result = a + b;
@@ -1043,7 +1043,7 @@
  HI.Kind = SymbolKind::Property;
  HI.Type = "const int";
  HI.NamespaceScope = "";
- HI.LocalScope = "Add::";
+ HI.LocalScope = "Add::";
}},
   {R"cpp(
 const char *[[ba^r]] = "1234";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -414,12 +414,12 @@
 static std::string getLocalScope(const Decl *D) {
   std::vector Scopes;
   const DeclContext *DC = D->getDeclContext();
-  auto GetName = [](const Decl *D) {
-const NamedDecl *ND = dyn_cast(D);
-std::string Name = ND->getNameAsString();
-// FIXME(sammccall): include template params/specialization args?.
-if (!Name.empty())
-  return Name;
+  auto GetName = [](const TypeDecl *D) {
+if (!D->getDeclName().isEmpty()) {
+  PrintingPolicy Policy = D->getASTContext().getPrintingPolicy();
+  Policy.SuppressScope = true;
+  return declaredType(D).getAsString(Policy);
+}
 if (auto RD = dyn_cast(D))
   return ("(anonymous " + RD->getKindName() + ")").str();
 return std::string("");


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -910,13 +910,13 @@
}},
   // Constructor of partially-specialized class template
   {R"cpp(
-  template struct X;
+  template struct X;
   template struct X{ [[^X]](); };
   )cpp",
[](HoverInfo ) {
  HI.NamespaceScope = "";
  HI.Name = "X";
- HI.LocalScope = "X::";// FIXME: Should be X::
+ HI.LocalScope = "X::"; // FIXME: X::
  HI.Kind = SymbolKind::Constructor;
  HI.ReturnType = "X";
  HI.Definition = "X()";
@@ -1029,8 +1029,8 @@
  HI.Type = "enum Color";
  HI.Value = "1";
}},
-  // FIXME: We should use the Decl referenced, even if it comes from an
-  // implicit instantiation.
+  // FIXME: We should use the Decl referenced, even if from an implicit
+  // instantiation. Then the scope would be Add<1, 2> and the value 3.
   {R"cpp(
 template struct Add {
   static constexpr int result = a + b;
@@ -1043,7 +1043,7 @@
  HI.Kind = SymbolKind::Property;
  HI.Type = "const int";
  HI.NamespaceScope = "";
- HI.LocalScope = "Add::";
+ HI.LocalScope = "Add::";
}},
   {R"cpp(
 const char *[[ba^r]] = "1234";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -414,12 +414,12 @@
 static std::string getLocalScope(const Decl *D) {
   std::vector Scopes;
   const DeclContext *DC = D->getDeclContext();
-  auto GetName = [](const Decl *D) {
-const NamedDecl *ND = dyn_cast(D);
-std::string Name = ND->getNameAsString();
-// FIXME(sammccall): include template params/specialization args?.
-if (!Name.empty())
-  

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

2019-11-19 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 229986.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,16 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: shell
+
+// RUN: mkdir -p %t; cd %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   %t/driver-test2.o %t/object.o %t/weak.o -S -o - 2>&1 | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1 2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o 

[PATCH] D70355: [clang-format] [NFC] add recent changes to release notes

2019-11-19 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b5f6c16476c: [clang-format] [NFC] add recent changes to 
release notes (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70355

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -58,7 +58,7 @@
   access to determine if two operand expressions are the same.
 - -Wtautological-bitwise-compare is a new warning group.  This group has the
   current warning which diagnoses the tautological comparison of a bitwise
-  operation and a constant.  The group also has the new warning which diagnoses
+  operation and a constant. The group also has the new warning which diagnoses
   when a bitwise-or with a non-negative value is converted to a bool, since
   that bool will always be true.
 - -Wbitwise-conditional-parentheses will warn on operator precedence issues
@@ -138,9 +138,6 @@
 
 error LNK2005: "bool const std::_Is_integral" 
(??$_Is_integral@H@std@@3_NB) already defined
 
-
-
-
 C Language Changes in Clang
 ---
 
@@ -296,13 +293,16 @@
   - ``Auto`` is the default and detects style from the code (this is 
unchanged).
 
   The previous values of ``Cpp03`` and ``Cpp11`` are deprecated. Note that
-  ``Cpp11`` is treated as ``Latest``, as this was always clang-format's 
behavior.
-  (One motivation for this change is the new name describes the behavior 
better).
+  ``Cpp11`` is treated as ``Latest``, as this was always clang-format's
+  behavior. (One motivation for this change is the new name describes the
+  behavior better).
 
-- clang-format gets a new option called ``--dry-run`` or ``-n`` to emit a
-  warning.
+- Clang-format has a new option called ``--dry-run`` or ``-n`` to emit a
+  warning for clang-format violations. This can be used together
+  with --ferror-limit=N to limit the number of warnings per file and --Werror
+  to make warnings into errors.
 
-- Option *IncludeIsMainSourceRegex* added to allow for additional
+- Option *IncludeIsMainSourceRegex* has been added to allow for additional
   suffixes and file extensions to be considered as a source file
   for execution of logic that looks for "main *include* file" to put
   it on top.
@@ -311,7 +311,7 @@
   they end with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``,
   ``.m`` or ``.mm`` extensions. This config option allows to
   extend this set of source files considered as "main".
-
+
   For example, if this option is configured to ``(Impl\.hpp)$``,
   then a file ``ClassImpl.hpp`` is considered "main" (in addition to
   ``Class.c``, ``Class.cc``, ``Class.cpp`` and so on) and "main
@@ -320,12 +320,31 @@
   ``ClassImpl.hpp`` would not have the main include file put on top
   before any other include.
 
+- Options ``DeriveLineEnding`` and  ``UseCRLF`` have been added to allow
+  clang-format to control the newlines. ``DeriveLineEnding`` is by default
+  ``true`` and reflects is the existing mechanism, which based is on majority
+  rule. The new options allows this to be turned off and ``UseCRLF`` to control
+  the decision as to which sort of line ending to use.
+
+- Option ``SpaceBeforeSquareBrackets`` has been added to insert a space before
+  array declarations.
+
+  .. code-block:: c++
+
+int a [5];vsint a[5];
+
+- Clang-format now supports JavaScript null operators.
+
+  .. code-block:: c++
+
+const x = foo ?? default;
+const z = foo?.bar?.baz;
+
 libclang
 
 
 - ...
 
-
 Static Analyzer
 ---
 
@@ -373,7 +392,6 @@
 return getelementpointer_inbounds(base, offset);
   }
 
-
 Core Analysis Improvements
 ==
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -58,7 +58,7 @@
   access to determine if two operand expressions are the same.
 - -Wtautological-bitwise-compare is a new warning group.  This group has the
   current warning which diagnoses the tautological comparison of a bitwise
-  operation and a constant.  The group also has the new warning which diagnoses
+  operation and a constant. The group also has the new warning which diagnoses
   when a bitwise-or with a non-negative value is converted to a bool, since
   that bool will always be true.
 - -Wbitwise-conditional-parentheses will warn on operator precedence issues
@@ -138,9 +138,6 @@
 
 error LNK2005: "bool const std::_Is_integral" (??$_Is_integral@H@std@@3_NB) already defined
 
-
-
-
 C Language Changes in Clang
 ---
 
@@ -296,13 +293,16 @@
   - ``Auto`` is the default and detects style from the code (this is unchanged).
 
   The previous values of ``Cpp03`` and 

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

IMHO these two should just not overlap. It makes sense, to have controversial 
or configurable stuff in clang-tidy. It should just be consistent with the 
warnings, as those are "always right" and clang-tidy can be 
opinionated/specialized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks! but again it looks as if rebasing went wrong :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325



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


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Nice!

Silly questions, but for my own education: I thought the key function concept 
only existed in the Itanium ABI, but from your numbers it sounds like it's a 
concept, at least for debug info, also on Windows?




Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

I worry that this is going to look obscure to most readers passing through. 
Maybe it could be expanded to more explicitly spell out that it reduces the 
size of the debug info?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

it feels like rebasing went wrong :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D70357: [clangd] Untangle Hover from XRefs, move into own file.

2019-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/Hover.cpp:374
+  } else {
+auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
+if (!Offset) {

sammccall wrote:
> kadircet wrote:
> > kadircet wrote:
> > > nit: `SM.getFileOffset(SourceLocationBeg)` ?
> > why not just expose getDeclAtPosition in `AST.h` ?
> > nit: SM.getFileOffset(SourceLocationBeg) ?
> I was deliberately trying to avoid using the "rewind token" logic where it's 
> not needed. We've had bugs with it before, as well as with selectiontree, and 
> composing them unneccesarily is harder to debug.
> 
> > why not just expose getDeclAtPosition in AST.h ?
> I don't think it's better than the expanded form - it's just plugging a 
> couple of functions together, and the choices it makes (flattening 
> SourceLocation into an offset, the DeclRelationSet, only looking at the 
> common ancestor and nothing higher up) aren't obvious.
> 
> It also privileges decls over other types of things (e.g. the followup patch 
> looks for Exprs in the selection tree to show their values, and that couldn't 
> happen if it was hidden behind getDeclAtPosition)
> I was deliberately trying to avoid using the "rewind token" logic where it's 
> not needed. We've had bugs with it before, as well as with selectiontree, and 
> composing them unneccesarily is harder to debug.

Sounds good, I was worried about performing this conversion twice, but I 
suppose it should be OK for hover's performance.



Comment at: clang-tools-extra/clangd/Hover.cpp:380
+SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
+std::vector Result;
+if (const SelectionTree::Node *N = Selection.commonAncestor()) {

nit: Unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70357



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


[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang/include/clang/Format/Format.h:1110
+  /// Different const alignment styles.
+  enum ConstAlignmentStyle {
+/// Don't change const to either East const or West const.

Drive-By question: https://reviews.llvm.org/D54395 implements clang-tidy part 
of adding `const` to variables. It would be great to use the same underlying 
functionality, e.g. this enum for determining which const-style to use.
Is this header-file always accessible from clang-tidy-code? Or can e.g. 
clang-format be deactivated while building leading to exclusion of these bits?


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

https://reviews.llvm.org/D69764



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment.

> IMHO these two should just not overlap. It makes sense, to have controversial 
> or configurable stuff in clang-tidy. It should just be consistent with the 
> warnings, as those are "always right" and clang-tidy can be 
> opinionated/specialized.

So to make sure I understand you're advocating for keeping the `const` version 
in the clang-tidy check but removing the `&&` detection from this check and let 
the warning deal with that ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70317: [clangd] More sensible output for constructors/destructors in hover.

2019-11-19 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ec071409807: [clangd] More sensible output for 
constructors/destructors in hover. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70317

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -918,11 +918,20 @@
  HI.Name = "X";
  HI.LocalScope = "X::";// FIXME: Should be X::
  HI.Kind = SymbolKind::Constructor;
- HI.Type = "void ()";  // FIXME: Should be None
- HI.ReturnType = "void";   // FIXME: Should be None or X
+ HI.ReturnType = "X";
  HI.Definition = "X()";
  HI.Parameters.emplace();
}},
+  {"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.Name = "~X";
+ HI.LocalScope = "X::";
+ HI.Kind = SymbolKind::Constructor;
+ HI.ReturnType = "void";
+ HI.Definition = "~X()";
+ HI.Parameters.emplace();
+   }},
 
   // auto on lambda
   {R"cpp(
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -566,6 +566,51 @@
   Req, [&](const Symbol ) { Hover.Documentation = S.Documentation; });
 }
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+static void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
+  const FunctionDecl *FD,
+  const PrintingPolicy ) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters()) {
+HI.Parameters->emplace_back();
+auto  = HI.Parameters->back();
+if (!PVD->getType().isNull()) {
+  P.Type.emplace();
+  llvm::raw_string_ostream OS(*P.Type);
+  PVD->getType().print(OS, Policy);
+} else {
+  std::string Param;
+  llvm::raw_string_ostream OS(Param);
+  PVD->dump(OS);
+  OS.flush();
+  elog("Got param with null type: {0}", Param);
+}
+if (!PVD->getName().empty())
+  P.Name = PVD->getNameAsString();
+if (PVD->hasDefaultArg()) {
+  P.Default.emplace();
+  llvm::raw_string_ostream Out(*P.Default);
+  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+}
+  }
+
+  if (const auto* CCD = llvm::dyn_cast(FD)) {
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.
+  } else if (const auto* CDD = llvm::dyn_cast(FD)){
+HI.ReturnType = "void";
+  } else {
+HI.ReturnType = FD->getReturnType().getAsString(Policy);
+
+QualType FunctionType = FD->getType();
+if (const VarDecl *VD = llvm::dyn_cast(D)) // Lambdas
+  FunctionType = VD->getType().getDesugaredType(D->getASTContext());
+HI.Type = FunctionType.getAsString(Policy);
+  }
+  // FIXME: handle variadics.
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -601,45 +646,7 @@
 
   // Fill in types and params.
   if (const FunctionDecl *FD = getUnderlyingFunction(D)) {
-HI.ReturnType.emplace();
-{
-  llvm::raw_string_ostream OS(*HI.ReturnType);
-  FD->getReturnType().print(OS, Policy);
-}
-
-HI.Parameters.emplace();
-for (const ParmVarDecl *PVD : FD->parameters()) {
-  HI.Parameters->emplace_back();
-  auto  = HI.Parameters->back();
-  if (!PVD->getType().isNull()) {
-P.Type.emplace();
-llvm::raw_string_ostream OS(*P.Type);
-PVD->getType().print(OS, Policy);
-  } else {
-std::string Param;
-llvm::raw_string_ostream OS(Param);
-PVD->dump(OS);
-OS.flush();
-elog("Got param with null type: {0}", Param);
-  }
-  if (!PVD->getName().empty())
-P.Name = PVD->getNameAsString();
-  if (PVD->hasDefaultArg()) {
-P.Default.emplace();
-llvm::raw_string_ostream Out(*P.Default);
-PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
-  }
-}
-
-HI.Type.emplace();
-llvm::raw_string_ostream TypeOS(*HI.Type);
-// Lambdas
-if (const VarDecl *VD = llvm::dyn_cast(D))
-  VD->getType().getDesugaredType(D->getASTContext()).print(TypeOS, Policy);
-// Functions
-else
-  FD->getType().print(TypeOS, Policy);
-// FIXME: 

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

2019-11-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 229996.
MyDeveloperDay added a comment.

- Rebase
- Add Release note
- Remove repeated lines cause by patch creation artefact


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -27,6 +27,10 @@
 
 FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
 
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style) \
+  verifyFormat(expect, actual, style, __LINE__)
+
 class FormatTest : public ::testing::Test {
 protected:
   enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
@@ -66,10 +70,12 @@
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle  = getLLVMStyle()) {
+const FormatStyle  = getLLVMStyle(),
+int line = __LINE__) {
 EXPECT_EQ(Expected.str(), format(Expected, Style))
-<< "Expected code is not stable";
-EXPECT_EQ(Expected.str(), format(Code, Style));
+<< "Expected code is not stable at " << __FILE__ << ":" << line;
+EXPECT_EQ(Expected.str(), format(Code, Style))
+<< " at " << __FILE__ << ":" << line;
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
@@ -80,8 +86,9 @@
   }
 
   void verifyFormat(llvm::StringRef Code,
-const FormatStyle  = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+const FormatStyle  = getLLVMStyle(),
+int line = __LINE__) {
+verifyFormat(Code, test::messUp(Code), Style, line);
   }
 
   void verifyIncompleteFormat(llvm::StringRef Code,
@@ -12609,6 +12616,15 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -14994,6 +15010,194 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  VERIFYFORMAT("const int a;", Style);
+  VERIFYFORMAT("const int *a;", Style);
+  VERIFYFORMAT("const int ", Style);
+  VERIFYFORMAT("const int &", Style);
+  VERIFYFORMAT("int const b;", Style);
+  VERIFYFORMAT("int const *b;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const *b const;", Style);
+  VERIFYFORMAT("int *const c;", Style);
+
+  VERIFYFORMAT("const Foo a;", Style);
+  VERIFYFORMAT("const Foo *a;", Style);
+  VERIFYFORMAT("const Foo ", Style);
+  VERIFYFORMAT("const Foo &", Style);
+  VERIFYFORMAT("Foo const b;", Style);
+  VERIFYFORMAT("Foo const *b;", Style);
+  VERIFYFORMAT("Foo const ", Style);
+  VERIFYFORMAT("Foo const &", Style);
+  VERIFYFORMAT("Foo const *b const;", Style);
+
+  VERIFYFORMAT("LLVM_NODISCARD const int ();", Style);
+  VERIFYFORMAT("LLVM_NODISCARD int const ();", Style);
+
+  VERIFYFORMAT("volatile const int *restrict;", Style);
+  VERIFYFORMAT("const volatile int *restrict;", Style);
+  VERIFYFORMAT("const int volatile *restrict;", Style);
+
+  Style.ConstStyle = FormatStyle::CS_Right;
+
+  VERIFYFORMAT("int const a;", Style);
+  VERIFYFORMAT("int const *a;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const b;", Style);
+  VERIFYFORMAT("int const *b;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const *b const;", Style);
+  VERIFYFORMAT("int *const c;", Style);
+
+  VERIFYFORMAT("Foo const a;", Style);
+  VERIFYFORMAT("Foo const *a;", Style);
+  

[clang-tools-extra] 7db1230 - Reland "[clangd] Implement rename by using SelectionTree and findExplicitReferences."

2019-11-19 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-19T10:20:25+01:00
New Revision: 7db1230a9f5e0185a88019c9aa5b55bd85498285

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

LOG: Reland "[clangd] Implement rename by using SelectionTree and 
findExplicitReferences."

this reland the commit 4f80fc2491cc35730a9a84b86975278b7daa8522 which
has been reverted at f805c60a093325c16ce4200d2615ef48555d9cb8.

Fixed windows buildbot failure (by adding -fno-delayed-template-parsing flag).

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 3969f3e2e4e2..fb83083384f9 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -8,14 +8,16 @@
 
 #include "refactor/Rename.h"
 #include "AST.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 
 namespace clang {
 namespace clangd {
@@ -34,6 +36,17 @@ llvm::Optional filePath(const SymbolLocation 
,
   return *Path;
 }
 
+// Returns true if the given location is expanded from any macro body.
+bool isInMacroBody(const SourceManager , SourceLocation Loc) {
+  while (Loc.isMacroID()) {
+if (SM.isMacroBodyExpansion(Loc))
+  return true;
+Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+
+  return false;
+}
+
 // Query the index to find some other files where the Decl is referenced.
 llvm::Optional getOtherRefFile(const Decl , StringRef MainFile,
 const SymbolIndex ) {
@@ -56,12 +69,41 @@ llvm::Optional getOtherRefFile(const Decl , 
StringRef MainFile,
   return OtherFile;
 }
 
+llvm::DenseSet locateDeclAt(ParsedAST ,
+  SourceLocation TokenStartLoc) {
+  unsigned Offset =
+  AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
+
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
+  if (!SelectedNode)
+return {};
+
+  // If the location points to a Decl, we check it is actually on the name
+  // range of the Decl. This would avoid allowing rename on unrelated tokens.
+  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
+  // // we don't attempt to trigger rename on this position.
+  // FIXME: make this work on destructors, e.g. "~F^oo()".
+  if (const auto *D = SelectedNode->ASTNode.get()) {
+if (D->getLocation() != TokenStartLoc)
+  return {};
+  }
+
+  llvm::DenseSet Result;
+  for (const auto *D :
+   targetDecl(SelectedNode->ASTNode,
+  DeclRelation::Alias | DeclRelation::TemplatePattern))
+Result.insert(D);
+  return Result;
+}
+
 enum ReasonToReject {
   NoSymbolFound,
   NoIndexProvided,
   NonIndexable,
   UsedOutsideFile,
   UnsupportedSymbol,
+  AmbiguousSymbol,
 };
 
 // Check the symbol Decl is renameable (per the index) within the file.
@@ -125,6 +167,8 @@ llvm::Error makeError(ReasonToReject Reason) {
   return "symbol may be used in other files (not eligible for indexing)";
 case UnsupportedSymbol:
   return "symbol is not a supported kind (e.g. namespace, macro)";
+case AmbiguousSymbol:
+  return "there are multiple symbols at the given location";
 }
 llvm_unreachable("unhandled reason kind");
   };
@@ -134,22 +178,38 @@ llvm::Error makeError(ReasonToReject Reason) {
 }
 
 // Return all rename occurrences in the main file.
-tooling::SymbolOccurrences
-findOccurrencesWithinFile(ParsedAST , const NamedDecl *RenameDecl) {
-  const NamedDecl *CanonicalRenameDecl =
-  tooling::getCanonicalSymbolDeclaration(RenameDecl);
-  assert(CanonicalRenameDecl && "RenameDecl must be not null");
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext());
-  std::string OldName = CanonicalRenameDecl->getNameAsString();
-  tooling::SymbolOccurrences Result;
+std::vector findOccurrencesWithinFile(ParsedAST ,
+  const NamedDecl ) {
+  // In theory, locateDeclAt should return the primary template. However, if 
the
+  // cursor is under the underlying CXXRecordDecl of the 

[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry about the bad rebase, I landed the right version i think :-)

In D70325#1750496 , @sammccall wrote:

> In D70325#1749432 , @kadircet wrote:
>
> > LGTM, with a question. What about default template params? I believe we 
> > would also like to print them, could you add a test case for that?
>
>
> They're not printed, as the type is "as written". Added a testcase and a 
> FIXME.
>  (I don't think this case is terribly important - either behavior seems to 
> have its advantages, the combination of default parameters and partial 
> specialization is fairly rare, and not much confusion seems likely in 
> practice)


Forgot to mention, I didn't fix this because it's significantly more work: for 
Type you have access to the canonical args (with defaults, but with 
type-parameter-0-0 instead of T), and the as-written args (where defaults are 
omitted).
I think you have to trawl around special casing various Decls if you want this 
to work. I wanted to get the cheap improvement in because realistically I won't 
get to that refinement soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325



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


[clang] 8b5f6c1 - [clang-format] [NFC] add recent changes to release notes

2019-11-19 Thread via cfe-commits

Author: mydeveloperday
Date: 2019-11-19T08:44:27Z
New Revision: 8b5f6c16476c7a8f50b660fb6e1b549759a783b6

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

LOG: [clang-format] [NFC] add recent changes to release notes

Summary: clang-tidy keeps nice release notes of what is added, have 
clang-format do the same.

Reviewers: klimek, mitchell-stellar, sylvestre.ledru, sammccall

Reviewed By: mitchell-stellar

Subscribers: merge_guards_bot, Eugene.Zelenko, cfe-commits

Tags: #clang-format, #clang

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5c8f3bd13d5a..4ac300deb589 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -58,7 +58,7 @@ Improvements to Clang's diagnostics
   access to determine if two operand expressions are the same.
 - -Wtautological-bitwise-compare is a new warning group.  This group has the
   current warning which diagnoses the tautological comparison of a bitwise
-  operation and a constant.  The group also has the new warning which diagnoses
+  operation and a constant. The group also has the new warning which diagnoses
   when a bitwise-or with a non-negative value is converted to a bool, since
   that bool will always be true.
 - -Wbitwise-conditional-parentheses will warn on operator precedence issues
@@ -138,9 +138,6 @@ Windows Support
 
 error LNK2005: "bool const std::_Is_integral" 
(??$_Is_integral@H@std@@3_NB) already defined
 
-
-
-
 C Language Changes in Clang
 ---
 
@@ -296,13 +293,16 @@ clang-format
   - ``Auto`` is the default and detects style from the code (this is 
unchanged).
 
   The previous values of ``Cpp03`` and ``Cpp11`` are deprecated. Note that
-  ``Cpp11`` is treated as ``Latest``, as this was always clang-format's 
behavior.
-  (One motivation for this change is the new name describes the behavior 
better).
+  ``Cpp11`` is treated as ``Latest``, as this was always clang-format's
+  behavior. (One motivation for this change is the new name describes the
+  behavior better).
 
-- clang-format gets a new option called ``--dry-run`` or ``-n`` to emit a
-  warning.
+- Clang-format has a new option called ``--dry-run`` or ``-n`` to emit a
+  warning for clang-format violations. This can be used together
+  with --ferror-limit=N to limit the number of warnings per file and --Werror
+  to make warnings into errors.
 
-- Option *IncludeIsMainSourceRegex* added to allow for additional
+- Option *IncludeIsMainSourceRegex* has been added to allow for additional
   suffixes and file extensions to be considered as a source file
   for execution of logic that looks for "main *include* file" to put
   it on top.
@@ -311,7 +311,7 @@ clang-format
   they end with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``,
   ``.m`` or ``.mm`` extensions. This config option allows to
   extend this set of source files considered as "main".
-
+
   For example, if this option is configured to ``(Impl\.hpp)$``,
   then a file ``ClassImpl.hpp`` is considered "main" (in addition to
   ``Class.c``, ``Class.cc``, ``Class.cpp`` and so on) and "main
@@ -320,12 +320,31 @@ clang-format
   ``ClassImpl.hpp`` would not have the main include file put on top
   before any other include.
 
+- Options ``DeriveLineEnding`` and  ``UseCRLF`` have been added to allow
+  clang-format to control the newlines. ``DeriveLineEnding`` is by default
+  ``true`` and reflects is the existing mechanism, which based is on majority
+  rule. The new options allows this to be turned off and ``UseCRLF`` to control
+  the decision as to which sort of line ending to use.
+
+- Option ``SpaceBeforeSquareBrackets`` has been added to insert a space before
+  array declarations.
+
+  .. code-block:: c++
+
+int a [5];vsint a[5];
+
+- Clang-format now supports JavaScript null operators.
+
+  .. code-block:: c++
+
+const x = foo ?? default;
+const z = foo?.bar?.baz;
+
 libclang
 
 
 - ...
 
-
 Static Analyzer
 ---
 
@@ -373,7 +392,6 @@ Undefined Behavior Sanitizer (UBSan)
 return getelementpointer_inbounds(base, offset);
   }
 
-
 Core Analysis Improvements
 ==
 



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


[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

2019-11-19 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:176
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  ConstraintManager  = C.getConstraintManager();
+  auto *CE = dyn_cast_or_null(Call.getOriginExpr());

NoQ wrote:
> balazske wrote:
> > baloghadamsoftware wrote:
> > > The four lines above are typical cases for using auto, since the type is 
> > > duplicated in the line. See: 
> > > https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> > This is not totally clear if we can trust the pattern that at `auto X = 
> > Y.getXxx()` type of `X` will be `Xxx`.
> @balazske +1. I'm personally in favor of using `auto` here, but the 
> community's stance is to use `auto` only for `dyn_cast`/`getAs` etc. calls 
> where type is explicitly spelled out, and also for iterators where the type 
> is going to be too long and annoying to spell out (but in the latter case i'm 
> suddenly in favor of spelling it out, as i always get confused about how many 
> `*`s do i need to add in order to properly dereference whatever's in there).
`SValBuilder` is explicitly spelled out here, also `DefinedSVal` below, which 
comes from a `getAs()`. Btw, is it right to call the actual variable 
`SValBuilder` as well? I would never do that. I is usually called `SVB`.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:189-190
+  // Check if error was generated.
+  if (C.isDifferent())
+return;
+

balazske wrote:
> NoQ wrote:
> > That's actually one more good reason to move all `checkNullStream` to 
> > `PreCall`: less spaghetti. The same checker should feel free to subscribe 
> > for the same function in multiple different callbacks and do different 
> > things in them.
> > 
> > Rule of a thumb (i'm not sure if it's //always// applicable but it's 
> > applicable to most "typical" checkers even if they aren't currently written 
> > this way):
> > 1. Pre-condition checking goes in PreCall;
> > 2. Modeling that affects everybody goes in evalCall;
> > 3. Checker's internal bookkeeping goes in PostCall.
> > 
> Precondition (check for NULL stream) can go into PreCall. Probably the 
> bookkeeping task is more difficult to do, if the state is split we have 
> already the different states available in `evalCall` but need to check the 
> state in `postCall`.
@NoQ +1 I completely agree: All precondition checks in this checker should be 
moved into `PreCall`. I would even go further: separate the checker into a 
`StreamModeling` which models the calls using `evalCall` and `StreamChecker` 
which does the checks. However, these NFCs should be done in subsequent 
patches, not this one.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:69
   {{"fclose", 1}, ::evalFclose},
+  {{"freopen", 3}, ::evalFreopen},
   {{"fread", 4},

I would move this `freopen` line and everything else (`evalFreopen` header and 
body) directly after `freopen`. To me it seems more logical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-19 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

Build result: fail - 60136 tests passed, 2 failed and 729 were skipped.

  failed: LLVM.CodeGen/NVPTX/vectorize-misaligned.ll
  failed: LLVM.Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[clang-tools-extra] 6ec0714 - [clangd] More sensible output for constructors/destructors in hover.

2019-11-19 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2019-11-19T10:43:22+01:00
New Revision: 6ec07140980770fccfcdf53fe43e7425a9f51a7b

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

LOG: [clangd] More sensible output for constructors/destructors in hover.

Summary:
Previously: both had type void() and return type void.
Now: neither have a type. Constructors return T, destructors return void.

Reviewers: hokein

Subscribers: merge_guards_bot, ilya-biryukov, MaskRay, jkorous, arphaman, 
kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index af04fbd0d4d4..d04ebcf22a88 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -246,6 +246,12 @@ std::string printType(const QualType QT, const DeclContext 
& Context){
   printNamespaceScope(Context) );
 }
 
+QualType declaredType(const TypeDecl *D) {
+  if (const auto *CTSD = llvm::dyn_cast(D))
+if (const auto *TSI = CTSD->getTypeAsWritten())
+  return TSI->getType();
+  return D->getASTContext().getTypeDeclType(D);
+}
 
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 8f1abdd3297d..b106e06f8d91 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -111,6 +111,11 @@ bool isExplicitTemplateSpecialization(const NamedDecl *D);
 /// void foo() -> returns null
 NestedNameSpecifierLoc getQualifierLoc(const NamedDecl );
 
+// Returns a type corresponding to a declaration of that type.
+// Unlike the method on ASTContext, attempts to preserve the type as-written
+// (i.e. vector rather than vector.
+QualType declaredType(const TypeDecl *D);
+
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 9697b8eec19a..ce8e59553622 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -566,6 +566,51 @@ static void enhanceFromIndex(HoverInfo , const Decl 
*D,
   Req, [&](const Symbol ) { Hover.Documentation = S.Documentation; });
 }
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+static void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
+  const FunctionDecl *FD,
+  const PrintingPolicy ) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters()) {
+HI.Parameters->emplace_back();
+auto  = HI.Parameters->back();
+if (!PVD->getType().isNull()) {
+  P.Type.emplace();
+  llvm::raw_string_ostream OS(*P.Type);
+  PVD->getType().print(OS, Policy);
+} else {
+  std::string Param;
+  llvm::raw_string_ostream OS(Param);
+  PVD->dump(OS);
+  OS.flush();
+  elog("Got param with null type: {0}", Param);
+}
+if (!PVD->getName().empty())
+  P.Name = PVD->getNameAsString();
+if (PVD->hasDefaultArg()) {
+  P.Default.emplace();
+  llvm::raw_string_ostream Out(*P.Default);
+  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+}
+  }
+
+  if (const auto* CCD = llvm::dyn_cast(FD)) {
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.
+  } else if (const auto* CDD = llvm::dyn_cast(FD)){
+HI.ReturnType = "void";
+  } else {
+HI.ReturnType = FD->getReturnType().getAsString(Policy);
+
+QualType FunctionType = FD->getType();
+if (const VarDecl *VD = llvm::dyn_cast(D)) // Lambdas
+  FunctionType = VD->getType().getDesugaredType(D->getASTContext());
+HI.Type = FunctionType.getAsString(Policy);
+  }
+  // FIXME: handle variadics.
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -601,45 +646,7 @@ static HoverInfo getHoverContents(const Decl *D, const 
SymbolIndex *Index) {
 
   // Fill in types and params.
   if (const FunctionDecl *FD = getUnderlyingFunction(D)) {
-HI.ReturnType.emplace();
-{
-  llvm::raw_string_ostream OS(*HI.ReturnType);
-  FD->getReturnType().print(OS, Policy);
-}
-
-HI.Parameters.emplace();
-for (const ParmVarDecl *PVD : FD->parameters()) {
-  HI.Parameters->emplace_back();
-  auto  = HI.Parameters->back();
-  if (!PVD->getType().isNull()) {
-

[clang-tools-extra] f0021f9 - [clangd] Fix ps4 buildbot failure.

2019-11-19 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-19T10:42:27+01:00
New Revision: f0021f95a1f40369e30bb94be1b0322747631047

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

LOG: [clangd] Fix ps4 buildbot failure.

dynamic_cast on ps4 buildbot seems relying on rtti flag which is off by
default. Remove the dynamic_cast in the tests.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp 
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index b7678f1d5fe4..8dedcf579fd3 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -210,8 +210,6 @@ TEST(RenameTest, WithinFileRename) {
   [[Foo]] foo;
   const Baz  = foo;
   const Baz *BazPointer = 
-  dynamic_cast(BazReference).getValue();
-  dynamic_cast(BazPointer)->getValue();
   reinterpret_cast(BazPointer)->getValue();
   static_cast(BazReference).getValue();
   static_cast(BazPointer)->getValue();



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


[clang-tools-extra] e51484a - [clangd] Fix hover 'local scope' to include class template params

2019-11-19 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2019-11-19T10:52:38+01:00
New Revision: e51484abd402016a385e17896e87877b77bf4c7b

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

LOG: [clangd] Fix hover 'local scope' to include class template params

Summary: Fixes the last part of https://github.com/clangd/clangd/issues/76

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index ce8e59553622..3165633e60f9 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -414,12 +414,12 @@ static PrintingPolicy 
printingPolicyForDecls(PrintingPolicy Base) {
 static std::string getLocalScope(const Decl *D) {
   std::vector Scopes;
   const DeclContext *DC = D->getDeclContext();
-  auto GetName = [](const Decl *D) {
-const NamedDecl *ND = dyn_cast(D);
-std::string Name = ND->getNameAsString();
-// FIXME(sammccall): include template params/specialization args?.
-if (!Name.empty())
-  return Name;
+  auto GetName = [](const TypeDecl *D) {
+if (!D->getDeclName().isEmpty()) {
+  PrintingPolicy Policy = D->getASTContext().getPrintingPolicy();
+  Policy.SuppressScope = true;
+  return declaredType(D).getAsString(Policy);
+}
 if (auto RD = dyn_cast(D))
   return ("(anonymous " + RD->getKindName() + ")").str();
 return std::string("");

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 9817ba643894..483f216ca666 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -910,13 +910,13 @@ void foo())cpp";
}},
   // Constructor of partially-specialized class template
   {R"cpp(
-  template struct X;
+  template struct X;
   template struct X{ [[^X]](); };
   )cpp",
[](HoverInfo ) {
  HI.NamespaceScope = "";
  HI.Name = "X";
- HI.LocalScope = "X::";// FIXME: Should be X::
+ HI.LocalScope = "X::"; // FIXME: X::
  HI.Kind = SymbolKind::Constructor;
  HI.ReturnType = "X";
  HI.Definition = "X()";
@@ -1029,8 +1029,8 @@ void foo())cpp";
  HI.Type = "enum Color";
  HI.Value = "1";
}},
-  // FIXME: We should use the Decl referenced, even if it comes from an
-  // implicit instantiation.
+  // FIXME: We should use the Decl referenced, even if from an implicit
+  // instantiation. Then the scope would be Add<1, 2> and the value 3.
   {R"cpp(
 template struct Add {
   static constexpr int result = a + b;
@@ -1043,7 +1043,7 @@ void foo())cpp";
  HI.Kind = SymbolKind::Property;
  HI.Type = "const int";
  HI.NamespaceScope = "";
- HI.LocalScope = "Add::";
+ HI.LocalScope = "Add::";
}},
   {R"cpp(
 const char *[[ba^r]] = "1234";



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


[libunwind] 4fb8ecd - [libunwind] Adjust the signal_frame test for Arm

2019-11-19 Thread Mikhail Maltsev via cfe-commits

Author: Mikhail Maltsev
Date: 2019-11-19T09:58:46Z
New Revision: 4fb8ecdef4c9b19563e428a151c376a4103d65fc

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

LOG: [libunwind] Adjust the signal_frame test for Arm

Summary:
This patch adjusts the signal_frame.pass.cpp to pass on Arm targets:
* When Arm EHABI is used the unwinder does not use DWARF, hence the
  DWARF-specific check unw_is_signal_frame() must be disabled.
* Certain C libraries don't include EH tables, so the unwinder must
  not try to step out of main(). The patch moves the test code out of
  main() into a separate function to avoid this.

Reviewers: saugustine, ostannard, phosek, jfb, mclow.lists

Reviewed By: saugustine

Subscribers: dexonsmith, aprantl, kristof.beyls, christof, libcxx-commits, 
pbarrio, labrinea

Tags: #libc

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

Added: 


Modified: 
libunwind/test/signal_frame.pass.cpp

Removed: 




diff  --git a/libunwind/test/signal_frame.pass.cpp 
b/libunwind/test/signal_frame.pass.cpp
index b14e95a51528..a6f3f483bea5 100644
--- a/libunwind/test/signal_frame.pass.cpp
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -13,13 +13,19 @@
 #include 
 #include 
 
-int main(void) {
+void test() {
   asm(".cfi_signal_frame");
   unw_cursor_t cursor;
   unw_context_t uc;
   unw_getcontext();
   unw_init_local(, );
   assert(unw_step() > 0);
+#if !defined(_LIBUNWIND_ARM_EHABI)
   assert(unw_is_signal_frame());
+#endif
+}
+
+int main() {
+  test();
   return 0;
 }



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Yay, i see the "Schrödinger handle" pattern: until the return value of release 
is checked, the handle is believed to be both dead and alive at the same time.

We usually use this pattern because a lot of code that's already written 
doesn't check their return values. But do you really need this for a brand-new 
API? Maybe aggressively assume that the release may always fail and fix all the 
places in your code where the return value is not checked before use?




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:99-100
+
+static const StringRef HandleTypeName = "zx_handle_t";
+static const StringRef ErrorTypeName = "zx_status_t";
+

So you plan to unhardcode these as well eventually?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:123
+
+  // Only use this in checkDeadSymbols!
+  bool hasError(ProgramStateRef State) const {

For now it doesn't seem to be used at all.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:141-146
+#define CASE(ID)   
\
+  case ID: 
\
+OS << #ID; 
\
+break;
+  CASE(Kind::Allocated)
+  CASE(Kind::Released)

This macro has literally saved exactly as many lines as it has caused :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:156-168
+  std::unique_ptr LeakBugType;
+  std::unique_ptr DoubleReleaseBugType;
+  std::unique_ptr UseAfterFreeBugType;
+
+public:
+  FuchsiaHandleChecker()
+  : LeakBugType(new BugType(this, "Fuchsia handle leak",

We've recently dumbed down this idiom to
```lang=c++
class FuchsiaHandleChecker : ... {
  LeakBugType{this, "Fuchsia handle leak",
  "Fuchsia Handle Error", /*SuppressOnSink=*/true};
  ...
};
```
:)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

This is never necessary, just call all the methods directly on `QT` - it has an 
overloaded operator `->()` for this.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

Ok, so what's the deal with arrays here? If the function receives an array of 
handles, do you ultimately plan to return multiple symbols - one for each 
element of the array?

Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:212
+  }
+  if (const auto *HandleType = dyn_cast(T)) {
+if (HandleType->getDecl()->getName() != HandleTypeName)

Here `QT->getAs()` respectively.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:302-303
+   Pred);
+// Avoid further reports on this symbol.
+State = State->remove(HandleSym);
+  } else

Why not generate a fatal error instead?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:336-338
+ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) const {

Ok, so you're basically saying that whenever the code under analysis does 
anything to check the return value, this callback will fire immediately? Thus 
guaranteeing that whenever the Schrödinger state is accessed, it's either 
already collapsed or indeed not checked yet?

This sounds right but i wish i had your confidence :D

My usual approach to Schrödinger states will be to do the same thing but in 
`checkDeadSymbols()`. I.e., when the return value symbol dies, see if it's 
constrained and collapse the state accordingly. If the access occurs earlier 
than symbol death, update the state according to the current constraints and 
forget about the symbol. If current constraints are insufficient, 
conservatively mark it as a successful release (as that's how non-paranoid code 
under analysis would behave).

Regardless of the approach, you still need to update the state upon access as 
if the release has succeeded (when the current state is a Schrödinger state). 
But with your approach you can avoid checking the constraints upon access, and 
instead blindly assume that the symbol is underconstrained.

I like this! Can we prove that it works by adding an 

Re: [PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Akira Hatanaka via cfe-commits
Can you check the `#0` at the end of the globals and other strings that precede 
that? If you do so, we can also check that `no_dead_strip` isn’t added.

> On Nov 19, 2019, at 6:05 PM, Duncan P. N. Exon Smith via Phabricator via 
> cfe-commits  wrote:
> 
> dexonsmith added a comment.
> 
> For some reason this revision is locked down and I'm not allowed to "edit" 
> it, which includes adding inline review comments.  Can you add me as a 
> reviewer?
> 
> The two comments:
> 
> - Please add a period at the end of the sentence in the comment.
> - Can you give more context about what `objc_arc_inert` is doing, and why 
> it's necessary now that `no_dead_strip` is gone?
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D70284/new/
> 
> https://reviews.llvm.org/D70284
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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


[clang] f37356d - [clang][IFS][test] Removing driver-test.c. Test is still too brittle.

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:42:17-05:00
New Revision: f37356d6f60ae5db978611621d3a375ed87ec0f0

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

LOG: [clang][IFS][test] Removing driver-test.c. Test is still too brittle.

Removing this test because if I add a triple then there are link falures
on targets like ppc and s390x. If I don't add a triple then on PS4
targets the clang driver tries to invoke orbis-ld which ends up being
not found.

Added: 


Modified: 


Removed: 
clang/test/InterfaceStubs/driver-test.c



diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
deleted file mode 100644
index 425d87271f33..
--- a/clang/test/InterfaceStubs/driver-test.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// REQUIRES: x86-registered-target
-// REQUIRES: !powerpc-registered-target
-// REQUIRES: !system-darwin && !system-windows
-
-// RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \
-// RUN:   -emit-interface-stubs -emit-merged-ifs %s %S/object.c %S/weak.cpp
-// RUN: cat %t1.ifs | FileCheck %s
-
-// CHECK-DAG: data
-// CHECK-DAG: foo
-// CHECK-DAG: strongFunc
-// CHECK-DAG: weakFunc
-
-int foo(int bar) { return 42 + 1844; }
-int main() { return foo(23); }



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


[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:279
+if (PredNode->getState() == ErrorNode->getState()) {
+  IsFalsePositiveFound = true;
+  PR->markInvalid(nullptr, nullptr);

Why is this a false positive?

You're bringing in a completely brand-new machinery here, could you explain how 
it works and why do you need it?


Repository:
  rC Clang

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

https://reviews.llvm.org/D70411



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


[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

2019-11-19 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added subscribers: cfe-commits, apazos, sameer.abuasal, pzheng, 
s.egerton, lenary, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, mgorny.
Herald added a project: clang.

We don't have a full sysroot yet, so for now we only include compiler
support and compiler-rt builtins, the rest of the runtimes will get
enabled later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70477

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Basic/Targets.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/riscv64-fuchsia/libclang_rt.builtins.a
  clang/test/Driver/fuchsia.c
  clang/test/Driver/fuchsia.cpp

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -1,9 +1,22 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes --target=x86_64-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 | FileCheck %s
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes --target=aarch64-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes --target=riscv64-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
-// CHECK: "-triple" "x86_64-fuchsia"
+// CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
+// CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
+// CHECK-RISCV64: "-triple" "riscv64-unknown-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
@@ -23,7 +36,9 @@
 // CHECK: "-lc++"
 // CHECK: "-lm"
 // CHECK: "--pop-state"
-// CHECK: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-X86_64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-RISCV64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}riscv64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -6,7 +6,14 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: %clang %s -### -no-canonical-prefixes --target=riscv64-fuchsia \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
+// CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
+// CHECK-RISCV64: "-triple" "riscv64-unknown-fuchsia"
 // CHECK: "--mrelax-relocations"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
@@ -29,6 +36,7 @@
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
 // CHECK-X86_64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-RISCV64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}riscv64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -224,7 +224,7 @@
 std::string Fuchsia::ComputeEffectiveClangTriple(const ArgList ,
  types::ID InputType) const {
   llvm::Triple Triple(ComputeLLVMTriple(Args, InputType));
-  return (Triple.getArchName() + "-" + Triple.getOSName()).str();
+  

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

mibintc wrote:
> michele.scandale wrote:
> > With this change if I run `clang -### -ffast-math test.c` I don't see 
> > `-fno-trapping-math` passed to the CC1. 
> > This is changing the changes the value of the function level attribute 
> > "no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
> > Is this an intended change?
> > 
> > Moreover since with this patch the default value for trapping math changed, 
> > the "no-trapping-math" function level attribute is incorrect also for 
> > default case.
> Before this patch, ftrapping-math was added to the Driver and also a 
> bitfield, ``NoTrappingFPMath`` was created in the LLVM to describe the state 
> of trapping-math, but otherwise that bit wasn't consulted and the option had 
> no effect.  Gcc describes ftrapping-math as the default, but in llvm by 
> default floating point exceptions are masked and this corresponds to the 
> floating point Constrained Intrinsics having exception behavior set to 
> ignored.  This patch changed the llvm constructor to set the trapping bit to 
> "no trap".  In fact I'd like to get rid of the ``NoTrappingFPMath`` bitfield 
> since it's not being used, but I didn't make that change at this point. 
> 
> If I remember correctly, there are a bunch of driver tests that failed if 
> fno-trapping-math is output to cc1. I'd have to reconstruct the details.  
> Since fno-trapping-math is the default, it isn't passed through on the cc1 
> command line: the Clang.cpp driver doesn't pass through the positive and 
> negative for each existing option.
> 
> Thanks for pointing out the line in CGCall.cpp, it seems the CodeGenOpts 
> aren't getting set up perfectly I'll fix that in CompilerInvocation.cpp; I 
> don't see anything setting trapping-math as part of function level attribute, 
> @michele.scandale  did I overlook that/can you point out where that is?
I guess you are referring to the code in `TargetMachine.cpp` where the function 
level attributes are used to reset the `TargetOptions` state whenever we 
initiate the backend codegen for a given function. Considering that the 
trapping math option as stated in the documentation did not have any effect, 
I'm not surprised to see not many uses. The only one I can see is in 
`llvm/lib/Target/ARM/ARMAsmPrinter.cpp : 687` where the function level 
attribute affects the emission of some ARM specific attributes.

My only concern was that the change of the default value for trapping math was 
not propagated entirely causing this function level attribute to be initialized 
incorrectly.
Fixing the logic in `CompilerInvocation.cpp` considering the change of default 
it is fine for me.

Given that `ffp-exception-behavior={ignore,maytrap,strict}` supersedes 
`-f{,no-}trapping-math` I would expect long term to see the internal state of 
the compiler frontend to only care about the new state `FPExceptionBehavior` 
for both language and code generation options. And I guess the same would apply 
to the backend stage as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Ben D. Jones via Phabricator via cfe-commits
bendjones added a comment.

Any additional thoughts @dexonsmith @erik.pilkington @ahatanak?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


[clang] 27b229d - Revert "[clang][IFS][test] Removing driver-test.c. Test is still too brittle."

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:59:10-05:00
New Revision: 27b229dc17b2ea1d06fe566df8631bb2fff7b1c8

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

LOG: Revert "[clang][IFS][test] Removing driver-test.c. Test is still too 
brittle."

This reverts commit f37356d6f60ae5db978611621d3a375ed87ec0f0.

Added: 
clang/test/InterfaceStubs/driver-test.c

Modified: 


Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
new file mode 100644
index ..425d87271f33
--- /dev/null
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -0,0 +1,15 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: !powerpc-registered-target
+// REQUIRES: !system-darwin && !system-windows
+
+// RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \
+// RUN:   -emit-interface-stubs -emit-merged-ifs %s %S/object.c %S/weak.cpp
+// RUN: cat %t1.ifs | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }



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


[clang] 6c6d348 - Revert "[clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot."

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:59:22-05:00
New Revision: 6c6d34883a3003693cbf0ab3edf19eb999c1a62d

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

LOG: Revert "[clang][IFS] Fixing unsupported emulation mode on 
clang-ppc64be-linux bot."

This reverts commit 1b387484b9b38a4a1e98a9d22a9a26065b0d184e.

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index 425d87271f33..62d522fb7b1f 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: !powerpc-registered-target
 // REQUIRES: !system-darwin && !system-windows
 
 // RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \



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


[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  auto  = DiffedLines.back();
+  for (auto Iter = AddedLine.Tokens.begin();
+   Iter != AddedLine.Tokens.end();) {

hokein wrote:
> nridge wrote:
> > hokein wrote:
> > > it took me a while to understand this code,
> > > 
> > > If the NewLine is `IsInactive`, it just contains a single token whose 
> > > range is [0, 0), can't we just?
> > > 
> > > ```
> > > 
> > > if (NewLine.back().Tokens.empty()) continue;
> > > 
> > > bool InactiveLine = NewLine.back().Tokens.front().Kind == InactiveCode;
> > > assert(InactiveLine && NewLine.back().Tokens.size() == 1 && "IncativeCode 
> > > must have a single token");
> > > DiffedLines.back().IsInactive = true;
> > > ```
> > An inactive line can contain token highlightings as well. For example, we 
> > highlight macro references in the condition of an `#ifdef`, and that line 
> > is also inactive if the condition is false. Clients can merge the line 
> > style (which is typically a background color) with the token styles 
> > (typically a foreground color).
> > 
> > I did expand the comment to explain what the loop is doing more clearly.
> thanks, I see. 
> 
> I think we can still simplify the code like below, this could improve the 
> code readability, and avoid the comment explaining it.
> 
> ```
> llvm::erase_if(AddedLine.Tokens, [](const Token& T) { return T.Kind == 
> InactiveCode;});
> ```
Done. Note that we still need to set `AddedLine.IsInactive` appropriately. I 
did that in the lambda, which feels like a strange thing for an `erase_if` 
predicate to do. The alternative would be doing a `count_if` first (but then 
we're looping over the tokens twice).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 230195.
nridge marked 4 inline comments as done.
nridge added a comment.

Address nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto  : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -493,11 +493,11 @@
 
   #define $Macro[[test]]
   #undef $Macro[[test]]
-  #ifdef $Macro[[test]]
-  #endif
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  #endif
 
-  #if defined($Macro[[test]])
-  #endif
+$InactiveCode[[]]  #if defined($Macro[[test]])
+$InactiveCode[[]]  #endif
 )cpp",
   R"cpp(
   struct $Class[[S]] {
@@ -598,6 +598,33 @@
 $Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
   *$Field[[t]];
   }
+)cpp",
+  // Inactive code highlighting
+  R"cpp(
+  // Code in the preamble.
+  // Inactive lines get an empty InactiveCode token at the beginning.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  #endif
+
+  // A declaration to cause the preamble to end.
+  int $Variable[[EndPreamble]];
+
+  // Code after the preamble.
+  // Code inside inactive blocks does not get regular highlightings
+  // because it's not part of the AST.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive2;
+$InactiveCode[[]]  #endif
+
+  #ifndef $Macro[[test]]
+  int $Variable[[Active1]];
+  #endif
+
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive3;
+$InactiveCode[[]]  #else
+  int $Variable[[Active2]];
+  #endif
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
@@ -665,10 +692,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:

[clang] 1b38748 - [clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot.

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T19:27:46-05:00
New Revision: 1b387484b9b38a4a1e98a9d22a9a26065b0d184e

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

LOG: [clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot.

I am in another pickle here where if I specify a triple, I get the wrong elf
target arch on the PPC bot (error from the PPC elf Linker). To avoid this I am
going to turn this test off on the PPC bots for now.

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index 62d522fb7b1f..425d87271f33 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,4 +1,5 @@
 // REQUIRES: x86-registered-target
+// REQUIRES: !powerpc-registered-target
 // REQUIRES: !system-darwin && !system-windows
 
 // RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \



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


[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: lib/Driver/ToolChains/AMDGPU.h:25
+/// TODO: Generalize to handle libclc.
+class RocmInstallationDetector {
+private:

arsenm wrote:
> yaxunl wrote:
> > I don't think we should detect ROCm installation here. We are compiling 
> > code for amdgpu not only on ROCm, but also on amdgpu-pro and windows. Many 
> > cases, people want to compile code for amdgpu on systems without ROCm 
> > installed.
> > 
> > Compiilng code for amdgpu does not really depend on ROCm. We only depend on 
> > device library.
> > 
> > It makes more sense to have a AMDGPUDeviceLibDetector which works on ROCm, 
> > amdgpu-pro, and windows. Also we need an option -amdgpu-device-lib-path to 
> > override the default amdgpu device lib path.
> Being able to cross compile without a rocm installation is one of my primary 
> goals here. I don't know what the install paths look like for those other 
> drivers. It would be best if everything could agree on a single default 
> install location for these to search, which is mostly a naming problem. In 
> the patch as-is, the rocm-path is serving as -amdgpu-device-lib-path, but 
> this is a naming question
Except we don't have an omni-purpose bitcode library set. It's the 
ROCm-Device-Libs. rocm is the only semi-coherently defined platform at the 
moment


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

https://reviews.llvm.org/D59321



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


[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 230197.
arsenm added a comment.

Rebase


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

https://reviews.llvm.org/D59321

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/test/CodeGenOpenCL/amdgpu-debug-info-pointer-address-space.cl
  clang/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  clang/test/Driver/Inputs/rocm-device-libs/lib/irif.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ockl.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_off.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_803.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_900.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ocml.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/opencl.amdgcn.bc
  clang/test/Driver/amdgpu-visibility.cl
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-device-libs.cl
  clang/test/Driver/rocm-not-found.cl

Index: clang/test/Driver/rocm-not-found.cl
===
--- /dev/null
+++ clang/test/Driver/rocm-not-found.cl
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+
+// Check that we raise an error if we're trying to compile OpenCL for amdhsa code but can't
+// find a ROCm install, unless -nodefaultlibs was passed.
+
+// RUN: %clang -### --sysroot=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// RUN: %clang -### --rocm-path=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// ERR: cannot find ROCm installation. Provide its path via --rocm-path, or pass -nodefaultlibs.
+
+// RUN: %clang -### -nodefaultlibs --rocm-path=%s/no-rocm-there %s 2>&1 | FileCheck %s --check-prefix OK
+// OK-NOT: cannot find ROCm installation.
Index: clang/test/Driver/rocm-device-libs.cl
===
--- /dev/null
+++ clang/test/Driver/rocm-device-libs.cl
@@ -0,0 +1,121 @@
+// REQUIRES: clang-driver
+// REQUIRES: amdgpu-registered-target
+
+// Test flush-denormals-to-zero enabled uses oclc_daz_opt_on
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900 %s
+
+
+
+// Make sure the different denormal default is respected for gfx8
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// Make sure the non-canonical name works
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=fiji \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX900 %s
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-finite-math-only \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-FINITE-ONLY,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa\
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-fp32-correctly-rounded-divide-sqrt \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck 

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

NoQ wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > The next obvious step here would be to type-check the declaration to make 
> > > sure that it's actually a handle (and emit a warning if it isn't).
> > Yeah, I do agree, but I think this depends on the role of the attribute. 
> > Adding a type check would make this more restrictive in a sense other users 
> > who want to write for example a Posix API checker and want to reuse this 
> > attribute might not be able to do so without touching the type-check code. 
> > Which may or may not be good. I do not have any strong feelings about 
> > either of the directions. 
> So you envision this working on completely arbitrary types? Fair!
> 
> Maybe check that when `AcquireHandleAttr` is on a parameter, it's actually an 
> out-parameter?
Hmm, I think this makes a lot of sense! So basically, I would exclude 
primitive, non-pointer types that are taken by value. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

In D70111#1751981 , @aprantl wrote:

> There are two options here:
>
> - leave the C bindings as is (fine with me)
> - add an overloaded function to the C bindings that has the extra argument 
> (also fine with me).


In my opinon, we should be doing both of these. Off-course Step 1 first and 
subsequently Step 2.
Otherwise consumers using / utilizing C bindings will again try to revert this, 
If we don't do Step 2.
As without an updated binding and backward compaitibilty -- this will break 
things for C-binding users ??


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:67
+// functions:
+//  1. __attribute__((clang::acquire_handle)) |handle will be acquired
+//  2. __attribute__((clang::release_handle)) |handle will be released

Whoops, the spelling of the attribute is wrong here, will fix in a followup 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70470



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


[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-19 Thread Zachary Turner via Phabricator via cfe-commits
zturner updated this revision to Diff 230162.
zturner added a comment.

Forgot to remove spurious `llvm::` namespace qualifications.


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

https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -8,75 +8,51 @@
 template 
 bind_rt bind(Fp &&, Arguments &&...);
 }
+
+template 
+T ref(T );
 }
 
-int add(int x, int y) { return x + y; }
+namespace boost {
+template 
+class bind_rt {};
 
-void f() {
-  auto clj = std::bind(add, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
-}
+template 
+bind_rt bind(const Fp &, Arguments...);
+} // namespace boost
 
-void g() {
-  int x = 2;
-  int y = 2;
-  auto clj = std::bind(add, x, y);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
-}
+namespace C {
+int add(int x, int y) { return x + y; }
+} // namespace C
 
-struct placeholder {};
-placeholder _1;
-placeholder _2;
+struct Foo {
+  static int add(int x, int y) { return x + y; }
+};
 
-void h() {
-  int x = 2;
-  auto clj = std::bind(add, x, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
-}
+struct D {
+  D() = default;
+  void operator()(int x, int y) const {}
 
-struct A;
-struct B;
-bool ABTest(const A &, const B &);
+  void MemberFunction(int x) {}
 
-void i() {
-  auto BATest = std::bind(ABTest, _2, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
-}
+  static D *create();
+};
 
-void j() {
-  auto clj = std::bind(add, 2, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for argument mismatches.
-  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
-}
+struct F {
+  F(int x) {}
+  ~F() {}
 
-void k() {
-  auto clj = std::bind(add, _1, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for reused placeholders.
-  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
-}
+  int get() { return 42; }
+};
 
-void m() {
-  auto clj = std::bind(add, 1, add(2, 5));
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for nested calls.
-  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
-}
+void UseF(F);
 
-namespace C {
-  int add(int x, int y){ return x + y; }
-}
+struct placeholder {};
+placeholder _1;
+placeholder _2;
 
-void n() {
-  auto clj = std::bind(C::add, 1, 1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
-}
+int add(int x, int y) { return x + y; }
+int addThree(int x, int y, int z) { return x + y + z; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -114,10 +90,188 @@
   void Reset(std::function);
 };
 
-void test(Thing *t) {
+int GlobalVariable = 42;
+
+struct TestCaptureByValueStruct {
+  int MemberVariable;
+  static int StaticMemberVariable;
+  F MemberStruct;
+
+  void testCaptureByValue(int Param, F f) {
+int x = 3;
+int y = 4;
+auto AAA = std::bind(add, x, y);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto AAA = [x, y] { return add(x, y); };
+
+// When the captured variable is repeated, it should only appear in the capture list once.
+auto BBB = std::bind(add, x, x);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto BBB = [x] { return add(x, x); };
+
+int LocalVariable;
+// Global variables shouldn't be captured at all, and members should be captured through this.
+auto CCC = std::bind(add, MemberVariable, GlobalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto CCC = [this] { return add(MemberVariable, GlobalVariable); };
+
+// Static member variables shouldn't be captured, but locals should
+auto DDD = std::bind(add, TestCaptureByValueStruct::StaticMemberVariable, LocalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// 

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 13 inline comments as done.
xazax.hun added a comment.

Thanks for the review!

I am not very well versed in Fuchsia's syscalls yet but my understanding is 
that not all of the syscalls can fail so we do not need all the users to check 
for errors. But this is something I will verify with the rest of the team, so 
please treat my answer with a grain of salt for now. An alternative would be to 
introduce an annotation to tell which APIs will never fail, but I am afraid 
that is also sometimes subjective. For example it is pretty much possible to 
have no available port in a system but is very unlikely to happen so some non 
mission critical applications might not check for errors there. But this is 
also something that I will follow up with the team.




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:99-100
+
+static const StringRef HandleTypeName = "zx_handle_t";
+static const StringRef ErrorTypeName = "zx_status_t";
+

NoQ wrote:
> So you plan to unhardcode these as well eventually?
I have no specific plans at this point. I do not see these types changing 
frequently, so I have no urge to unhardcode them. Are there any downsides I am 
not being aware of?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:123
+
+  // Only use this in checkDeadSymbols!
+  bool hasError(ProgramStateRef State) const {

NoQ wrote:
> For now it doesn't seem to be used at all.
Wow, indeed! This is from an eariler iteration :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:141-146
+#define CASE(ID)   
\
+  case ID: 
\
+OS << #ID; 
\
+break;
+  CASE(Kind::Allocated)
+  CASE(Kind::Released)

NoQ wrote:
> This macro has literally saved exactly as many lines as it has caused :)
Good point! Initially I had a separate escaped state, but now I guess I should 
just expand them :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:156-168
+  std::unique_ptr LeakBugType;
+  std::unique_ptr DoubleReleaseBugType;
+  std::unique_ptr UseAfterFreeBugType;
+
+public:
+  FuchsiaHandleChecker()
+  : LeakBugType(new BugType(this, "Fuchsia handle leak",

NoQ wrote:
> We've recently dumbed down this idiom to
> ```lang=c++
> class FuchsiaHandleChecker : ... {
>   LeakBugType{this, "Fuchsia handle leak",
>   "Fuchsia Handle Error", /*SuppressOnSink=*/true};
>   ...
> };
> ```
> :)
+1 :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

NoQ wrote:
> This is never necessary, just call all the methods directly on `QT` - it has 
> an overloaded operator `->()` for this.
`getPointeeOrArrayElementType` returns a `Type *`, so I cannot really continue 
to use `QualType` and I am not interested in the qualifiers at all.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

NoQ wrote:
> Ok, so what's the deal with arrays here? If the function receives an array of 
> handles, do you ultimately plan to return multiple symbols - one for each 
> element of the array?
> 
> Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?
I do not have a real plan with arrays just yet. Creating eagerly a symbol for 
all the elements might look a bit wasteful but also users are probably not 
expected to have large arrays of handles? Probably I should remove that code 
for now.

I would expect non-Fuchsia checkers to introduce their own `getHandleSymbol` 
logic. Does that make sense? Maybe I should rename this to 
`getFuchsiaHandleSymbol`. 



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:302-303
+   Pred);
+// Avoid further reports on this symbol.
+State = State->remove(HandleSym);
+  } else

NoQ wrote:
> Why not generate a fatal error instead?
The motivation was to not to reduce coverage, but I have no strong feelings 
about making this a sink. But in case of leaks I certainly prefer non-fatal 
errors.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:336-338
+ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) 

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

For some reason this revision is locked down and I'm not allowed to "edit" it, 
which includes adding inline review comments.  Can you add me as a reviewer?

The two comments:

- Please add a period at the end of the sentence in the comment.
- Can you give more context about what `objc_arc_inert` is doing, and why it's 
necessary now that `no_dead_strip` is gone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 230187.
xazax.hun added a comment.

- Addressed **some** of the review comments. More changes are planned :)


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

https://reviews.llvm.org/D70470

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.c

Index: clang/test/Analysis/fuchsia_handle.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.c
@@ -0,0 +1,114 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  if (tag) {
+escape1();
+escape2(sb);
+  }
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  use1();
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  use1(); // expected-warning {{Using a previously released handle}}
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -0,0 +1,436 @@
+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- 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
+//
+//===--===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before execution
+//ends.
+//   - If a handle is released, it should not be released again.
+//   - If a handle is released, it should not be used for other purposes
+//such as I/O.
+//
+// In this checker, each tracked handle is associated with a state. When the
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//
+//+---+
+//|   |
+//|release_func failed|
+//|   +-+ |
+//|   | |  

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

2019-11-19 Thread Tim Wojtulewicz via Phabricator via cfe-commits
timwoj added a comment.

Thanks for the consideration on committer access, but I'm going to have to pass 
for the time being.

That said, can someone land this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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


LLVM buildmaster will be updated and restarted tonight

2019-11-19 Thread Galina Kistanova via cfe-commits
 Hello everyone,

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

Thanks

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


[PATCH] D70446: [clangd] Treat UserDefinedLiteral as a leaf in SelectionTree, sidestepping tokenization issues

2019-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70446



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


[clang] 69242e9 - clang/Modules: Sink ASTReadResult in ReadControlBlock, NFC

2019-11-19 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2019-11-19T16:10:44-08:00
New Revision: 69242e986823e3fdd11a8e51f47f36bec714363c

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

LOG: clang/Modules: Sink ASTReadResult in ReadControlBlock, NFC

Simplify the code by avoiding some state that wasn't being used.  The
function-level `Result` was only assigned a value other than `Success`
in the handler for `OPTIONS_BLOCK_ID`, but in that case it also hits an
early return.  Remove it at the function-level to make it obvious that
the normal case always returns `Success`.

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 9111b60a7179..36c2952346a7 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2561,7 +2561,6 @@ ASTReader::ReadControlBlock(ModuleFile ,
 const ModuleFile *ImportedBy,
 unsigned ClientLoadCapabilities) {
   BitstreamCursor  = F.Stream;
-  ASTReadResult Result = Success;
 
   if (llvm::Error Err = Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
 Error(std::move(Err));
@@ -2652,7 +2651,7 @@ ASTReader::ReadControlBlock(ModuleFile ,
 }
   }
 
-  return Result;
+  return Success;
 }
 
 case llvm::BitstreamEntry::SubBlock:
@@ -2682,9 +2681,10 @@ ASTReader::ReadControlBlock(ModuleFile ,
   bool AllowCompatibleConfigurationMismatch =
   F.Kind == MK_ExplicitModule || F.Kind == MK_PrebuiltModule;
 
-  Result = ReadOptionsBlock(Stream, ClientLoadCapabilities,
-AllowCompatibleConfigurationMismatch,
-*Listener, SuggestedPredefines);
+  ASTReadResult Result =
+  ReadOptionsBlock(Stream, ClientLoadCapabilities,
+   AllowCompatibleConfigurationMismatch, *Listener,
+   SuggestedPredefines);
   if (Result == Failure) {
 Error("malformed block record in AST file");
 return Result;



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

xazax.hun wrote:
> NoQ wrote:
> > This is never necessary, just call all the methods directly on `QT` - it 
> > has an overloaded operator `->()` for this.
> `getPointeeOrArrayElementType` returns a `Type *`, so I cannot really 
> continue to use `QualType` and I am not interested in the qualifiers at all.
Whoops ^.^"



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

xazax.hun wrote:
> NoQ wrote:
> > Ok, so what's the deal with arrays here? If the function receives an array 
> > of handles, do you ultimately plan to return multiple symbols - one for 
> > each element of the array?
> > 
> > Also, in the generic non-Fuchsia case, what if the handle itself is a 
> > pointer?
> I do not have a real plan with arrays just yet. Creating eagerly a symbol for 
> all the elements might look a bit wasteful but also users are probably not 
> expected to have large arrays of handles? Probably I should remove that code 
> for now.
> 
> I would expect non-Fuchsia checkers to introduce their own `getHandleSymbol` 
> logic. Does that make sense? Maybe I should rename this to 
> `getFuchsiaHandleSymbol`. 
But, i mean, why do you even consider arrays in this code? Why not simply `QT = 
QT->getPointeeType()`?


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

https://reviews.llvm.org/D70470



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


[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Ben D. Jones via Phabricator via cfe-commits
bendjones added a comment.

In D70284#1752811 , @dexonsmith wrote:

> For some reason this revision is locked down and I'm not allowed to "edit" 
> it, which includes adding inline review comments.  Can you add me as a 
> reviewer?


Thought I did.

> The two comments:
> 
> - Please add a period at the end of the sentence in the comment.

Will do.

> - Can you give more context about what `objc_arc_inert` is doing, and why 
> it's necessary now that `no_dead_strip` is gone?

The objc_arc_inert was added to other similar things so in the spirt of making 
things match I added it here. I can keep it simple though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

Some how, this{eric response} doesn't got it's way to phabricator, anyways FYI

> The C bindings, in general, don't have stability guarantees outside of a few 
> cases.



> Whitequark owns all of this now though :)



> -eric


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

xazax.hun wrote:
> NoQ wrote:
> > The next obvious step here would be to type-check the declaration to make 
> > sure that it's actually a handle (and emit a warning if it isn't).
> Yeah, I do agree, but I think this depends on the role of the attribute. 
> Adding a type check would make this more restrictive in a sense other users 
> who want to write for example a Posix API checker and want to reuse this 
> attribute might not be able to do so without touching the type-check code. 
> Which may or may not be good. I do not have any strong feelings about either 
> of the directions. 
So you envision this working on completely arbitrary types? Fair!

Maybe check that when `AcquireHandleAttr` is on a parameter, it's actually an 
out-parameter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

The next obvious step here would be to type-check the declaration to make sure 
that it's actually a handle (and emit a warning if it isn't).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

NoQ wrote:
> The next obvious step here would be to type-check the declaration to make 
> sure that it's actually a handle (and emit a warning if it isn't).
Yeah, I do agree, but I think this depends on the role of the attribute. Adding 
a type check would make this more restrictive in a sense other users who want 
to write for example a Posix API checker and want to reuse this attribute might 
not be able to do so without touching the type-check code. Which may or may not 
be good. I do not have any strong feelings about either of the directions. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[clang] 8700831 - clang/Modules: Early return in CompilerInstance::createModuleManager, NFC

2019-11-19 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2019-11-19T18:16:23-08:00
New Revision: 8700831734811cb89eafb72b75206f21e9f047e9

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

LOG: clang/Modules: Early return in CompilerInstance::createModuleManager, NFC

Reduce nesting with an early `return`.

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index a0663217453a..c9c66f011193 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1472,51 +1472,52 @@ static void pruneModuleCache(const HeaderSearchOptions 
) {
 }
 
 void CompilerInstance::createModuleManager() {
-  if (!ModuleManager) {
-if (!hasASTContext())
-  createASTContext();
-
-// If we're implicitly building modules but not currently recursively
-// building a module, check whether we need to prune the module cache.
-if (getSourceManager().getModuleBuildStack().empty() &&
-!getPreprocessor().getHeaderSearchInfo().getModuleCachePath().empty() 
&&
-getHeaderSearchOpts().ModuleCachePruneInterval > 0 &&
-getHeaderSearchOpts().ModuleCachePruneAfter > 0) {
-  pruneModuleCache(getHeaderSearchOpts());
-}
+  if (ModuleManager)
+return;
 
-HeaderSearchOptions  = getHeaderSearchOpts();
-std::string Sysroot = HSOpts.Sysroot;
-const PreprocessorOptions  = getPreprocessorOpts();
-std::unique_ptr ReadTimer;
-if (FrontendTimerGroup)
-  ReadTimer = std::make_unique("reading_modules",
- "Reading modules",
- *FrontendTimerGroup);
-ModuleManager = new ASTReader(
-getPreprocessor(), getModuleCache(), (),
-getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions,
-Sysroot.empty() ? "" : Sysroot.c_str(), PPOpts.DisablePCHValidation,
-/*AllowASTWithCompilerErrors=*/false,
-/*AllowConfigurationMismatch=*/false,
-HSOpts.ModulesValidateSystemHeaders,
-HSOpts.ValidateASTInputFilesContent,
-getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
-if (hasASTConsumer()) {
-  ModuleManager->setDeserializationListener(
-getASTConsumer().GetASTDeserializationListener());
-  getASTContext().setASTMutationListener(
-getASTConsumer().GetASTMutationListener());
-}
-getASTContext().setExternalSource(ModuleManager);
-if (hasSema())
-  ModuleManager->InitializeSema(getSema());
-if (hasASTConsumer())
-  ModuleManager->StartTranslationUnit(());
-
-for (auto  : DependencyCollectors)
-  Listener->attachToASTReader(*ModuleManager);
+  if (!hasASTContext())
+createASTContext();
+
+  // If we're implicitly building modules but not currently recursively
+  // building a module, check whether we need to prune the module cache.
+  if (getSourceManager().getModuleBuildStack().empty() &&
+  !getPreprocessor().getHeaderSearchInfo().getModuleCachePath().empty() &&
+  getHeaderSearchOpts().ModuleCachePruneInterval > 0 &&
+  getHeaderSearchOpts().ModuleCachePruneAfter > 0) {
+pruneModuleCache(getHeaderSearchOpts());
   }
+
+  HeaderSearchOptions  = getHeaderSearchOpts();
+  std::string Sysroot = HSOpts.Sysroot;
+  const PreprocessorOptions  = getPreprocessorOpts();
+  std::unique_ptr ReadTimer;
+  if (FrontendTimerGroup)
+ReadTimer = std::make_unique("reading_modules",
+"Reading modules",
+*FrontendTimerGroup);
+  ModuleManager = new ASTReader(
+  getPreprocessor(), getModuleCache(), (),
+  getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions,
+  Sysroot.empty() ? "" : Sysroot.c_str(), PPOpts.DisablePCHValidation,
+  /*AllowASTWithCompilerErrors=*/false,
+  /*AllowConfigurationMismatch=*/false,
+  HSOpts.ModulesValidateSystemHeaders,
+  HSOpts.ValidateASTInputFilesContent,
+  getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
+  if (hasASTConsumer()) {
+ModuleManager->setDeserializationListener(
+  getASTConsumer().GetASTDeserializationListener());
+getASTContext().setASTMutationListener(
+  getASTConsumer().GetASTMutationListener());
+  }
+  getASTContext().setExternalSource(ModuleManager);
+  if (hasSema())
+ModuleManager->InitializeSema(getSema());
+  if (hasASTConsumer())
+ModuleManager->StartTranslationUnit(());
+
+  for (auto  : DependencyCollectors)
+Listener->attachToASTReader(*ModuleManager);
 }
 
 bool CompilerInstance::loadModuleFile(StringRef FileName) {




[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2337
+<< "-ffp-contract=fast";
+optID = options::OPT_ffast_math;
+FPModel = Val;

Here it seems you are changing `optID` to `OPT_ffast_math` to reuse the logic 
specified below for that case to reset the state of the floating point options.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2345
+<< "-ffp-contract=fast";
+optID = options::OPT_ffp_contract;
+FPModel = Val;

Here the state of the floating point options seems unchanged except for 
`FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
expect the state of the floating point options to match the one of 
`-fno-fast-math` except for `FPContract` which you want to be set to "fast".

I think you might need to replicate the reset for all the option here as well, 
so at this point I don't know how much worth is to use the optID reset trick 
for the "fast" case only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

2019-11-19 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal ) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

NoQ wrote:
> Maybe move this function to `Iterator.cpp` as well, and then move the 
> definitions for iterator maps from `Iterator.h` to `Iterator.cpp`, which will 
> allow you to use the usual `REGISTER_MAP_WITH_PROGRAMSTATE` macros, and 
> additionally guarantee that all access to the maps goes through the accessor 
> methods that you provide?
Hmm, I was trying hard to use these macros but failed so I reverted to the 
manual solution. I will retry now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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


[clang] b6d7bbf - [RISCV] Support mutilib in baremetal environment

2019-11-19 Thread Zakk Chen via cfe-commits

Author: Zakk Chen
Date: 2019-11-19T02:10:39-08:00
New Revision: b6d7bbfa004310777cd41448ffc377aea082fc8c

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

LOG: [RISCV] Support mutilib in baremetal environment

Currently only support the set of multilibs same to riscv-gnu-toolchain.

Reviewers: espindola, asb, kito-cheng, lenary

Reviewed By: lenary

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

Added: 

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/bin/ld

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imafdc/lp64d/crt0.o

Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 2dd3450dd1ba..eb84a99a16b7 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1503,9 +1503,65 @@ static bool findMSP430Multilibs(const Driver ,
   return false;
 }
 
+static void findRISCVBareMetalMultilibs(const Driver ,
+const llvm::Triple ,
+StringRef Path, const ArgList ,
+DetectedMultilibs ) {
+  FilterNonExistent NonExistent(Path, "/crtbegin.o", D.getVFS());
+  struct RiscvMultilib {
+StringRef march;
+StringRef mabi;
+  };
+  // currently only support the set of multilibs like riscv-gnu-toolchain does.
+  // TODO: support MULTILIB_REUSE
+  SmallVector RISCVMultilibSet = {
+  {"rv32i", "ilp32"}, {"rv32im", "ilp32"}, {"rv32iac", "ilp32"},
+  {"rv32imac", "ilp32"},  {"rv32imafc", "ilp32f"}, {"rv64imac", "lp64"},
+  {"rv64imafdc", "lp64d"}};
+
+  std::vector Ms;
+  for (auto Element : RISCVMultilibSet) {
+// multilib path rule is ${march}/${mabi}
+Ms.emplace_back(
+makeMultilib((Twine(Element.march) + "/" + Twine(Element.mabi)).str())
+.flag(Twine("+march=", Element.march).str())
+.flag(Twine("+mabi=", Element.mabi).str()));
+  }
+  MultilibSet RISCVMultilibs =
+  MultilibSet()
+  .Either(ArrayRef(Ms))
+  .FilterOut(NonExistent)
+  .setFilePathsCallback([](const 

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

2019-11-19 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6d7bbfa0043: [RISCV] Support mutilib in baremetal 
environment (authored by khchen, committed by Zakk Chen 
zakk.c...@sifive.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67508

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/bin/ld
  clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imafdc/lp64d/crt0.o
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -14,8 +14,8 @@
 // C-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
 // C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
 // C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
@@ -29,8 +29,8 @@
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib{{/|}}crt0.o"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf{{/|}}lib"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: 

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-19 Thread Ehud Katz via Phabricator via cfe-commits
ekatz added a comment.

Ping


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

https://reviews.llvm.org/D69770



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


[PATCH] D70183: Detect source location overflow due includes

2019-11-19 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio marked an inline comment as done.
dnsampaio added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70183



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D70165#1751402 , @mitchell-stellar 
wrote:

> Oops, it looks like I mixed up this ticket with 
> https://reviews.llvm.org/D69238. Sorry about that. Would you like me to 
> revert both commits and then re-commit with the correct links, etc.?


No strong opinion, might be good to do (and reopen the differentials so they 
are closed with proper diffs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar reopened this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Reopening in order to correct the accidentally swapped commits between this 
ticket and https://reviews.llvm.org/D69238.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar reopened this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Reopening in order to correct the accidentally swapped commits between this 
ticket and https://reviews.llvm.org/D70165.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69238



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


[clang-tools-extra] 41ee54e - Revert "[clang-tidy] Fix readability-redundant-string-init for c++17/c++2a"

2019-11-19 Thread Mitchell Balan via cfe-commits

Author: Mitchell Balan
Date: 2019-11-19T07:52:31-05:00
New Revision: 41ee54e5d18858c56725485ef637ad5a686368f4

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

LOG: Revert "[clang-tidy] Fix readability-redundant-string-init for c++17/c++2a"

This reverts commit 06f3dabe4a2e85a32ade27c0769b6084c828a206.

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index 8ee77ccd16ff..2f15213dca8c 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -20,13 +20,11 @@ namespace modernize {
 UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreDestructors(Options.get("IgnoreDestructors", false)),
-  AllowOverrideAndFinal(Options.get("AllowOverrideAndFinal", false)),
   OverrideSpelling(Options.get("OverrideSpelling", "override")),
   FinalSpelling(Options.get("FinalSpelling", "final")) {}
 
 void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
-  Options.store(Opts, "AllowOverrideAndFinal", AllowOverrideAndFinal);
   Options.store(Opts, "OverrideSpelling", OverrideSpelling);
   Options.store(Opts, "FinalSpelling", FinalSpelling);
 }
@@ -105,8 +103,7 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult 
) {
   bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal;
   unsigned KeywordCount = HasVirtual + HasOverride + HasFinal;
 
-  if ((!OnlyVirtualSpecified && KeywordCount == 1) ||
-  (!HasVirtual && HasOverride && HasFinal && AllowOverrideAndFinal))
+  if (!OnlyVirtualSpecified && KeywordCount == 1)
 return; // Nothing to do.
 
   std::string Message;
@@ -116,9 +113,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult 
) {
 Message = "annotate this function with '%0' or (rarely) '%1'";
   } else {
 StringRef Redundant =
-HasVirtual ? (HasOverride && HasFinal && !AllowOverrideAndFinal
-  ? "'virtual' and '%0' are"
-  : "'virtual' is")
+HasVirtual ? (HasOverride && HasFinal ? "'virtual' and '%0' are"
+  : "'virtual' is")
: "'%0' is";
 StringRef Correct = HasFinal ? "'%1'" : "'%0'";
 
@@ -215,7 +211,7 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult 
) {
 Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);
   }
 
-  if (HasFinal && HasOverride && !AllowOverrideAndFinal) {
+  if (HasFinal && HasOverride) {
 SourceLocation OverrideLoc = 
Method->getAttr()->getLocation();
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));

diff  --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h 
b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
index 082778f2957c..ed163956ecdb 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -26,7 +26,6 @@ class UseOverrideCheck : public ClangTidyCheck {
 
 private:
   const bool IgnoreDestructors;
-  const bool AllowOverrideAndFinal;
   const std::string OverrideSpelling;
   const std::string FinalSpelling;
 };

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index b9feee29d137..4e773d62bf6b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,12 +170,6 @@ Improvements to clang-tidy
   Finds non-static member functions that can be made ``const``
   because the functions don't use ``this`` in a non-const way.
 
-- Improved :doc:`modernize-use-override
-  ` check.
-
-  The check now supports the ``AllowOverrideAndFinal`` option to eliminate
-  conflicts with ``gcc -Wsuggest-override`` or ``gcc 
-Werror=suggest-override``.
-
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst 
b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
index d20c1d88520e..4273c6e57708 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
+++ 

[clang] 3de7cc9 - Revert "[RISCV] Support mutilib in baremetal environment"

2019-11-19 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2019-11-19T08:16:55-05:00
New Revision: 3de7cc9fc01c8eb19eb344691c4747def1e00311

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

LOG: Revert "[RISCV] Support mutilib in baremetal environment"

This reverts commit b6d7bbfa004310777cd41448ffc377aea082fc8c.
Driver/riscv64-toolchain.c fails on Windows.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/bin/ld

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o

clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imafdc/lp64d/crt0.o



diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index eb84a99a16b7..2dd3450dd1ba 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1503,65 +1503,9 @@ static bool findMSP430Multilibs(const Driver ,
   return false;
 }
 
-static void findRISCVBareMetalMultilibs(const Driver ,
-const llvm::Triple ,
-StringRef Path, const ArgList ,
-DetectedMultilibs ) {
-  FilterNonExistent NonExistent(Path, "/crtbegin.o", D.getVFS());
-  struct RiscvMultilib {
-StringRef march;
-StringRef mabi;
-  };
-  // currently only support the set of multilibs like riscv-gnu-toolchain does.
-  // TODO: support MULTILIB_REUSE
-  SmallVector RISCVMultilibSet = {
-  {"rv32i", "ilp32"}, {"rv32im", "ilp32"}, {"rv32iac", "ilp32"},
-  {"rv32imac", "ilp32"},  {"rv32imafc", "ilp32f"}, {"rv64imac", "lp64"},
-  {"rv64imafdc", "lp64d"}};
-
-  std::vector Ms;
-  for (auto Element : RISCVMultilibSet) {
-// multilib path rule is ${march}/${mabi}
-Ms.emplace_back(
-makeMultilib((Twine(Element.march) + "/" + Twine(Element.mabi)).str())
-.flag(Twine("+march=", Element.march).str())
-.flag(Twine("+mabi=", Element.mabi).str()));
-  }
-  MultilibSet RISCVMultilibs =
-  MultilibSet()
-  .Either(ArrayRef(Ms))
-  .FilterOut(NonExistent)
-  .setFilePathsCallback([](const Multilib ) {
-return std::vector(
-{M.gccSuffix(),
-

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-19 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 230031.
lh123 edited the summary of this revision.
lh123 added a comment.

Address comment


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

https://reviews.llvm.org/D70222

Files:
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp

Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace tooling {
@@ -859,5 +860,98 @@
 "clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
 }
 
+TEST(ExpandResponseFileTest, JSONCompilationDatabase) {
+  SmallString<128> TestDir;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory("unittest", TestDir);
+  EXPECT_TRUE(!EC);
+
+  SmallString<128> TestMainFileName;
+  llvm::sys::path::append(TestMainFileName, TestDir, "main.cpp");
+
+  SmallString<128> FooFileName;
+  llvm::sys::path::append(FooFileName, TestDir, "foo.rsp");
+  std::ofstream FooFile(FooFileName.c_str());
+  FooFile << "-DFOO @bar.rsp";
+  FooFile.close();
+
+  SmallString<128> BarFileName;
+  llvm::sys::path::append(BarFileName, TestDir, "bar.rsp");
+  std::ofstream BarFile(BarFileName.c_str());
+  BarFile << "-DBAR";
+  BarFile.close();
+
+  SmallString<128> CompileCommandsFileName;
+  llvm::sys::path::append(CompileCommandsFileName, TestDir,
+  "compile_commands.json");
+  std::ofstream CompileCommandsFile(CompileCommandsFileName.c_str());
+  // clang-format off
+  CompileCommandsFile << ("[{\"directory\": \"" + TestDir + "\","
+  "\"command\": \"clang @foo.rsp\","
+  "\"file\": \"" + TestMainFileName + "\"}]").str();
+  // clang-format on
+  CompileCommandsFile.close();
+  std::string ErrorMessage;
+  auto JsonDatabase =
+  JSONCompilationDatabase::loadFromDirectory(TestDir, ErrorMessage);
+
+  EXPECT_TRUE(JsonDatabase);
+  auto FoundCommand = JsonDatabase->getCompileCommands(TestMainFileName);
+
+  EXPECT_TRUE(FoundCommand.size() == 1u) << ErrorMessage;
+  EXPECT_EQ(FoundCommand[0].Directory, TestDir) << ErrorMessage;
+  EXPECT_THAT(FoundCommand[0].CommandLine,
+  ElementsAre("clang", "-DFOO", "-DBAR"))
+  << ErrorMessage;
+  EXPECT_EQ(FoundCommand[0].Filename, TestMainFileName) << ErrorMessage;
+
+  llvm::sys::fs::remove(BarFileName);
+  llvm::sys::fs::remove(FooFileName);
+  llvm::sys::fs::remove(TestDir);
+}
+
+TEST(ExpandResponseFileTest, FixedCompilationDatabase) {
+  SmallString<128> TestDir;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory("unittest", TestDir);
+  EXPECT_TRUE(!EC);
+
+  SmallString<128> TestMainFileName;
+  llvm::sys::path::append(TestMainFileName, TestDir, "main.cpp");
+
+  SmallString<128> FooFileName;
+  llvm::sys::path::append(FooFileName, TestDir, "foo.rsp");
+  std::ofstream FooFile(FooFileName.c_str());
+  FooFile << "-DFOO @bar.rsp";
+  FooFile.close();
+
+  SmallString<128> BarFileName;
+  llvm::sys::path::append(BarFileName, TestDir, "bar.rsp");
+  std::ofstream BarFile(BarFileName.c_str());
+  BarFile << "-DBAR";
+  BarFile.close();
+
+  SmallString<128> FixedCompilationFileName;
+  llvm::sys::path::append(FixedCompilationFileName, TestDir,
+  "compile_flags.txt");
+  std::ofstream CompileCommandsFile(FixedCompilationFileName.c_str());
+  CompileCommandsFile << "@foo.rsp";
+  CompileCommandsFile.close();
+  std::string ErrorMessage;
+  auto FixedDatabase =
+  FixedCompilationDatabase::loadFromDirectory(TestDir, ErrorMessage);
+
+  EXPECT_TRUE(FixedDatabase);
+  auto FoundCommand = FixedDatabase->getCompileCommands(TestMainFileName);
+
+  EXPECT_TRUE(FoundCommand.size() == 1u) << ErrorMessage;
+  EXPECT_THAT(FoundCommand[0].CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO", "-DBAR",
+  EndsWith("main.cpp")))
+  << ErrorMessage;
+
+  llvm::sys::fs::remove(BarFileName);
+  llvm::sys::fs::remove(FooFileName);
+  llvm::sys::fs::remove(TestDir);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -313,16 +313,28 @@
 void JSONCompilationDatabase::getCommands(
 ArrayRef CommandsRef,
 std::vector ) const {
+  auto GetTokenizer = [](JSONCommandLineSyntax Syntax) {
+if (Syntax == JSONCommandLineSyntax::AutoDetect) {
+  Syntax = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+   ? 

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Awanish Pandey via Phabricator via cfe-commits
awpandey added a comment.

Hi @aprantl. Can we drop C binding for this feature ? What will be the impact 
of this.
As noted by @sammccall this does not conform / break LLVM-C ABI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70242: [OpenCL] Allow addr space qualifiers on lambda call expressions

2019-11-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D70242#1748313 , @rjmccall wrote:

> Well that was easy.
>
> Do we accept the address-space attribute in this position, or is that TBD?


We accept the OpenCL one right at the end. I might need to test more in C++ 
though...


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

https://reviews.llvm.org/D70242



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment.

Oops, it looks like I mixed up this ticket with 
https://reviews.llvm.org/D69238. Sorry about that. Would you like me to revert 
both commits and then re-commit with the correct links, etc.?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D67508: [RISCV] support mutilib in baremetal environment

2019-11-19 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

The test fails on Windows: http://45.33.8.238/win/2576/step_6.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67508



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


[PATCH] D70183: Detect source location overflow due includes

2019-11-19 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:587
+Diag.Report(IncludePos, diag::err_include_too_large);
+exit(1);
+  }

dnsampaio wrote:
> miyuki wrote:
> > dnsampaio wrote:
> > > miyuki wrote:
> > > > dnsampaio wrote:
> > > > > For debug builds, I could not find any other way to not reach an 
> > > > > assert failure other than exiting here. Perhaps there is a more llvm 
> > > > > specific way to die? llvm_unreachable() ?
> > > > There is a similar error `err_file_too_large`. How is it handled? 
> > > > `llvm_unreachable` is intended for code that is unreachable: it causes 
> > > > an assertion failure in debug builds and undefined behavior in release 
> > > > builds.
> > > The `err_file_too_large` above in this file allows the function to finish 
> > > and return further on. Here, we assert false before the message being 
> > > printed.
> > > If `llvm_unreachable` is undefined behavior, then I should stick to 
> > > exit(1) ?
> > I think you should return an invalid file ID (i.e. `return FileID();` and 
> > propagate the error through the call stack. Clang can be used as a library, 
> > so you can't just call exit(), this would terminate the user's program.
> It will still reach an false assert in builds that enable them. But in 
> release it linger on and ends with the correct warning.
Has this problem been fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70183



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


[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1315f4e009b0: [clang-tidy] Fix 
readability-redundant-string-init for c++17/c++2a (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69238?vs=229651=230039#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69238

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-init.StringNames, \
+// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
@@ -131,6 +135,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std::string d = "u", e = "u", f = "u";
+
+  std::string g = "u", h = "", i = "uuu", j = "", k;
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:43: warning: redundant string initialization
+  // CHECK-FIXES: std::string g = "u", h, i = "uuu", j, k;
 }
 
 // These cases should not generate warnings.
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
 
 namespace std {
 template 
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -73,7 +73,7 @@
   namedDecl(
   varDecl(
   hasType(hasUnqualifiedDesugaredType(recordType(
-  hasDeclaration(cxxRecordDecl(hasName("basic_string")),
+  hasDeclaration(cxxRecordDecl(hasStringTypeName),
   hasInitializer(expr(ignoringImplicit(anyOf(
   EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)
   .bind("vardecl"),
@@ -82,11 +82,12 @@
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult ) {
-  const auto *CtorExpr = Result.Nodes.getNodeAs("expr");
-  const auto *Decl = Result.Nodes.getNodeAs("decl");
-  diag(CtorExpr->getExprLoc(), "redundant string initialization")
-  << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
-  Decl->getName());
+  const auto *VDecl = Result.Nodes.getNodeAs("vardecl");
+  // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+  // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+  SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+  diag(VDecl->getLocation(), "redundant string initialization")
+  << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
 }
 
 } // namespace readability


Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-init.StringNames, \
+// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
@@ -131,6 +135,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std::string d = "u", e = "u", f = "u";
+
+  std::string g 

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf7086fe: [clang-tidy] modernize-use-override new option 
AllowOverrideAndFinal (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D70165?vs=229632=230040#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165

Files:
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.AllowOverrideAndFinal, value: 1}]}"
+
+struct Base {
+  virtual ~Base();
+  virtual void a();
+  virtual void b();
+  virtual void c();
+  virtual void d();
+  virtual void e();
+  virtual void f();
+  virtual void g();
+  virtual void h();
+  virtual void i();
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  ~Simple() override;
+  virtual void a() override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'override' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+  virtual void b() final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void b() final;
+  virtual void c() final override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void c() final override;
+  virtual void d() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void d() override final;
+  void e() final override;
+  void f() override final;
+  void g() final;
+  void h() override;
+  void i();
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void i() override;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
@@ -27,6 +27,14 @@
 
If set to non-zero, this check will not diagnose destructors. Default is `0`.
 
+.. option:: AllowOverrideAndFinal
+
+   If set to non-zero, this check will not diagnose ``override`` as redundant
+   with ``final``. This is useful when code will be compiled by a compiler with
+   warning/error checking flags requiring ``override`` explicitly on overriden
+   members, such as ``gcc -Wsuggest-override``/``gcc -Werror=suggest-override``.
+   Default is `0`.
+
 .. option:: OverrideSpelling
 
Specifies a macro to use instead of ``override``. This is useful when
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   Finds non-static member functions that can be made ``const``
   because the functions don't use ``this`` in a non-const way.
 
+- Improved :doc:`modernize-use-override
+  ` check.
+
+  The check now supports the ``AllowOverrideAndFinal`` option to eliminate
+  conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -26,6 +26,7 @@
 
 private:
   const bool IgnoreDestructors;
+  const bool AllowOverrideAndFinal;
   const std::string OverrideSpelling;
   const std::string FinalSpelling;
 };
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp

[PATCH] D70437: [AArch64][SVE] Implement shift intrinsics

2019-11-19 Thread Kerry McLaughlin via Phabricator via cfe-commits
kmclaughlin created this revision.
kmclaughlin added reviewers: huntergr, sdesmalen, dancgr, mgudim.
Herald added subscribers: psnobl, rkruppe, hiraditya, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: LLVM.

Adds the following intrinsics:

- asr & asrd
- insr
- lsl & lsr

This patch also adds a new AArch64ISD node (INSR) to represent the 
int_aarch64_sve_insr intrinsic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70437

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-shifts.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-shifts.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-shifts.ll
@@ -0,0 +1,367 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; ASR
+;
+
+define  @asr_i8( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_i8:
+; CHECK: asr z0.b, p0/m, z0.b, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.nxv16i8( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @asr_i16( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_i16:
+; CHECK: asr z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.nxv8i16( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @asr_i32( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_i32:
+; CHECK: asr z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.nxv4i32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @asr_i64( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_i64:
+; CHECK: asr z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.nxv2i64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @asr_wide_i8( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_wide_i8:
+; CHECK: asr z0.b, p0/m, z0.b, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.wide.nxv16i8( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+define  @asr_wide_i16( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_wide_i16:
+; CHECK: asr z0.h, p0/m, z0.h, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.wide.nxv8i16( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+define  @asr_wide_i32( %pg,  %a,  %b) {
+; CHECK-LABEL: asr_wide_i32:
+; CHECK: asr z0.s, p0/m, z0.s, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asr.wide.nxv4i32( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; ASRD
+;
+
+define  @asrd_i8( %pg,  %a) {
+; CHECK-LABEL: asrd_i8:
+; CHECK: asrd z0.b, p0/m, z0.b, #1
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asrd.nxv16i8( %pg,
+ %a,
+i32 1)
+  ret  %out
+}
+
+define  @asrd_i16( %pg,  %a) {
+; CHECK-LABEL: asrd_i16:
+; CHECK: asrd z0.h, p0/m, z0.h, #2
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asrd.nxv8i16( %pg,
+ %a,
+i32 2)
+  ret  %out
+}
+
+define  @asrd_i32( %pg,  %a) {
+; CHECK-LABEL: asrd_i32:
+; CHECK: asrd z0.s, p0/m, z0.s, #31
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asrd.nxv4i32( %pg,
+ %a,
+i32 31)
+  ret  %out
+}
+
+define  @asrd_i64( %pg,  %a) {
+; CHECK-LABEL: asrd_i64:
+; CHECK: asrd z0.d, p0/m, z0.d, #64
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.asrd.nxv2i64( %pg,
+ %a,
+i32 64)
+  ret  %out
+}
+
+;
+; INSR
+;
+
+define  @insr_i8( %a, i8 %b) {
+; CHECK-LABEL: insr_i8:
+; CHECK: insr z0.b, w0
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.insr.nxv16i8( %a, i8 %b)
+  ret  %out
+}
+
+define  

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

2019-11-19 Thread Paul Mulders via Phabricator via cfe-commits
justinkb reopened this revision.
justinkb added a comment.
This revision is now accepted and ready to land.
Herald added a project: LLVM.

The check doesn't work correctly if LLVM is built as a dylib and the 
LLVMTestingSupport static library isn't installed (which it normally wouldn't 
be)

On my machine llvm-config --components (correctly) doesn't print 
testingsupport, but since llvm-config --libs "testingsupport" does return 
"-lLLVM-9" since "If LLVM_LINK_DYLIB is ON, the single shared library will be 
return for "--libs" (quote from llvm-config source). Since testingsupport never 
seems to get built into the dylib, we end up with the failing check.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55891



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


[PATCH] D70183: Detect source location overflow due includes

2019-11-19 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio marked 2 inline comments as done.
dnsampaio added a comment.

Yes. It does return a non-valid FileID, and in builds without assert you get 
the expected error message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70183



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

In D70390#1751159 , @courbet wrote:

> > IMHO these two should just not overlap. It makes sense, to have 
> > controversial or configurable stuff in clang-tidy. It should just be 
> > consistent with the warnings, as those are "always right" and clang-tidy 
> > can be opinionated/specialized.
>
> So to make sure I understand you're advocating for keeping the `const` 
> version in the clang-tidy check but removing the `&&` detection from this 
> check and let the warning deal with that ?


Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-11-19 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.

LGTM. Thanks for adding the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55326



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


[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-19 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8a4c74f1157: [clang-tidy] Added DefaultOperatorNewCheck. 
(authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D67545?vs=226206=230019#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp
  clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp
  clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t
+
+namespace std {
+typedef __typeof(sizeof(int)) size_t;
+void *aligned_alloc(size_t, size_t);
+void free(void *);
+} // namespace std
+
+struct alignas(32) Vector1 {
+  char elems[32];
+};
+
+struct Vector2 {
+  char elems[32];
+};
+
+struct alignas(32) Vector3 {
+  char elems[32];
+  static void *operator new(std::size_t nbytes) noexcept(true) {
+return std::aligned_alloc(alignof(Vector3), nbytes);
+  }
+  static void operator delete(void *p) {
+std::free(p);
+  }
+};
+
+struct alignas(8) Vector4 {
+  char elems[32];
+};
+
+void f() {
+  auto *V1 = new Vector1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+  auto *V2 = new Vector2;
+  auto *V3 = new Vector3;
+  auto *V4 = new Vector4;
+  auto *V1_Arr = new Vector1[2];
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+}
Index: clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t
+// RUN: clang-tidy --extra-arg='-std=c++17' -checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
+// RUN: clang-tidy --extra-arg='-std=c++2a' -checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
+
+struct alignas(32) Vector {
+  char Elems[32];
+};
+
+void f() {
+  auto *V1 = new Vector;// CHECK-MESSAGES: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+  auto *V1_Arr = new Vector[2]; // CHECK-MESSAGES: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+}
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
@@ -100,6 +100,7 @@
cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) 
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
+   cert-mem57-cpp
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc32-c (redirects to cert-msc51-cpp) 
cert-msc50-cpp
Index: clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - cert-mem57-cpp
+
+cert-mem57-cpp
+==
+
+This check flags uses of default ``operator new`` where the type has extended
+alignment (an alignment greater than the fundamental alignment). (The default
+``operator new`` is guaranteed to provide the correct alignmment if the
+requested alignment is less or equal to the fundamental alignment).
+Only cases are detected (by design) where the ``operator new`` is not
+user-defined and is not a placement new (the reason is that in these cases we
+assume that the user provided the correct memory allocation).
+
+This check corresponds to the CERT C++ Coding Standard rule
+`MEM57-CPP. Avoid using default operator new for over-aligned types
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

2019-11-19 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Reverted in 3de7cc9fc01c8 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67508



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


[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-19 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 230033.
lh123 added a comment.

fix typo in function document.


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

https://reviews.llvm.org/D70222

Files:
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp

Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace tooling {
@@ -859,5 +860,98 @@
 "clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
 }
 
+TEST(ExpandResponseFileTest, JSONCompilationDatabase) {
+  SmallString<128> TestDir;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory("unittest", TestDir);
+  EXPECT_TRUE(!EC);
+
+  SmallString<128> TestMainFileName;
+  llvm::sys::path::append(TestMainFileName, TestDir, "main.cpp");
+
+  SmallString<128> FooFileName;
+  llvm::sys::path::append(FooFileName, TestDir, "foo.rsp");
+  std::ofstream FooFile(FooFileName.c_str());
+  FooFile << "-DFOO @bar.rsp";
+  FooFile.close();
+
+  SmallString<128> BarFileName;
+  llvm::sys::path::append(BarFileName, TestDir, "bar.rsp");
+  std::ofstream BarFile(BarFileName.c_str());
+  BarFile << "-DBAR";
+  BarFile.close();
+
+  SmallString<128> CompileCommandsFileName;
+  llvm::sys::path::append(CompileCommandsFileName, TestDir,
+  "compile_commands.json");
+  std::ofstream CompileCommandsFile(CompileCommandsFileName.c_str());
+  // clang-format off
+  CompileCommandsFile << ("[{\"directory\": \"" + TestDir + "\","
+  "\"command\": \"clang @foo.rsp\","
+  "\"file\": \"" + TestMainFileName + "\"}]").str();
+  // clang-format on
+  CompileCommandsFile.close();
+  std::string ErrorMessage;
+  auto JsonDatabase =
+  JSONCompilationDatabase::loadFromDirectory(TestDir, ErrorMessage);
+
+  EXPECT_TRUE(JsonDatabase);
+  auto FoundCommand = JsonDatabase->getCompileCommands(TestMainFileName);
+
+  EXPECT_TRUE(FoundCommand.size() == 1u) << ErrorMessage;
+  EXPECT_EQ(FoundCommand[0].Directory, TestDir) << ErrorMessage;
+  EXPECT_THAT(FoundCommand[0].CommandLine,
+  ElementsAre("clang", "-DFOO", "-DBAR"))
+  << ErrorMessage;
+  EXPECT_EQ(FoundCommand[0].Filename, TestMainFileName) << ErrorMessage;
+
+  llvm::sys::fs::remove(BarFileName);
+  llvm::sys::fs::remove(FooFileName);
+  llvm::sys::fs::remove(TestDir);
+}
+
+TEST(ExpandResponseFileTest, FixedCompilationDatabase) {
+  SmallString<128> TestDir;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory("unittest", TestDir);
+  EXPECT_TRUE(!EC);
+
+  SmallString<128> TestMainFileName;
+  llvm::sys::path::append(TestMainFileName, TestDir, "main.cpp");
+
+  SmallString<128> FooFileName;
+  llvm::sys::path::append(FooFileName, TestDir, "foo.rsp");
+  std::ofstream FooFile(FooFileName.c_str());
+  FooFile << "-DFOO @bar.rsp";
+  FooFile.close();
+
+  SmallString<128> BarFileName;
+  llvm::sys::path::append(BarFileName, TestDir, "bar.rsp");
+  std::ofstream BarFile(BarFileName.c_str());
+  BarFile << "-DBAR";
+  BarFile.close();
+
+  SmallString<128> FixedCompilationFileName;
+  llvm::sys::path::append(FixedCompilationFileName, TestDir,
+  "compile_flags.txt");
+  std::ofstream CompileCommandsFile(FixedCompilationFileName.c_str());
+  CompileCommandsFile << "@foo.rsp";
+  CompileCommandsFile.close();
+  std::string ErrorMessage;
+  auto FixedDatabase =
+  FixedCompilationDatabase::loadFromDirectory(TestDir, ErrorMessage);
+
+  EXPECT_TRUE(FixedDatabase);
+  auto FoundCommand = FixedDatabase->getCompileCommands(TestMainFileName);
+
+  EXPECT_TRUE(FoundCommand.size() == 1u) << ErrorMessage;
+  EXPECT_THAT(FoundCommand[0].CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO", "-DBAR",
+  EndsWith("main.cpp")))
+  << ErrorMessage;
+
+  llvm::sys::fs::remove(BarFileName);
+  llvm::sys::fs::remove(FooFileName);
+  llvm::sys::fs::remove(TestDir);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -313,16 +313,28 @@
 void JSONCompilationDatabase::getCommands(
 ArrayRef CommandsRef,
 std::vector ) const {
+  auto GetTokenizer = [](JSONCommandLineSyntax Syntax) {
+if (Syntax == JSONCommandLineSyntax::AutoDetect) {
+  Syntax = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+   ? JSONCommandLineSyntax::Windows
+  

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

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 230048.
JonasToth added a comment.

- port patch to monorepo
- Merge branch 'master' into feature_transform_const.patch
- merge current master and new version for previous steps into this patch
- fix compilation issues after type renaming
- work on fixing support and find out issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
+  AddConstTest.cpp
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,982 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template 
+class ConstTransform : public ClangTidyCheck {
+public:
+  ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+  }
+
+  void check(const MatchFinder::MatchResult ) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::addQualifierToVarDecl;
+Optional Fix = addQualifierToVarDecl(*D, DeclSpec::TQ::TQ_const,
+CT, CP, Result.Context);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+  Diag << *Fix;
+  }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform =
+ConstTransform;
+using ValueRTransform =
+ConstTransform;
+
+// 
+// Test Value-like types. Everything with indirection is done later.
+// 
+
+TEST(Values, Builtin) {
+  StringRef Snippet = "int target = 0;";
+
+  EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+  EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+  StringRef T = "typedef int MyInt;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) 

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

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

I know it has been a long time, but this patch is ready, i think :)
i would appreciate some reviews ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395



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


[PATCH] D70175: [libTooling] Extend `buildASTFromCodeWithArgs` to take files argument.

2019-11-19 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

gentle ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70175



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


[PATCH] D70183: Detect source location overflow due includes

2019-11-19 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

In D70183#1751552 , @dnsampaio wrote:

> Yes. It does return a non-valid FileID, and in builds without assert you get 
> the expected error message.


I was asking about "It will still reach an false assert in builds that enable 
them". Has this been fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70183



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


[PATCH] D60455: [SYCL] Implement SYCL device code outlining

2019-11-19 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

@aaron.ballman, @Anastasia, could you take a look at new version of the patch, 
please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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


[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

2019-11-19 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

That sounds like a bug in llvm-config. It shouldn't really return dylib for 
components that aren't part of it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55891



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:262
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);

rjmccall wrote:
> kpn wrote:
> > This looks reasonable to me. 
> > 
> > It smells like there's a larger strictfp IRBuilder issue, but that's not an 
> > issue for this patch here. The larger issue won't be hit since the new 
> > options affect the entire compilation. It therefore shouldn't block this 
> > patch.
> Does IRBuilder actually support inserting into an unparented basic block?  I 
> feel like this is exposing a much more serious mis-use of IRBuilder.
I suspect you are correct. If we let this "F && " change go in we'll have a 
situation where whether or not a block is currently in a function when a 
function call is emitted will affect whether or not the eventual function 
definition gets the strictfp attribute. That seems like an unfortunate 
inconsistency.

I'm still looking into it. I hope to have an IRBuilder review up today or 
tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-19 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG33d93c3d0b4a: [clangd] Show values of more expressions on 
hover (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -275,6 +275,7 @@
  {std::string("int"), std::string("T"), llvm::None},
  {std::string("bool"), std::string("B"), llvm::None},
  };
+ HI.Value = "false";
  return HI;
}},
   // Lambda variable
@@ -443,10 +444,23 @@
  HI.Definition = "GREEN";
  HI.Kind = SymbolKind::EnumMember;
  HI.Type = "enum Color";
- HI.Value = "1";
+ HI.Value = "1"; // Numeric when hovering on the enumerator name.
+   }},
+  {R"cpp(
+enum Color { RED, GREEN, };
+Color x = GREEN;
+Color y = [[^x]];
+   )cpp",
+   [](HoverInfo ) {
+ HI.Name = "x";
+ HI.NamespaceScope = "";
+ HI.Definition = "enum Color x = GREEN";
+ HI.Kind = SymbolKind::Variable;
+ HI.Type = "enum Color";
+ HI.Value = "GREEN (1)"; // Symbolic when hovering on an expression.
}},
   // FIXME: We should use the Decl referenced, even if from an implicit
-  // instantiation. Then the scope would be Add<1, 2> and the value 3.
+  // instantiation. Then the scope would be Add<1, 2>.
   {R"cpp(
 template struct Add {
   static constexpr int result = a + b;
@@ -460,6 +474,21 @@
  HI.Type = "const int";
  HI.NamespaceScope = "";
  HI.LocalScope = "Add::";
+ HI.Value = "3";
+   }},
+  {R"cpp(
+constexpr int answer() { return 40 + 2; }
+int x = [[ans^wer]]();
+)cpp",
+   [](HoverInfo ) {
+ HI.Name = "answer";
+ HI.Definition = "constexpr int answer()";
+ HI.Kind = SymbolKind::Function;
+ HI.Type = "int ()";
+ HI.ReturnType = "int";
+ HI.Parameters.emplace();
+ HI.NamespaceScope = "";
+ HI.Value = "42";
}},
   {R"cpp(
 const char *[[ba^r]] = "1234";
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
@@ -239,6 +240,46 @@
   // FIXME: handle variadics.
 }
 
+llvm::Optional printExprValue(const Expr *E, const ASTContext ) {
+  Expr::EvalResult Constant;
+  // Evaluating [[foo]]() as "" isn't useful, and prevents us walking up
+  // to the enclosing call.
+  QualType T = E->getType();
+  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  T->isFunctionReferenceType())
+return llvm::None;
+  // Attempt to evaluate. If expr is dependent, evaluation crashes!
+  if (E->isValueDependent() || !E->EvaluateAsRValue(Constant, Ctx))
+return llvm::None;
+
+  // Show enums symbolically, not numerically like APValue::printPretty().
+  if (T->isEnumeralType() && Constant.Val.getInt().getMinSignedBits() <= 64) {
+// Compare to int64_t to avoid bit-width match requirements.
+int64_t Val = Constant.Val.getInt().getExtValue();
+for (const EnumConstantDecl *ECD :
+ T->castAs()->getDecl()->enumerators())
+  if (ECD->getInitVal() == Val)
+return llvm::formatv("{0} ({1})", ECD->getNameAsString(), Val).str();
+  }
+  return Constant.Val.getAsString(Ctx, E->getType());
+}
+
+llvm::Optional printExprValue(const SelectionTree::Node *N,
+   const ASTContext ) {
+  for (; N; N = N->Parent) {
+// Try to evaluate the first evaluable enclosing expression.
+if (const Expr *E = N->ASTNode.get()) {
+  if (auto Val = printExprValue(E, Ctx))
+return Val;
+} else if (N->ASTNode.get() || N->ASTNode.get()) {
+  // Refuse to cross certain non-exprs. (TypeLoc are OK as part of Exprs).
+  // This tries to ensure we're showing a value related to the cursor.
+  break;
+}
+  }
+  return llvm::None;
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -282,18 +323,9 @@
   }
 
   // Fill in value with evaluated initializer if possible.
-  // FIXME(kadircet): Also set Value field for expressions like "sizeof" and
-  // function 

[PATCH] D70319: [ARM,MVE] Add intrinsics for scalar shifts.

2019-11-19 Thread Dave Green via Phabricator via cfe-commits
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

> That makes even LSLL and ASRL different enough from standard LLVM IR shift 
> semantics that I couldn't see any better alternative than to simply model the 
> whole family as a set of MVE-specific IR intrinsics.

Ah I see, that makes sense. I was wondering if the mapping of these would be 
simpler in one direction that the other.

Looks sensible to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70319



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


[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

2019-11-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr.
Herald added subscribers: llvm-commits, hiraditya, mgorny.
Herald added projects: clang, LLVM.

This is a follow-up to 590f279c456bbde632eca8ef89a85c478f15a249 
, which
moved some of the callers to use VFS.

It turned out more code in Driver calls into real filesystem APIs and
also needs an update.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70440

Files:
  clang/lib/Basic/SanitizerSpecialCaseList.cpp
  clang/lib/Basic/XRayLists.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/unittests/Driver/CMakeLists.txt
  clang/unittests/Driver/SanitizerArgsTest.cpp
  llvm/include/llvm/Support/SpecialCaseList.h
  llvm/lib/Support/SpecialCaseList.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -88,6 +88,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SpecialCaseList.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -480,7 +481,9 @@
   std::vector AllABIListFiles(std::move(ABIListFiles));
   AllABIListFiles.insert(AllABIListFiles.end(), ClABIListFiles.begin(),
  ClABIListFiles.end());
-  ABIList.set(SpecialCaseList::createOrDie(AllABIListFiles));
+  // FIXME: should we propagate vfs::FileSystem to this constructor?
+  ABIList.set(
+  SpecialCaseList::createOrDie(AllABIListFiles, *vfs::getRealFileSystem()));
 }
 
 FunctionType *DataFlowSanitizer::getArgsFunctionType(FunctionType *T) {
Index: llvm/lib/Support/SpecialCaseList.cpp
===
--- llvm/lib/Support/SpecialCaseList.cpp
+++ llvm/lib/Support/SpecialCaseList.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include 
 #include 
 #include 
@@ -71,9 +72,9 @@
 
 std::unique_ptr
 SpecialCaseList::create(const std::vector ,
-std::string ) {
+llvm::vfs::FileSystem , std::string ) {
   std::unique_ptr SCL(new SpecialCaseList());
-  if (SCL->createInternal(Paths, Error))
+  if (SCL->createInternal(Paths, FS, Error))
 return SCL;
   return nullptr;
 }
@@ -87,15 +88,16 @@
 }
 
 std::unique_ptr
-SpecialCaseList::createOrDie(const std::vector ) {
+SpecialCaseList::createOrDie(const std::vector ,
+ llvm::vfs::FileSystem ) {
   std::string Error;
-  if (auto SCL = create(Paths, Error))
+  if (auto SCL = create(Paths, FS, Error))
 return SCL;
   report_fatal_error(Error);
 }
 
 bool SpecialCaseList::createInternal(const std::vector ,
- std::string , vfs::FileSystem ) {
+ vfs::FileSystem , std::string ) {
   StringMap Sections;
   for (const auto  : Paths) {
 ErrorOr> FileOrErr =
Index: llvm/include/llvm/Support/SpecialCaseList.h
===
--- llvm/include/llvm/Support/SpecialCaseList.h
+++ llvm/include/llvm/Support/SpecialCaseList.h
@@ -69,7 +69,8 @@
   /// Parses the special case list entries from files. On failure, returns
   /// 0 and writes an error message to string.
   static std::unique_ptr
-  create(const std::vector , std::string );
+  create(const std::vector , llvm::vfs::FileSystem ,
+ std::string );
   /// Parses the special case list from a memory buffer. On failure, returns
   /// 0 and writes an error message to string.
   static std::unique_ptr create(const MemoryBuffer *MB,
@@ -77,7 +78,7 @@
   /// Parses the special case list entries from files. On failure, reports a
   /// fatal error.
   static std::unique_ptr
-  createOrDie(const std::vector );
+  createOrDie(const std::vector , llvm::vfs::FileSystem );
 
   ~SpecialCaseList();
 
@@ -103,8 +104,8 @@
 protected:
   // Implementations of the create*() functions that can also be used by derived
   // classes.
-  bool createInternal(const std::vector , std::string ,
-  vfs::FileSystem  = *vfs::getRealFileSystem());
+  bool createInternal(const std::vector ,
+  vfs::FileSystem , std::string );
   bool createInternal(const MemoryBuffer *MB, std::string );
 
   SpecialCaseList() = default;
Index: clang/unittests/Driver/SanitizerArgsTest.cpp
===
--- /dev/null
+++ clang/unittests/Driver/SanitizerArgsTest.cpp
@@ 

[PATCH] D69263: [clangd] Implement cross-file rename.

2019-11-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 230068.
hokein marked an inline comment as done.
hokein added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer , PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer , PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer , PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer , PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,10 +96,9 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer ,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
+llvm::Expected runRename(ClangdServer , PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
   Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
   return std::move(*Result);
 }
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,8 +20,45 @@
 namespace clangd {
 namespace {
 
-MATCHER_P2(RenameRange, Code, Range, "") {
-  return replacementToEdit(Code, arg).range == Range;
+using testing::Eq;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
+// Build a RefSlab from all marked ranges in the annotation. The ranges are
+// assumed to associate with the given SymbolName.
+std::unique_ptr buildRefSlab(const Annotations ,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto  = findSymbol(Symbols, SymbolName).ID;
+  for (const auto  : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+std::vector<
+std::pair>
+applyEdits(FileEdits FE) {
+  std::vector> Results;
+  for (auto  : FE)
+Results.emplace_back(
+It.first().str(),
+llvm::cantFail(tooling::applyAllReplacements(
+It.getValue().InitialCode, It.getValue().Replacements)));
+  return Results;
 }
 
 // Generates an expected rename result by replacing all ranges in the given
@@ -363,11 +402,11 @@
 llvm::StringRef NewName = "abcde";
 for (const auto  : Code.points()) {
   auto RenameResult =
-  renameWithinFile(AST, testPath(TU.Filename), RenamePos, NewName);
-  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T;
-  auto ApplyResult = llvm::cantFail(
-  tooling::applyAllReplacements(Code.code(), *RenameResult));
-  EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
+  rename({RenamePos, NewName, AST, testPath(TU.Filename)});
+  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+  ASSERT_EQ(1u, RenameResult->size());
+  

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

2019-11-19 Thread Kiran Chandramohan via Phabricator via cfe-commits
kiranchandramohan added a comment.

Thanks @jdoerfert for working on this.

Sorry for being late to the party. I am trying to implement one trivial 
directive (Flush) and one slightly more involved (not decided).

I applied D69853 , D69785 
, D69922  to 
my local build and found that D69922  is 
referring to OpenMPIRBuilder.h in llvm/Frontend whereas in D69785 
 it was introduced in 
llvm/Transforms/Utils/OpenMPIRBuilder.h




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:29
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Frontend/OpenMPIRBuilder.h"
 #include 

D69785 has this file in llvm/Transforms/Utils/OpenMPIRBuilder.h



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:55
 #include "llvm/IR/Module.h"
+#include "llvm/Frontend/OpenMPIRBuilder.h"
 #include "llvm/IR/ProfileSummary.h"

D69785 has this file in llvm/Transforms/Utils/OpenMPIRBuilder.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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


[PATCH] D69878: Consoldiate internal denormal flushing controls

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1775
-if (getLangOpts().OpenCL)
-  FuncAttrs.addAttribute("denorms-are-zero",
- llvm::toStringRef(CodeGenOpts.FlushDenorm));

Anastasia wrote:
> so where would `denorms-are-zero` be emitted now (in case some out of tree 
> implementations rely on this)?
Rely on in what sense? Do you have a concrete use of this?


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

https://reviews.llvm.org/D69878



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


[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

2019-11-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

> hence not throwing the warning on any platform?

I'm not sure I understand? The way I read the buildbot breakage, an existing 
ClangTidy test passed before and after this change, but broke on Windows. The 
breakage was that the warnings stopped being produced. Usually that is related 
to delayed parsing of templates on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950



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


[clang-tools-extra] 33d93c3 - [clangd] Show values of more expressions on hover

2019-11-19 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2019-11-19T15:34:04+01:00
New Revision: 33d93c3d0b4a331632902f5fb9874f4e021a2f58

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

LOG: [clangd] Show values of more expressions on hover

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 0e28e30482eb..feba29dd605d 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
@@ -239,6 +240,46 @@ void fillFunctionTypeAndParams(HoverInfo , const Decl 
*D,
   // FIXME: handle variadics.
 }
 
+llvm::Optional printExprValue(const Expr *E, const ASTContext 
) {
+  Expr::EvalResult Constant;
+  // Evaluating [[foo]]() as "" isn't useful, and prevents us walking up
+  // to the enclosing call.
+  QualType T = E->getType();
+  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  T->isFunctionReferenceType())
+return llvm::None;
+  // Attempt to evaluate. If expr is dependent, evaluation crashes!
+  if (E->isValueDependent() || !E->EvaluateAsRValue(Constant, Ctx))
+return llvm::None;
+
+  // Show enums symbolically, not numerically like APValue::printPretty().
+  if (T->isEnumeralType() && Constant.Val.getInt().getMinSignedBits() <= 64) {
+// Compare to int64_t to avoid bit-width match requirements.
+int64_t Val = Constant.Val.getInt().getExtValue();
+for (const EnumConstantDecl *ECD :
+ T->castAs()->getDecl()->enumerators())
+  if (ECD->getInitVal() == Val)
+return llvm::formatv("{0} ({1})", ECD->getNameAsString(), Val).str();
+  }
+  return Constant.Val.getAsString(Ctx, E->getType());
+}
+
+llvm::Optional printExprValue(const SelectionTree::Node *N,
+   const ASTContext ) {
+  for (; N; N = N->Parent) {
+// Try to evaluate the first evaluable enclosing expression.
+if (const Expr *E = N->ASTNode.get()) {
+  if (auto Val = printExprValue(E, Ctx))
+return Val;
+} else if (N->ASTNode.get() || N->ASTNode.get()) {
+  // Refuse to cross certain non-exprs. (TypeLoc are OK as part of Exprs).
+  // This tries to ensure we're showing a value related to the cursor.
+  break;
+}
+  }
+  return llvm::None;
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -282,18 +323,9 @@ HoverInfo getHoverContents(const Decl *D, const 
SymbolIndex *Index) {
   }
 
   // Fill in value with evaluated initializer if possible.
-  // FIXME(kadircet): Also set Value field for expressions like "sizeof" and
-  // function calls.
   if (const auto *Var = dyn_cast(D)) {
-if (const Expr *Init = Var->getInit()) {
-  Expr::EvalResult Result;
-  if (!Init->isValueDependent() && Init->EvaluateAsRValue(Result, Ctx)) {
-HI.Value.emplace();
-llvm::raw_string_ostream ValueOS(*HI.Value);
-Result.Val.printPretty(ValueOS, const_cast(Ctx),
-   Init->getType());
-  }
-}
+if (const Expr *Init = Var->getInit())
+  HI.Value = printExprValue(Init, Ctx);
   } else if (const auto *ECD = dyn_cast(D)) {
 // Dependent enums (e.g. nested in template classes) don't have values yet.
 if (!ECD->getType()->isDependentType())
@@ -381,8 +413,16 @@ llvm::Optional getHover(ParsedAST , 
Position Pos,
 if (const SelectionTree::Node *N = Selection.commonAncestor()) {
   DeclRelationSet Rel = DeclRelation::TemplatePattern | 
DeclRelation::Alias;
   auto Decls = targetDecl(N->ASTNode, Rel);
-  if (!Decls.empty())
+  if (!Decls.empty()) {
 HI = getHoverContents(Decls.front(), Index);
+// Look for a close enclosing expression to show the value of.
+if (!HI->Value)
+  HI->Value = printExprValue(N, AST.getASTContext());
+  }
+  // FIXME: support hovers for other nodes?
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
+  //  - literals (esp user-defined)
 }
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 7b5907faaa2c..530dfe600ecf 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 

  1   2   3   >