[PATCH] D40451: [OpenMP] Add function attribute for triggering shared memory lowering in the LLVM backend

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 124246.

Repository:
  rL LLVM

https://reviews.llvm.org/D40451

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void 
@__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** 
[[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -934,6 +934,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's somehting to share, add the attribute
+  CGF.CurFn->addFnAttr("has-nvptx-shared-depot");
   // Prepare for parallel region. Indicate the outlined function.
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.VoidPtrPtrTy,


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -934,6 +934,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's 

[PATCH] D40451: [OpenMP] Add function attribute for triggering shared memory lowering in the LLVM backend

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
Herald added a subscriber: jholewinski.

Since OpenMP and CUDA share the same toolchain we need to disable:

- the lowering of variables to shared memory in the LLVM NVPTX backend
- the emission of the shared depot
- the emission of shared stack pointers

when compiling:

- CUDA programs
- OpenMP programs that do not require data sharing.


Repository:
  rL LLVM

https://reviews.llvm.org/D40451

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void 
@__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** 
[[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,17 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attribute must contain
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -934,6 +934,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's somehting to share, add the attribute
+  CGF.CurFn->addFnAttr("has-nvptx-shared-depot");
   // Prepare for parallel region. Indicate the outlined function.
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.VoidPtrPtrTy,


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,17 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attribute must contain
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: 

[PATCH] D40450: [clangd] Refactoring of GlobalCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.

The interface used is now tooling::CompilationDatabase.
The various behaviors of the existing CDB implementation is decomposed into
several classes responsible for one aspect each.

- The fallback commands are now a FixedCompilationDatabase. For now, this is 
done by both ClangdLSPServer and ClangdServer (preserving behavior) but I'd 
like to eliminate the latter, as embedders of ClangdServer are in a better 
position to know what fallback is appropriate.
- the fallback now uses '.' as the directory, rather than the file's parent.
- The compilation database file scan caches missing as well as present files.
- -fsyntax-only is now applied to all commands, not just the fallback. We also 
string output flags.
- The final command line used for each file is now logged.
- The -resource-dir flag is managed by ClangdServer rather than being done 
deeper in the stack. All flag manipulation is done by CDBs set up by 
ClangdLSPServer and ClangdServer.


https://reviews.llvm.org/D40450

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -203,7 +203,7 @@
   VFSTag LastVFSTag = VFSTag();
 };
 
-class MockCompilationDatabase : public GlobalCompilationDatabase {
+class MockCompilationDatabase : public tooling::CompilationDatabase {
 public:
   MockCompilationDatabase(bool AddFreestandingFlag) {
 // We have to add -ffreestanding to VFS-specific tests to avoid errors on
@@ -213,7 +213,7 @@
   }
 
   std::vector
-  getCompileCommands(PathRef File) override {
+  getCompileCommands(PathRef File) const override {
 if (ExtraClangFlags.empty())
   return {};
 
Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -6,73 +6,77 @@
 // License. See LICENSE.TXT for details.
 //
 //===-===//
+//
+// Defines CompilationDatabase that help clangd determine compile commands.
+// We plug together a few of these, depending on configuration.
+//
+//===-===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 
 #include "Path.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/StringMap.h"
 #include 
 #include 
 #include 
 
 namespace clang {
-
-namespace tooling {
-class CompilationDatabase;
-struct CompileCommand;
-} // namespace tooling
-
 namespace clangd {
-
 class Logger;
 
-/// Returns a default compile command to use for \p File.
-tooling::CompileCommand getDefaultCompileCommand(PathRef File);
-
-/// Provides compilation arguments used for parsing C and C++ files.
-class GlobalCompilationDatabase {
+/// Provides compile commands for any file by scanning parent directories
+/// looking for compilation databases. These are cached for efficiency.
+class GlobalCompilationDatabase : public tooling::CompilationDatabase {
 public:
-  virtual ~GlobalCompilationDatabase() = default;
+  std::vector
+  getCompileCommands(PathRef File) const override;
 
-  virtual std::vector
-  getCompileCommands(PathRef File) = 0;
+private:
+  tooling::CompilationDatabase *getCompilationDatabase(PathRef File) const;
 
-  /// FIXME(ibiryukov): add facilities to track changes to compilation flags of
-  /// existing targets.
+  mutable std::mutex Mutex;
+  /// Caches compilation databases from directories. Dir -> DB or null.
+  mutable llvm::StringMap
+  Cache;
 };
 
-/// Gets compile args from tooling::CompilationDatabases built for parent
-/// directories.
-class DirectoryBasedGlobalCompilationDatabase
-: public GlobalCompilationDatabase {
+/// Adds extra flags into the compile commands of another database.
+/// The extra flags are configured per file.
+class ExtraFlagsCompilationDatabase : public tooling::CompilationDatabase {
 public:
-  DirectoryBasedGlobalCompilationDatabase(
-  clangd::Logger , llvm::Optional CompileCommandsDir);
+  ExtraFlagsCompilationDatabase(
+  std::unique_ptr Inner)
+  : Inner(std::move(Inner)) {}
 
   std::vector
-  getCompileCommands(PathRef File) override;
+  getCompileCommands(PathRef File) const override;
 
-  void setExtraFlagsForFile(PathRef File, std::vector ExtraFlags);
+  void setExtraFlagsForFile(PathRef File, std::vector Flags);
 
 private:
-  tooling::CompilationDatabase *getCompilationDatabase(PathRef File);
-  

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D30610#934891, @malcolm.parsons wrote:

> I have 1000s of warnings from this check.
>
> A lot of them are about google tests:
>  warning: class 'Foo_Bar_Test' defines a copy constructor and a copy 
> assignment operator but does not define a destructor 
> [cppcoreguidelines-special-member-functions]
>  I'd like an option to suppress these.
>
> It warns about classes that use boost::noncopyable:
>  warning: class 'Foo' defines a non-default destructor but does not define a 
> copy constructor or a copy assignment operator 
> [cppcoreguidelines-special-member-functions]
>  class Foo : boost::noncopyable
>  I think this is a bug in the check.


It is not a bug in the check; the check implements the C++ Core Guideline rule. 
Hence the comment about discussing it with them.

I think your use cases are compelling and might warrant an option. However, 
with that in mind combined with `AllowMissingMoveFunctions`, it makes me think 
this check should not have any of the options -- it's a sign that your code 
base isn't ready to comply with C.21 in C++ Core Guidelines. The wording for 
the guideline is pretty specific about what the authors intend: "Compilers 
enforce much of this rule and ideally warn about any violation." and its 
enforcement "(Simple) A class should have a declaration (even a =delete one) 
for either all or none of the special functions." I'm not saying we shouldn't 
add this option you're looking for, but I'm still a bit uncomfortable with 
coming up with option combinations that effectively disable the entire point to 
a check in a coding standard -- that's why we allow you to disable the check 
more visibly.

> In https://reviews.llvm.org/D30610#934862, @fgross wrote:
> 
>> My 2 cents on this one: Even with `AllowMissingMoveFunctions=1` at least the 
>> missing destructor definition should be diagnosed, because it violates the 
>> classic rule of three. If you delete your copy operations, you likely have 
>> some resources that need to be taken care of in your destructor, so this 
>> piece of code would worry me. Better be clear about it and explicitly 
>> default the destructor.
> 
> 
> This is a rule of zero class, but with copying disabled; the resources are 
> just as safe as with a normal rule of zero class.

Depending on the resource; you might allocate a resource in the constructor, 
deleted copy (and thus no move) operators, but still want to release the 
resource in the destructor.


https://reviews.llvm.org/D30610



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

[PATCH] D38978: [OpenMP] Enable the lowering of implicitly shared variables in OpenMP GPU-offloaded target regions to the GPU shared memory

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 124243.
gtbercea added a comment.

Add regression tests and allow for shared memory lowering to be disabled at 
function level.


Repository:
  rL LLVM

https://reviews.llvm.org/D38978

Files:
  include/llvm/CodeGen/TargetPassConfig.h
  lib/CodeGen/TargetPassConfig.cpp
  lib/Target/NVPTX/CMakeLists.txt
  lib/Target/NVPTX/NVPTX.h
  lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  lib/Target/NVPTX/NVPTXFrameLowering.cpp
  lib/Target/NVPTX/NVPTXFunctionDataSharing.cpp
  lib/Target/NVPTX/NVPTXFunctionDataSharing.h
  lib/Target/NVPTX/NVPTXInstrInfo.td
  lib/Target/NVPTX/NVPTXLowerAlloca.cpp
  lib/Target/NVPTX/NVPTXLowerSharedFrameIndicesPass.cpp
  lib/Target/NVPTX/NVPTXRegisterInfo.cpp
  lib/Target/NVPTX/NVPTXRegisterInfo.h
  lib/Target/NVPTX/NVPTXRegisterInfo.td
  lib/Target/NVPTX/NVPTXTargetMachine.cpp
  lib/Target/NVPTX/NVPTXUtilities.cpp
  lib/Target/NVPTX/NVPTXUtilities.h
  test/CodeGen/NVPTX/insert-shared-depot.ll
  test/CodeGen/NVPTX/lower-alloca-shared.ll
  test/CodeGen/NVPTX/no-shared-depot.ll
  test/CodeGen/NVPTX/nvptx-function-data-sharing.ll

Index: test/CodeGen/NVPTX/nvptx-function-data-sharing.ll
===
--- /dev/null
+++ test/CodeGen/NVPTX/nvptx-function-data-sharing.ll
@@ -0,0 +1,31 @@
+; RUN: opt < %s -S -nvptx-function-data-sharing -infer-address-spaces | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_35 | FileCheck %s --check-prefix PTX
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v16:16:16-v32:32:32-v64:64:64-v128:128:128-n16:32:64"
+target triple = "nvptx64-unknown-unknown"
+
+define void @kernel() #0 {
+; LABEL: @lower_shared_alloca
+; PTX-LABEL: .visible .entry kernel(
+  %A = alloca i32
+; CHECK: addrspacecast i32* %A to i32 addrspace(3)*
+; CHECK: addrspacecast i32 addrspace(3)* %A1 to i32*
+; CHECK: store i32 0, i32 addrspace(3)* {{%.+}}
+; PTX: add.u64 {{%rd[0-9]+}}, %SPS, 0;
+; PTX: cvta.to.shared.u64 {{%rd[0-9]+}}, {{%rd[0-9]+}};
+; PTX: st.shared.u32 [{{%rd[0-9]+}}], {{%r[0-9]+}}
+  %shared_args = alloca i32**
+  call void @callee(i32*** %shared_args)
+  %1 = load i32**, i32*** %shared_args
+  %2 = getelementptr inbounds i32*, i32** %1, i64 0
+  store i32* %A, i32** %2
+  store i32 0, i32* %A
+  ret void
+}
+
+declare void @callee(i32***)
+
+attributes #0 = {"has-nvptx-shared-depot"}
+
+!nvvm.annotations = !{!0}
+!0 = !{void ()* @kernel, !"kernel", i32 1}
Index: test/CodeGen/NVPTX/no-shared-depot.ll
===
--- /dev/null
+++ test/CodeGen/NVPTX/no-shared-depot.ll
@@ -0,0 +1,40 @@
+; RUN: llc < %s -march=nvptx -mcpu=sm_20 | FileCheck %s --check-prefix=PTX32
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_20 | FileCheck %s --check-prefix=PTX64
+
+; PTX32: {{.*}}kernel()
+; PTX64: {{.*}}kernel()
+
+; PTX32: .local .align 8{{.*}}.b8{{.*}}__local_depot0
+; PTX64: .local .align 8{{.*}}.b8{{.*}}__local_depot0
+
+; PTX32-NOT: .shared .align 8{{.*}}.b8{{.*}}__shared_depot0
+; PTX64-NOT: .shared .align 8{{.*}}.b8{{.*}}__shared_depot0
+
+; PTX32-NOT: .reg .b32{{.*}}%SPS;
+; PTX64-NOT: .reg .b64{{.*}}%SPS;
+
+; PTX32-NOT: .reg .b32{{.*}}%SPSH;
+; PTX64-NOT: .reg .b64{{.*}}%SPSH;
+
+; PTX32-NOT: mov.u32{{.*}}%SPSH, __shared_depot0;
+; PTX64-NOT: mov.u64{{.*}}%SPSH, __shared_depot0;
+
+; PTX32-NOT: cvta.shared.u32{{.*}}%SPS, %SPSH;
+; PTX64-NOT: cvta.shared.u64{{.*}}%SPS, %SPSH;
+
+target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64"
+target triple = "nvptx64-unknown-unknown"
+
+define void @kernel() {
+; LABEL: @linsert_shared_depot
+  %A = alloca i32, align 4
+  %shared_args = alloca i8**, align 8
+  call void @callee(i8*** %shared_args)
+  store i32 10, i32* %A
+  ret void
+}
+
+declare void @callee(i8***)
+
+!nvvm.annotations = !{!0}
+!0 = !{void ()* @kernel, !"kernel", i32 1}
Index: test/CodeGen/NVPTX/lower-alloca-shared.ll
===
--- /dev/null
+++ test/CodeGen/NVPTX/lower-alloca-shared.ll
@@ -0,0 +1,31 @@
+; RUN: opt < %s -S -nvptx-lower-alloca -infer-address-spaces | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_35 | FileCheck %s --check-prefix PTX
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v16:16:16-v32:32:32-v64:64:64-v128:128:128-n16:32:64"
+target triple = "nvptx64-unknown-unknown"
+
+define void @kernel() #0 {
+; LABEL: @lower_shared_alloca
+; PTX-LABEL: .visible .entry kernel(
+  %A = alloca i32
+; CHECK: addrspacecast i32* %A to i32 addrspace(3)*
+; CHECK: addrspacecast i32 addrspace(3)* %1 to i32*
+; CHECK: store i32 0, i32 addrspace(3)* {{%.+}}
+; PTX: add.u64 {{%rd[0-9]+}}, %SPS, 0;
+; PTX: cvta.to.shared.u64 {{%rd[0-9]+}}, {{%rd[0-9]+}};
+; PTX: st.shared.u32 [{{%rd[0-9]+}}], {{%r[0-9]+}}
+  %shared_args = alloca i32**
+  call void @callee(i32*** %shared_args)
+  %1 = load i32**, i32*** %shared_args
+  %2 = getelementptr inbounds i32*, i32** 

[PATCH] D37903: Fix assume-filename handling in clang-format.el

[Micah Werbitt via Phabricator via cfe-commits]

werbitt updated this revision to Diff 124242.
werbitt edited the summary of this revision.
werbitt added a comment.

Updating for upstream head


https://reviews.llvm.org/D37903

Files:
  tools/clang-format/clang-format.el


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
   (byte-to-position (1+ byte)
 
 ;;;###autoload
-(defun clang-format-region (start end  style)
+(defun clang-format-region (start end  style assume-file-name)
   "Use clang-format to format the code between START and END according to 
STYLE.
-If called interactively uses the region or the current statement if there
-is no active region.  If no style is given uses `clang-format-style'."
+If called interactively uses the region or the current statement if there is no
+no active region. If no STYLE is given uses `clang-format-style'. Use
+ASSUME-FILE-NAME to locate a style config file, if no ASSUME-FILE-NAME is given
+uses the function `buffer-file-name'."
   (interactive
(if (use-region-p)
(list (region-beginning) (region-end))
  (list (point) (point
 
   (unless style
 (setq style clang-format-style))
 
+  (unless assume-file-name
+(setq assume-file-name buffer-file-name))
+
   (let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
 'utf-8-unix))
 (file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -144,16 +149,21 @@
 ;; always use ‘utf-8-unix’ and ignore the buffer coding system.
 (default-process-coding-system '(utf-8-unix . utf-8-unix)))
 (unwind-protect
-(let ((status (call-process-region
-   nil nil clang-format-executable
-   nil `(,temp-buffer ,temp-file) nil
-
-   "-output-replacements-xml"
-   "-assume-filename" (or (buffer-file-name) "")
-   "-style" style
-   "-offset" (number-to-string file-start)
-   "-length" (number-to-string (- file-end file-start))
-   "-cursor" (number-to-string cursor)))
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+   ;; Gaurd against a nil assume-file-name.
+   ;; If the clang-format option -assume-filename
+   ;; is given a blank string it will crash as per
+   ;; the following bug report
+   ;; https://bugs.llvm.org/show_bug.cgi?id=34667
+   ,@(and assume-file-name
+  (list "-assume-filename" 
assume-file-name))
+   "-style" ,style
+   "-offset" ,(number-to-string file-start)
+   "-length" ,(number-to-string (- file-end 
file-start))
+   "-cursor" ,(number-to-string cursor
   (stderr (with-temp-buffer
 (unless (zerop (cadr (insert-file-contents temp-file)))
   (insert ": "))
@@ -181,10 +191,13 @@
   (when (buffer-name temp-buffer) (kill-buffer temp-buffer)
 
 ;;;###autoload
-(defun clang-format-buffer ( style)
-  "Use clang-format to format the current buffer according to STYLE."
+(defun clang-format-buffer ( style assume-file-name)
+  "Use clang-format to format the current buffer according to STYLE.
+If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME
+to locate a style config file. If no ASSUME-FILE-NAME is given uses
+the function `buffer-file-name'."
   (interactive)
-  (clang-format-region (point-min) (point-max) style))
+  (clang-format-region (point-min) (point-max) style assume-file-name))
 
 ;;;###autoload
 (defalias 'clang-format 'clang-format-region)


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
   (byte-to-position (1+ byte)
 
 ;;;###autoload
-(defun clang-format-region (start end  style)
+(defun clang-format-region (start end  style assume-file-name)
   "Use clang-format to format the code between START and END according to STYLE.
-If called interactively uses the region or the current statement if there
-is no active region.  If no style is given uses `clang-format-style'."
+If called interactively uses the region or the current statement if there is no
+no active region. If no STYLE is given uses `clang-format-style'. Use
+ASSUME-FILE-NAME to locate a 

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

I have 1000s of warnings from this check.

A lot of them are about google tests:
warning: class 'Foo_Bar_Test' defines a copy constructor and a copy assignment 
operator but does not define a destructor 
[cppcoreguidelines-special-member-functions]
I'd like an option to suppress these.

It warns about classes that use boost::noncopyable:
warning: class 'Foo' defines a non-default destructor but does not define a 
copy constructor or a copy assignment operator 
[cppcoreguidelines-special-member-functions]
class Foo : boost::noncopyable
I think this is a bug in the check.

In https://reviews.llvm.org/D30610#934862, @fgross wrote:

> My 2 cents on this one: Even with `AllowMissingMoveFunctions=1` at least the 
> missing destructor definition should be diagnosed, because it violates the 
> classic rule of three. If you delete your copy operations, you likely have 
> some resources that need to be taken care of in your destructor, so this 
> piece of code would worry me. Better be clear about it and explicitly default 
> the destructor.


This is a rule of zero class, but with copying disabled; the resources are just 
as safe as with a normal rule of zero class.


https://reviews.llvm.org/D30610



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

[PATCH] D40448: Add a printing policy for AST dumping

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.

The AST dump functionality does not currently make use of a printing policy, so 
when dumping C++ code, you will see `_Bool`, `struct Foo` and other C-isms in 
the output. This patch adds a `PrintingPolicy` object to the AST dumper to get 
slightly improved output for the various language modes, and corrects the 
impacted test cases.


https://reviews.llvm.org/D40448

Files:
  include/clang/AST/Type.h
  lib/AST/ASTDumper.cpp
  lib/AST/TypePrinter.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-examples.cpp
  test/Frontend/float16.cpp
  test/Misc/ast-dump-attr.cpp
  test/Misc/ast-dump-decl.cpp
  test/Misc/ast-dump-invalid.cpp
  test/OpenMP/dump.cpp
  test/Parser/objc-default-ctor-init.mm
  test/SemaCXX/compound-literal.cpp
  test/SemaCXX/sourceranges.cpp
  test/SemaCXX/warn-redundant-move.cpp
  test/SemaObjCXX/block-cleanup.mm
  test/SemaTemplate/default-expr-arguments-2.cpp
  test/SemaTemplate/default-expr-arguments-3.cpp

Index: test/SemaTemplate/default-expr-arguments-3.cpp
===
--- test/SemaTemplate/default-expr-arguments-3.cpp
+++ test/SemaTemplate/default-expr-arguments-3.cpp
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -std=c++14 -verify -ast-dump %s | FileCheck %s
 // expected-no-diagnostics
 
-// CHECK: FunctionDecl {{.*}} used func 'void (void)'
+// CHECK: FunctionDecl {{.*}} used func 'void ()'
 // CHECK-NEXT: TemplateArgument type 'int'
-// CHECK: LambdaExpr {{.*}} 'class (lambda at
-// CHECK: ParmVarDecl {{.*}} used f 'enum foo' cinit
-// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo'
+// CHECK: LambdaExpr {{.*}} '(lambda at
+// CHECK: ParmVarDecl {{.*}} used f 'foo' cinit
+// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo'
 
 namespace PR28795 {
   template
@@ -22,9 +22,9 @@
 
 // CHECK: ClassTemplateSpecializationDecl {{.*}} struct class2 definition
 // CHECK: TemplateArgument type 'int'
-// CHECK: LambdaExpr {{.*}} 'class (lambda at
-// CHECK: ParmVarDecl {{.*}} used f 'enum foo' cinit
-// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo'
+// CHECK: LambdaExpr {{.*}} '(lambda at
+// CHECK: ParmVarDecl {{.*}} used f 'foo' cinit
+// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo'
 
 // Template struct case:
 template  struct class2 {
@@ -38,11 +38,11 @@
 
 // CHECK: FunctionTemplateDecl {{.*}} f1
 // CHECK-NEXT: TemplateTypeParmDecl {{.*}} typename depth 0 index 0 T
-// CHECK-NEXT: FunctionDecl {{.*}} f1 'void (void)'
-// CHECK: FunctionDecl {{.*}} f1 'void (void)'
+// CHECK-NEXT: FunctionDecl {{.*}} f1 'void ()'
+// CHECK: FunctionDecl {{.*}} f1 'void ()'
 // CHECK-NEXT: TemplateArgument type 'int'
-// CHECK: ParmVarDecl {{.*}} n 'enum foo' cinit
-// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo'
+// CHECK: ParmVarDecl {{.*}} n 'foo' cinit
+// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo'
 
 template
 void f1() {
Index: test/SemaTemplate/default-expr-arguments-2.cpp
===
--- test/SemaTemplate/default-expr-arguments-2.cpp
+++ test/SemaTemplate/default-expr-arguments-2.cpp
@@ -9,8 +9,8 @@
   public: enum { kSomeConst = 128 };
 bar(int x = kSomeConst) {}
   };
-  
-  // CHECK: FunctionDecl{{.*}}f 'void (void)'
+
+  // CHECK: FunctionDecl{{.*}}f 'void ()'
   void f() {
 // CHECK: VarDecl{{.*}}tmp 'bar'
 // CHECK: CXXDefaultArgExpr{{.*}}'int'
Index: test/SemaObjCXX/block-cleanup.mm
===
--- test/SemaObjCXX/block-cleanup.mm
+++ test/SemaObjCXX/block-cleanup.mm
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -std=gnu++11 -o /dev/null -x objective-c++ -fblocks -ast-dump %s 2>&1 | FileCheck %s
 
-// CHECK:  -FunctionDecl {{.*}} test 'id (void)'
+// CHECK:  -FunctionDecl {{.*}} test 'id ()'
 // CHECK-NEXT:   -CompoundStmt
 // CHECK-NEXT: -ReturnStmt
 // CHECK-NEXT:   -ExprWithCleanups
Index: test/SemaCXX/warn-redundant-move.cpp
===
--- test/SemaCXX/warn-redundant-move.cpp
+++ test/SemaCXX/warn-redundant-move.cpp
@@ -75,7 +75,7 @@
   return d;
   // Verify the implicit move from the AST dump
   // CHECK-AST: ReturnStmt{{.*}}line:[[@LINE-2]]
-  // CHECK-AST-NEXT: CXXConstructExpr{{.*}}struct D{{.*}}void (struct D &&)
+  // CHECK-AST-NEXT: CXXConstructExpr{{.*}}D{{.*}}void (D &&)
   // CHECK-AST-NEXT: ImplicitCastExpr
   // CHECK-AST-NEXT: DeclRefExpr{{.*}}ParmVar{{.*}}'d'
 
Index: test/SemaCXX/sourceranges.cpp
===
--- test/SemaCXX/sourceranges.cpp
+++ test/SemaCXX/sourceranges.cpp
@@ -46,7 +46,7 @@
 void construct() {
   using namespace foo;
   A a = A(12);
-  // CHECK: CXXConstructExpr {{0x[0-9a-fA-F]+}} 

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 124239.
Rakete edited the summary of this revision.
Rakete added a comment.

Added more test coverage for compatibility warnings, and fixed a bug at the 
same time :).


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp

Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
 
 int f();
 
@@ -10,10 +11,68 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+void ifInitStatement() {
+  int Var = 0;
+
+  if (int I = 0; true) {}
+  if (Var + Var; true) {}
+  if (; true) {}
+#ifdef CPP17
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{if initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'if' initialization statements are a C++17 extension}}
+#endif
+}
+
+void switchInitStatement() {
+  int Var = 0;
+
+  switch (int I = 0; Var) {}
+  switch (Var + Var; Var) {}
+  switch (; Var) {}
+#ifdef CPP17
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+  // expected-warning@-4 {{switch initialization statements are incompatible with C++ standards before C++17}}
+#else
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+  // expected-warning@-8 {{'switch' initialization statements are a C++17 extension}}
+#endif
+}
+
+// TODO: Better diagnostics for while init statements.
+void whileInitStatement() {
+  while (int I = 10; I--); // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{use of undeclared identifier 'I'}}
+
+  int Var = 10;
+  while (Var + Var; Var--) {} // expected-error {{expected ')'}}
+  // expected-note@-1 {{to match this '('}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+  // expected-warning@-4 {{while loop has empty body}}
+  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
+}
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
+  while (; false) {} // expected-error {{expected expression}}
+  // expected-warning@-1 {{expression result unused}}
+  // expected-error@-2 {{expected ';' after expression}}
+  // expected-error@-3 {{expected expression}}
+}
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1740,17 +1740,32 @@
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  const auto WarnOnInit = [this, ] {
+Diag(Tok.getLocation(), getLangOpts().CPlusPlus1z
+? diag::warn_cxx14_compat_init_statement
+: diag::ext_init_statement)
+<< (CK == Sema::ConditionKind::Switch);
+  };
+
   // Determine what kind of thing we have.
   switch (isCXXConditionDeclarationOrInitStatement(InitStmt)) {
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (InitStmt && TryConsumeToken(tok::semi)) {
+  WarnOnInit();
+  return ParseCXXCondition(nullptr, Loc, CK);
+}
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
   return Sema::ConditionError();
 
 if (InitStmt && Tok.is(tok::semi)) {
+  WarnOnInit();
   *InitStmt = Actions.ActOnExprStmt(Expr.get());
   ConsumeToken();
   return ParseCXXCondition(nullptr, Loc, CK);
@@ -1760,10 +1775,7 @@
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
-Diag(Tok.getLocation(), getLangOpts().CPlusPlus1z
-? diag::warn_cxx14_compat_init_statement
-: diag::ext_init_statement)
-   

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124238.
Nebiroth marked 3 inline comments as done.
Nebiroth added a comment.

Fixed test checking for values from an incorrect bit shift


https://reviews.llvm.org/D38425

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/main.cpp
  test/clangd/documenthighlight.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/documenthighlight.test
===
--- /dev/null
+++ test/clangd/documenthighlight.test
@@ -0,0 +1,38 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 455
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();}\n"}}}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":12,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}}]}
+
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":9,"line":12},"start":{"character":4,"line":12}}},{"kind":1,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":3,"range":{"end":{"character":8,"line":19},"start":{"character":4,"line":19}}}]}
+
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":22,"line":4},"start":{"character":5,"line":4}}},{"kind":1,"range":{"end":{"character":25,"line":21},"start":{"character":8,"line":21}}}]}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}			
\ No newline at end of file
Index: clangd/main.cpp
===
--- /dev/null
+++ clangd/main.cpp
@@ -0,0 +1,22 @@
+#define MACRO 1
+namespace ns1 {
+struct MyClass {
+int xasd;
+void anotherOperation(){
+}
+static int foo(MyClass*) {
+return 0;
+}
+
+};
+struct Foo {
+int xasd;
+};
+}
+int main() {
+int bonjour;
+bonjour = 2;
+ns1::Foo bar = { xasd : 1};
+bar.xasd = 3;
+ns1::MyClass* Params;
+Params->anotherOperation();}
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -54,6 +54,8 @@
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams ) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams ) = 0;
   

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Florian Gross via Phabricator via cfe-commits]

fgross added a comment.

> If you've written your own copy functions then you probably want to write 
> your own move functions to allow moving, so `AllowMissingMoveFunctions` 
> doesn't make sense.

The scenario I had in mind was legacy code which was written back when it still 
was the "rule of three" instead of the "rule of five". Those classes with 
defined destructor and copy operations are still perfectly safe, because moving 
them falls back to copying. They may not be perfectly tuned for performance, 
but having no move operations is not an indication for some resoure management 
error. That's why I do think this options makes sense.

> I'd like an `AllowDeletedCopyFunctions` option that allows move and 
> destructor functions to be missing when copying is disabled.
> 
>   struct A {
> A(const A&) = delete;
> A& operator=(const A&) = delete;
>   }
>

My 2 cents on this one: Even with `AllowMissingMoveFunctions=1` at least the 
missing destructor definition should be diagnosed, because it violates the 
classic rule of three. If you delete your copy operations, you likely have some 
resources that need to be taken care of in your destructor, so this piece of 
code would worry me. Better be clear about it and explicitly default the 
destructor.


https://reviews.llvm.org/D30610



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

[PATCH] D38425: [clangd] Document highlights for clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

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



Comment at: test/clangd/documenthighlight.test:1
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.

we should test that we can get kind == 1, 2, 3



Comment at: test/clangd/documenthighlight.test:16
+# Go to local variable
+# CHECK: 
{"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":12,"line":16},"start":{"character":4,"line":16}}},{"kind":0,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}}]}
+

kind 0 is invalid



Comment at: test/clangd/documenthighlight.test:23
+# Go to local variable
+# CHECK: 
{"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":9,"line":12},"start":{"character":4,"line":12}}},{"kind":1,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":216,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":220,"range":{"end":{"character":8,"line":19},"start":{"character":4,"line":19}}}]}
+

kind 216? 220?



Comment at: test/clangd/documenthighlight.test:29
+# Go to local variable
+# CHECK: 
{"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":22,"line":4},"start":{"character":5,"line":4}}},{"kind":0,"range":{"end":{"character":25,"line":21},"start":{"character":8,"line":21}}}]}
+

kind 0 is invalid


https://reviews.llvm.org/D38425



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

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 124235.
Rakete added a comment.

Added a test for the switch statement and added full context to the diff.


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -10,10 +11,41 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int ifInitStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+
+int switchInitStatement() {
+  switch (int I = 1; I) {
+  case 1:
+return I;
+  }
+
+  int Var = 0;
+  switch (Var + Var; Var) {
+  case 0:
+return Var;
+  }
+
+  switch (; 0) {
+  case 0:
+return 0;
+  }
+}
+#endif
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -10,10 +11,41 @@
   }
 }
 
-
 void h() {
   if (int x = f()) // expected-note 2{{previous definition}}
 int x; // expected-error{{redefinition of 'x'}}
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int ifInitStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+
+int switchInitStatement() {
+  switch (int I = 1; I) {
+  case 1:
+return I;
+  }
+
+  int Var = 0;
+  switch (Var + Var; Var) {
+  case 0:
+return Var;
+  }
+
+  switch (; 0) {
+  case 0:
+return 0;
+  }
+}
+#endif
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Hi, thanks for working on this!
Can you add tests to make sure that this also works with switch statements 
(which also have this bug), and not with while?  Also, it makes it a lot easier 
to review these patches if you add context lines to the diff.
Thanks,
Erik




Comment at: lib/Parse/ParseExprCXX.cpp:1750
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);

We should only be doing this if InitStmt != nullptr, right? I think this would 
lead us to be too permissive with while statements, which don't have this 
feature. Also, it would be nice to emit a c++14-compat warning here, like below.


https://reviews.llvm.org/D40445



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39722#934783, @aaron.ballman wrote:

> In https://reviews.llvm.org/D39722#934781, @tk1012 wrote:
>
> > Hello Aaron,
> >
> > I remove the semicolon.
> >
> > > Is this type actually correct for C++?
> >
> > Yes, it is.
> >  clang generates the AST for `declToImport` struct like this.
> >
> >   |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct 
> > declToImport
> >   `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
> > `-CompoundStmt 0x8b1a1e8 
> >   `-TypeTraitExpr 0x8b1a1c8  '_Bool'
> >   
>
>
> Hmm, that looks like a bug to me; but not one that should impact this review.


Yeah, it seems as though the ASTDumper does not receive a printing policy, and 
the default behavior of `QualType::getAsString()` is to gin up a very dumb 
printing policy from default language options, hence the poor quality of the 
name here.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D39722#934781, @tk1012 wrote:

> Hello Aaron,
>
> I remove the semicolon.
>
> > Is this type actually correct for C++?
>
> Yes, it is.
>  clang generates the AST for `declToImport` struct like this.
>
>   |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct 
> declToImport
>   `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
> `-CompoundStmt 0x8b1a1e8 
>   `-TypeTraitExpr 0x8b1a1c8  '_Bool'
>   


Hmm, that looks like a bug to me; but not one that should impact this review.

LGTM!


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 124232.
tk1012 added a comment.

Hello Aaron,

I remove the semicolon.

> Is this type actually correct for C++?

Yes, it is.
clang generates the AST for `declToImport` struct like this.

  |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct declToImport
  `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
`-CompoundStmt 0x8b1a1e8 
  `-TypeTraitExpr 0x8b1a1c8  '_Bool'




https://reviews.llvm.org/D39722

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); }"
+ "};"
+ "void f() { declToImport().m(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(
+   has(
+ cxxRecordDecl(
+   has(
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(
+   hasType(asString("_Bool"))
+   )));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+   

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 124231.
hamzasood added a comment.

Good idea, I've added that to the test.
I'll give Richard some time to look over this before committing.


https://reviews.llvm.org/D40443

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/export-kw.cpp
  
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp

Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -fmodules-ts %s -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -verify -emit-module-interface -o %t
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -verify -fmodule-file=%t -o /dev/null
-//
-// RUN: %clang_cc1 -fmodules-ts %s -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-
-#if INTERFACE
-// expected-no-diagnostics
-export module A;
-#elif IMPLEMENTATION
-module A;
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@-2 {{missing 'export' specifier in module declaration while building module interface}}
-  #define INTERFACE
- #endif
-#else
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@1 {{missing 'export module' declaration in module interface unit}}
- #endif
-#endif
-
-#ifndef INTERFACE
-export int b; // expected-error {{export declaration can only be used within a module interface unit}}
-#else
-export int a;
-#endif
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module a; export class A { };' > %t/a.cppm
+// RUN: echo 'export module b; export class B { };' > %t/b.cppm
+//
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/b.cppm -o %t/b.pcm
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/test.pcm -DTEST_INTERFACE
+//
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DTEST_IMPLEMENTATION
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DOTHER_TU
+
+
+#ifdef TEST_INTERFACE
+import a;
+export module test;
+import b;
+#endif
+
+#ifdef TEST_IMPLEMENTATION
+module test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+// Module b is imported within the purview of this module's interface unit.
+// So its exported definitions should be visible here.
+B b;
+#endif
+
+
+#ifdef OTHER_TU
+import test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+B b; // expected-error {{must be imported from module 'b'}}
+ // expected-n...@b.cppm:1 {{here}}
+#endif
Index: test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
===
--- test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
+++ test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
@@ -10,9 +10,7 @@
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N.pcm -DMODULE_INTERFACE -DNO_ERRORS
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -verify
-// FIXME: Once we start importing "import" declarations properly, this should
-// be rejected (-verify should be added to the following line).
-// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT
+// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT -verify
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N-no-M.pcm -DMODULE_INTERFACE -DNO_ERRORS -DNO_IMPORT
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N-no-M.pcm -verify
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16308,6 +16308,15 @@
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
 
+  // Modules TS + p0731r0 [dcl.module.interface]p1:
+  

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124230.
Nebiroth marked 6 inline comments as done.
Nebiroth added a comment.

Fixed a few outstanding issues that were reported as completed


https://reviews.llvm.org/D38425

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/documenthighlight.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/documenthighlight.test
===
--- /dev/null
+++ test/clangd/documenthighlight.test
@@ -0,0 +1,37 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 455
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();}\n"}}}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":12,"line":16},"start":{"character":4,"line":16}}},{"kind":0,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}}]}
+
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":9,"line":12},"start":{"character":4,"line":12}}},{"kind":1,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":216,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":220,"range":{"end":{"character":8,"line":19},"start":{"character":4,"line":19}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":22,"line":4},"start":{"character":5,"line":4}}},{"kind":0,"range":{"end":{"character":25,"line":21},"start":{"character":8,"line":21}}}]}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}			
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -54,6 +54,8 @@
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams ) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams ) = 0;
   virtual void onRename(Ctx C, RenameParams ) = 0;
+  virtual void onDocumentHighlight(Ctx C,
+   TextDocumentPositionParams ) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher , JSONOutput ,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -74,4 

[clang-tools-extra] r318961 - Fix MSVC double-float implicit truncation warning. NFCI

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Nov 24 10:18:42 2017
New Revision: 318961

URL: http://llvm.org/viewvc/llvm-project?rev=318961=rev
Log:
Fix MSVC double-float implicit truncation warning. NFCI

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

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=318961=318960=318961=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Nov 24 10:18:42 2017
@@ -406,7 +406,7 @@ private:
   // No penalty.
   break;
 case CXAvailability_Deprecated:
-  Score *= 0.1;
+  Score *= 0.1f;
   break;
 case CXAvailability_NotAccessible:
 case CXAvailability_NotAvailable:


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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: unittests/AST/ASTImporterTest.cpp:553
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); };"
+ "};"

Drop the spurious semicolon at the end of the function definition.



Comment at: unittests/AST/ASTImporterTest.cpp:566
+ typeTraitExpr(
+   hasType(asString("_Bool"))
+   )));

Is this type actually correct for C++? I would expect that for C code, but not 
for C++.


https://reviews.llvm.org/D39722



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

[PATCH] D40435: [clang-format] Deduplicate using declarations

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318960: [clang-format] Deduplicate using declarations 
(authored by krasimir).

Repository:
  rL LLVM

https://reviews.llvm.org/D40435

Files:
  cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
  cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp


Index: cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
===
--- cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
+++ cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
@@ -130,7 +130,27 @@
   UsingDeclarations->begin(), UsingDeclarations->end());
   std::stable_sort(SortedUsingDeclarations.begin(),
SortedUsingDeclarations.end());
+  SortedUsingDeclarations.erase(
+  std::unique(SortedUsingDeclarations.begin(),
+  SortedUsingDeclarations.end(),
+  [](const UsingDeclaration , const UsingDeclaration ) {
+return a.Label == b.Label;
+  }),
+  SortedUsingDeclarations.end());
   for (size_t I = 0, E = UsingDeclarations->size(); I < E; ++I) {
+if (I >= SortedUsingDeclarations.size()) {
+  // This using declaration has been deduplicated, delete it.
+  auto Begin =
+  (*UsingDeclarations)[I].Line->First->WhitespaceRange.getBegin();
+  auto End = (*UsingDeclarations)[I].Line->Last->Tok.getEndLoc();
+  auto Range = CharSourceRange::getCharRange(Begin, End);
+  auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, ""));
+  if (Err) {
+llvm::errs() << "Error while sorting using declarations: "
+ << llvm::toString(std::move(Err)) << "\n";
+  }
+  continue;
+}
 if ((*UsingDeclarations)[I].Line == SortedUsingDeclarations[I].Line)
   continue;
 auto Begin = (*UsingDeclarations)[I].Line->First->Tok.getLocation();
Index: cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
+++ cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -142,20 +142,16 @@
 
 TEST_F(UsingDeclarationsSorterTest, SortsStably) {
   EXPECT_EQ("using a;\n"
-"using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;",
 sortUsingDeclarations("using a;\n"
@@ -355,6 +351,25 @@
   "/* comment */ using a;"));
 }
 
+TEST_F(UsingDeclarationsSorterTest, DeduplicatesUsingDeclarations) {
+  EXPECT_EQ("using a;\n"
+"using b;\n"
+"using c;\n"
+"\n"
+"using a;\n"
+"using e;",
+sortUsingDeclarations("using c;\n"
+  "using a;\n"
+  "using b;\n"
+  "using a;\n"
+  "using b;\n"
+  "\n"
+  "using e;\n"
+  "using a;\n"
+  "using e;"));
+
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


Index: cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
===
--- cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
+++ cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
@@ -130,7 +130,27 @@
   UsingDeclarations->begin(), UsingDeclarations->end());
   std::stable_sort(SortedUsingDeclarations.begin(),
SortedUsingDeclarations.end());
+  SortedUsingDeclarations.erase(
+  std::unique(SortedUsingDeclarations.begin(),
+  SortedUsingDeclarations.end(),
+  [](const UsingDeclaration , const UsingDeclaration ) {
+return a.Label == b.Label;
+  }),
+  SortedUsingDeclarations.end());
   for (size_t I = 0, E = UsingDeclarations->size(); I < E; ++I) {
+if (I >= SortedUsingDeclarations.size()) {
+  // This using declaration has been deduplicated, delete it.
+  auto Begin =
+  (*UsingDeclarations)[I].Line->First->WhitespaceRange.getBegin();
+  auto End = (*UsingDeclarations)[I].Line->Last->Tok.getEndLoc();
+  auto Range = CharSourceRange::getCharRange(Begin, End);
+  auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, ""));
+  if (Err) {
+llvm::errs() << "Error while sorting using declarations: "
+ << llvm::toString(std::move(Err)) << "\n";
+  }
+  continue;
+}
 if 

r318960 - [clang-format] Deduplicate using declarations

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Fri Nov 24 10:00:01 2017
New Revision: 318960

URL: http://llvm.org/viewvc/llvm-project?rev=318960=rev
Log:
[clang-format] Deduplicate using declarations

Summary: This deduplicated equivalent using declarations within a block.

Reviewers: bkramer

Reviewed By: bkramer

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp

Modified: cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp?rev=318960=318959=318960=diff
==
--- cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp (original)
+++ cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp Fri Nov 24 10:00:01 2017
@@ -130,7 +130,27 @@ void endUsingDeclarationBlock(
   UsingDeclarations->begin(), UsingDeclarations->end());
   std::stable_sort(SortedUsingDeclarations.begin(),
SortedUsingDeclarations.end());
+  SortedUsingDeclarations.erase(
+  std::unique(SortedUsingDeclarations.begin(),
+  SortedUsingDeclarations.end(),
+  [](const UsingDeclaration , const UsingDeclaration ) {
+return a.Label == b.Label;
+  }),
+  SortedUsingDeclarations.end());
   for (size_t I = 0, E = UsingDeclarations->size(); I < E; ++I) {
+if (I >= SortedUsingDeclarations.size()) {
+  // This using declaration has been deduplicated, delete it.
+  auto Begin =
+  (*UsingDeclarations)[I].Line->First->WhitespaceRange.getBegin();
+  auto End = (*UsingDeclarations)[I].Line->Last->Tok.getEndLoc();
+  auto Range = CharSourceRange::getCharRange(Begin, End);
+  auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, ""));
+  if (Err) {
+llvm::errs() << "Error while sorting using declarations: "
+ << llvm::toString(std::move(Err)) << "\n";
+  }
+  continue;
+}
 if ((*UsingDeclarations)[I].Line == SortedUsingDeclarations[I].Line)
   continue;
 auto Begin = (*UsingDeclarations)[I].Line->First->Tok.getLocation();

Modified: cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp?rev=318960=318959=318960=diff
==
--- cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp (original)
+++ cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp Fri Nov 24 
10:00:01 2017
@@ -142,7 +142,6 @@ TEST_F(UsingDeclarationsSorterTest, Usin
 
 TEST_F(UsingDeclarationsSorterTest, SortsStably) {
   EXPECT_EQ("using a;\n"
-"using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
@@ -151,11 +150,8 @@ TEST_F(UsingDeclarationsSorterTest, Sort
 "using a;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;",
 sortUsingDeclarations("using a;\n"
@@ -355,6 +351,25 @@ TEST_F(UsingDeclarationsSorterTest, Sort
   "/* comment */ using a;"));
 }
 
+TEST_F(UsingDeclarationsSorterTest, DeduplicatesUsingDeclarations) {
+  EXPECT_EQ("using a;\n"
+"using b;\n"
+"using c;\n"
+"\n"
+"using a;\n"
+"using e;",
+sortUsingDeclarations("using c;\n"
+  "using a;\n"
+  "using b;\n"
+  "using a;\n"
+  "using b;\n"
+  "\n"
+  "using e;\n"
+  "using a;\n"
+  "using e;"));
+
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


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

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D30610#934704, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D30610#934617, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D30610#934452, @malcolm.parsons wrote:
> >
> > > I'd like an `AllowDeletedCopyFunctions` option that allows move and 
> > > destructor functions to be missing when copying is disabled.
> > >
> > >   struct A {
> > > A(const A&) = delete;
> > > A& operator=(const A&) = delete;
> > >   }
> > >   
> >
> >
> > Doesn't `AllowMissingMoveFunctions` do almost that? If not, it should -- 
> > that code produces a class that does not declare a move constructor or move 
> > assignment operator per [class.copy]p8 and [class.copy.assign]p4, so that 
> > would be a "missing move function". Granted, that doesn't handle the dtor 
> > case, but I think an `AllowMissingDestructor` option might be overkill -- 
> > the destructor isn't missing, it's implicitly declared as defaulted in that 
> > case, but if the C++ Core Guidelines folks want it spelled out explicitly 
> > then, it might be worth the option. Have they weighed in on your exception?
>
>
> The check is about providing a consistent set of special member functions.


Understood.

> If you've written a destructor then you probably need to write copy functions 
> to avoid double free.

Sometimes. RAII is a good example where that's not always true. However, in the 
general case, I agree.

> If you've defaulted the destructor, as often needed in a base class, then the 
> default copy functions are still valid, so `AllowSoleDefaultDtor` makes sense.

Agreed.

> If you've written your own copy functions then you probably want to write 
> your own move functions to allow moving, so `AllowMissingMoveFunctions` 
> doesn't make sense.

I disagree. Move constructors can gracefully degenerate into a copy operation 
when the copy and move semantics are identical. Think about classes that only 
contain scalar values as a common example of this. `AllowMissingMoveFunctions` 
is the option which serves that common purpose.

> If you've deleted the copy functions, then you probably don't want the move 
> functions either, so `AllowDeletedCopyFunctions` would make sense.

I agree with the premise, but disagree with your conclusion. If you've deleted 
the copy functions *you don't have the move functions* unless you explicitly 
spell them out (they are not implicitly generated for you). To that end, I 
don't think this check should diagnose the lack of move operations in the 
presence of deleted copy operations. However, I'd like to know what the Core 
Guidelines people think about this scenario. If the copy operations are 
explicitly deleted, the primary difference between the default behavior and 
explicitly deleted move operations is that the diagnostics change from "no such 
operator" to "explicitly deleted operator". Do they think that's worth the 
extra typing for the user?


https://reviews.llvm.org/D30610



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

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124224.
Nebiroth marked an inline comment as done.
Nebiroth added a comment.

Getting DocumentHighlightKind is now done in DocumentHighlightsFinder
Removed duplicated and unused code
Refactored method and variable names


https://reviews.llvm.org/D38425

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/documenthighlight.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/documenthighlight.test
===
--- /dev/null
+++ test/clangd/documenthighlight.test
@@ -0,0 +1,37 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 455
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();}\n"}}}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":12,"line":16},"start":{"character":4,"line":16}}},{"kind":0,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}}]}
+
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":9,"line":12},"start":{"character":4,"line":12}}},{"kind":1,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":216,"range":{"end":{"character":21,"line":18},"start":{"character":17,"line":18}}},{"kind":220,"range":{"end":{"character":8,"line":19},"start":{"character":4,"line":19}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":22,"line":4},"start":{"character":5,"line":4}}},{"kind":0,"range":{"end":{"character":25,"line":21},"start":{"character":8,"line":21}}}]}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}			
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -54,6 +54,8 @@
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams ) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams ) = 0;
   virtual void onRename(Ctx C, RenameParams ) = 0;
+  virtual void onDocumentHighlight(Ctx C,
+   TextDocumentPositionParams ) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher , JSONOutput ,
Index: clangd/ProtocolHandlers.cpp

[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete created this revision.
Rakete added a project: clang.

This fixes PR35381 .


https://reviews.llvm.org/D40445

Files:
  lib/Parse/ParseExprCXX.cpp
  test/CXX/stmt.stmt/stmt.select/p3.cpp


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -17,3 +18,18 @@
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int initStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+#endif
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())


Index: test/CXX/stmt.stmt/stmt.select/p3.cpp
===
--- test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s -DCXX17
 
 int f();
 
@@ -17,3 +18,18 @@
   else
 int x; // expected-error{{redefinition of 'x'}}
 }
+
+#ifdef CXX17
+int initStatement() {
+  if (int I = 0; ++I == 1)
+return I;
+
+  int Var = 0;
+  if (Var + Var; Var == 0)
+return Var;
+
+  if (; true)
+return 1;
+}
+#endif
+
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1745,6 +1745,11 @@
   case ConditionOrInitStatement::Expression: {
 ProhibitAttributes(attrs);
 
+// We can have an empty expression here.
+//   if (; true);
+if (TryConsumeToken(tok::semi))
+  return ParseCXXCondition(nullptr, Loc, CK);
+
 // Parse the expression.
 ExprResult Expr = ParseExpression(); // expression
 if (Expr.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

[Boris Kolpackov via Phabricator via cfe-commits]

boris added a comment.

LGTM. Maybe it makes sense to also test that an unrelated translation unit that 
imports module 'test' sees neither 'a' nor 'b'.


https://reviews.llvm.org/D40443



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

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D30610#934617, @aaron.ballman wrote:

> In https://reviews.llvm.org/D30610#934452, @malcolm.parsons wrote:
>
> > I'd like an `AllowDeletedCopyFunctions` option that allows move and 
> > destructor functions to be missing when copying is disabled.
> >
> >   struct A {
> > A(const A&) = delete;
> > A& operator=(const A&) = delete;
> >   }
> >   
>
>
> Doesn't `AllowMissingMoveFunctions` do almost that? If not, it should -- that 
> code produces a class that does not declare a move constructor or move 
> assignment operator per [class.copy]p8 and [class.copy.assign]p4, so that 
> would be a "missing move function". Granted, that doesn't handle the dtor 
> case, but I think an `AllowMissingDestructor` option might be overkill -- the 
> destructor isn't missing, it's implicitly declared as defaulted in that case, 
> but if the C++ Core Guidelines folks want it spelled out explicitly then, it 
> might be worth the option. Have they weighed in on your exception?


The check is about providing a consistent set of special member functions.
If you've written a destructor then you probably need to write copy functions 
to avoid double free.
If you've defaulted the destructor, as often needed in a base class, then the 
default copy functions are still valid, so `AllowSoleDefaultDtor` makes sense.
If you've written your own copy functions then you probably want to write your 
own move functions to allow moving, so `AllowMissingMoveFunctions` doesn't make 
sense.
If you've deleted the copy functions, then you probably don't want the move 
functions either, so `AllowDeletedCopyFunctions` would make sense.

I haven't asked the Core Guidelines folks.


https://reviews.llvm.org/D30610



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

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth marked 31 inline comments as done.
Nebiroth added inline comments.



Comment at: clangd/ClangdServer.h:291
+  /// Get document highlights for a symbol hovered on.
+  Tagged findDocumentHighlights(PathRef File,
+Position Pos);

malaperle wrote:
> can this thing fail? Should it be Expectedhttps://reviews.llvm.org/D38425



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

r318959 - clang-format: [JS] do not break in ArrayType[].

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Nov 24 09:05:56 2017
New Revision: 318959

URL: http://llvm.org/viewvc/llvm-project?rev=318959=rev
Log:
clang-format: [JS] do not break in ArrayType[].

Summary:
Wrapping between the type name and the array type indicator creates
invalid syntax in TypeScript.

Before:
const xIsALongIdent:
YJustBarelyFitsLinex
[];  // illegal syntax.

After:
const xIsALongIdent:
YJustBarelyFitsLinex[];

Reviewers: djasper

Subscribers: klimek

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=318959=318958=318959=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Nov 24 09:05:56 2017
@@ -2706,6 +2706,9 @@ bool TokenAnnotator::canBreakBefore(cons
 Keywords.kw_readonly, Keywords.kw_abstract,
 Keywords.kw_get, Keywords.kw_set))
   return false; // Otherwise automatic semicolon insertion would trigger.
+if (Left.Tok.getIdentifierInfo() &&
+Right.startsSequence(tok::l_square, tok::r_square))
+  return false;  // breaking in "foo[]" creates illegal TS type syntax.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;
 if (Left.is(TT_JsTypeColon))

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=318959=318958=318959=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Nov 24 09:05:56 2017
@@ -1426,6 +1426,8 @@ TEST_F(FormatTestJS, TypeAnnotations) {
   verifyFormat(
   "var someValue = (v as [])\n"
   ".someFunction(aa);");
+  verifyFormat("const xIsALongIdent:\n""YJustBarelyFitsLinex[];",
+  getGoogleJSStyleWithColumns(20));
 }
 
 TEST_F(FormatTestJS, UnionIntersectionTypes) {


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

r318958 - clang-format: [JS] do not wrap before yield.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Nov 24 09:05:35 2017
New Revision: 318958

URL: http://llvm.org/viewvc/llvm-project?rev=318958=rev
Log:
clang-format: [JS] do not wrap before yield.

Summary: The same rules apply as for `return`.

Reviewers: djasper

Subscribers: klimek

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=318958=318957=318958=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Nov 24 09:05:35 2017
@@ -2699,8 +2699,8 @@ bool TokenAnnotator::canBreakBefore(cons
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 const FormatToken *NonComment = Right.getPreviousNonComment();
 if (NonComment &&
-NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
-tok::kw_throw, Keywords.kw_interface,
+NonComment->isOneOf(tok::kw_return, Keywords.kw_yield, 
tok::kw_continue,
+tok::kw_break, tok::kw_throw, 
Keywords.kw_interface,
 Keywords.kw_type, tok::kw_static, tok::kw_public,
 tok::kw_private, tok::kw_protected,
 Keywords.kw_readonly, Keywords.kw_abstract,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=318958=318957=318958=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Nov 24 09:05:35 2017
@@ -1123,6 +1123,7 @@ TEST_F(FormatTestJS, WrapRespectsAutomat
   // would change due to automatic semicolon insertion.
   // See http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.1.
   verifyFormat("return a;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("yield a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("return /* hello! */ a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("continue a;", getGoogleJSStyleWithColumns(10));
   verifyFormat("continue /* hello! */ a;", 
getGoogleJSStyleWithColumns(10));


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

r318957 - clang-format: [JS] space between ! assert and in.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Nov 24 09:04:40 2017
New Revision: 318957

URL: http://llvm.org/viewvc/llvm-project?rev=318957=rev
Log:
clang-format: [JS] space between ! assert and in.

Summary:
Before:
x = y!in z;
After:
x = y! in z;

Reviewers: djasper

Subscribers: klimek

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=318957=318956=318957=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Nov 24 09:04:40 2017
@@ -2433,8 +2433,9 @@ bool TokenAnnotator::spaceRequiredBefore
   return false;
 if (Right.is(TT_JsNonNullAssertion))
   return false;
-if (Left.is(TT_JsNonNullAssertion) && Right.is(Keywords.kw_as))
-  return true; // "x! as string"
+if (Left.is(TT_JsNonNullAssertion) &&
+Right.isOneOf(Keywords.kw_as, Keywords.kw_in))
+  return true; // "x! as string", "x! in y"
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=318957=318956=318957=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Nov 24 09:04:40 2017
@@ -1908,6 +1908,7 @@ TEST_F(FormatTestJS, CastSyntax) {
   verifyFormat("x = x as {a: string};");
   verifyFormat("x = x as (string);");
   verifyFormat("x = x! as (string);");
+  verifyFormat("x = y! in z;");
   verifyFormat("var x = something.someFunction() as\n"
"something;",
getGoogleJSStyleWithColumns(40));


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

[PATCH] D40424: clang-format: [JS] handle semis in generic types.

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D40424



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

[PATCH] D40435: [clang-format] Deduplicate using declarations

[Benjamin Kramer via Phabricator via cfe-commits]

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

lg


https://reviews.llvm.org/D40435



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

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.

This provides an implementation for the changes outlined in section 4.1 of 
P0731r0, which clarifies the intended behaviour regarding implementation units 
being able to see imports made within their corresponding interface unit.


https://reviews.llvm.org/D40443

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/export-kw.cpp
  
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp

Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -fmodules-ts %s -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -verify -emit-module-interface -o %t
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -verify -fmodule-file=%t -o /dev/null
-//
-// RUN: %clang_cc1 -fmodules-ts %s -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-
-#if INTERFACE
-// expected-no-diagnostics
-export module A;
-#elif IMPLEMENTATION
-module A;
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@-2 {{missing 'export' specifier in module declaration while building module interface}}
-  #define INTERFACE
- #endif
-#else
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@1 {{missing 'export module' declaration in module interface unit}}
- #endif
-#endif
-
-#ifndef INTERFACE
-export int b; // expected-error {{export declaration can only be used within a module interface unit}}
-#else
-export int a;
-#endif
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module a; export class A { };' > %t/a.cppm
+// RUN: echo 'export module b; export class B { };' > %t/b.cppm
+//
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/b.cppm -o %t/b.pcm
+//
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/test.pcm -DINTERFACE
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify
+
+#ifdef INTERFACE
+
+import a;
+export module test;
+import b;
+
+#else // IMPLEMENTATION
+
+module test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+// Module b is imported within the purview of this module's interface unit.
+// So its exported definitions should be visible here.
+B b;
+
+#endif
Index: test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
===
--- test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
+++ test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
@@ -10,9 +10,7 @@
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N.pcm -DMODULE_INTERFACE -DNO_ERRORS
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -verify
-// FIXME: Once we start importing "import" declarations properly, this should
-// be rejected (-verify should be added to the following line).
-// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT
+// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT -verify
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N-no-M.pcm -DMODULE_INTERFACE -DNO_ERRORS -DNO_IMPORT
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N-no-M.pcm -verify
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16308,6 +16308,15 @@
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
 
+  // Modules TS + p0731r0 [dcl.module.interface]p1:
+  //   Every name of an entity with linkage other than internal linkage made
+  //   visible in the purview of the module interface unit of a module M is
+  //   visible in the purview of all module implementation units of M.
+  if (MDK == ModuleDeclKind::Implementation) {
+for (Module 

[PATCH] D40435: [clang-format] Deduplicate using declarations

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 124215.
krasimir added a comment.

- Address review comments


https://reviews.llvm.org/D40435

Files:
  lib/Format/UsingDeclarationsSorter.cpp
  unittests/Format/UsingDeclarationsSorterTest.cpp


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -142,20 +142,16 @@
 
 TEST_F(UsingDeclarationsSorterTest, SortsStably) {
   EXPECT_EQ("using a;\n"
-"using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;",
 sortUsingDeclarations("using a;\n"
@@ -355,6 +351,25 @@
   "/* comment */ using a;"));
 }
 
+TEST_F(UsingDeclarationsSorterTest, DeduplicatesUsingDeclarations) {
+  EXPECT_EQ("using a;\n"
+"using b;\n"
+"using c;\n"
+"\n"
+"using a;\n"
+"using e;",
+sortUsingDeclarations("using c;\n"
+  "using a;\n"
+  "using b;\n"
+  "using a;\n"
+  "using b;\n"
+  "\n"
+  "using e;\n"
+  "using a;\n"
+  "using e;"));
+
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -130,7 +130,27 @@
   UsingDeclarations->begin(), UsingDeclarations->end());
   std::stable_sort(SortedUsingDeclarations.begin(),
SortedUsingDeclarations.end());
+  SortedUsingDeclarations.erase(
+  std::unique(SortedUsingDeclarations.begin(),
+  SortedUsingDeclarations.end(),
+  [](const UsingDeclaration , const UsingDeclaration ) {
+return a.Label == b.Label;
+  }),
+  SortedUsingDeclarations.end());
   for (size_t I = 0, E = UsingDeclarations->size(); I < E; ++I) {
+if (I >= SortedUsingDeclarations.size()) {
+  // This using declaration has been deduplicated, delete it.
+  auto Begin =
+  (*UsingDeclarations)[I].Line->First->WhitespaceRange.getBegin();
+  auto End = (*UsingDeclarations)[I].Line->Last->Tok.getEndLoc();
+  auto Range = CharSourceRange::getCharRange(Begin, End);
+  auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, ""));
+  if (Err) {
+llvm::errs() << "Error while sorting using declarations: "
+ << llvm::toString(std::move(Err)) << "\n";
+  }
+  continue;
+}
 if ((*UsingDeclarations)[I].Line == SortedUsingDeclarations[I].Line)
   continue;
 auto Begin = (*UsingDeclarations)[I].Line->First->Tok.getLocation();


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -142,20 +142,16 @@
 
 TEST_F(UsingDeclarationsSorterTest, SortsStably) {
   EXPECT_EQ("using a;\n"
-"using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using A;\n"
 "using a;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;\n"
-"using b;\n"
-"using b;\n"
 "using B;\n"
 "using b;",
 sortUsingDeclarations("using a;\n"
@@ -355,6 +351,25 @@
   "/* comment */ using a;"));
 }
 
+TEST_F(UsingDeclarationsSorterTest, DeduplicatesUsingDeclarations) {
+  EXPECT_EQ("using a;\n"
+"using b;\n"
+"using c;\n"
+"\n"
+"using a;\n"
+"using e;",
+sortUsingDeclarations("using c;\n"
+  "using a;\n"
+  "using b;\n"
+  "using a;\n"
+  "using b;\n"
+  "\n"
+  "using e;\n"
+  "using a;\n"
+  "using e;"));
+
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang

[PATCH] D40381: Parse concept definition

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: include/clang/Sema/Sema.h:6194
+SourceLocation TemplateLoc,
+const TemplateArgumentListInfo *TemplateArgs);
+

Indentation issue here too.



Comment at: lib/Parse/ParseTemplate.cpp:57
 ///
+///   template-declaration: [Concepts TS]
+/// template-head declaration

C++2a Concepts



Comment at: lib/Parse/ParseTemplate.cpp:374
+
+  ExprResult ConstraintExpr = ParseConstraintExpression();
+

saar.raz wrote:
> Add a check to ParseConstraintExpression that the type is either dependent or 
> bool, and add an apropriate diagnostic.
> 
> ```
> if (!ConstraintExpr->isTypeDependent() && ConstraintExpr->getType() != 
> Context.BoolTy) {
>   Diag(Init->getSourceRange().getBegin(),
>diag::err_concept_initialized_with_non_bool_type) << 
> Init->getType();
>   Concept->setInvalidDecl();
>   return;
> }
> ```
I think that would still need a TODO to instead walk the constraint expression 
for atomic constraints and diagnose those.
```
template 
concept C = 1 || T::value; // error
```



Comment at: lib/Sema/SemaTemplate.cpp:3903
+  // /*FoundD=*/nullptr, TemplateArgs);
+  return Template->getConstraintExpr();
+  return true;

Add more comments here. It looks like this allows us to get id-expressions 
naming concepts defined with non-dependent `bool` constraint expressions to 
"work" for now?



Comment at: test/Parser/cxx-concept-declaration.cpp:5
 // RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
-// template concept C1 = true;
+template concept C1 = true;
+

Should add tests to prevent redeclaring concept names as either a "normal" 
(e.g., variable) or tag name.
Also the reverse for redeclaring a tag name as a concept.

Should add tests to prevent multiple definitions of the same concept.
Should eventually add tests to prevent explicit specialization, partial 
specialization and explicit instantiation of concepts: while the restriction is 
syntactic in the wording, that does not necessarily translate directly to the 
implementation strategy. In any case, we may discover that we want a nicer 
message.

Should add tests to prevent defining concepts at class scope.



https://reviews.llvm.org/D40381



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

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D30610#934452, @malcolm.parsons wrote:

> I'd like an `AllowDeletedCopyFunctions` option that allows move and 
> destructor functions to be missing when copying is disabled.
>
>   struct A {
> A(const A&) = delete;
> A& operator=(const A&) = delete;
>   }
>   


Doesn't `AllowMissingMoveFunctions` do almost that? If not, it should -- that 
code produces a class that does not declare a move constructor or move 
assignment operator per [class.copy]p8 and [class.copy.assign]p4, so that would 
be a "missing move function". Granted, that doesn't handle the dtor case, but I 
think an `AllowMissingDestructor` option might be overkill -- the destructor 
isn't missing, it's implicitly declared as defaulted in that case, but if the 
C++ Core Guidelines folks want it spelled out explicitly then, it might be 
worth the option. Have they weighed in on your exception?


https://reviews.llvm.org/D30610



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

[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:694
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (BlockFront.getKind() == CFGElement::Statement) {

MTC wrote:
> dcoughlin wrote:
> > MTC wrote:
> > > szepet wrote:
> > > > I think it would be more correct to use the location what is used in 
> > > > case of the BlockEdge. (So on the entranced block terminator 
> > > > condition.) The reason is because the BlockEntrance display message 
> > > > will be displayed before the message of the BlockEdge (since it is an 
> > > > "earlier" node in the ExplodedGraph). So it would result that if check 
> > > > these notes in a viewer then the earlier note would belong to the later 
> > > > location which could be confusing.
> > > Yes, it would be better to use the location of the TerminatorCondition :D.
> > Thanks for looking into fixing this.
> > 
> > I don't think using the terminator condition of the entered block is the 
> > right thing to do. The terminator is at the *end* of a basic block and this 
> > program point represents the entrance to the block. I think it is better to 
> > use the location corresponding to the first element in in the entered block.
> Thank you, dcoughlin!
> 
> I have updated the diff.  The first element  kind I can think of is only 
> `Stmt` and `NewAllocator`. I don't know if it's enough? 
> 
It would be good to add an llvm_unreachable() assertion expressing this 
expectation so that we'll get an assertion failure if this turns out not to be 
enough. (Or if we add a new CFGElement kind in the future).



Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:695
+CFGElement BlockFront = BE->getBlock()->front();
+if (BlockFront.getKind() == CFGElement::Kind::Statement) {
+  return PathDiagnosticLocation(

You can use `getAs<>` in the `if` guard condition to make this more concise:

```
if (auto StmtElt = BlockFront.getAs()) {
  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
}
```


https://reviews.llvm.org/D37187



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

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir removed a reviewer: krasimir.
krasimir accepted this revision.
krasimir added a comment.
This revision is now accepted and ready to land.

Thank you! Do you have commit access?


Repository:
  rL LLVM

https://reviews.llvm.org/D40288



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

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for the review! I think you're right about new names.




Comment at: clangd/JSONExpr.h:368
+// Typed accessors return None/nullptr if the element has the wrong type.
+llvm::Optional null(size_t I) const {
+  return (*this)[I].null();

sammccall wrote:
> ioeric wrote:
> > Why is this needed? `v[x].null()` seems to be more intuitive than 
> > `v.null(x)`.
> In the overwhelmingly common case when parsing, v is a pointer, because it 
> resulted from a call to array().
> 
> So it's `(*v)[x].null()` vs `v->null(x)` - I think the latter is slightly 
> more readable.
> 
> But more compelling to me is having consistency between object/array.
> 
> We *can* live without these if you like, though - most of the time we iterate 
> over arrays rather than indexed access. 
I've kept these for now, for readability and symmetry.



Comment at: unittests/clangd/JSONExprTests.cpp:203
+
+  EXPECT_FALSE(O->null("missing"));
+  EXPECT_FALSE(O->null("boolean"));

sammccall wrote:
> ioeric wrote:
> > It's not very obvious that this accesses a KV and converts the value, by 
> > only reading this line. Would it make sense to make the APIs more explicit 
> > e.g. `get_as_xxx(...)`? 
> I think this is made worse because it's an artificial example.
> I've sent D40406 which has "real life" code - what do you think about the 
> getters there?
> 
> I think the best variant names would be `asString()` or `getString()`.
> 
> I'm a bit reluctant to add a fixed prefix to these common accessors as they 
> seem noisy. I also think we'd need to change all the names on Expr(), which 
> seems like a shame.
> 
> So I'm not sure about changing this, but curious what you think of the 
> Protocol conversion code.
After offline discussion, renamed to `Expr::asString()` and `obj::getString()`.
These aren't much longer and are more accessible to casual inspection.


https://reviews.llvm.org/D40399



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

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 124203.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Renames to address review comments


https://reviews.llvm.org/D40399

Files:
  clangd/ClangdUnit.cpp
  clangd/JSONExpr.cpp
  clangd/JSONExpr.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Trace.cpp
  clangd/Trace.h
  clangd/tool/ClangdMain.cpp
  test/clangd/trace.test
  unittests/clangd/JSONExprTests.cpp
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -116,7 +116,7 @@
   ASSERT_NE(++Event, Events->end()) << "Expected span start";
   EXPECT_TRUE(VerifyObject(*Event, {{"ph", "B"}, {"name", "A"}}));
   ASSERT_NE(++Event, Events->end()) << "Expected log message";
-  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "B"}}));
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "Log"}}));
   ASSERT_NE(++Event, Events->end()) << "Expected span end";
   EXPECT_TRUE(VerifyObject(*Event, {{"ph", "E"}}));
   ASSERT_EQ(++Event, Events->end());
Index: unittests/clangd/JSONExprTests.cpp
===
--- unittests/clangd/JSONExprTests.cpp
+++ unittests/clangd/JSONExprTests.cpp
@@ -167,7 +167,6 @@
   ExpectErr("Unexpected EOF", "");
   ExpectErr("Unexpected EOF", "[");
   ExpectErr("Text after end of document", "[][]");
-  ExpectErr("Text after end of document", "[][]");
   ExpectErr("Invalid bareword", "fuzzy");
   ExpectErr("Expected , or ]", "[2?]");
   ExpectErr("Expected object key", "{a:2}");
@@ -185,6 +184,49 @@
 })");
 }
 
+TEST(JSONTest, Inspection) {
+  llvm::Expected Doc = parse(R"(
+{
+  "null": null,
+  "boolean": false,
+  "number": 2.78,
+  "string": "json",
+  "array": [null, true, 3.14, "hello", [1,2,3], {"time": "arrow"}],
+  "object": {"fruit": "banana"}
+}
+  )");
+  EXPECT_TRUE(!!Doc);
+
+  obj *O = Doc->asObject();
+  ASSERT_TRUE(O);
+
+  EXPECT_FALSE(O->getNull("missing"));
+  EXPECT_FALSE(O->getNull("boolean"));
+  EXPECT_TRUE(O->getNull("null"));
+
+  EXPECT_EQ(O->getNumber("number"), llvm::Optional(2.78));
+  EXPECT_EQ(O->getString("string"), llvm::Optional("json"));
+  ASSERT_FALSE(O->getObject("missing"));
+  ASSERT_FALSE(O->getObject("array"));
+  ASSERT_TRUE(O->getObject("object"));
+  EXPECT_EQ(*O->getObject("object"), (obj{{"fruit", "banana"}}));
+
+  ary *A = O->getArray("array");
+  ASSERT_TRUE(A);
+  EXPECT_EQ(A->getBoolean(1), llvm::Optional(true));
+  ASSERT_TRUE(A->getArray(4));
+  EXPECT_EQ(*A->getArray(4), (ary{1, 2, 3}));
+  int I = 0;
+  for (Expr  : *A) {
+if (I++ == 5) {
+  ASSERT_TRUE(E.asObject());
+  EXPECT_EQ(E.asObject()->getString("time"),
+llvm::Optional("arrow"));
+} else
+  EXPECT_FALSE(E.asObject());
+  }
+}
+
 } // namespace
 } // namespace json
 } // namespace clangd
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -1,4 +1,4 @@
-# RUN: clangd -run-synchronously -trace %t < %s && FileCheck %s < %t
+# RUN: clangd -pretty -run-synchronously -trace %t < %s && FileCheck %s < %t
 # It is absolutely vital that this file has CRLF line endings.
 #
 Content-Length: 125
@@ -8,9 +8,19 @@
 Content-Length: 152
 
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
-# CHECK: "textDocument/didOpen"
-# CHECK: "Preamble: /foo.c"
-# CHECK: "Build: /foo.c"
+#  CHECK: {"displayTimeUnit":"ns","traceEvents":[
+# Start opening the doc.
+#  CHECK: "name": "textDocument/didOpen"
+#  CHECK: "ph": "E"
+# Start building the preamble.
+#  CHECK: "name": "Preamble"
+#  CHECK: },
+# Finish building the preamble, with filename.
+#  CHECK: "File": "/foo.c"
+# CHECK-NEXT: },
+# CHECK-NEXT: "ph": "E"
+# Start building the file.
+#  CHECK: "name": "Build"
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -124,7 +124,7 @@
   TraceFile.reset();
   llvm::errs() << "Error while opening trace file: " << EC.message();
 } else {
-  TraceSession = trace::Session::create(*TraceStream);
+  TraceSession = trace::Session::create(*TraceStream, PrettyPrint);
 }
   }
 
Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -21,6 +21,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
+#include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -35,7 +36,8 @@
 class Session {
 public:

[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Started the review. It would take a few cycles 




Comment at: lib/Format/ContinuationIndenter.cpp:1518
+  unsigned RemainingTokenColumns = 0;
+  // The column number we're currently at.
+  unsigned ContentStartColumn = 0;

Could you please spell out the invariants that we maintain about `TailOffset`, 
`RemainingTokenColumns` and `ContentStartColumn` (at least) at the beginning of 
every main loop iteration below? That would surely make it easier to review.



Comment at: lib/Format/ContinuationIndenter.cpp:1533
+  // ContentStartColumn is either
+  // - at the start of the line, directly after a break
+  // - the end of the last line +1, when continuing a reflow over multiple

If the previous iteration requested that we try to reflow, how can 
`ContentStartColumn` be at the start of the (current) line?


https://reviews.llvm.org/D40310



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

[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz added inline comments.



Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1
+// RUN:  %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s
+// expected-no-diagnostics

hubert.reinterpretcast wrote:
> saar.raz wrote:
> > Rakete wrote:
> > > There is no `-fconcepts-ts` flag AFAIK. Also, why `-fcxx-exceptions`, 
> > > it's not needed here. The `-x c++` part too. It should also be 
> > > `-std=c++2a`.
> > There is a -fconcepts-ts flag (clang -Xclang -fconcepts-ts), I believe now 
> > that there is no more Concepts TS the flag should be removed as well and 
> > all concepts-related code be put under C++2a ifs.
> This was discussed with Richard, Faisal, and Aaron Ballman before. We would 
> like to use -Xclang -fconcepts until there is enough of an implementation. 
> When we are ready, then we switch to having it directly under C++2a. There is 
> no reason why dealing with the option cannot be done as a separate patch, so 
> I don't think we need to hold up this one for that work.
OK, wasn't aware this was discussed before.
I agree then, leave the -fconcepts-ts for now.


https://reviews.llvm.org/D40381



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

[PATCH] D40381: Parse concept definition

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1
+// RUN:  %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s
+// expected-no-diagnostics

saar.raz wrote:
> Rakete wrote:
> > There is no `-fconcepts-ts` flag AFAIK. Also, why `-fcxx-exceptions`, it's 
> > not needed here. The `-x c++` part too. It should also be `-std=c++2a`.
> There is a -fconcepts-ts flag (clang -Xclang -fconcepts-ts), I believe now 
> that there is no more Concepts TS the flag should be removed as well and all 
> concepts-related code be put under C++2a ifs.
This was discussed with Richard, Faisal, and Aaron Ballman before. We would 
like to use -Xclang -fconcepts until there is enough of an implementation. When 
we are ready, then we switch to having it directly under C++2a. There is no 
reason why dealing with the option cannot be done as a separate patch, so I 
don't think we need to hold up this one for that work.


https://reviews.llvm.org/D40381



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

[clang-tools-extra] r318951 - [clang-tidy] rename_check.py: fix header guard handling

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Fri Nov 24 06:33:06 2017
New Revision: 318951

URL: http://llvm.org/viewvc/llvm-project?rev=318951=rev
Log:
[clang-tidy] rename_check.py: fix header guard handling

Modified:
clang-tools-extra/trunk/clang-tidy/rename_check.py

Modified: clang-tools-extra/trunk/clang-tidy/rename_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/rename_check.py?rev=318951=318950=318951=diff
==
--- clang-tools-extra/trunk/clang-tidy/rename_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/rename_check.py Fri Nov 24 06:33:06 2017
@@ -196,8 +196,8 @@ def main():
 
   clang_tidy_path = os.path.dirname(__file__)
 
-  header_guard_old = args.old_check_name.upper().replace('-', '_')
-  header_guard_new = args.new_check_name.upper().replace('-', '_')
+  header_guard_old = (old_module + '_' + check_name_camel).upper()
+  header_guard_new = (new_module + '_' + new_check_name_camel).upper()
 
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)


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

[PATCH] D40435: [clang-format] Deduplicate using declarations

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added inline comments.



Comment at: lib/Format/UsingDeclarationsSorter.cpp:115
 
+bool usingDeclarationsEqual(const UsingDeclaration ,
+const UsingDeclaration ) {

I'd use a lambda instead.


https://reviews.llvm.org/D40435



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

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

I'd like an `AllowDeletedCopyFunctions` option that allows move and destructor 
functions to be missing when copying is disabled.

  struct A {
A(const A&) = delete;
A& operator=(const A&) = delete;
  }


https://reviews.llvm.org/D30610



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

[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz added inline comments.



Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1
+// RUN:  %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s
+// expected-no-diagnostics

Rakete wrote:
> There is no `-fconcepts-ts` flag AFAIK. Also, why `-fcxx-exceptions`, it's 
> not needed here. The `-x c++` part too. It should also be `-std=c++2a`.
There is a -fconcepts-ts flag (clang -Xclang -fconcepts-ts), I believe now that 
there is no more Concepts TS the flag should be removed as well and all 
concepts-related code be put under C++2a ifs.


https://reviews.llvm.org/D40381



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

[PATCH] D40426: [clang-tidy] Move a few more checks from misc to bugprone.

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D40426



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

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet updated this revision to Diff 124173.
szepet marked 3 inline comments as done.
szepet added a comment.
Herald added a subscriber: rnkovacs.

Thank you for the review!

Updated according to review comments.


https://reviews.llvm.org/D38843

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -589,5 +589,21 @@
  binaryOperator(has(cxxUnresolvedConstructExpr()));
 }
 
+const internal::VariadicDynCastAllOfMatcher
+cxxPseudoDestructorExpr;
+
+TEST(ImportExpr, ImportCXXPseudoDestructorExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("typedef int T;"
+ "void declToImport(int *p) {"
+ "  T t;"
+ "  p->T::~T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(has(compoundStmt(has(
+ callExpr(has(cxxPseudoDestructorExpr();
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -292,6 +292,7 @@
 Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
 Expr *VisitCXXThisExpr(CXXThisExpr *E);
 Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
+Expr *VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E);
 Expr *VisitMemberExpr(MemberExpr *E);
 Expr *VisitCallExpr(CallExpr *E);
 Expr *VisitInitListExpr(InitListExpr *E);
@@ -5910,6 +5911,39 @@
   E->isOverloaded(), ToDecls.begin(), ToDecls.end());
 }
 
+Expr *ASTNodeImporter::VisitCXXPseudoDestructorExpr(
+CXXPseudoDestructorExpr *E) {
+
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE)
+return nullptr;
+
+  TypeSourceInfo *ScopeInfo = Importer.Import(E->getScopeTypeInfo());
+  if (!ScopeInfo && E->getScopeTypeInfo())
+return nullptr;
+
+  PseudoDestructorTypeStorage Storage;
+  if (IdentifierInfo *FromII = E->getDestroyedTypeIdentifier()) {
+IdentifierInfo *ToII = Importer.Import(FromII);
+if (!ToII)
+  return nullptr;
+Storage = PseudoDestructorTypeStorage(
+  ToII, Importer.Import(E->getDestroyedTypeLoc()));
+  } else {
+TypeSourceInfo *TI = Importer.Import(E->getDestroyedTypeInfo());
+if (!TI)
+  return nullptr;
+Storage = PseudoDestructorTypeStorage(TI);
+  }
+
+  return new (Importer.getToContext()) CXXPseudoDestructorExpr(
+Importer.getToContext(), BaseE, E->isArrow(),
+Importer.Import(E->getOperatorLoc()),
+Importer.Import(E->getQualifierLoc()),
+ScopeInfo, Importer.Import(E->getColonColonLoc()),
+Importer.Import(E->getTildeLoc()), Storage);
+}
+
 Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
   QualType T = Importer.Import(E->getType());
   if (T.isNull())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -589,5 +589,21 @@
  binaryOperator(has(cxxUnresolvedConstructExpr()));
 }
 
+const internal::VariadicDynCastAllOfMatcher
+cxxPseudoDestructorExpr;
+
+TEST(ImportExpr, ImportCXXPseudoDestructorExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("typedef int T;"
+ "void declToImport(int *p) {"
+ "  T t;"
+ "  p->T::~T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(has(compoundStmt(has(
+ callExpr(has(cxxPseudoDestructorExpr();
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -292,6 +292,7 @@
 Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
 Expr *VisitCXXThisExpr(CXXThisExpr *E);
 Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
+Expr *VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E);
 Expr *VisitMemberExpr(MemberExpr *E);
 Expr *VisitCallExpr(CallExpr *E);
 Expr *VisitInitListExpr(InitListExpr *E);
@@ -5910,6 +5911,39 @@
   E->isOverloaded(), ToDecls.begin(), ToDecls.end());
 }
 
+Expr *ASTNodeImporter::VisitCXXPseudoDestructorExpr(
+CXXPseudoDestructorExpr *E) {
+
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE)
+return nullptr;
+
+  TypeSourceInfo *ScopeInfo = Importer.Import(E->getScopeTypeInfo());
+  if (!ScopeInfo && E->getScopeTypeInfo())
+return nullptr;
+
+  PseudoDestructorTypeStorage Storage;
+  if (IdentifierInfo *FromII = 

[clang-tools-extra] r318946 - [clangd] Sort list of sources in CMakeLists.txt. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Nov 24 05:13:41 2017
New Revision: 318946

URL: http://llvm.org/viewvc/llvm-project?rev=318946=rev
Log:
[clangd] Sort list of sources in CMakeLists.txt. NFC

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=318946=318945=318946=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Fri Nov 24 05:13:41 2017
@@ -9,8 +9,8 @@ add_clang_library(clangDaemon
   ClangdUnitStore.cpp
   DraftStore.cpp
   GlobalCompilationDatabase.cpp
-  JSONRPCDispatcher.cpp
   JSONExpr.cpp
+  JSONRPCDispatcher.cpp
   Logger.cpp
   Protocol.cpp
   ProtocolHandlers.cpp


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

[PATCH] D40302: Avoid copying the data of in-memory preambles

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318945: Avoid copying the data of in-memory preambles 
(authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D40302

Files:
  cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
  cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp


Index: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
===
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
@@ -699,9 +699,7 @@
 StringRef PCHPath = getInMemoryPreamblePath();
 PreprocessorOpts.ImplicitPCHInclude = PCHPath;
 
-// FIMXE(ibiryukov): Preambles can be large. We should allow shared access
-// to the preamble data instead of copying it here.
-auto Buf = llvm::MemoryBuffer::getMemBufferCopy(Storage.asMemory().Data);
+auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.asMemory().Data);
 VFS = createVFSOverlayForPreamblePCH(PCHPath, std::move(Buf), VFS);
   }
 }
Index: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
===
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
@@ -98,6 +98,10 @@
   /// Changes options inside \p CI to use PCH from this preamble. Also remaps
   /// main file to \p MainFileBuffer and updates \p VFS to ensure the preamble
   /// is accessible.
+  /// For in-memory preambles, PrecompiledPreamble instance continues to own
+  /// the MemoryBuffer with the Preamble after this method returns. The caller
+  /// is reponsible for making sure the PrecompiledPreamble instance outlives
+  /// the compiler run and the AST that will be using the PCH.
   void AddImplicitPreamble(CompilerInvocation ,
IntrusiveRefCntPtr ,
llvm::MemoryBuffer *MainFileBuffer) const;


Index: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
===
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
@@ -699,9 +699,7 @@
 StringRef PCHPath = getInMemoryPreamblePath();
 PreprocessorOpts.ImplicitPCHInclude = PCHPath;
 
-// FIMXE(ibiryukov): Preambles can be large. We should allow shared access
-// to the preamble data instead of copying it here.
-auto Buf = llvm::MemoryBuffer::getMemBufferCopy(Storage.asMemory().Data);
+auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.asMemory().Data);
 VFS = createVFSOverlayForPreamblePCH(PCHPath, std::move(Buf), VFS);
   }
 }
Index: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
===
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
@@ -98,6 +98,10 @@
   /// Changes options inside \p CI to use PCH from this preamble. Also remaps
   /// main file to \p MainFileBuffer and updates \p VFS to ensure the preamble
   /// is accessible.
+  /// For in-memory preambles, PrecompiledPreamble instance continues to own
+  /// the MemoryBuffer with the Preamble after this method returns. The caller
+  /// is reponsible for making sure the PrecompiledPreamble instance outlives
+  /// the compiler run and the AST that will be using the PCH.
   void AddImplicitPreamble(CompilerInvocation ,
IntrusiveRefCntPtr ,
llvm::MemoryBuffer *MainFileBuffer) const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r318945 - Avoid copying the data of in-memory preambles

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Nov 24 05:12:38 2017
New Revision: 318945

URL: http://llvm.org/viewvc/llvm-project?rev=318945=rev
Log:
Avoid copying the data of in-memory preambles

Summary: Preambles are large and we should avoid copying them.

Reviewers: bkramer, klimek

Reviewed By: bkramer

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp

Modified: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h?rev=318945=318944=318945=diff
==
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h (original)
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h Fri Nov 24 05:12:38 
2017
@@ -98,6 +98,10 @@ public:
   /// Changes options inside \p CI to use PCH from this preamble. Also remaps
   /// main file to \p MainFileBuffer and updates \p VFS to ensure the preamble
   /// is accessible.
+  /// For in-memory preambles, PrecompiledPreamble instance continues to own
+  /// the MemoryBuffer with the Preamble after this method returns. The caller
+  /// is reponsible for making sure the PrecompiledPreamble instance outlives
+  /// the compiler run and the AST that will be using the PCH.
   void AddImplicitPreamble(CompilerInvocation ,
IntrusiveRefCntPtr ,
llvm::MemoryBuffer *MainFileBuffer) const;

Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=318945=318944=318945=diff
==
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original)
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Fri Nov 24 05:12:38 2017
@@ -699,9 +699,7 @@ void PrecompiledPreamble::setupPreambleS
 StringRef PCHPath = getInMemoryPreamblePath();
 PreprocessorOpts.ImplicitPCHInclude = PCHPath;
 
-// FIMXE(ibiryukov): Preambles can be large. We should allow shared access
-// to the preamble data instead of copying it here.
-auto Buf = llvm::MemoryBuffer::getMemBufferCopy(Storage.asMemory().Data);
+auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.asMemory().Data);
 VFS = createVFSOverlayForPreamblePCH(PCHPath, std::move(Buf), VFS);
   }
 }


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

[PATCH] D40301: [clangd] Ensure preamble outlives the AST

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318944: [clangd] Ensure preamble outlives the AST (authored 
by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D40301

Files:
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h

Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -48,15 +48,25 @@
   llvm::SmallVector FixIts;
 };
 
+// Stores Preamble and associated data.
+struct PreambleData {
+  PreambleData(PrecompiledPreamble Preamble,
+   std::vector TopLevelDeclIDs,
+   std::vector Diags);
+
+  PrecompiledPreamble Preamble;
+  std::vector TopLevelDeclIDs;
+  std::vector Diags;
+};
+
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
   /// it is reused during parsing.
   static llvm::Optional
   Build(std::unique_ptr CI,
-const PrecompiledPreamble *Preamble,
-ArrayRef PreambleDeclIDs,
+std::shared_ptr Preamble,
 std::unique_ptr Buffer,
 std::shared_ptr PCHs,
 IntrusiveRefCntPtr VFS, clangd::Logger );
@@ -80,15 +90,18 @@
   const std::vector () const;
 
 private:
-  ParsedAST(std::unique_ptr Clang,
+  ParsedAST(std::shared_ptr Preamble,
+std::unique_ptr Clang,
 std::unique_ptr Action,
 std::vector TopLevelDecls,
-std::vector PendingTopLevelDecls,
 std::vector Diags);
 
 private:
   void ensurePreambleDeclsDeserialized();
 
+  // In-memory preambles must outlive the AST, it is important that this member
+  // goes before Clang and Action.
+  std::shared_ptr Preamble;
   // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called
   // on it) and CompilerInstance used to run it. That way we don't have to do
   // complex memory management of all Clang structures on our own. (They are
@@ -100,7 +113,7 @@
   // Data, stored after parsing.
   std::vector Diags;
   std::vector TopLevelDecls;
-  std::vector PendingTopLevelDecls;
+  bool PreambleDeclsDeserialized;
 };
 
 // Provides thread-safe access to ParsedAST.
@@ -124,17 +137,6 @@
   mutable llvm::Optional AST;
 };
 
-// Stores Preamble and associated data.
-struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble,
-   std::vector TopLevelDeclIDs,
-   std::vector Diags);
-
-  PrecompiledPreamble Preamble;
-  std::vector TopLevelDeclIDs;
-  std::vector Diags;
-};
-
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
 class CppFile : public std::enable_shared_from_this {
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -879,18 +879,19 @@
 
 llvm::Optional
 ParsedAST::Build(std::unique_ptr CI,
- const PrecompiledPreamble *Preamble,
- ArrayRef PreambleDeclIDs,
+ std::shared_ptr Preamble,
  std::unique_ptr Buffer,
  std::shared_ptr PCHs,
  IntrusiveRefCntPtr VFS,
  clangd::Logger ) {
 
   std::vector ASTDiags;
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
 
+  const PrecompiledPreamble *PreamblePCH =
+  Preamble ? >Preamble : nullptr;
   auto Clang = prepareCompilerInstance(
-  std::move(CI), Preamble, std::move(Buffer), std::move(PCHs),
+  std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
   std::move(VFS), /*ref*/ UnitDiagsConsumer);
 
   // Recover resources if we crash before exiting this method.
@@ -912,15 +913,8 @@
   Clang->getDiagnostics().setClient(new EmptyDiagsConsumer);
 
   std::vector ParsedDecls = Action->takeTopLevelDecls();
-  std::vector PendingDecls;
-  if (Preamble) {
-PendingDecls.reserve(PreambleDeclIDs.size());
-PendingDecls.insert(PendingDecls.begin(), PreambleDeclIDs.begin(),
-PreambleDeclIDs.end());
-  }
-
-  return ParsedAST(std::move(Clang), std::move(Action), std::move(ParsedDecls),
-   std::move(PendingDecls), std::move(ASTDiags));
+  return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
+   std::move(ParsedDecls), std::move(ASTDiags));
 }
 
 namespace {
@@ -1061,24 +1055,25 @@
 }
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
-  if (PendingTopLevelDecls.empty())
+  if (PreambleDeclsDeserialized || !Preamble)
 return;
 
   std::vector Resolved;
-  Resolved.reserve(PendingTopLevelDecls.size());
+  Resolved.reserve(Preamble->TopLevelDeclIDs.size());
 
   ExternalASTSource  

[clang-tools-extra] r318944 - [clangd] Ensure preamble outlives the AST

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Nov 24 05:04:21 2017
New Revision: 318944

URL: http://llvm.org/viewvc/llvm-project?rev=318944=rev
Log:
[clangd] Ensure preamble outlives the AST

Summary:
In-memory preambles will not be copied anymore, so we need to make
sure they outlive the AST.

Reviewers: bkramer, sammccall, klimek

Reviewed By: sammccall

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=318944=318943=318944=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Nov 24 05:04:21 2017
@@ -879,8 +879,7 @@ void clangd::dumpAST(ParsedAST , llv
 
 llvm::Optional
 ParsedAST::Build(std::unique_ptr CI,
- const PrecompiledPreamble *Preamble,
- ArrayRef PreambleDeclIDs,
+ std::shared_ptr Preamble,
  std::unique_ptr Buffer,
  std::shared_ptr PCHs,
  IntrusiveRefCntPtr VFS,
@@ -889,8 +888,10 @@ ParsedAST::Build(std::unique_ptr ASTDiags;
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
 
+  const PrecompiledPreamble *PreamblePCH =
+  Preamble ? >Preamble : nullptr;
   auto Clang = prepareCompilerInstance(
-  std::move(CI), Preamble, std::move(Buffer), std::move(PCHs),
+  std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
   std::move(VFS), /*ref*/ UnitDiagsConsumer);
 
   // Recover resources if we crash before exiting this method.
@@ -912,15 +913,8 @@ ParsedAST::Build(std::unique_ptrgetDiagnostics().setClient(new EmptyDiagsConsumer);
 
   std::vector ParsedDecls = Action->takeTopLevelDecls();
-  std::vector PendingDecls;
-  if (Preamble) {
-PendingDecls.reserve(PreambleDeclIDs.size());
-PendingDecls.insert(PendingDecls.begin(), PreambleDeclIDs.begin(),
-PreambleDeclIDs.end());
-  }
-
-  return ParsedAST(std::move(Clang), std::move(Action), std::move(ParsedDecls),
-   std::move(PendingDecls), std::move(ASTDiags));
+  return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
+   std::move(ParsedDecls), std::move(ASTDiags));
 }
 
 namespace {
@@ -1061,24 +1055,25 @@ std::vector clangd::findDefini
 }
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
-  if (PendingTopLevelDecls.empty())
+  if (PreambleDeclsDeserialized || !Preamble)
 return;
 
   std::vector Resolved;
-  Resolved.reserve(PendingTopLevelDecls.size());
+  Resolved.reserve(Preamble->TopLevelDeclIDs.size());
 
   ExternalASTSource  = *getASTContext().getExternalSource();
-  for (serialization::DeclID TopLevelDecl : PendingTopLevelDecls) {
+  for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) {
 // Resolve the declaration ID to an actual declaration, possibly
 // deserializing the declaration in the process.
 if (Decl *D = Source.GetExternalDecl(TopLevelDecl))
   Resolved.push_back(D);
   }
 
-  TopLevelDecls.reserve(TopLevelDecls.size() + PendingTopLevelDecls.size());
+  TopLevelDecls.reserve(TopLevelDecls.size() +
+Preamble->TopLevelDeclIDs.size());
   TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), 
Resolved.end());
 
-  PendingTopLevelDecls.clear();
+  PreambleDeclsDeserialized = true;
 }
 
 ParsedAST::ParsedAST(ParsedAST &) = default;
@@ -1112,14 +1107,21 @@ const std::vector 
   return Diags;
 }
 
-ParsedAST::ParsedAST(std::unique_ptr Clang,
+PreambleData::PreambleData(PrecompiledPreamble Preamble,
+   std::vector TopLevelDeclIDs,
+   std::vector Diags)
+: Preamble(std::move(Preamble)),
+  TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {}
+
+ParsedAST::ParsedAST(std::shared_ptr Preamble,
+ std::unique_ptr Clang,
  std::unique_ptr Action,
  std::vector TopLevelDecls,
- std::vector PendingTopLevelDecls,
  std::vector Diags)
-: Clang(std::move(Clang)), Action(std::move(Action)),
-  Diags(std::move(Diags)), TopLevelDecls(std::move(TopLevelDecls)),
-  PendingTopLevelDecls(std::move(PendingTopLevelDecls)) {
+: Preamble(std::move(Preamble)), Clang(std::move(Clang)),
+  Action(std::move(Action)), Diags(std::move(Diags)),
+  TopLevelDecls(std::move(TopLevelDecls)),
+  PreambleDeclsDeserialized(false) {
   assert(this->Clang);
   assert(this->Action);
 }
@@ -1130,12 +1132,6 @@ ParsedASTWrapper::ParsedASTWrapper(Parse
 ParsedASTWrapper::ParsedASTWrapper(llvm::Optional AST)
 : AST(std::move(AST)) {}
 

[PATCH] D40301: [clangd] Ensure preamble outlives the AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 124167.
ilya-biryukov added a comment.

- Moved PreambleData declaration up to avoid fwd-decl.


https://reviews.llvm.org/D40301

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h

Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -48,15 +48,25 @@
   llvm::SmallVector FixIts;
 };
 
+// Stores Preamble and associated data.
+struct PreambleData {
+  PreambleData(PrecompiledPreamble Preamble,
+   std::vector TopLevelDeclIDs,
+   std::vector Diags);
+
+  PrecompiledPreamble Preamble;
+  std::vector TopLevelDeclIDs;
+  std::vector Diags;
+};
+
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
   /// it is reused during parsing.
   static llvm::Optional
   Build(std::unique_ptr CI,
-const PrecompiledPreamble *Preamble,
-ArrayRef PreambleDeclIDs,
+std::shared_ptr Preamble,
 std::unique_ptr Buffer,
 std::shared_ptr PCHs,
 IntrusiveRefCntPtr VFS, clangd::Logger );
@@ -80,15 +90,18 @@
   const std::vector () const;
 
 private:
-  ParsedAST(std::unique_ptr Clang,
+  ParsedAST(std::shared_ptr Preamble,
+std::unique_ptr Clang,
 std::unique_ptr Action,
 std::vector TopLevelDecls,
-std::vector PendingTopLevelDecls,
 std::vector Diags);
 
 private:
   void ensurePreambleDeclsDeserialized();
 
+  // In-memory preambles must outlive the AST, it is important that this member
+  // goes before Clang and Action.
+  std::shared_ptr Preamble;
   // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called
   // on it) and CompilerInstance used to run it. That way we don't have to do
   // complex memory management of all Clang structures on our own. (They are
@@ -100,7 +113,7 @@
   // Data, stored after parsing.
   std::vector Diags;
   std::vector TopLevelDecls;
-  std::vector PendingTopLevelDecls;
+  bool PreambleDeclsDeserialized;
 };
 
 // Provides thread-safe access to ParsedAST.
@@ -124,17 +137,6 @@
   mutable llvm::Optional AST;
 };
 
-// Stores Preamble and associated data.
-struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble,
-   std::vector TopLevelDeclIDs,
-   std::vector Diags);
-
-  PrecompiledPreamble Preamble;
-  std::vector TopLevelDeclIDs;
-  std::vector Diags;
-};
-
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
 class CppFile : public std::enable_shared_from_this {
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -879,18 +879,19 @@
 
 llvm::Optional
 ParsedAST::Build(std::unique_ptr CI,
- const PrecompiledPreamble *Preamble,
- ArrayRef PreambleDeclIDs,
+ std::shared_ptr Preamble,
  std::unique_ptr Buffer,
  std::shared_ptr PCHs,
  IntrusiveRefCntPtr VFS,
  clangd::Logger ) {
 
   std::vector ASTDiags;
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
 
+  const PrecompiledPreamble *PreamblePCH =
+  Preamble ? >Preamble : nullptr;
   auto Clang = prepareCompilerInstance(
-  std::move(CI), Preamble, std::move(Buffer), std::move(PCHs),
+  std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
   std::move(VFS), /*ref*/ UnitDiagsConsumer);
 
   // Recover resources if we crash before exiting this method.
@@ -912,15 +913,8 @@
   Clang->getDiagnostics().setClient(new EmptyDiagsConsumer);
 
   std::vector ParsedDecls = Action->takeTopLevelDecls();
-  std::vector PendingDecls;
-  if (Preamble) {
-PendingDecls.reserve(PreambleDeclIDs.size());
-PendingDecls.insert(PendingDecls.begin(), PreambleDeclIDs.begin(),
-PreambleDeclIDs.end());
-  }
-
-  return ParsedAST(std::move(Clang), std::move(Action), std::move(ParsedDecls),
-   std::move(PendingDecls), std::move(ASTDiags));
+  return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
+   std::move(ParsedDecls), std::move(ASTDiags));
 }
 
 namespace {
@@ -1061,24 +1055,25 @@
 }
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
-  if (PendingTopLevelDecls.empty())
+  if (PreambleDeclsDeserialized || !Preamble)
 return;
 
   std::vector Resolved;
-  Resolved.reserve(PendingTopLevelDecls.size());
+  Resolved.reserve(Preamble->TopLevelDeclIDs.size());
 
   ExternalASTSource  = *getASTContext().getExternalSource();
-  for (serialization::DeclID TopLevelDecl : PendingTopLevelDecls) {
+  for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) {
 // Resolve the declaration ID to an actual declaration, 

[PATCH] D39279: Stringizing raw string literals containing newline

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added inline comments.



Comment at: lib/Lex/Lexer.cpp:217
+void StringifyImpl(T& Str, char Quote) {
+  unsigned i = 0, e = Str.size();
+  while (i < e) {

Wouldn't **auto** or **typename T::size_type** instead of **unsigned** be more 
appropriate here?

Both of your supported use cases have this member type.
http://llvm.org/doxygen/classllvm_1_1StringRef.html#a54e59e2d53e5ee736ee060be7c457508
http://llvm.org/doxygen/classllvm_1_1SmallVectorImpl.html#acc72e8846802a1e703501219cf19458e



Comment at: lib/Lex/Lexer.cpp:231
+  Str.erase(Str.begin() + i, Str.begin() + i + Size);
+  Str.insert(Str.begin() + i, '\\');
+  Str.insert(Str.begin() + i + 1, 'n');

I am just wondering if potential performance benefit of counting all the extra 
space in advance and resizing the string just once might be interesting here.
Basically with current approach characters at the end of the string are moved 
as many times as there are endlines in the string.


https://reviews.llvm.org/D39279



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

[PATCH] D40381: Parse concept definition

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: lib/Parse/ParseTemplate.cpp:161
   // Parse the actual template declaration.
-  return ParseSingleDeclarationAfterTemplate(Context,
- ParsedTemplateInfo(,
- isSpecialization,
- 
LastParamListWasEmpty),
- ParsingTemplateParams,
- DeclEnd, AS, AccessAttrs);
+  if (!TryConsumeToken(tok::kw_concept)) {
+return ParseSingleDeclarationAfterTemplate(Context,

No braces here please.



Comment at: lib/Parse/ParseTemplate.cpp:168
+   DeclEnd, AS, AccessAttrs);
+  } else {
+return ParseConceptDefinition(Context,

No `else` needed here.



Comment at: lib/Parse/ParseTemplate.cpp:382
+
+  return ThisDecl;
+}

Do we really need `ThisDecl`?



Comment at: lib/Sema/SemaTemplate.cpp:3936
 
+  if (R.getAsSingle()) {
+return CheckConceptTemplateId(SS, R.getLookupNameInfo(),

saar.raz wrote:
> We're gonna want to check
> ```
> !TemplateSpecializationType::anyDependentTemplateArguments(*TemplateArgs, 
> InstantiationDependent)
> ``` 
> here as well - so that we can instantiate a ConceptSpecializationExpr later 
> when we have it (we're not gonna want to instantiate a 
> ConceptSpecializationExpr with dependent arguments.
No braces here please.



Comment at: lib/Sema/SemaTemplate.cpp:3938
+return CheckConceptTemplateId(SS, R.getLookupNameInfo(),
+  R.getAsSingle(),
+  TemplateKWLoc, TemplateArgs);

clang-format please :)



Comment at: lib/Sema/SemaTemplate.cpp:7693
+Decl *Sema::ActOnConceptDefinition(Scope *S,
+  MultiTemplateParamsArg TemplateParameterLists,
+   IdentifierInfo *Name, SourceLocation L,

Did you run this through clang-format?



Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1
+// RUN:  %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s
+// expected-no-diagnostics

There is no `-fconcepts-ts` flag AFAIK. Also, why `-fcxx-exceptions`, it's not 
needed here. The `-x c++` part too. It should also be `-std=c++2a`.


https://reviews.llvm.org/D40381



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

[PATCH] D40426: [clang-tidy] Move a few more checks from misc to bugprone.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh created this revision.
Herald added subscribers: kbarton, xazax.hun, javed.absar, mgorny, nemanjai.

clang_tidy/rename_check.py misc-assert-side-effect bugprone-assert-side-effect
clang_tidy/rename_check.py misc-bool-pointer-implicit-conversion 
bugprone-bool-pointer-implicit-conversion
clang_tidy/rename_check.py misc-fold-init-type bugprone-fold-init-type
clang_tidy/rename_check.py misc-forward-declaration-namespace 
bugprone-forward-declaration-namespace
clang_tidy/rename_check.py misc-inaccurate-erase bugprone-inaccurate-erase
clang_tidy/rename_check.py misc-move-forwarding-reference 
bugprone-move-forwarding-reference
clang_tidy/rename_check.py misc-multiple-statement-macro 
bugprone-multiple-statement-macro
clang_tidy/rename_check.py misc-use-after-move bugprone-use-after-move
clang_tidy/rename_check.py misc-virtual-near-miss bugprone-virtual-near-miss

Manually fix a reference to UseAfterMoveCheck in the hicpp module.


https://reviews.llvm.org/D40426

Files:
  clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  clang-tidy/bugprone/BoolPointerImplicitConversionCheck.h
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/FoldInitTypeCheck.cpp
  clang-tidy/bugprone/FoldInitTypeCheck.h
  clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp
  clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.h
  clang-tidy/bugprone/InaccurateEraseCheck.cpp
  clang-tidy/bugprone/InaccurateEraseCheck.h
  clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp
  clang-tidy/bugprone/MoveForwardingReferenceCheck.h
  clang-tidy/bugprone/MultipleStatementMacroCheck.cpp
  clang-tidy/bugprone/MultipleStatementMacroCheck.h
  clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tidy/bugprone/UseAfterMoveCheck.h
  clang-tidy/bugprone/VirtualNearMissCheck.cpp
  clang-tidy/bugprone/VirtualNearMissCheck.h
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/misc/AssertSideEffectCheck.cpp
  clang-tidy/misc/AssertSideEffectCheck.h
  clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
  clang-tidy/misc/BoolPointerImplicitConversionCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/FoldInitTypeCheck.cpp
  clang-tidy/misc/FoldInitTypeCheck.h
  clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp
  clang-tidy/misc/ForwardDeclarationNamespaceCheck.h
  clang-tidy/misc/InaccurateEraseCheck.cpp
  clang-tidy/misc/InaccurateEraseCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/MoveForwardingReferenceCheck.cpp
  clang-tidy/misc/MoveForwardingReferenceCheck.h
  clang-tidy/misc/MultipleStatementMacroCheck.cpp
  clang-tidy/misc/MultipleStatementMacroCheck.h
  clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tidy/misc/UseAfterMoveCheck.h
  clang-tidy/misc/VirtualNearMissCheck.cpp
  clang-tidy/misc/VirtualNearMissCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-assert-side-effect.rst
  docs/clang-tidy/checks/bugprone-bool-pointer-implicit-conversion.rst
  docs/clang-tidy/checks/bugprone-fold-init-type.rst
  docs/clang-tidy/checks/bugprone-forward-declaration-namespace.rst
  docs/clang-tidy/checks/bugprone-inaccurate-erase.rst
  docs/clang-tidy/checks/bugprone-move-forwarding-reference.rst
  docs/clang-tidy/checks/bugprone-multiple-statement-macro.rst
  docs/clang-tidy/checks/bugprone-use-after-move.rst
  docs/clang-tidy/checks/bugprone-virtual-near-miss.rst
  docs/clang-tidy/checks/hicpp-invalid-access-moved.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-assert-side-effect.rst
  docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.rst
  docs/clang-tidy/checks/misc-fold-init-type.rst
  docs/clang-tidy/checks/misc-forward-declaration-namespace.rst
  docs/clang-tidy/checks/misc-inaccurate-erase.rst
  docs/clang-tidy/checks/misc-move-forwarding-reference.rst
  docs/clang-tidy/checks/misc-multiple-statement-macro.rst
  docs/clang-tidy/checks/misc-use-after-move.rst
  docs/clang-tidy/checks/misc-virtual-near-miss.rst
  test/clang-tidy/bugprone-assert-side-effect.cpp
  test/clang-tidy/bugprone-bool-pointer-implicit-conversion.cpp
  test/clang-tidy/bugprone-fold-init-type.cpp
  test/clang-tidy/bugprone-forward-declaration-namespace.cpp
  test/clang-tidy/bugprone-inaccurate-erase.cpp
  test/clang-tidy/bugprone-move-forwarding-reference.cpp
  test/clang-tidy/bugprone-multiple-statement-macro.cpp
  test/clang-tidy/bugprone-use-after-move.cpp
  test/clang-tidy/bugprone-virtual-near-miss.cpp
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp
  test/clang-tidy/misc-assert-side-effect.cpp
  test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp
  test/clang-tidy/misc-fold-init-type.cpp
  test/clang-tidy/misc-forward-declaration-namespace.cpp
  test/clang-tidy/misc-inaccurate-erase.cpp
  test/clang-tidy/misc-move-forwarding-reference.cpp
  test/clang-tidy/misc-multiple-statement-macro.cpp
  test/clang-tidy/misc-use-after-move.cpp
  

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/JSONRPCDispatcher.h:47
+  // Whether output should be pretty-printed.
+  const bool Pretty;
+

ioeric wrote:
> It seems that we are piggybacking a state of clangd with `JSONOutput`. It 
> might make sense to store the option in clangd (or the dispatcher?) and pass 
> it via `writeMessage`, but what you have is also fine if it saves trouble :)
Yeah, this isn't great. In the short term I don't think it does much harm. 
Longer-term we'll revisit logging organization as part of RequestContext that 
Ilya's working on.


https://reviews.llvm.org/D40406



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

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 124163.
sammccall marked an inline comment as done.
sammccall added a comment.
Herald added a subscriber: ilya-biryukov.

Use llvm::toString


https://reviews.llvm.org/D40406

Files:
  clangd/ClangdLSPServer.cpp
  clangd/JSONExpr.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/Trace.cpp
  test/clangd/authority-less-uri.test
  test/clangd/definitions.test
  test/clangd/diagnostics.test
  test/clangd/did-change-watch-files.test
  test/clangd/protocol.test
  unittests/clangd/JSONExprTests.cpp

Index: unittests/clangd/JSONExprTests.cpp
===
--- unittests/clangd/JSONExprTests.cpp
+++ unittests/clangd/JSONExprTests.cpp
@@ -205,6 +205,7 @@
   EXPECT_TRUE(O->null("null"));
 
   EXPECT_EQ(O->number("number"), llvm::Optional(2.78));
+  EXPECT_FALSE(O->integer("number"));
   EXPECT_EQ(O->string("string"), llvm::Optional("json"));
   ASSERT_FALSE(O->object("missing"));
   ASSERT_FALSE(O->object("array"));
@@ -216,6 +217,7 @@
   EXPECT_EQ(A->boolean(1), llvm::Optional(true));
   ASSERT_TRUE(A->array(4));
   EXPECT_EQ(*A->array(4), (ary{1, 2, 3}));
+  EXPECT_EQ(A->array(4)->integer(1), llvm::Optional(2));
   int I = 0;
   for (Expr  : *A) {
 if (I++ == 5) {
Index: test/clangd/protocol.test
===
--- test/clangd/protocol.test
+++ test/clangd/protocol.test
@@ -79,7 +79,7 @@
 {"jsonrpc":"2.0","id":4,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
 # Test message with malformed Content-Length
 #
-# STDERR: JSON dispatch failed!
+# STDERR: JSON parse error
 # Ensure we recover by sending another (valid) message
 
 Content-Length: 146
Index: test/clangd/did-change-watch-files.test
===
--- test/clangd/did-change-watch-files.test
+++ /dev/null
@@ -1,53 +0,0 @@
-# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
-# It is absolutely vital that this file has CRLF line endings.
-#
-# Test initialize request parameters with rootUri
-Content-Length: 143
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# Normal case.
-Content-Length: 217
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file.cpp","type":1},{"uri":"file:///path/to/file2.cpp","type":2},{"uri":"file:///path/to/file3.cpp","type":3}]}}
-
-# Wrong event type, integer
-Content-Length: 173
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":0},{"uri":"file:///path/to/file3.cpp","type":4}]}}
-# STDERR: Failed to decode a FileEvent.
-# Wrong event type, string
-Content-Length: 132
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":"foo"}]}}
-# STDERR: Failed to decode a FileEvent.
-#Custom event field
-Content-Length: 143
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":1,"custom":"foo"}]}}
-# STDERR: Failed to decode a FileEvent.
-#Event field with object
-Content-Length: 140
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":{"foo":"bar"}}]}}
-# STDERR: Failed to decode a FileEvent.
-# Changes field with sequence but no object
-Content-Length: 86
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[""]}}
-# STDERR: Failed to decode workspace/didChangeWatchedFiles request.
-# Changes field with no sequence
-Content-Length: 84
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":""}}
-# STDERR: Failed to decode workspace/didChangeWatchedFiles request.
-# Custom field
-Content-Length: 86
-
-{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"custom":"foo"}}
-# STDERR: Ignored unknown field "custom"
-Content-Length: 44
-
-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
-Content-Length: 33
-
-{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/diagnostics.test
===
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -47,4 +47,4 @@
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 Content-Length: 33
 
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/definitions.test
===
--- test/clangd/definitions.test
+++ test/clangd/definitions.test
@@ -101,7 +101,7 @@
 # CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318943: [Tooling] Acknowledge that many CompilationDatabases 
don't support enumeration. (authored by sammccall).

Repository:
  rL LLVM

https://reviews.llvm.org/D40409

Files:
  cfe/trunk/include/clang/Tooling/CompilationDatabase.h
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp

Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -64,10 +64,12 @@
 
 /// \brief Interface for compilation databases.
 ///
-/// A compilation database allows the user to retrieve all compile command lines
-/// that a specified file is compiled with in a project.
-/// The retrieved compile command lines can be used to run clang tools over
-/// a subset of the files in a project.
+/// A compilation database allows the user to retrieve compile command lines
+/// for the files in a project.
+///
+/// Many implementations are enumerable, allowing all command lines to be
+/// retrieved. These can be used to run clang tools over a subset of the files
+/// in a project.
 class CompilationDatabase {
 public:
   virtual ~CompilationDatabase();
@@ -114,15 +116,21 @@
 StringRef FilePath) const = 0;
 
   /// \brief Returns the list of all files available in the compilation database.
-  virtual std::vector getAllFiles() const = 0;
+  ///
+  /// By default, returns nothing. Implementations should override this if they
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 
   /// \brief Returns all compile commands for all the files in the compilation
   /// database.
   ///
   /// FIXME: Add a layer in Tooling that provides an interface to run a tool
   /// over all files in a compilation database. Not all build systems have the
   /// ability to provide a feasible implementation for \c getAllCompileCommands.
-  virtual std::vector getAllCompileCommands() const = 0;
+  ///
+  /// By default, this is implemented in terms of getAllFiles() and
+  /// getCompileCommands(). Subclasses may override this for efficiency.
+  virtual std::vector getAllCompileCommands() const;
 };
 
 /// \brief Interface for compilation database plugins.
@@ -149,6 +157,7 @@
 /// \brief A compilation database that returns a single compile command line.
 ///
 /// Useful when we want a tool to behave more like a compiler invocation.
+/// This compilation database is not enumerable: getAllFiles() returns {}.
 class FixedCompilationDatabase : public CompilationDatabase {
 public:
   /// \brief Creates a FixedCompilationDatabase from the arguments after "--".
@@ -199,17 +208,6 @@
   std::vector
   getCompileCommands(StringRef FilePath) const override;
 
-  /// \brief Returns the list of all files available in the compilation database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllFiles() const override;
-
-  /// \brief Returns all compile commands for all the files in the compilation
-  /// database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllCompileCommands() const override;
-
 private:
   /// This is built up to contain a single entry vector to be returned from
   /// getCompileCommands after adding the positional argument.
Index: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp
@@ -112,6 +112,15 @@
   return DB;
 }
 
+std::vector CompilationDatabase::getAllCompileCommands() const {
+  std::vector Result;
+  for (const auto  : getAllFiles()) {
+auto C = getCompileCommands(File);
+std::move(C.begin(), C.end(), std::back_inserter(Result));
+  }
+  return Result;
+}
+
 CompilationDatabasePlugin::~CompilationDatabasePlugin() {}
 
 namespace {
@@ -342,16 +351,6 @@
   return Result;
 }
 
-std::vector
-FixedCompilationDatabase::getAllFiles() const {
-  return std::vector();
-}
-
-std::vector
-FixedCompilationDatabase::getAllCompileCommands() const {
-  return std::vector();
-}
-
 namespace {
 
 class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r318943 - [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Nov 24 04:13:55 2017
New Revision: 318943

URL: http://llvm.org/viewvc/llvm-project?rev=318943=rev
Log:
[Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

Summary: Provide default implementations so that only getCompileCommands() is 
mandatory.

Reviewers: ioeric

Subscribers: cfe-commits, bkramer, klimek

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

Modified:
cfe/trunk/include/clang/Tooling/CompilationDatabase.h
cfe/trunk/lib/Tooling/CompilationDatabase.cpp

Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=318943=318942=318943=diff
==
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original)
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Fri Nov 24 04:13:55 
2017
@@ -64,10 +64,12 @@ struct CompileCommand {
 
 /// \brief Interface for compilation databases.
 ///
-/// A compilation database allows the user to retrieve all compile command 
lines
-/// that a specified file is compiled with in a project.
-/// The retrieved compile command lines can be used to run clang tools over
-/// a subset of the files in a project.
+/// A compilation database allows the user to retrieve compile command lines
+/// for the files in a project.
+///
+/// Many implementations are enumerable, allowing all command lines to be
+/// retrieved. These can be used to run clang tools over a subset of the files
+/// in a project.
 class CompilationDatabase {
 public:
   virtual ~CompilationDatabase();
@@ -114,7 +116,10 @@ public:
 StringRef FilePath) const = 0;
 
   /// \brief Returns the list of all files available in the compilation 
database.
-  virtual std::vector getAllFiles() const = 0;
+  ///
+  /// By default, returns nothing. Implementations should override this if they
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 
   /// \brief Returns all compile commands for all the files in the compilation
   /// database.
@@ -122,7 +127,10 @@ public:
   /// FIXME: Add a layer in Tooling that provides an interface to run a tool
   /// over all files in a compilation database. Not all build systems have the
   /// ability to provide a feasible implementation for \c 
getAllCompileCommands.
-  virtual std::vector getAllCompileCommands() const = 0;
+  ///
+  /// By default, this is implemented in terms of getAllFiles() and
+  /// getCompileCommands(). Subclasses may override this for efficiency.
+  virtual std::vector getAllCompileCommands() const;
 };
 
 /// \brief Interface for compilation database plugins.
@@ -149,6 +157,7 @@ public:
 /// \brief A compilation database that returns a single compile command line.
 ///
 /// Useful when we want a tool to behave more like a compiler invocation.
+/// This compilation database is not enumerable: getAllFiles() returns {}.
 class FixedCompilationDatabase : public CompilationDatabase {
 public:
   /// \brief Creates a FixedCompilationDatabase from the arguments after "--".
@@ -199,17 +208,6 @@ public:
   std::vector
   getCompileCommands(StringRef FilePath) const override;
 
-  /// \brief Returns the list of all files available in the compilation 
database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllFiles() const override;
-
-  /// \brief Returns all compile commands for all the files in the compilation
-  /// database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllCompileCommands() const override;
-
 private:
   /// This is built up to contain a single entry vector to be returned from
   /// getCompileCommands after adding the positional argument.

Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CompilationDatabase.cpp?rev=318943=318942=318943=diff
==
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Fri Nov 24 04:13:55 2017
@@ -112,6 +112,15 @@ CompilationDatabase::autoDetectFromDirec
   return DB;
 }
 
+std::vector CompilationDatabase::getAllCompileCommands() const 
{
+  std::vector Result;
+  for (const auto  : getAllFiles()) {
+auto C = getCompileCommands(File);
+std::move(C.begin(), C.end(), std::back_inserter(Result));
+  }
+  return Result;
+}
+
 CompilationDatabasePlugin::~CompilationDatabasePlugin() {}
 
 namespace {
@@ -342,16 +351,6 @@ FixedCompilationDatabase::getCompileComm
   return Result;
 }
 
-std::vector
-FixedCompilationDatabase::getAllFiles() const {
-  return std::vector();
-}
-
-std::vector
-FixedCompilationDatabase::getAllCompileCommands() const {
-  return std::vector();
-}
-
 namespace {
 
 class 

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:122
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 

grandinj wrote:
> I know very little about LLVM's standards, so ignore me if I'm wrong, but 
> shouldn't this be returning a pair of (begin,end) iterators rather than 
> potentially a copy of a very large array of strings?
> 
> And shouldn't it be returning an iteration over StringRef rather then 
> std::string, which will require copying the actual data?
You might well be right. But this is an existing interface designed as an 
extension point for out-of-tree build systems. I'd rather not break them unless 
we have evidence of an actual performance problem.
(Note my change here is backwards compatible and doesn't change the signature)


https://reviews.llvm.org/D40409



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

[PATCH] D40424: clang-format: [JS] handle semis in generic types.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
Herald added a subscriber: klimek.

TypeScript generic type arguments can contain object (literal) types,
which in turn can contain semicolons:

  const x: Array<{a: number; b: string;} = [];

Previously, clang-format would incorrectly categorize the braced list as
a block and terminate the line at the openening `{`, and then format the
entire expression badly.

With this change, clang-format recognizes `<` preceding a `{` as
introducing a type expression. In JS, `<` comparison with an object
literal can never be true, so the chance of introducing false positives
here is very low.


https://reviews.llvm.org/D40424

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJS.cpp


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1406,6 +1406,7 @@
   verifyFormat("function x(y: {a?: number;} = {}): number {\n"
"  return 12;\n"
"}");
+  verifyFormat("const x: Array<{a: number; b: string;}> = [];");
   verifyFormat("((a: string, b: number): string => a + b);");
   verifyFormat("var x: (y: number) => string;");
   verifyFormat("var x: P string>;");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -377,13 +377,16 @@
 switch (Tok->Tok.getKind()) {
 case tok::l_brace:
   if (Style.Language == FormatStyle::LK_JavaScript && PrevTok) {
-if (PrevTok->is(tok::colon))
-  // A colon indicates this code is in a type, or a braced list
-  // following a label in an object literal ({a: {b: 1}}). The code
-  // below could be confused by semicolons between the individual
-  // members in a type member list, which would normally trigger
-  // BK_Block. In both cases, this must be parsed as an inline braced
-  // init.
+if (PrevTok->isOneOf(tok::colon, tok::less))
+  // A ':' indicates this code is in a type, or a braced list
+  // following a label in an object literal ({a: {b: 1}}).
+  // A '<' could be an object used in a comparison, but that is 
nonsense
+  // code (can never return true), so more likely it is a generic type
+  // argument (`X<{a: string; b: number}>`).
+  // The code below could be confused by semicolons between the
+  // individual members in a type member list, which would normally
+  // trigger BK_Block. In both cases, this must be parsed as an inline
+  // braced init.
   Tok->BlockKind = BK_BracedInit;
 else if (PrevTok->is(tok::r_paren))
   // `) { }` can only occur in function or method declarations in JS.


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1406,6 +1406,7 @@
   verifyFormat("function x(y: {a?: number;} = {}): number {\n"
"  return 12;\n"
"}");
+  verifyFormat("const x: Array<{a: number; b: string;}> = [];");
   verifyFormat("((a: string, b: number): string => a + b);");
   verifyFormat("var x: (y: number) => string;");
   verifyFormat("var x: P string>;");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -377,13 +377,16 @@
 switch (Tok->Tok.getKind()) {
 case tok::l_brace:
   if (Style.Language == FormatStyle::LK_JavaScript && PrevTok) {
-if (PrevTok->is(tok::colon))
-  // A colon indicates this code is in a type, or a braced list
-  // following a label in an object literal ({a: {b: 1}}). The code
-  // below could be confused by semicolons between the individual
-  // members in a type member list, which would normally trigger
-  // BK_Block. In both cases, this must be parsed as an inline braced
-  // init.
+if (PrevTok->isOneOf(tok::colon, tok::less))
+  // A ':' indicates this code is in a type, or a braced list
+  // following a label in an object literal ({a: {b: 1}}).
+  // A '<' could be an object used in a comparison, but that is nonsense
+  // code (can never return true), so more likely it is a generic type
+  // argument (`X<{a: string; b: number}>`).
+  // The code below could be confused by semicolons between the
+  // individual members in a type member list, which would normally
+  // trigger BK_Block. In both cases, this must be parsed as an inline
+  // braced init.
   Tok->BlockKind = BK_BracedInit;
 else if 

[PATCH] D40415: [libcxx

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki created this revision.
Herald added a reviewer: EricWF.

https://reviews.llvm.org/D40415

Files:
  include/iterator
  
test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp


Index: 
test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
===
--- 
test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
+++ 
test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
@@ -49,4 +49,7 @@
 
 assert(i4 == i4);
 assert(i4 == i5);
+
+assert(std::operator==(i1, i2));
+assert(std::operator!=(i1, i3));
 }
Index: include/iterator
===
--- include/iterator
+++ include/iterator
@@ -904,15 +904,37 @@
 _LIBCPP_INLINE_VISIBILITY istream_iterator  operator++(int)
 {istream_iterator __t(*this); ++(*this); return __t;}
 
+template 
 friend _LIBCPP_INLINE_VISIBILITY
-bool operator==(const istream_iterator& __x, const istream_iterator& __y)
-{return __x.__in_stream_ == __y.__in_stream_;}
+bool
+operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x,
+   const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y);
 
+template 
 friend _LIBCPP_INLINE_VISIBILITY
-bool operator!=(const istream_iterator& __x, const istream_iterator& __y)
-{return !(__x == __y);}
+bool
+operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x,
+   const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y);
 };
 
+template 
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+operator==(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x,
+   const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y)
+{
+return __x.__in_stream_ == __y.__in_stream_;
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+operator!=(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x,
+   const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y)
+{
+return !(__x == __y);
+}
+
 template  >
 class _LIBCPP_TEMPLATE_VIS ostream_iterator
 : public iterator


Index: test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
===
--- test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
+++ test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp
@@ -49,4 +49,7 @@
 
 assert(i4 == i4);
 assert(i4 == i5);
+
+assert(std::operator==(i1, i2));
+assert(std::operator!=(i1, i3));
 }
Index: include/iterator
===
--- include/iterator
+++ include/iterator
@@ -904,15 +904,37 @@
 _LIBCPP_INLINE_VISIBILITY istream_iterator  operator++(int)
 {istream_iterator __t(*this); ++(*this); return __t;}
 
+template 
 friend _LIBCPP_INLINE_VISIBILITY
-bool operator==(const istream_iterator& __x, const istream_iterator& __y)
-{return __x.__in_stream_ == __y.__in_stream_;}
+bool
+operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x,
+   const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y);
 
+template 
 friend _LIBCPP_INLINE_VISIBILITY
-bool operator!=(const istream_iterator& __x, const istream_iterator& __y)
-{return !(__x == __y);}
+bool
+operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x,
+   const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y);
 };
 
+template 
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+operator==(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x,
+   const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y)
+{
+return __x.__in_stream_ == __y.__in_stream_;
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+operator!=(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x,
+   const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y)
+{
+return !(__x == __y);
+}
+
 template  >
 class _LIBCPP_TEMPLATE_VIS ostream_iterator
 : public iterator
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 124154.
sberg added a comment.

Oh, I'd meant to upload the version of the patch where the newly added calls to 
EmitTypeCheck use the default Alignment and SkippedChecks arguments, instead of 
explicitly skipping Alignment and ObjectSize checks.   (That's the version I 
had successfully used in a LibreOffice UBSan 'make check' test build.  But I 
still don't know which version is better.)


https://reviews.llvm.org/D40295

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  compiler-rt/lib/ubsan/ubsan_handlers.cc
  compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
@@ -1,41 +1,53 @@
-// RUN: %clangxx -frtti -fsanitize=null,vptr -fno-sanitize-recover=null,vptr -g %s -O3 -o %t -mllvm -enable-tail-merge=false
-// RUN: %run %t rT && %run %t mT && %run %t fT && %run %t cT
-// RUN: %run %t rU && %run %t mU && %run %t fU && %run %t cU
-// RUN: %run %t rS && %run %t rV && %run %t oV
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t mS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t fS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t cS 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t mV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t fV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t cV 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t oU 2>&1 | FileCheck %s --check-prefix=CHECK-OFFSET --check-prefix=CHECK-%os-OFFSET --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
-// RUN: not %run %t nN 2>&1 | FileCheck %s --check-prefix=CHECK-NULL-MEMFUN --strict-whitespace
+// RUN: %clangxx -frtti -fsanitize=null,vptr -g %s -O3 -o %t -mllvm -enable-tail-merge=false
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t mT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t fT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t cT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t mU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t fU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t cU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rS
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rV
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t oV
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t zN
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t mS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t fS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t cS 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t mV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t fV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t cV 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t oU 2>&1 | FileCheck %s --check-prefix=CHECK-OFFSET --check-prefix=CHECK-%os-OFFSET --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
+// RUN: 

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D40409



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

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

@dcoughlin do you have some input on this?


https://reviews.llvm.org/D37437



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

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/JSONRPCDispatcher.cpp:221
+Out.log("<-- " + JSONRef + "\n");
+handleAllErrors(Doc.takeError(), [&](const llvm::ErrorInfoBase ) {
+  Out.log(llvm::Twine("JSON parse error: ") + Err.message() + "\n");

Use `llvm::toString`?



Comment at: clangd/JSONRPCDispatcher.h:47
+  // Whether output should be pretty-printed.
+  const bool Pretty;
+

It seems that we are piggybacking a state of clangd with `JSONOutput`. It might 
make sense to store the option in clangd (or the dispatcher?) and pass it via 
`writeMessage`, but what you have is also fine if it saves trouble :)


https://reviews.llvm.org/D40406



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

r318942 - clang-format: [JS] handle destructuring `of`.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Nov 24 02:48:25 2017
New Revision: 318942

URL: http://llvm.org/viewvc/llvm-project?rev=318942=rev
Log:
clang-format: [JS] handle destructuring `of`.

Summary:
Previously, clang-format would drop a space character between `of` and
then following (non-identifier) token if the preceding token was part of
a destructuring assignment (`}` or `]`).

Before:
for (const [a, b] of[]) {}

After:
for (const [a, b] of []) {}

Reviewers: djasper

Subscribers: klimek

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=318942=318941=318942=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Nov 24 02:48:25 2017
@@ -2397,9 +2397,11 @@ bool TokenAnnotator::spaceRequiredBefore
 if ((Left.isOneOf(Keywords.kw_let, Keywords.kw_var, Keywords.kw_in,
   tok::kw_const) ||
  // "of" is only a keyword if it appears after another identifier
- // (e.g. as "const x of y" in a for loop).
+ // (e.g. as "const x of y" in a for loop), or after a destructuring
+ // operation (const [x, y] of z, const {a, b} of c).
  (Left.is(Keywords.kw_of) && Left.Previous &&
-  Left.Previous->Tok.getIdentifierInfo())) &&
+  (Left.Previous->Tok.getIdentifierInfo() ||
+   Left.Previous->isOneOf(tok::r_square, tok::r_brace &&
 (!Left.Previous || !Left.Previous->is(tok::period)))
   return true;
 if (Left.isOneOf(tok::kw_for, Keywords.kw_as) && Left.Previous &&

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=318942=318941=318942=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Nov 24 02:48:25 2017
@@ -1110,6 +1110,10 @@ TEST_F(FormatTestJS, ForLoops) {
"}");
   verifyFormat("for (let {a, b} of x) {\n"
"}");
+  verifyFormat("for (let {a, b} of [x]) {\n"
+   "}");
+  verifyFormat("for (let [a, b] of [x]) {\n"
+   "}");
   verifyFormat("for (let {a, b} in x) {\n"
"}");
 }


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

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Noel Grandin via Phabricator via cfe-commits]

grandinj added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:122
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 

I know very little about LLVM's standards, so ignore me if I'm wrong, but 
shouldn't this be returning a pair of (begin,end) iterators rather than 
potentially a copy of a very large array of strings?

And shouldn't it be returning an iteration over StringRef rather then 
std::string, which will require copying the actual data?



Comment at: include/clang/Tooling/CompilationDatabase.h:133
+  /// getCompileCommands(). Subclasses may override this for efficiency.
+  virtual std::vector getAllCompileCommands() const;
 };

similarly here


https://reviews.llvm.org/D40409



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

[PATCH] D40410: clang-format: [JS] disable ASI on decorators.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
Herald added a subscriber: klimek.

Automatic Semicolon Insertion in clang-format tries to guess if a line
wrap should insert an implicit semicolong. The previous heuristic would
not trigger ASI if a token was immediately preceded by an `@` sign:

  function foo(@Bar  // <-- does not trigger due to preceding @
  baz) {}

However decorators can have arbitrary parameters:

  function foo(@Bar(param, param, param)  // <-- precending @ missed
  baz) {}

While it would be possible to precisely find the matching `@`, just
conversatively disabling ASI for the entire line is simpler, while also
not regressing ASI substatially.


https://reviews.llvm.org/D40410

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJS.cpp


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1192,6 +1192,8 @@
   "String");
   verifyFormat("function f(@Foo bar) {}", "function f(@Foo\n"
   "  bar) {}");
+  verifyFormat("function f(@Foo(Param) bar) {}", "function f(@Foo(Param)\n"
+ "  bar) {}");
   verifyFormat("a = true\n"
"return 1",
"a = true\n"
@@ -1557,7 +1559,7 @@
"}");
 }
 
-TEST_F(FormatTestJS, MetadataAnnotations) {
+TEST_F(FormatTestJS, Decorators) {
   verifyFormat("@A\nclass C {\n}");
   verifyFormat("@A({arg: 'value'})\nclass C {\n}");
   verifyFormat("@A\n@B\nclass C {\n}");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -18,6 +18,8 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
+#include 
+
 #define DEBUG_TYPE "format-parser"
 
 namespace clang {
@@ -891,11 +893,14 @@
   bool PreviousMustBeValue = mustBeJSIdentOrValue(Keywords, Previous);
   bool PreviousStartsTemplateExpr =
   Previous->is(TT_TemplateString) && Previous->TokenText.endswith("${");
-  if (PreviousMustBeValue && Line && Line->Tokens.size() > 1) {
-// If the token before the previous one is an '@', the previous token is an
-// annotation and can precede another identifier/value.
-const FormatToken *PrePrevious = std::prev(Line->Tokens.end(), 2)->Tok;
-if (PrePrevious->is(tok::at))
+  if (PreviousMustBeValue || Previous->is(tok::r_paren)) {
+// If the line contains an '@' sign, the previous token might be an
+// annotation, which can precede another identifier/value.
+bool HasAt = std::find_if(Line->Tokens.begin(), Line->Tokens.end(),
+  [](UnwrappedLineNode ) {
+return LineNode.Tok->is(tok::at);
+  }) != Line->Tokens.end();
+if (HasAt)
   return;
   }
   if (Next->is(tok::exclaim) && PreviousMustBeValue)


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1192,6 +1192,8 @@
   "String");
   verifyFormat("function f(@Foo bar) {}", "function f(@Foo\n"
   "  bar) {}");
+  verifyFormat("function f(@Foo(Param) bar) {}", "function f(@Foo(Param)\n"
+ "  bar) {}");
   verifyFormat("a = true\n"
"return 1",
"a = true\n"
@@ -1557,7 +1559,7 @@
"}");
 }
 
-TEST_F(FormatTestJS, MetadataAnnotations) {
+TEST_F(FormatTestJS, Decorators) {
   verifyFormat("@A\nclass C {\n}");
   verifyFormat("@A({arg: 'value'})\nclass C {\n}");
   verifyFormat("@A\n@B\nclass C {\n}");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -18,6 +18,8 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
+#include 
+
 #define DEBUG_TYPE "format-parser"
 
 namespace clang {
@@ -891,11 +893,14 @@
   bool PreviousMustBeValue = mustBeJSIdentOrValue(Keywords, Previous);
   bool PreviousStartsTemplateExpr =
   Previous->is(TT_TemplateString) && Previous->TokenText.endswith("${");
-  if (PreviousMustBeValue && Line && Line->Tokens.size() > 1) {
-// If the token before the previous one is an '@', the previous token is an
-// annotation and can precede another identifier/value.
-const FormatToken *PrePrevious = std::prev(Line->Tokens.end(), 2)->Tok;
-if (PrePrevious->is(tok::at))
+  if (PreviousMustBeValue || Previous->is(tok::r_paren)) {
+// If the line contains an '@' sign, the previous token might be an
+// annotation, which can precede 

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added a subscriber: klimek.

Provide default implementations so that only getCompileCommands() is mandatory.


https://reviews.llvm.org/D40409

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp

Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -112,6 +112,15 @@
   return DB;
 }
 
+std::vector CompilationDatabase::getAllCompileCommands() const {
+  std::vector Result;
+  for (const auto  : getAllFiles()) {
+auto C = getCompileCommands(File);
+std::move(C.begin(), C.end(), std::back_inserter(Result));
+  }
+  return Result;
+}
+
 CompilationDatabasePlugin::~CompilationDatabasePlugin() {}
 
 namespace {
@@ -342,16 +351,6 @@
   return Result;
 }
 
-std::vector
-FixedCompilationDatabase::getAllFiles() const {
-  return std::vector();
-}
-
-std::vector
-FixedCompilationDatabase::getAllCompileCommands() const {
-  return std::vector();
-}
-
 namespace {
 
 class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -64,10 +64,12 @@
 
 /// \brief Interface for compilation databases.
 ///
-/// A compilation database allows the user to retrieve all compile command lines
-/// that a specified file is compiled with in a project.
-/// The retrieved compile command lines can be used to run clang tools over
-/// a subset of the files in a project.
+/// A compilation database allows the user to retrieve compile command lines
+/// for the files in a project.
+///
+/// Many implementations are enumerable, allowing all command lines to be
+/// retrieved. These can be used to run clang tools over a subset of the files
+/// in a project.
 class CompilationDatabase {
 public:
   virtual ~CompilationDatabase();
@@ -114,15 +116,21 @@
 StringRef FilePath) const = 0;
 
   /// \brief Returns the list of all files available in the compilation database.
-  virtual std::vector getAllFiles() const = 0;
+  ///
+  /// By default, returns nothing. Implementations should override this if they
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 
   /// \brief Returns all compile commands for all the files in the compilation
   /// database.
   ///
   /// FIXME: Add a layer in Tooling that provides an interface to run a tool
   /// over all files in a compilation database. Not all build systems have the
   /// ability to provide a feasible implementation for \c getAllCompileCommands.
-  virtual std::vector getAllCompileCommands() const = 0;
+  ///
+  /// By default, this is implemented in terms of getAllFiles() and
+  /// getCompileCommands(). Subclasses may override this for efficiency.
+  virtual std::vector getAllCompileCommands() const;
 };
 
 /// \brief Interface for compilation database plugins.
@@ -149,6 +157,7 @@
 /// \brief A compilation database that returns a single compile command line.
 ///
 /// Useful when we want a tool to behave more like a compiler invocation.
+/// This compilation database is not enumerable: getAllFiles() returns {}.
 class FixedCompilationDatabase : public CompilationDatabase {
 public:
   /// \brief Creates a FixedCompilationDatabase from the arguments after "--".
@@ -199,17 +208,6 @@
   std::vector
   getCompileCommands(StringRef FilePath) const override;
 
-  /// \brief Returns the list of all files available in the compilation database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllFiles() const override;
-
-  /// \brief Returns all compile commands for all the files in the compilation
-  /// database.
-  ///
-  /// Note: This is always an empty list for the fixed compilation database.
-  std::vector getAllCompileCommands() const override;
-
 private:
   /// This is built up to contain a single entry vector to be returned from
   /// getCompileCommands after adding the positional argument.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40389: [clang-tidy] rename_check.py misc-dangling-handle bugprone-dangling-handle

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318941: [clang-tidy] rename_check.py misc-dangling-handle 
bugprone-dangling-handle (authored by alexfh).

Changed prior to commit:
  https://reviews.llvm.org/D40389?vs=124072=124136#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40389

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h
  clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h
  clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst
  clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp

Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -57,6 +57,9 @@
 Improvements to clang-tidy
 --
 
+- The 'misc-dangling-handle' check was renamed to `bugprone-dangling-handle
+  `_
+
 - The 'misc-argument-comment' check was renamed to `bugprone-argument-comment
   `_
 
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - bugprone-dangling-handle
+
+bugprone-dangling-handle
+
+
+Detect dangling references in value handles like
+``std::experimental::string_view``.
+These dangling references can be a result of constructing handles from temporary
+values, where the temporary is destroyed soon after the handle is created.
+
+Examples:
+
+.. code-block:: c++
+
+  string_view View = string();  // View will dangle.
+  string A;
+  View = A + "A";  // still dangle.
+
+  vector V;
+  V.push_back(string());  // V[0] is dangling.
+  V.resize(3, string());  // V[1] and V[2] will also dangle.
+
+  string_view f() {
+// All these return values will dangle.
+return string();
+string S;
+return S;
+char Array[10]{};
+return Array;
+  }
+
+Options
+---
+
+.. option:: HandleClasses
+
+   A semicolon-separated list of class names that should be treated as handles.
+   By default only ``std::experimental::basic_string_view`` is considered.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
@@ -19,6 +19,7 @@
boost-use-to-string
bugprone-argument-comment
bugprone-copy-constructor-init
+   bugprone-dangling-handle
bugprone-integer-division
bugprone-misplaced-operator-in-strlen-in-alloc
bugprone-string-constructor
@@ -108,7 +109,6 @@
llvm-twine-local
misc-assert-side-effect
misc-bool-pointer-implicit-conversion
-   misc-dangling-handle
misc-definitions-in-headers
misc-fold-init-type
misc-forward-declaration-namespace
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp
@@ -0,0 +1,191 @@
+// RUN: %check_clang_tidy %s bugprone-dangling-handle %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-dangling-handle.HandleClasses, \
+// RUN:   value: 'std::basic_string_view; ::llvm::StringRef;'}]}" \
+// RUN:   -- -std=c++11
+
+namespace std {
+
+template 
+class vector {
+ public:
+  using const_iterator = const T*;
+  using iterator = T*;
+  using size_type = int;
+
+  void assign(size_type count, const T& value);
+  iterator insert(const_iterator pos, const T& value);
+  iterator insert(const_iterator pos, T&& value);
+  iterator insert(const_iterator pos, size_type count, const T& value);
+  void push_back(const T&);
+  void push_back(T&&);
+  void resize(size_type count, const T& value);
+};

[clang-tools-extra] r318941 - [clang-tidy] rename_check.py misc-dangling-handle bugprone-dangling-handle

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Fri Nov 24 01:52:05 2017
New Revision: 318941

URL: http://llvm.org/viewvc/llvm-project?rev=318941=rev
Log:
[clang-tidy] rename_check.py misc-dangling-handle bugprone-dangling-handle

Reviewers: hokein

Reviewed By: hokein

Subscribers: mgorny, xazax.hun, cfe-commits

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

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp
  - copied, changed from r318926, 
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h
  - copied, changed from r318926, 
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-dangling-handle.rst
  - copied, changed from r318926, 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-dangling-handle.cpp
  - copied, changed from r318926, 
clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp
Removed:
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst
clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=318941=318940=318941=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Fri Nov 
24 01:52:05 2017
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "CopyConstructorInitCheck.h"
+#include "DanglingHandleCheck.h"
 #include "IntegerDivisionCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "StringConstructorCheck.h"
@@ -29,6 +30,8 @@ public:
 "bugprone-argument-comment");
 CheckFactories.registerCheck(
 "bugprone-copy-constructor-init");
+CheckFactories.registerCheck(
+"bugprone-dangling-handle");
 CheckFactories.registerCheck(
 "bugprone-integer-division");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=318941=318940=318941=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Fri Nov 24 
01:52:05 2017
@@ -4,6 +4,7 @@ add_clang_library(clangTidyBugproneModul
   ArgumentCommentCheck.cpp
   BugproneTidyModule.cpp
   CopyConstructorInitCheck.cpp
+  DanglingHandleCheck.cpp
   IntegerDivisionCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   StringConstructorCheck.cpp

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp 
(from r318926, clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp?p2=clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp=clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp=318926=318941=318941=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.cpp Fri Nov 
24 01:52:05 2017
@@ -18,7 +18,7 @@ using namespace clang::tidy::matchers;
 
 namespace clang {
 namespace tidy {
-namespace misc {
+namespace bugprone {
 
 namespace {
 
@@ -179,6 +179,6 @@ void DanglingHandleCheck::check(const Ma
   << Handle->getQualifiedNameAsString();
 }
 
-} // namespace misc
+} // namespace bugprone
 } // namespace tidy
 } // namespace clang

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h (from 
r318926, clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h?p2=clang-tools-extra/trunk/clang-tidy/bugprone/DanglingHandleCheck.h=clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h=318926=318941=318941=diff

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/JSONExpr.h:368
+// Typed accessors return None/nullptr if the element has the wrong type.
+llvm::Optional null(size_t I) const {
+  return (*this)[I].null();

ioeric wrote:
> Why is this needed? `v[x].null()` seems to be more intuitive than `v.null(x)`.
In the overwhelmingly common case when parsing, v is a pointer, because it 
resulted from a call to array().

So it's `(*v)[x].null()` vs `v->null(x)` - I think the latter is slightly more 
readable.

But more compelling to me is having consistency between object/array.

We *can* live without these if you like, though - most of the time we iterate 
over arrays rather than indexed access. 



Comment at: unittests/clangd/JSONExprTests.cpp:203
+
+  EXPECT_FALSE(O->null("missing"));
+  EXPECT_FALSE(O->null("boolean"));

ioeric wrote:
> It's not very obvious that this accesses a KV and converts the value, by only 
> reading this line. Would it make sense to make the APIs more explicit e.g. 
> `get_as_xxx(...)`? 
I think this is made worse because it's an artificial example.
I've sent D40406 which has "real life" code - what do you think about the 
getters there?

I think the best variant names would be `asString()` or `getString()`.

I'm a bit reluctant to add a fixed prefix to these common accessors as they 
seem noisy. I also think we'd need to change all the names on Expr(), which 
seems like a shame.

So I'm not sure about changing this, but curious what you think of the Protocol 
conversion code.


https://reviews.llvm.org/D40399



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

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/JSONExpr.h:368
+// Typed accessors return None/nullptr if the element has the wrong type.
+llvm::Optional null(size_t I) const {
+  return (*this)[I].null();

Why is this needed? `v[x].null()` seems to be more intuitive than `v.null(x)`.



Comment at: unittests/clangd/JSONExprTests.cpp:203
+
+  EXPECT_FALSE(O->null("missing"));
+  EXPECT_FALSE(O->null("boolean"));

It's not very obvious that this accesses a KV and converts the value, by only 
reading this line. Would it make sense to make the APIs more explicit e.g. 
`get_as_xxx(...)`? 


https://reviews.llvm.org/D40399



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

[PATCH] D40389: [clang-tidy] rename_check.py misc-dangling-handle bugprone-dangling-handle

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D40389



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

We take the conservative approach there: if a function has no `throw` specifier 
we handle it as it would have a `noexcept` specifier.


https://reviews.llvm.org/D33537



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