Re: [PATCH] D11781: Refactored pthread usage in libcxx

2015-08-11 Thread David Chisnall via cfe-commits
theraven added inline comments.


Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \

espositofulvio wrote:
> jroelofs wrote:
> > espositofulvio wrote:
> > > jroelofs wrote:
> > > > jroelofs wrote:
> > > > > @espositofulvio: @ed meant this:
> > > > > 
> > > > > ```
> > > > > #ifndef _WIN32
> > > > > #  include 
> > > > > #  if _POSIX_THREADS > 0
> > > > > ...
> > > > > #  endif
> > > > > #endif
> > > > > ```
> > > > > 
> > > > > Which //is// the correct way to test for this.
> > > > That being said, there have been discussions before about whether or 
> > > > not we should #include  in <__config>, with the conclusion 
> > > > being that we shouldn't.
> > > > 
> > > > It would be better if this were a CMake configure-time check that sets 
> > > > _LIBCPP_THREAD_API, rather than these build-time guards.
> > > Tried adding that as configure time checks, but then libcxxabi fails to 
> > > compile because of the guard in __config to check that _LIBCPP_THREAD_API 
> > > has beed defined when _LIBCPP_HAS_NO_THREADS is not. 
> > > 
> > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > Tried adding that as configure time checks...
> > 
> > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > I'll take a look.
> > 
> > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > 
> > For the platforms libcxx currently builds on, yes.
> > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > I'll take a look.
> 
> It's here https://gist.github.com/espositofulvio/eac2fb08acf2e430c516
I'm sorry to have triggered this bikeshed.  Given that, of the currently 
supported platforms, Windows is the only one where we want to use threads but 
don't want to use PTHREADS, I think it's fine to have this check be !WIN32.  
The important thing is having the check *in one place* so that the person who 
wants to add the *second* non-pthread threading implementation doesn't have a 
load of different places to look: they can just change it in `__config` and 
then chase all of the compile failures.


Repository:
  rL LLVM

http://reviews.llvm.org/D11781



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


Re: [PATCH] D11781: Refactored pthread usage in libcxx

2015-08-11 Thread Fulvio Esposito via cfe-commits
espositofulvio added inline comments.


Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \

theraven wrote:
> espositofulvio wrote:
> > jroelofs wrote:
> > > espositofulvio wrote:
> > > > jroelofs wrote:
> > > > > jroelofs wrote:
> > > > > > @espositofulvio: @ed meant this:
> > > > > > 
> > > > > > ```
> > > > > > #ifndef _WIN32
> > > > > > #  include 
> > > > > > #  if _POSIX_THREADS > 0
> > > > > > ...
> > > > > > #  endif
> > > > > > #endif
> > > > > > ```
> > > > > > 
> > > > > > Which //is// the correct way to test for this.
> > > > > That being said, there have been discussions before about whether or 
> > > > > not we should #include  in <__config>, with the conclusion 
> > > > > being that we shouldn't.
> > > > > 
> > > > > It would be better if this were a CMake configure-time check that 
> > > > > sets _LIBCPP_THREAD_API, rather than these build-time guards.
> > > > Tried adding that as configure time checks, but then libcxxabi fails to 
> > > > compile because of the guard in __config to check that 
> > > > _LIBCPP_THREAD_API has beed defined when _LIBCPP_HAS_NO_THREADS is not. 
> > > > 
> > > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > > Tried adding that as configure time checks...
> > > 
> > > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > > I'll take a look.
> > > 
> > > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > 
> > > For the platforms libcxx currently builds on, yes.
> > > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > > I'll take a look.
> > 
> > It's here https://gist.github.com/espositofulvio/eac2fb08acf2e430c516
> I'm sorry to have triggered this bikeshed.  Given that, of the currently 
> supported platforms, Windows is the only one where we want to use threads but 
> don't want to use PTHREADS, I think it's fine to have this check be !WIN32.  
> The important thing is having the check *in one place* so that the person who 
> wants to add the *second* non-pthread threading implementation doesn't have a 
> load of different places to look: they can just change it in `__config` and 
> then chase all of the compile failures.
Given that it's something I suggested in a comment on the first revision, I was 
happy to try it out. It was also a quick change. Unfortunately it brakes 
libcxxabi build (at least the way I've done it), so I'm happy to make the 
change if @jroelofs finds another way 


Repository:
  rL LLVM

http://reviews.llvm.org/D11781



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


[PATCH] D11932: [OPENMP] Link libomp.lib on Windows

2015-08-11 Thread Alexey Bataev via cfe-commits
ABataev created this revision.
ABataev added reviewers: chandlerc, rsmith.
ABataev added a subscriber: cfe-commits.

Adds libomp.lib for -fopenmp=libomp and libiomp5md.lib for -fopenmp=libiomp5 on 
Windows

http://reviews.llvm.org/D11932

Files:
  lib/Driver/Tools.cpp
  test/OpenMP/linking.c

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8878,6 +8878,25 @@
 
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+   options::OPT_fno_openmp, false)) {
+CmdArgs.push_back("-nodefaultlib:vcomp.lib");
+CmdArgs.push_back("-nodefaultlib:vcompd.lib");
+switch (getOpenMPRuntime(getToolChain(), Args)) {
+case OMPRT_OMP:
+  CmdArgs.push_back("-defaultlib:libomp.lib");
+  break;
+case OMPRT_IOMP5:
+  CmdArgs.push_back("-defaultlib:libiomp5md.lib");
+  break;
+case OMPRT_GOMP:
+  break;
+case OMPRT_Unknown:
+  // Already diagnosed.
+  break;
+}
+  }
+
   // Add filenames, libraries, and other linker inputs.
   for (const auto &Input : Inputs) {
 if (Input.isFilename()) {
Index: test/OpenMP/linking.c
===
--- test/OpenMP/linking.c
+++ test/OpenMP/linking.c
@@ -69,3 +69,15 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt" "-lgcc"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libomp -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
+// CHECK-MSVC-LINK-64: "link.exe"
+// CHECK-MSVC-LINK-64-SAME: "-nodefaultlib:vcomp.lib" 
"-nodefaultlib:vcompd.lib" "-defaultlib:libomp.lib"
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
+// CHECK-MSVC-ILINK-64: "link.exe"
+// CHECK-MSVC-ILINK-64-SAME: "-nodefaultlib:vcomp.lib" 
"-nodefaultlib:vcompd.lib" "-defaultlib:libiomp5md.lib"
+//


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8878,6 +8878,25 @@
 
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+   options::OPT_fno_openmp, false)) {
+CmdArgs.push_back("-nodefaultlib:vcomp.lib");
+CmdArgs.push_back("-nodefaultlib:vcompd.lib");
+switch (getOpenMPRuntime(getToolChain(), Args)) {
+case OMPRT_OMP:
+  CmdArgs.push_back("-defaultlib:libomp.lib");
+  break;
+case OMPRT_IOMP5:
+  CmdArgs.push_back("-defaultlib:libiomp5md.lib");
+  break;
+case OMPRT_GOMP:
+  break;
+case OMPRT_Unknown:
+  // Already diagnosed.
+  break;
+}
+  }
+
   // Add filenames, libraries, and other linker inputs.
   for (const auto &Input : Inputs) {
 if (Input.isFilename()) {
Index: test/OpenMP/linking.c
===
--- test/OpenMP/linking.c
+++ test/OpenMP/linking.c
@@ -69,3 +69,15 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt" "-lgcc"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libomp -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
+// CHECK-MSVC-LINK-64: "link.exe"
+// CHECK-MSVC-LINK-64-SAME: "-nodefaultlib:vcomp.lib" "-nodefaultlib:vcompd.lib" "-defaultlib:libomp.lib"
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
+// CHECK-MSVC-ILINK-64: "link.exe"
+// CHECK-MSVC-ILINK-64-SAME: "-nodefaultlib:vcomp.lib" "-nodefaultlib:vcompd.lib" "-defaultlib:libiomp5md.lib"
+//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r244583 - Revert "[CUDA] Add implicit __attribute__((used)) to all __global__ functions."

2015-08-11 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Tue Aug 11 06:02:09 2015
New Revision: 244583

URL: http://llvm.org/viewvc/llvm-project?rev=244583&view=rev
Log:
Revert "[CUDA] Add implicit __attribute__((used)) to all __global__ functions."

This is breaking internal test. I'll provide a reproduction.

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGenCUDA/ptx-kernels.cu

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=244583&r1=244582&r2=244583&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug 11 06:02:09 2015
@@ -3350,10 +3350,6 @@ static void handleGlobalAttr(Sema &S, De
   D->addAttr(::new (S.Context)
   CUDAGlobalAttr(Attr.getRange(), S.Context,
  Attr.getAttributeSpellingListIndex()));
-
-  // Add implicit attribute((used)) so we don't eliminate kernels
-  // because there is nothing referencing them on device side.
-  D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
 static void handleGNUInlineAttr(Sema &S, Decl *D, const AttributeList &Attr) {

Modified: cfe/trunk/test/CodeGenCUDA/ptx-kernels.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/ptx-kernels.cu?rev=244583&r1=244582&r2=244583&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/ptx-kernels.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/ptx-kernels.cu Tue Aug 11 06:02:09 2015
@@ -1,16 +1,7 @@
-// Make sure that __global__ functions are emitted along with correct
-// annotations and are added to @llvm.used to prevent their elimination.
-// REQUIRES: nvptx-registered-target
-//
 // RUN: %clang_cc1 %s -triple nvptx-unknown-unknown -fcuda-is-device 
-emit-llvm -o - | FileCheck %s
 
 #include "Inputs/cuda.h"
 
-// Make sure that all __global__ functiona are added to @llvm.used
-// CHECK: @llvm.used = appending global
-// CHECK-SAME: @global_function
-// CHECK-SAME: @_Z16templated_kernelIiEvT_
-
 // CHECK-LABEL: define void @device_function
 extern "C"
 __device__ void device_function() {}
@@ -22,10 +13,4 @@ __global__ void global_function() {
   device_function();
 }
 
-// Make sure host-instantiated kernels are preserved on device side.
-template  __global__ void templated_kernel(T param) {}
-// CHECK-LABEL: define linkonce_odr void @_Z16templated_kernelIiEvT_
-void host_function() { templated_kernel<<<0,0>>>(0); }
-
 // CHECK: !{{[0-9]+}} = !{void ()* @global_function, !"kernel", i32 1}
-// CHECK: !{{[0-9]+}} = !{void (i32)* @_Z16templated_kernelIiEvT_, !"kernel", 
i32 1}


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


Re: [PATCH] D11932: [OPENMP] Link libomp.lib on Windows

2015-08-11 Thread İsmail Dönmez via cfe-commits
ismail added a comment.

OpenMP/linking.c fails here with VS2015 (building with 
-DCLANG_DEFAULT_OPENMP_RUNTIME=libomp)

Command 21 Stderr:
C:\cygwin64\home\ismail\src\llvm\tools\clang\test\OpenMP\linking.c:75:24: 
error: expected string not found in input
// CHECK-MSVC-LINK-64: "link.exe"

  ^

:1:1: note: scanning from here
clang version 3.8.0 (http://llvm.org/git/clang 
d7efa4b285d2464f1844b52b220381c64cd64bf6) (http://llvm.org/git/llvm bee32461e9d
466ed85439a026c62cb896b638653)
^
:6:73: note: possible intended match here
 "C:\\Program Files (x86)\\Microsoft Visual Studio 
14.0\\VC\\BIN\\amd64\\link.exe" "-out:C:\\cygwin64\\home\\ismail\\src\\llvm
\\dist\\tools\\clang\\test\\OpenMP\\Output\\linking.c.tmp.o" 
"-defaultlib:libcmt" "-libpath:C:\\Program Files (x86)\\Microsoft
 Visual Studio 14.0\\VC\\lib\\amd64" "-libpath:C:\\Program Files (x86)\\Windows 
Kits\\8.1\\Lib\\winv6.3\\um\\x64" "-nologo" "-
nodefaultlib:vcomp.lib" "-nodefaultlib:vcompd.lib" "-defaultlib:libomp.lib" 
"C:\\cygwin64\\tmp\\linking-19e90e.o"

  ^


http://reviews.llvm.org/D11932



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


Re: [PATCH] D11932: [OPENMP] Link libomp.lib on Windows

2015-08-11 Thread Alexey Bataev via cfe-commits
ABataev updated this revision to Diff 31792.
ABataev added a comment.

Test fix


http://reviews.llvm.org/D11932

Files:
  lib/Driver/Tools.cpp
  test/OpenMP/linking.c

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8878,6 +8878,25 @@
 
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+   options::OPT_fno_openmp, false)) {
+CmdArgs.push_back("-nodefaultlib:vcomp.lib");
+CmdArgs.push_back("-nodefaultlib:vcompd.lib");
+switch (getOpenMPRuntime(getToolChain(), Args)) {
+case OMPRT_OMP:
+  CmdArgs.push_back("-defaultlib:libomp.lib");
+  break;
+case OMPRT_IOMP5:
+  CmdArgs.push_back("-defaultlib:libiomp5md.lib");
+  break;
+case OMPRT_GOMP:
+  break;
+case OMPRT_Unknown:
+  // Already diagnosed.
+  break;
+}
+  }
+
   // Add filenames, libraries, and other linker inputs.
   for (const auto &Input : Inputs) {
 if (Input.isFilename()) {
Index: test/OpenMP/linking.c
===
--- test/OpenMP/linking.c
+++ test/OpenMP/linking.c
@@ -69,3 +69,19 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt" "-lgcc"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libomp -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
+// CHECK-MSVC-LINK-64: link.exe
+// CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcomp.lib
+// CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcompd.lib
+// CHECK-MSVC-LINK-64-SAME: -defaultlib:libomp.lib
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
+// CHECK-MSVC-ILINK-64: link.exe
+// CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcomp.lib
+// CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcompd.lib
+// CHECK-MSVC-ILINK-64-SAME: -defaultlib:libiomp5md.lib
+//


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8878,6 +8878,25 @@
 
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+   options::OPT_fno_openmp, false)) {
+CmdArgs.push_back("-nodefaultlib:vcomp.lib");
+CmdArgs.push_back("-nodefaultlib:vcompd.lib");
+switch (getOpenMPRuntime(getToolChain(), Args)) {
+case OMPRT_OMP:
+  CmdArgs.push_back("-defaultlib:libomp.lib");
+  break;
+case OMPRT_IOMP5:
+  CmdArgs.push_back("-defaultlib:libiomp5md.lib");
+  break;
+case OMPRT_GOMP:
+  break;
+case OMPRT_Unknown:
+  // Already diagnosed.
+  break;
+}
+  }
+
   // Add filenames, libraries, and other linker inputs.
   for (const auto &Input : Inputs) {
 if (Input.isFilename()) {
Index: test/OpenMP/linking.c
===
--- test/OpenMP/linking.c
+++ test/OpenMP/linking.c
@@ -69,3 +69,19 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt" "-lgcc"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libomp -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
+// CHECK-MSVC-LINK-64: link.exe
+// CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcomp.lib
+// CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcompd.lib
+// CHECK-MSVC-LINK-64-SAME: -defaultlib:libomp.lib
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 \
+// RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
+// CHECK-MSVC-ILINK-64: link.exe
+// CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcomp.lib
+// CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcompd.lib
+// CHECK-MSVC-ILINK-64-SAME: -defaultlib:libiomp5md.lib
+//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r244586 - Add an IncludeInserter to clang-tidy.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 06:37:48 2015
New Revision: 244586

URL: http://llvm.org/viewvc/llvm-project?rev=244586&view=rev
Log:
Add an IncludeInserter to clang-tidy.

Will be used to allow checks to insert includes at the right position.

Added:
clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp
clang-tools-extra/trunk/clang-tidy/IncludeInserter.h
clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp
clang-tools-extra/trunk/clang-tidy/IncludeSorter.h
clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=244586&r1=244585&r2=244586&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Tue Aug 11 06:37:48 2015
@@ -7,6 +7,8 @@ add_clang_library(clangTidy
   ClangTidyModule.cpp
   ClangTidyDiagnosticConsumer.cpp
   ClangTidyOptions.cpp
+  IncludeInserter.cpp
+  IncludeSorter.cpp
 
   DEPENDS
   ClangSACheckers

Added: clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp?rev=244586&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp Tue Aug 11 06:37:48 
2015
@@ -0,0 +1,77 @@
+//=== IncludeInserter.cpp - clang-tidy 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+
+class IncludeInserterCallback : public PPCallbacks {
+public:
+  explicit IncludeInserterCallback(IncludeInserter *IncludeInserter)
+  : IncludeInserter(IncludeInserter) {}
+  // Implements PPCallbacks::InclusionDerective(). Records the names and source
+  // locations of the inclusions in the main source file being processed.
+  void InclusionDirective(SourceLocation HashLocation,
+  const Token & /*include_token*/,
+  StringRef FileNameRef, bool IsAngled,
+  CharSourceRange FileNameRange,
+  const FileEntry * /*IncludedFile*/,
+  StringRef /*SearchPath*/, StringRef /*RelativePath*/,
+  const Module * /*ImportedModule*/) override {
+IncludeInserter->AddInclude(FileNameRef, IsAngled, HashLocation,
+FileNameRange.getEnd());
+  }
+
+private:
+  IncludeInserter *IncludeInserter;
+};
+
+IncludeInserter::IncludeInserter(const SourceManager &SourceMgr,
+ const LangOptions &LangOpts,
+ IncludeSorter::IncludeStyle Style)
+: SourceMgr(SourceMgr), LangOpts(LangOpts), Style(Style) {}
+
+IncludeInserter::~IncludeInserter() {}
+
+std::unique_ptr IncludeInserter::CreatePPCallbacks() {
+  return llvm::make_unique(this);
+}
+
+llvm::Optional
+IncludeInserter::CreateIncludeInsertion(FileID FileID, StringRef Header,
+bool IsAngled) {
+  // We assume the same Header will never be included both angled and not
+  // angled.
+  if (!InsertedHeaders.insert(std::make_pair(FileID, std::set()))
+   .second) {
+return llvm::None;
+  }
+  if (IncludeSorterByFile.find(FileID) == IncludeSorterByFile.end()) {
+return llvm::None;
+  }
+  return IncludeSorterByFile[FileID]->CreateIncludeInsertion(Header, IsAngled);
+}
+
+void IncludeInserter::AddInclude(StringRef file_name, bool IsAngled,
+ SourceLocation HashLocation,
+ SourceLocation end_location) {
+  FileID FileID = SourceMgr.getFileID(HashLocation);
+  if (IncludeSorterByFile.find(FileID) == IncludeSorterByFile.end()) {
+IncludeSorterByFile.insert(std::make_pair(
+FileID, llvm::make_unique(
+&SourceMgr, &LangOpts, FileID,
+SourceMgr.getFilename(HashLocation), Style)));
+  }
+  IncludeSorterByFile[FileID]->AddInclude(file_name, IsAngled, HashLocation,
+  end_location);
+}
+
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/IncludeInserter.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/Inc

Re: r244413 - [modules] When building a dependency file, include module maps parsed in the

2015-08-11 Thread Manuel Klimek via cfe-commits
I believe this breaks -fno-module-file-deps.

On Sun, Aug 9, 2015 at 6:47 AM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Sat Aug  8 23:46:57 2015
> New Revision: 244413
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244413&view=rev
> Log:
> [modules] When building a dependency file, include module maps parsed in
> the
> current compilation, not just those from imported modules.
>
> Modified:
> cfe/trunk/include/clang/Lex/ModuleMap.h
> cfe/trunk/lib/Frontend/DependencyFile.cpp
> cfe/trunk/lib/Lex/ModuleMap.cpp
> cfe/trunk/test/Modules/dependency-gen-pch.m
> cfe/trunk/test/Modules/dependency-gen.m
> cfe/trunk/test/Modules/dependency-gen.modulemap
> cfe/trunk/test/Modules/relative-dep-gen.cpp
>
> Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=244413&r1=244412&r2=244413&view=diff
>
> ==
> --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
> +++ cfe/trunk/include/clang/Lex/ModuleMap.h Sat Aug  8 23:46:57 2015
> @@ -35,6 +35,22 @@ class DiagnosticConsumer;
>  class DiagnosticsEngine;
>  class HeaderSearch;
>  class ModuleMapParser;
> +
> +/// \brief A mechanism to observe the actions of the module map parser as
> it
> +/// reads module map files.
> +class ModuleMapCallbacks {
> +public:
> +  virtual ~ModuleMapCallbacks() {}
> +
> +  /// \brief Called when a module map file has been read.
> +  ///
> +  /// \param FileStart A SourceLocation referring to the start of the
> file's
> +  /// contents.
> +  /// \param File The file itself.
> +  /// \param IsSystem Whether this is a module map from a system include
> path.
> +  virtual void moduleMapFileRead(SourceLocation FileStart,
> + const FileEntry &File, bool IsSystem) {}
> +};
>
>  class ModuleMap {
>SourceManager &SourceMgr;
> @@ -42,6 +58,8 @@ class ModuleMap {
>const LangOptions &LangOpts;
>const TargetInfo *Target;
>HeaderSearch &HeaderInfo;
> +
> +  llvm::SmallVector, 1> Callbacks;
>
>/// \brief The directory used for Clang-supplied, builtin include
> headers,
>/// such as "stdint.h".
> @@ -263,6 +281,11 @@ public:
>  BuiltinIncludeDir = Dir;
>}
>
> +  /// \brief Add a module map callback.
> +  void addModuleMapCallbacks(std::unique_ptr
> Callback) {
> +Callbacks.push_back(std::move(Callback));
> +  }
> +
>/// \brief Retrieve the module that owns the given header file, if any.
>///
>/// \param File The header file that is likely to be included.
>
> Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=244413&r1=244412&r2=244413&view=diff
>
> ==
> --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
> +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Sat Aug  8 23:46:57 2015
> @@ -18,6 +18,7 @@
>  #include "clang/Frontend/FrontendDiagnostic.h"
>  #include "clang/Lex/DirectoryLookup.h"
>  #include "clang/Lex/LexDiagnostic.h"
> +#include "clang/Lex/ModuleMap.h"
>  #include "clang/Lex/PPCallbacks.h"
>  #include "clang/Lex/Preprocessor.h"
>  #include "clang/Serialization/ASTReader.h"
> @@ -82,6 +83,20 @@ struct DepCollectorPPCallbacks : public
>}
>  };
>
> +struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
> +  DependencyCollector &DepCollector;
> +  DepCollectorMMCallbacks(DependencyCollector &DC) : DepCollector(DC) {}
> +
> +  void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
> + bool IsSystem) override {
> +StringRef Filename = Entry.getName();
> +DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
> +/*IsSystem*/IsSystem,
> +/*IsModuleFile*/false,
> +/*IsMissing*/false);
> +  }
> +};
> +
>  struct DepCollectorASTListener : public ASTReaderListener {
>DependencyCollector &DepCollector;
>DepCollectorASTListener(DependencyCollector &L) : DepCollector(L) { }
> @@ -132,6 +147,8 @@ DependencyCollector::~DependencyCollecto
>  void DependencyCollector::attachToPreprocessor(Preprocessor &PP) {
>PP.addPPCallbacks(
>llvm::make_unique(*this,
> PP.getSourceManager()));
> +  PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks(
> +  llvm::make_unique(*this));
>  }
>  void DependencyCollector::attachToASTReader(ASTReader &R) {
>R.addListener(llvm::make_unique(*this));
> @@ -185,6 +202,17 @@ public:
>bool includeModuleFiles() const { return IncludeModuleFiles; }
>  };
>
> +class DFGMMCallback : public ModuleMapCallbacks {
> +  DFGImpl &Parent;
> +public:
> +  DFGMMCallback(DFGImpl &Parent) : Parent(Parent) {}
> +  void moduleMapFileRead(SourceLocation Loc, const FileEn

[clang-tools-extra] r244587 - Fix shadowing of type with variable.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 07:02:28 2015
New Revision: 244587

URL: http://llvm.org/viewvc/llvm-project?rev=244587&view=rev
Log:
Fix shadowing of type with variable.

Modified:
clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp

Modified: clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp?rev=244587&r1=244586&r2=244587&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp Tue Aug 11 07:02:28 
2015
@@ -14,8 +14,8 @@ namespace tidy {
 
 class IncludeInserterCallback : public PPCallbacks {
 public:
-  explicit IncludeInserterCallback(IncludeInserter *IncludeInserter)
-  : IncludeInserter(IncludeInserter) {}
+  explicit IncludeInserterCallback(IncludeInserter *Inserter)
+  : Inserter(Inserter) {}
   // Implements PPCallbacks::InclusionDerective(). Records the names and source
   // locations of the inclusions in the main source file being processed.
   void InclusionDirective(SourceLocation HashLocation,
@@ -25,12 +25,12 @@ public:
   const FileEntry * /*IncludedFile*/,
   StringRef /*SearchPath*/, StringRef /*RelativePath*/,
   const Module * /*ImportedModule*/) override {
-IncludeInserter->AddInclude(FileNameRef, IsAngled, HashLocation,
+Inserter->AddInclude(FileNameRef, IsAngled, HashLocation,
 FileNameRange.getEnd());
   }
 
 private:
-  IncludeInserter *IncludeInserter;
+  IncludeInserter *Inserter;
 };
 
 IncludeInserter::IncludeInserter(const SourceManager &SourceMgr,


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


[clang-tools-extra] r244596 - Default initialize from explicitly constructed object.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 07:13:15 2015
New Revision: 244596

URL: http://llvm.org/viewvc/llvm-project?rev=244596&view=rev
Log:
Default initialize from explicitly constructed object.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244596&r1=244595&r2=244596&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug 11 
07:13:15 2015
@@ -48,7 +48,8 @@ runCheckOnCode(StringRef Code, std::vect
const Twine &Filename = "input.cc",
ArrayRef ExtraArgs = None,
const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
-   std::map PathsToContent = {}) {
+   std::map PathsToContent =
+   std::map()) {
   ClangTidyOptions Options = ExtraOptions;
   Options.Checks = "*";
   ClangTidyContext Context(llvm::make_unique(
@@ -70,7 +71,7 @@ runCheckOnCode(StringRef Code, std::vect
   tooling::ToolInvocation Invocation(
   ArgCXX11, new TestClangTidyAction(Check, Finder, Context), Files.get());
   Invocation.mapVirtualFile(Filename.str(), Code);
-  for (const auto & FileContent : PathsToContent) {
+  for (const auto &FileContent : PathsToContent) {
 Invocation.mapVirtualFile(Twine("include/" + FileContent.first).str(),
   FileContent.second);
   }


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


Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

2015-08-11 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added a comment.

John,
(Others are welcome to chime in as well)

I started to implement your suggestion, but faced with problems.

Consider the following test case:

  1: struct T {
  2:   ~T() {}
  3: };
  4: 
  5: extern "C" void _ZN1TD1Ev();
  6: 
  7: int main() {
  8:   _ZN1TD1Ev();
  9:   T t;
  10: }

First time we visit "GetOrCreateLLVMFunction" for "_ZN1TD1Ev" mangled name is 
when we are dealing with the call at line 8. No global values are associated 
with "_ZN1TD1Ev" name yet, so we simply create a new llvm::Function and add it 
to the list of module's functions 
(ParentModule->getFunctionList().push_back(this); at Function.cpp:264).

Next time we visit "GetOrCreateLLVMFunction" for the same "_ZN1TD1Ev" name is 
when we are dealing with the implicit t's destructor call at the end of "main" 
routine (say, at line 10). We already have a global value associated with 
"_ZN1TD1Ev" name *and* we have a function with this name in module's function 
list. We can't create a new llvm::Function and add it to the functions list -- 
since module's machinery asserts that there is only one function with a given 
mangled name in the list; we can't remove old Function either -- since it is 
referred to in the call at line 8. The best we can do is to bit cast old 
llvm::Function (from line 8) to destructor's type and return it. But this 
generates destructor call for a wrong llvm::Function which means problems and 
further asserts down the road.

Any hints how to resolve this?

Yours,
Andrey

P.S.: I honestly believe this code is incorrect (despite no definition for 
"ZN1TD1Ev" function), so in my opinion the way to go would be to issue a 
warning inside "GetOrCreateLLVMFunction". This is different from my initial 
patch and still compiles fine the following code -- which, as I believe, you 
meant as a correct use of explicitly mangled names:

  struct T {
~T() {}
  };
  
  extern "C" void _ZN1TD1Ev(T *this_p);
  
  int main() {
_ZN1TD1Ev(0);
T t;
  }

(My initial patch warned even on this code, which is probably too strict 
indeed.)


http://reviews.llvm.org/D11297



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


[clang-tools-extra] r244597 - Do not use inheriting constructors.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 07:59:22 2015
New Revision: 244597

URL: http://llvm.org/viewvc/llvm-project?rev=244597&view=rev
Log:
Do not use inheriting constructors.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp

Modified: clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp?rev=244597&r1=244596&r2=244597&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp Tue 
Aug 11 07:59:22 2015
@@ -19,7 +19,8 @@ namespace {
 
 class IncludeInserterCheckBase : public ClangTidyCheck {
 public:
-  using ClangTidyCheck::ClangTidyCheck;
+  IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
   void registerPPCallbacks(CompilerInstance &Compiler) override {
 Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
Compiler.getLangOpts(),
@@ -54,14 +55,16 @@ public:
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
 public:
-  using IncludeInserterCheckBase::IncludeInserterCheckBase;
+  NonSystemHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context) {}
   StringRef HeaderToInclude() const override { return "path/to/header.h"; }
   bool IsAngledInclude() const override { return false; }
 };
 
 class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
 public:
-  using IncludeInserterCheckBase::IncludeInserterCheckBase;
+  CXXSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context) {}
   StringRef HeaderToInclude() const override { return "set"; }
   bool IsAngledInclude() const override { return true; }
 };


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


Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-11 Thread Антон Ярцев via cfe-commits
ayartsev added inline comments.


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2359
@@ -2314,1 +2358,3 @@
+RegionAndSymbolInvalidationTraits ITraits;
+W.AddToWorkList(*I, ITraits);
   }

Too much unnecessary passing around of RegionAndSymbolInvalidationTraits 
parameter. What about moving "RegionAndSymbolInvalidationTraits" member from 
"invalidateRegionsWorker" class to the base class "ClusterAnalysis"?


http://reviews.llvm.org/D11832



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


[clang-tools-extra] r244598 - Fix strict dependency uncovered by windows bot.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 08:11:29 2015
New Revision: 244598

URL: http://llvm.org/viewvc/llvm-project?rev=244598&view=rev
Log:
Fix strict dependency uncovered by windows bot.

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

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=244598&r1=244597&r2=244598&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Tue Aug 11 08:11:29 2015
@@ -18,6 +18,7 @@ add_clang_library(clangTidy
   clangASTMatchers
   clangBasic
   clangFrontend
+  clangLex
   clangRewrite
   clangSema
   clangStaticAnalyzerCore


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


[clang-tools-extra] r244599 - Also ClangTidyTests requires clangLex.

2015-08-11 Thread NAKAMURA Takumi via cfe-commits
Author: chapuni
Date: Tue Aug 11 08:16:51 2015
New Revision: 244599

URL: http://llvm.org/viewvc/llvm-project?rev=244599&view=rev
Log:
Also ClangTidyTests requires clangLex.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt

Modified: clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt?rev=244599&r1=244598&r2=244599&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt Tue Aug 11 
08:16:51 2015
@@ -20,6 +20,7 @@ target_link_libraries(ClangTidyTests
   clangASTMatchers
   clangBasic
   clangFrontend
+  clangLex
   clangTidy
   clangTidyGoogleModule
   clangTidyLLVMModule


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


Re: [PATCH] D11035: trivial patch, improve constness

2015-08-11 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki added a comment.

ping. can somebody review?


http://reviews.llvm.org/D11035



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


[PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: krememek.
danielmarjamaki added a subscriber: cfe-commits.

Don't diagnose -Wunused-parameter in methods that override other methods 
because the overridden methods might use the parameter

Don't diagnose -Wunused-parameter in virtual methods because these might be 
overriden by other methods that use the parameter.

Such diagnostics could be more accurately written if they are based on 
whole-program-analysis that establish if such parameter is unused in all 
methods.



http://reviews.llvm.org/D11940

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/warn-unused-parameters.cpp

Index: test/SemaCXX/warn-unused-parameters.cpp
===
--- test/SemaCXX/warn-unused-parameters.cpp
+++ test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,20 @@
   auto l = [&t...]() { return sizeof...(s); };
   return l();
 }
+
+// Don't diagnose virtual methods or methods that override base class
+// methods.
+class Base {
+public:
+  virtual void f(int x);
+};
+
+class Derived : public Base {
+public:
+  // Don't warn in overridden methods.
+  virtual void f(int x) {}
+
+  // Don't warn in virtual methods.
+  virtual void a(int x) {}
+};
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10797,8 +10797,13 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
-DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+  if (!FD->isDeleted() && !FD->isDefaulted()) {
+// Don't diagnose unused parameters in virtual methods or
+// in methods that override base class methods.
+const auto MD = dyn_cast(FD);
+if (!MD || (MD->size_overridden_methods() == 0U && !MD->isVirtual()))
+  DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+  }
   DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), 
FD->param_end(),
  FD->getReturnType(), FD);
 


Index: test/SemaCXX/warn-unused-parameters.cpp
===
--- test/SemaCXX/warn-unused-parameters.cpp
+++ test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,20 @@
   auto l = [&t...]() { return sizeof...(s); };
   return l();
 }
+
+// Don't diagnose virtual methods or methods that override base class
+// methods.
+class Base {
+public:
+  virtual void f(int x);
+};
+
+class Derived : public Base {
+public:
+  // Don't warn in overridden methods.
+  virtual void f(int x) {}
+
+  // Don't warn in virtual methods.
+  virtual void a(int x) {}
+};
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10797,8 +10797,13 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
-DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+  if (!FD->isDeleted() && !FD->isDefaulted()) {
+// Don't diagnose unused parameters in virtual methods or
+// in methods that override base class methods.
+const auto MD = dyn_cast(FD);
+if (!MD || (MD->size_overridden_methods() == 0U && !MD->isVirtual()))
+  DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+  }
   DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), FD->param_end(),
  FD->getReturnType(), FD);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11916: [CONCEPTS] Add diagnostic; invalid tag when concept specified

2015-08-11 Thread Aaron Ballman via cfe-commits
On Mon, Aug 10, 2015 at 3:00 PM, Nathan Wilson  wrote:
> nwilson created this revision.
> nwilson added reviewers: rsmith, hubert.reinterpretcast, fraggamuffin, 
> faisalv, aaron.ballman.
> nwilson added a subscriber: cfe-commits.
>
> Adding check to emit diagnostic for invalid tag when concept is specified and 
> associated tests.
>
> http://reviews.llvm.org/D11916
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaDecl.cpp
>   test/SemaCXX/cxx-concept-declaration.cpp
>
> Index: test/SemaCXX/cxx-concept-declaration.cpp
> ===
> --- test/SemaCXX/cxx-concept-declaration.cpp
> +++ test/SemaCXX/cxx-concept-declaration.cpp
> @@ -23,3 +23,13 @@
>  template
>  concept bool D6; // expected-error {{variable concept declaration must be 
> initialized}}
>
> +// Tag
> +concept class CC1 {}; // expected-error {{class cannot be marked concept}}
> +concept struct CS1 {}; // expected-error {{struct cannot be marked concept}}
> +concept union CU1 {}; // expected-error {{union cannot be marked concept}}
> +concept enum CE1 {}; // expected-error {{enum cannot be marked concept}}
> +template  concept class TCC1 {}; // expected-error {{class 
> cannot be marked concept}}
> +template  concept struct TCS1 {}; // expected-error {{struct 
> cannot be marked concept}}
> +template  concept union TCU1 {}; // expected-error {{union 
> cannot be marked concept}}
> +
> +extern concept bool ExtC; // expected-error {{'concept' can only appear on 
> the definition of a function template or variable template}}
> Index: lib/Sema/SemaDecl.cpp
> ===
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -3662,6 +3662,19 @@
>  return TagD;
>}
>
> +  if (DS.isConceptSpecified()) {
> +// C++ Concepts TS [dcl.spec.concept]p1: A concept definition refers to
> +// either a function concept and its definition or a variable concept and
> +// its initializer.
> +if (Tag)
> +  Diag(DS.getConceptSpecLoc(), diag::err_concept_tag)
> +  << GetDiagnosticTypeSpecifierID(DS.getTypeSpecType());
> +else
> +  Diag(DS.getConceptSpecLoc(), diag::err_concept_decl_non_template);
> +// Don't emit warnings after this error.
> +return TagD;
> +  }

I'm not certain I understand why we need two different diagnostics for
this case. I think err_concept_decl_non_template is sufficiently clear
for both.

~Aaron

> +
>DiagnoseFunctionSpecifiers(DS);
>
>if (DS.isFriendSpecified()) {
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1973,6 +1973,8 @@
>"function concept declaration must be a definition">;
>  def err_var_concept_not_initialized : Error<
>"variable concept declaration must be initialized">;
> +def err_concept_tag : Error<
> +  "%select{class|struct|interface|union|enum}0 cannot be marked concept">;
>
>  // C++11 char16_t/char32_t
>  def warn_cxx98_compat_unicode_type : Warning<
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D4724: Add framework for iterative compilation to clang

2015-08-11 Thread Radovan Obradovic via cfe-commits
Radovan.Obradovic updated this revision to Diff 31807.
Radovan.Obradovic added a comment.

The patch is updated to be consistent with the patch for IC for LLVM.


http://reviews.llvm.org/D4724

Files:
  include/clang/Driver/Compilation.h
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/Driver.cpp
  lib/Driver/Tools.cpp
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -50,6 +50,10 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include "llvm/Analysis/DecisionTree.h"
+#include "llvm/Support/Program.h"
+#include 
+#include 
 using namespace clang;
 using namespace clang::driver;
 using namespace llvm::opt;
@@ -470,6 +474,35 @@
 Diags.takeClient(), std::move(SerializedConsumer)));
   }
 
+  // Get number of iterations for iterative compilation.
+  int ICNumberOfIterations = 0;
+  std::string ICInputFile;
+  std::string ICInputFile2;
+  std::string ICResultsFile;
+  std::string ICResultsFile2;
+
+  std::unique_ptr CCopts(createDriverOptTable());
+  unsigned MissingArgIndex, MissingArgCount;
+  ArrayRef argv_ref(argv);
+  InputArgList Args = CCopts->ParseArgs(argv_ref.slice(1),
+MissingArgIndex, MissingArgCount);
+  if (Args.getLastArg(options::OPT_fiterative_comp_EQ)) {
+std::string Val = Args.getLastArgValue(options::OPT_fiterative_comp_EQ, "1");
+std::stringstream Convert(Val);
+Convert >> ICNumberOfIterations;
+   }
+  if (!ICNumberOfIterations)
+ICNumberOfIterations = 1; // Default value is 1.
+
+  llvm::ModuleDecisionTrees moduleDecisionTrees;
+
+  int CurrentIteration = 0;
+  int Res = 0;
+
+  llvm::FnNameAndPhase::ICPhase CurrICPhase = llvm::FnNameAndPhase::ModulePhase;
+
+  for (; CurrentIteration < ICNumberOfIterations; CurrentIteration++) {
+
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
@@ -480,12 +513,138 @@
 
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
+  std::stringstream filename;
+
+  if (ICNumberOfIterations > 1) {
+std::string ErrMsg;
+SmallString<128> InputPath;
+filename << "ic-input-" << CurrentIteration;
+if (llvm::sys::fs::createTemporaryFile(filename.str(), ".json", InputPath)) {
+  llvm::errs() << "Error creating temporary file: " << ErrMsg << "\n";
+  ICNumberOfIterations = 1;
+}
+ICInputFile = InputPath.c_str();
+
+SmallString<128> InputPath2;
+
+filename.str("");
+filename << "ic-input2-" << CurrentIteration;
+if (llvm::sys::fs::createTemporaryFile(filename.str(), ".json", InputPath2)) {
+  llvm::errs() << "Error creating temporary file: " << ErrMsg << "\n";
+  ICNumberOfIterations = 1;
+}
+ICInputFile2 = InputPath2.c_str();
+
+SmallString<128> ResPath;
+filename.str("");
+filename << "ic-results-" << CurrentIteration;
+if (llvm::sys::fs::createTemporaryFile(filename.str(), ".json", ResPath)) {
+  llvm::errs() << "Error creating temporary file: " << ErrMsg << "\n";
+  ICNumberOfIterations = 1;
+}
+ICResultsFile = ResPath.c_str();
+
+SmallString<128> ResPath2;
+filename.str("");
+filename << "ic-results2-" << CurrentIteration;
+if (llvm::sys::fs::createTemporaryFile(filename.str(), ".json", ResPath2)) {
+  llvm::errs() << "Error creating temporary file: " << ErrMsg << "\n";
+  ICNumberOfIterations = 1;
+}
+ICResultsFile2 = ResPath2.c_str();
+
+moduleDecisionTrees.getICInputFile() = ICInputFile;
+moduleDecisionTrees.getICInputFile2() = ICInputFile2;
+moduleDecisionTrees.getICResultsFile() = ICResultsFile;
+moduleDecisionTrees.getICResultsFile2() = ICResultsFile2;
+TheDriver.setModuleDecisionTrees(&moduleDecisionTrees);
+
+std::map paths;
+std::map paths1;
+std::map paths2;
+moduleDecisionTrees.getPaths(paths);
+moduleDecisionTrees.startNewIteration();
+moduleDecisionTrees.splitPaths(paths, paths1, paths2);
+std::string pathsStr =
+  llvm::ModuleDecisionTrees::generateMdtPathsString(paths1);
+llvm::DecisionTree::writeToFile(ICInputFile, pathsStr);
+std::string pathsStr2 =
+  llvm::ModuleDecisionTrees::generateMdtPathsString(paths2);
+llvm::DecisionTree::writeToFile(ICInputFile2, pathsStr2);
+  }
+
   std::unique_ptr C(TheDriver.BuildCompilation(argv));
   int Res = 0;
   SmallVector, 4> FailingCommands;
   if (C.get())
 Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
+  if (ICNumberOfIterations > 1) {
+std::string ResultsStrJson = llvm::DecisionTree::readFromFile(ICResultsFile);
+std::vector ResultsJson;
+std::string ErrorMsg;
+if (!llvm::ModuleDecisionTrees::parseMdtResults(ResultsJson,
+  ResultsStrJson, ErrorMsg)) {
+  llvm::errs() << "ITERATIVE COMPILATION ERROR:" << ErrorMsg <<

Re: [PATCH] D3583: Sema: Implement DR244

2015-08-11 Thread Johnny Willemsen via cfe-commits
jwillemsen added a comment.

This isn't fixed in 3.6.2, any idea when this gets into a release?


Repository:
  rL LLVM

http://reviews.llvm.org/D3583



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


[clang-tools-extra] r244602 - 1. Disable tests that currently cannot work on windows due to missing path canonicalization in the file manager.

2015-08-11 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Aug 11 09:21:26 2015
New Revision: 244602

URL: http://llvm.org/viewvc/llvm-project?rev=244602&view=rev
Log:
1. Disable tests that currently cannot work on windows due to missing path 
canonicalization in the file manager.
2. Add better output when a clang-tidy unit test fails so it's clear what the 
error is.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244602&r1=244601&r2=244602&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug 11 
09:21:26 2015
@@ -17,6 +17,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
 #include 
 
 namespace clang {
@@ -76,8 +77,15 @@ runCheckOnCode(StringRef Code, std::vect
   FileContent.second);
   }
   Invocation.setDiagnosticConsumer(&DiagConsumer);
-  if (!Invocation.run())
+  bool Result = Invocation.run();
+  if (!Result) {
+std::string ErrorText;
+for (const auto &Error : Context.getErrors()) {
+  ErrorText += Error.Message.Message + "\n";
+}
+ADD_FAILURE() << ErrorText;
 return "";
+  }
 
   DiagConsumer.finish();
   tooling::Replacements Fixes;

Modified: clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp?rev=244602&r1=244601&r2=244602&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp Tue 
Aug 11 09:21:26 2015
@@ -13,6 +13,16 @@
 #include "ClangTidyTest.h"
 #include "gtest/gtest.h"
 
+// FIXME: Canonicalize paths correctly on windows.
+// Currently, adding virtual files will canonicalize the paths before
+// storing the virtual entries.
+// When resolving virtual entries in the FileManager, the paths (for
+// example coming from a #include directive) are not canonicalized
+// to native paths; thus, the virtual file is not found.
+// This needs to be fixed in the FileManager before we can make
+// clang-tidy tests work.
+#if !defined(_WIN32)
+
 namespace clang {
 namespace tidy {
 namespace {
@@ -76,7 +86,7 @@ std::string runCheckOnCode(StringRef Cod
   return test::runCheckOnCode(Code, &Errors, Filename, None,
  ClangTidyOptions(),
  {// Main file include
-  {"devtools/cymbal/clang_tidy/tests/"
+  {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
   // Non system headers
@@ -95,7 +105,7 @@ std::string runCheckOnCode(StringRef Cod
 
 TEST(IncludeInserterTest, InsertAfterLastNonSystemInclude) {
   const char *PreCode = R"(
-#include "devtools/cymbal/clang_tidy/tests/insert_includes_test_header.h"
+#include "clang_tidy/tests/insert_includes_test_header.h"
 
 #include 
 #include 
@@ -106,7 +116,7 @@ void foo() {
   int a = 0;
 })";
   const char *PostCode = R"(
-#include "devtools/cymbal/clang_tidy/tests/insert_includes_test_header.h"
+#include "clang_tidy/tests/insert_includes_test_header.h"
 
 #include 
 #include 
@@ -119,14 +129,14 @@ void foo() {
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "clang_tidy/tests/"
"insert_includes_test_input2.cc",
   1));
 }
 
 TEST(IncludeInserterTest, InsertBeforeFirstNonSystemInclude) {
   const char *PreCode = R"(
-#include "devtools/cymbal/clang_tidy/tests/insert_includes_test_header.h"
+#include "clang_tidy/tests/insert_includes_test_header.h"
 
 #include 
 #include 
@@ -137,7 +147,7 @@ void foo() {
   int a = 0;
 })";
   const char *PostCode = R"(
-#include "devtools/cymbal/clang_tidy/tests/insert_includes_test_header.h"
+#include "clang_tidy/tests/insert_includes_test_header.h"
 
 #include 
 #include 
@@ -150,14 +160,14 @@ void foo() {
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "clang_tidy/tests/"
"insert_includes_test_input2.cc",
   1));
 }
 
 TEST(IncludeInserterTest, Inser

[PATCH] D11944: Nativize filename in FileManager::getFile().

2015-08-11 Thread Manuel Klimek via cfe-commits
klimek created this revision.
klimek added reviewers: rsmith, chapuni.
klimek added a subscriber: cfe-commits.

For virtual files to work correctly on Windows, we need to canonicalize
the paths before looking into the caches.

This is basically a request for comments - if we want to go this route, I will
make all FileManager methods canonicalize the paths.

http://reviews.llvm.org/D11944

Files:
  lib/Basic/FileManager.cpp

Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -218,9 +218,12 @@
   bool CacheFailure) {
   ++NumFileLookups;
 
+  SmallString<1024> NativeFilename;
+  llvm::sys::path::native(Filename, NativeFilename);
+
   // See if there is already an entry in the map.
   auto &NamedFileEnt =
-  *SeenFileEntries.insert(std::make_pair(Filename, nullptr)).first;
+  *SeenFileEntries.insert(std::make_pair(NativeFilename, nullptr)).first;
 
   // See if there is already an entry in the map.
   if (NamedFileEnt.second)
@@ -241,11 +244,11 @@
   // subdirectory.  This will let us avoid having to waste time on 
known-to-fail
   // searches when we go to find sys/bar.h, because all the search directories
   // without a 'sys' subdir will get a cached failure result.
-  const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, Filename,
+  const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, NativeFilename,
CacheFailure);
   if (DirInfo == nullptr) { // Directory doesn't exist, file can't exist.
 if (!CacheFailure)
-  SeenFileEntries.erase(Filename);
+  SeenFileEntries.erase(NativeFilename);
 
 return nullptr;
   }
@@ -259,7 +262,7 @@
   if (getStatValue(InterndFileName, Data, true, openFile ? &F : nullptr)) {
 // There's no real file at the given path.
 if (!CacheFailure)
-  SeenFileEntries.erase(Filename);
+  SeenFileEntries.erase(NativeFilename);
 
 return nullptr;
   }
@@ -272,9 +275,9 @@
 
   NamedFileEnt.second = &UFE;
 
-  // If the name returned by getStatValue is different than Filename, re-intern
+  // If the name returned by getStatValue is different than NativeFilename, 
re-intern
   // the name.
-  if (Data.Name != Filename) {
+  if (Data.Name != NativeFilename) {
 auto &NamedFileEnt =
 *SeenFileEntries.insert(std::make_pair(Data.Name, nullptr)).first;
 if (!NamedFileEnt.second)


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -218,9 +218,12 @@
   bool CacheFailure) {
   ++NumFileLookups;
 
+  SmallString<1024> NativeFilename;
+  llvm::sys::path::native(Filename, NativeFilename);
+
   // See if there is already an entry in the map.
   auto &NamedFileEnt =
-  *SeenFileEntries.insert(std::make_pair(Filename, nullptr)).first;
+  *SeenFileEntries.insert(std::make_pair(NativeFilename, nullptr)).first;
 
   // See if there is already an entry in the map.
   if (NamedFileEnt.second)
@@ -241,11 +244,11 @@
   // subdirectory.  This will let us avoid having to waste time on known-to-fail
   // searches when we go to find sys/bar.h, because all the search directories
   // without a 'sys' subdir will get a cached failure result.
-  const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, Filename,
+  const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, NativeFilename,
CacheFailure);
   if (DirInfo == nullptr) { // Directory doesn't exist, file can't exist.
 if (!CacheFailure)
-  SeenFileEntries.erase(Filename);
+  SeenFileEntries.erase(NativeFilename);
 
 return nullptr;
   }
@@ -259,7 +262,7 @@
   if (getStatValue(InterndFileName, Data, true, openFile ? &F : nullptr)) {
 // There's no real file at the given path.
 if (!CacheFailure)
-  SeenFileEntries.erase(Filename);
+  SeenFileEntries.erase(NativeFilename);
 
 return nullptr;
   }
@@ -272,9 +275,9 @@
 
   NamedFileEnt.second = &UFE;
 
-  // If the name returned by getStatValue is different than Filename, re-intern
+  // If the name returned by getStatValue is different than NativeFilename, re-intern
   // the name.
-  if (Data.Name != Filename) {
+  if (Data.Name != NativeFilename) {
 auto &NamedFileEnt =
 *SeenFileEntries.insert(std::make_pair(Data.Name, nullptr)).first;
 if (!NamedFileEnt.second)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11944: Nativize filename in FileManager::getFile().

2015-08-11 Thread Yaron Keren via cfe-commits
yaron.keren added a subscriber: yaron.keren.
yaron.keren added a comment.

There is also the case insensitivity issue, see

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


http://reviews.llvm.org/D11944



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


Re: [clang-tools-extra] r244596 - Default initialize from explicitly constructed object.

2015-08-11 Thread David Blaikie via cfe-commits
Which compiler do we support that didn't accept this? I thought we'd
already grown a few uses of initializer lists like this...
On Aug 11, 2015 5:13 AM, "Manuel Klimek via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Author: klimek
> Date: Tue Aug 11 07:13:15 2015
> New Revision: 244596
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244596&view=rev
> Log:
> Default initialize from explicitly constructed object.
>
> Modified:
> clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>
> Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244596&r1=244595&r2=244596&view=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
> +++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug
> 11 07:13:15 2015
> @@ -48,7 +48,8 @@ runCheckOnCode(StringRef Code, std::vect
> const Twine &Filename = "input.cc",
> ArrayRef ExtraArgs = None,
> const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
> -   std::map PathsToContent = {}) {
> +   std::map PathsToContent =
> +   std::map()) {
>ClangTidyOptions Options = ExtraOptions;
>Options.Checks = "*";
>ClangTidyContext Context(llvm::make_unique(
> @@ -70,7 +71,7 @@ runCheckOnCode(StringRef Code, std::vect
>tooling::ToolInvocation Invocation(
>ArgCXX11, new TestClangTidyAction(Check, Finder, Context),
> Files.get());
>Invocation.mapVirtualFile(Filename.str(), Code);
> -  for (const auto & FileContent : PathsToContent) {
> +  for (const auto &FileContent : PathsToContent) {
>  Invocation.mapVirtualFile(Twine("include/" + FileContent.first).str(),
>FileContent.second);
>}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r244597 - Do not use inheriting constructors.

2015-08-11 Thread David Blaikie via cfe-commits
Unsupported (which compiler(s)?) or just not preferred?
On Aug 11, 2015 6:00 AM, "Manuel Klimek via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Author: klimek
> Date: Tue Aug 11 07:59:22 2015
> New Revision: 244597
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244597&view=rev
> Log:
> Do not use inheriting constructors.
>
> Modified:
> clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
>
> Modified:
> clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp?rev=244597&r1=244596&r2=244597&view=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
> Tue Aug 11 07:59:22 2015
> @@ -19,7 +19,8 @@ namespace {
>
>  class IncludeInserterCheckBase : public ClangTidyCheck {
>  public:
> -  using ClangTidyCheck::ClangTidyCheck;
> +  IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context)
> +  : ClangTidyCheck(CheckName, Context) {}
>void registerPPCallbacks(CompilerInstance &Compiler) override {
>  Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
> Compiler.getLangOpts(),
> @@ -54,14 +55,16 @@ public:
>
>  class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
>  public:
> -  using IncludeInserterCheckBase::IncludeInserterCheckBase;
> +  NonSystemHeaderInserterCheck(StringRef CheckName, ClangTidyContext
> *Context)
> +  : IncludeInserterCheckBase(CheckName, Context) {}
>StringRef HeaderToInclude() const override { return "path/to/header.h";
> }
>bool IsAngledInclude() const override { return false; }
>  };
>
>  class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
>  public:
> -  using IncludeInserterCheckBase::IncludeInserterCheckBase;
> +  CXXSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext
> *Context)
> +  : IncludeInserterCheckBase(CheckName, Context) {}
>StringRef HeaderToInclude() const override { return "set"; }
>bool IsAngledInclude() const override { return true; }
>  };
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r244596 - Default initialize from explicitly constructed object.

2015-08-11 Thread Manuel Klimek via cfe-commits
Don't remember which bot it was; the message said something about an
explicit constructor being called for std::map

On Tue, Aug 11, 2015 at 5:00 PM David Blaikie  wrote:

> Which compiler do we support that didn't accept this? I thought we'd
> already grown a few uses of initializer lists like this...
> On Aug 11, 2015 5:13 AM, "Manuel Klimek via cfe-commits" <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: klimek
>> Date: Tue Aug 11 07:13:15 2015
>> New Revision: 244596
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=244596&view=rev
>> Log:
>> Default initialize from explicitly constructed object.
>>
>> Modified:
>> clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>>
>> Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244596&r1=244595&r2=244596&view=diff
>>
>> ==
>> --- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>> (original)
>> +++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug
>> 11 07:13:15 2015
>> @@ -48,7 +48,8 @@ runCheckOnCode(StringRef Code, std::vect
>> const Twine &Filename = "input.cc",
>> ArrayRef ExtraArgs = None,
>> const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
>> -   std::map PathsToContent = {}) {
>> +   std::map PathsToContent =
>> +   std::map()) {
>>ClangTidyOptions Options = ExtraOptions;
>>Options.Checks = "*";
>>ClangTidyContext Context(llvm::make_unique(
>> @@ -70,7 +71,7 @@ runCheckOnCode(StringRef Code, std::vect
>>tooling::ToolInvocation Invocation(
>>ArgCXX11, new TestClangTidyAction(Check, Finder, Context),
>> Files.get());
>>Invocation.mapVirtualFile(Filename.str(), Code);
>> -  for (const auto & FileContent : PathsToContent) {
>> +  for (const auto &FileContent : PathsToContent) {
>>  Invocation.mapVirtualFile(Twine("include/" +
>> FileContent.first).str(),
>>FileContent.second);
>>}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11944: Nativize filename in FileManager::getFile().

2015-08-11 Thread Manuel Klimek via cfe-commits
klimek added a comment.

The case sensitivity stuff is related, but a much larger problem - while the 
native paths are system dependent, the case sensitivity is file system 
dependent - I can have case insensitive file systems on any OS.


http://reviews.llvm.org/D11944



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


Re: [clang-tools-extra] r244597 - Do not use inheriting constructors.

2015-08-11 Thread Manuel Klimek via cfe-commits
MSVC 2013

On Tue, Aug 11, 2015 at 5:02 PM David Blaikie  wrote:

> Unsupported (which compiler(s)?) or just not preferred?
> On Aug 11, 2015 6:00 AM, "Manuel Klimek via cfe-commits" <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: klimek
>> Date: Tue Aug 11 07:59:22 2015
>> New Revision: 244597
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=244597&view=rev
>> Log:
>> Do not use inheriting constructors.
>>
>> Modified:
>> clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp?rev=244597&r1=244596&r2=244597&view=diff
>>
>> ==
>> --- clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
>> (original)
>> +++ clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
>> Tue Aug 11 07:59:22 2015
>> @@ -19,7 +19,8 @@ namespace {
>>
>>  class IncludeInserterCheckBase : public ClangTidyCheck {
>>  public:
>> -  using ClangTidyCheck::ClangTidyCheck;
>> +  IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext
>> *Context)
>> +  : ClangTidyCheck(CheckName, Context) {}
>>void registerPPCallbacks(CompilerInstance &Compiler) override {
>>  Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
>> Compiler.getLangOpts(),
>> @@ -54,14 +55,16 @@ public:
>>
>>  class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
>>  public:
>> -  using IncludeInserterCheckBase::IncludeInserterCheckBase;
>> +  NonSystemHeaderInserterCheck(StringRef CheckName, ClangTidyContext
>> *Context)
>> +  : IncludeInserterCheckBase(CheckName, Context) {}
>>StringRef HeaderToInclude() const override { return
>> "path/to/header.h"; }
>>bool IsAngledInclude() const override { return false; }
>>  };
>>
>>  class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
>>  public:
>> -  using IncludeInserterCheckBase::IncludeInserterCheckBase;
>> +  CXXSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext
>> *Context)
>> +  : IncludeInserterCheckBase(CheckName, Context) {}
>>StringRef HeaderToInclude() const override { return "set"; }
>>bool IsAngledInclude() const override { return true; }
>>  };
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread Nico Weber via cfe-commits
Can't you just change your signature to

  virtual void a(int /* x */) {}

in these cases?

On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki <
cfe-commits@lists.llvm.org> wrote:

> danielmarjamaki created this revision.
> danielmarjamaki added a reviewer: krememek.
> danielmarjamaki added a subscriber: cfe-commits.
>
> Don't diagnose -Wunused-parameter in methods that override other methods
> because the overridden methods might use the parameter
>
> Don't diagnose -Wunused-parameter in virtual methods because these might
> be overriden by other methods that use the parameter.
>
> Such diagnostics could be more accurately written if they are based on
> whole-program-analysis that establish if such parameter is unused in all
> methods.
>
>
>
> http://reviews.llvm.org/D11940
>
> Files:
>   lib/Sema/SemaDecl.cpp
>   test/SemaCXX/warn-unused-parameters.cpp
>
> Index: test/SemaCXX/warn-unused-parameters.cpp
> ===
> --- test/SemaCXX/warn-unused-parameters.cpp
> +++ test/SemaCXX/warn-unused-parameters.cpp
> @@ -32,3 +32,20 @@
>auto l = [&t...]() { return sizeof...(s); };
>return l();
>  }
> +
> +// Don't diagnose virtual methods or methods that override base class
> +// methods.
> +class Base {
> +public:
> +  virtual void f(int x);
> +};
> +
> +class Derived : public Base {
> +public:
> +  // Don't warn in overridden methods.
> +  virtual void f(int x) {}
> +
> +  // Don't warn in virtual methods.
> +  virtual void a(int x) {}
> +};
> +
> Index: lib/Sema/SemaDecl.cpp
> ===
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -10797,8 +10797,13 @@
>
>  if (!FD->isInvalidDecl()) {
>// Don't diagnose unused parameters of defaulted or deleted
> functions.
> -  if (!FD->isDeleted() && !FD->isDefaulted())
> -DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
> +  if (!FD->isDeleted() && !FD->isDefaulted()) {
> +// Don't diagnose unused parameters in virtual methods or
> +// in methods that override base class methods.
> +const auto MD = dyn_cast(FD);
> +if (!MD || (MD->size_overridden_methods() == 0U &&
> !MD->isVirtual()))
> +  DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
> +  }
>DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(),
> FD->param_end(),
>   FD->getReturnType(), FD);
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread Nico Weber via cfe-commits
thakis added a subscriber: thakis.
thakis added a comment.

Can't you just change your signature to

  virtual void a(int /* x */) {}

in these cases?


http://reviews.llvm.org/D11940



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


Re: [PATCH] D11781: Refactored pthread usage in libcxx

2015-08-11 Thread Jonathan Roelofs via cfe-commits
jroelofs added a subscriber: EricWF.


Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \

espositofulvio wrote:
> theraven wrote:
> > espositofulvio wrote:
> > > jroelofs wrote:
> > > > espositofulvio wrote:
> > > > > jroelofs wrote:
> > > > > > jroelofs wrote:
> > > > > > > @espositofulvio: @ed meant this:
> > > > > > > 
> > > > > > > ```
> > > > > > > #ifndef _WIN32
> > > > > > > #  include 
> > > > > > > #  if _POSIX_THREADS > 0
> > > > > > > ...
> > > > > > > #  endif
> > > > > > > #endif
> > > > > > > ```
> > > > > > > 
> > > > > > > Which //is// the correct way to test for this.
> > > > > > That being said, there have been discussions before about whether 
> > > > > > or not we should #include  in <__config>, with the 
> > > > > > conclusion being that we shouldn't.
> > > > > > 
> > > > > > It would be better if this were a CMake configure-time check that 
> > > > > > sets _LIBCPP_THREAD_API, rather than these build-time guards.
> > > > > Tried adding that as configure time checks, but then libcxxabi fails 
> > > > > to compile because of the guard in __config to check that 
> > > > > _LIBCPP_THREAD_API has beed defined when _LIBCPP_HAS_NO_THREADS is 
> > > > > not. 
> > > > > 
> > > > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > > > Tried adding that as configure time checks...
> > > > 
> > > > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > > > I'll take a look.
> > > > 
> > > > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > > 
> > > > For the platforms libcxx currently builds on, yes.
> > > > Can you put the patch for that up on gist.github.com, or a pastebin?... 
> > > > I'll take a look.
> > > 
> > > It's here https://gist.github.com/espositofulvio/eac2fb08acf2e430c516
> > I'm sorry to have triggered this bikeshed.  Given that, of the currently 
> > supported platforms, Windows is the only one where we want to use threads 
> > but don't want to use PTHREADS, I think it's fine to have this check be 
> > !WIN32.  The important thing is having the check *in one place* so that the 
> > person who wants to add the *second* non-pthread threading implementation 
> > doesn't have a load of different places to look: they can just change it in 
> > `__config` and then chase all of the compile failures.
> Given that it's something I suggested in a comment on the first revision, I 
> was happy to try it out. It was also a quick change. Unfortunately it brakes 
> libcxxabi build (at least the way I've done it), so I'm happy to make the 
> change if @jroelofs finds another way 
I meant something like this: 
https://gist.github.com/jroelofs/279cb2639ad910b953d2

... which doesn't quite work yet, but should give you the idea. I don't know 
how to get CMake to treat `${CMAKE_BINARY_DIR}/include/c++/v1` as the includes 
directory instead of `${CMAKE_CURRENT_SOURCE_DIR}/../include`. Maybe @ericwf 
can help with that?

Basically the idea is that we have cmake create a __config_site header which 
__config includes.  This new header would then have the definitions for these 
kinds of configure-time decisions.


Repository:
  rL LLVM

http://reviews.llvm.org/D11781



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


[PATCH] Expose diagnostic formatting to Python bindings

2015-08-11 Thread Omar Sandoval via cfe-commits
This makes it easier for tools using the Python libclang bindings to
display diagnostics in a manner consistent with clang.
---
 bindings/python/clang/cindex.py | 33 +
 1 file changed, 33 insertions(+)

diff --git a/bindings/python/clang/cindex.py b/bindings/python/clang/cindex.py
index f5caca8572cb..f46084b140aa 100644
--- a/bindings/python/clang/cindex.py
+++ b/bindings/python/clang/cindex.py
@@ -305,6 +305,14 @@ class Diagnostic(object):
 Error   = 3
 Fatal   = 4
 
+DisplaySourceLocation = 0x01
+DisplayColumn = 0x02
+DisplaySourceRanges   = 0x04
+DisplayOption = 0x08
+DisplayCategoryId = 0x10
+DisplayCategoryName   = 0x20
+_FormatOptionsMask = 0x3f
+
 def __init__(self, ptr):
 self.ptr = ptr
 
@@ -382,10 +390,27 @@ class Diagnostic(object):
 
 return conf.lib.clang_getCString(disable)
 
+def format(self, options=None):
+"""
+Format this diagnostic for display. The options argument takes
+Diagnostic.DisplayFoo flags, which can be combined using bitwise OR. If
+the options argument is not provided, the default display options will
+be used.
+"""
+if options is None:
+options = conf.lib.clang_defaultDiagnosticDisplayOptions()
+elif options & ~Diagnostic._FormatOptionsMask:
+raise ValueError('Invalid format options')
+formatted = conf.lib.clang_formatDiagnostic(self, options)
+return conf.lib.clang_getCString(formatted)
+
 def __repr__(self):
 return "" % (
 self.severity, self.location, self.spelling)
 
+def __str__(self):
+return self.format()
+
 def from_param(self):
   return self.ptr
 
@@ -2889,6 +2914,10 @@ functionList = [
[Cursor],
bool),
 
+  ("clang_defaultDiagnosticDisplayOptions",
+   [],
+   c_uint),
+
   ("clang_defaultSaveOptions",
[TranslationUnit],
c_uint),
@@ -2930,6 +2959,10 @@ functionList = [
[Type, Type],
bool),
 
+  ("clang_formatDiagnostic",
+   [Diagnostic, c_uint],
+   _CXString),
+
   ("clang_getArgType",
[Type, c_uint],
Type,
-- 
2.5.0

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


[PATCH] D11946: Create clang-tidy module modernize. Add pass-by-value check.

2015-08-11 Thread Angel Garcia via cfe-commits
angelgarcia created this revision.
angelgarcia added a reviewer: alexfh.
angelgarcia added a subscriber: cfe-commits.
angelgarcia changed the visibility of this Differential Revision from "Public 
(No Login Required)" to "All Users".

This is the first step for migrating cppmodernize to clang-tidy.

http://reviews.llvm.org/D11946

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/Makefile
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/Makefile
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/PassByValueCheck.cpp
  clang-tidy/modernize/PassByValueCheck.h
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/tool/Makefile
  test/clang-tidy/modernize-pass-by-value.cpp

Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -0,0 +1,185 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s modernize-pass-by-value %t
+// REQUIRES: shell
+
+// CHECK: #include 
+
+namespace {
+// POD types are trivially move constructible
+struct Movable {
+  int a, b, c;
+};
+
+struct NotMovable {
+  NotMovable() = default;
+  NotMovable(const NotMovable &) = default;
+  NotMovable(NotMovable &&) = delete;
+  int a, b, c;
+};
+}
+
+struct A {
+  A(const Movable &M) : M(M) {}
+  // CHECK: A(Movable M) : M(std::move(M)) {}
+  // SAFE_RISK: A(const Movable &M) : M(M) {}
+  Movable M;
+};
+
+// Test that we aren't modifying other things than a parameter
+Movable GlobalObj;
+struct B {
+  B(const Movable &M) : M(GlobalObj) {}
+  // CHECK: B(const Movable &M) : M(GlobalObj) {}
+  Movable M;
+};
+
+// Test that a parameter with more than one reference to it won't be changed.
+struct C {
+  // Tests extra-reference in body
+  C(const Movable &M) : M(M) { this->i = M.a; }
+  // CHECK: C(const Movable &M) : M(M) { this->i = M.a; }
+
+  // Tests extra-reference in init-list
+  C(const Movable &M, int) : M(M), i(M.a) {}
+  // CHECK: C(const Movable &M, int) : M(M), i(M.a) {}
+  Movable M;
+  int i;
+};
+
+// Test that both declaration and definition are updated
+struct D {
+  D(const Movable &M);
+  // CHECK: D(Movable M);
+  Movable M;
+};
+D::D(const Movable &M) : M(M) {}
+// CHECK: D::D(Movable M) : M(std::move(M)) {}
+
+// Test with default parameter
+struct E {
+  E(const Movable &M = Movable()) : M(M) {}
+  // CHECK: E(Movable M = Movable()) : M(std::move(M)) {}
+  Movable M;
+};
+
+// Test with object that can't be moved
+struct F {
+  F(const NotMovable &NM) : NM(NM) {}
+  // CHECK: F(const NotMovable &NM) : NM(NM) {}
+  NotMovable NM;
+};
+
+// Test unnamed parameter in declaration
+struct G {
+  G(const Movable &);
+  // CHECK: G(Movable );
+  Movable M;
+};
+G::G(const Movable &M) : M(M) {}
+// CHECK: G::G(Movable M) : M(std::move(M)) {}
+
+// Test parameter with and without qualifier
+namespace ns_H {
+typedef ::Movable HMovable;
+}
+struct H {
+  H(const ns_H::HMovable &M);
+  // CHECK: H(ns_H::HMovable M);
+  ns_H::HMovable M;
+};
+using namespace ns_H;
+H::H(const HMovable &M) : M(M) {}
+// CHECK: H(HMovable M) : M(std::move(M)) {}
+
+// Try messing up with macros
+#define MOVABLE_PARAM(Name) const Movable & Name
+// CHECK: #define MOVABLE_PARAM(Name) const Movable & Name
+struct I {
+  I(MOVABLE_PARAM(M)) : M(M) {}
+  // CHECK: I(MOVABLE_PARAM(M)) : M(M) {}
+  Movable M;
+};
+#undef MOVABLE_PARAM
+
+// Test that templates aren't modified
+template  struct J {
+  J(const T &M) : M(M) {}
+  // CHECK: J(const T &M) : M(M) {}
+  T M;
+};
+J j1(Movable());
+J j2(NotMovable());
+
+struct K_Movable {
+  K_Movable() = default;
+  K_Movable(const K_Movable &) = default;
+  K_Movable(K_Movable &&o) { dummy = o.dummy; }
+  int dummy;
+};
+
+// Test with movable type with an user defined move constructor.
+struct K {
+  K(const K_Movable &M) : M(M) {}
+  // CHECK: K(K_Movable M) : M(std::move(M)) {}
+  K_Movable M;
+};
+
+template  struct L {
+  L(const Movable &M) : M(M) {}
+  // CHECK: L(Movable M) : M(std::move(M)) {}
+  Movable M;
+};
+L l(Movable());
+
+// Test with a non-instantiated template class
+template  struct N {
+  N(const Movable &M) : M(M) {}
+  // CHECK: N(Movable M) : M(std::move(M)) {}
+
+  Movable M;
+  T A;
+};
+
+// Test with value parameter
+struct O {
+  O(Movable M) : M(M) {}
+  // CHECK: O(Movable M) : M(std::move(M)) {}
+  Movable M;
+};
+
+// Test with a const-value parameter
+struct P {
+  P(const Movable M) : M(M) {}
+  // CHECK: P(const Movable M) : M(M) {}
+  Movable M;
+};
+
+// Test with multiples parameters where some need to be changed and some don't
+// need to.
+struct Q {
+  Q(const Movable &A, const Movable &B, const Movable &C, double D)
+  : A(A), B(B), C(C), D(D) {}
+  // CHECK:  Q(const Movable &A, Movable B, Movable C, double D)
+  // CHECK-NEXT: : A(A), B(std::move(B)), C(std::move(C)), D(D) {}
+  const Movable &A;
+  Movable B;
+  Movable C;
+  double D;
+};
+
+// 

[PATCH] clang-format: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Jon Chesterfield via cfe-commits
Hi,

I believe this is being sent to the correct list. Please let me know if
there is a better choice.

The clang-format option SpaceBeforeParens "Always" does not insert a space
before the opening parenthesis of an overloaded operator function. The
attached patch against trunk resolves this.

Kind regards,

Jon Chesterfield
SN Systems - Sony Computer Entertainment Group.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp   (revision 244436)
+++ lib/Format/TokenAnnotator.cpp   (working copy)
@@ -1997,7 +1997,7 @@
   if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
 return true;
   if (Right.is(TT_OverloadedOperatorLParen))
-return false;
+return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
   if (Right.is(tok::colon)) {
 if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
 !Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp (revision 244436)
+++ unittests/Format/FormatTest.cpp (working copy)
@@ -8276,6 +8276,8 @@
   verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");", NoSpace);
   verifyFormat("int f() throw(Deprecated);", NoSpace);
   verifyFormat("typedef void (*cb)(int);", NoSpace);
+  verifyFormat("T A::operator()();",NoSpace);
+  verifyFormat("X A::operator++(T);",NoSpace);
 
   FormatStyle Space = getLLVMStyle();
   Space.SpaceBeforeParens = FormatStyle::SBPO_Always;
@@ -8321,6 +8323,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
+  verifyFormat("T A::operator() ();",Space);
+  verifyFormat("X A::operator++ (T);",Space);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] [Rewriter] Fix RemoveText when IncludeInsertsAtBeginOfRange=true

2015-08-11 Thread Alex Corrado via cfe-commits
Hello,

I’m new to Clang, and I’d like to start off by saying that it's been a real 
treat to work with so far.

I’ve been putting the Rewriter through its paces and found a bug: RemoveText 
does not use the correct starting offset when IncludeInsertsAtBeginOfRange=true 
in the RewriteOptions and some text has been inserted at the beginning of the 
range.

I’ve attached a patch. Apologies if it isn’t in the correct format, as this is 
my first one.

Thanks again for all your great work!

-Alex



0001-Rewriter-Fix-RemoveText-when-IncludeInsertsAtBeginOf.patch
Description: Binary data


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


[PATCH] D11851: clang-format: Add "AllowShortNamespacesOnASingleLine" option

2015-08-11 Thread Chris Beck via cfe-commits
cbeck88 created this revision.
cbeck88 added a reviewer: djasper.
cbeck88 added a subscriber: cfe-commits.
cbeck88 set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.

Rationale:

I sometimes use a different clang tool, iwyu ("include what you use"), to clean 
up header file inclusions in my C++ projects. Iwyu seeks to correct the 
includes of a header or cpp unit so that definitions which are needed are 
included, and definitions which only need to be forward declared are forward 
declared. It often generates code like this at the top of your file:

namespace foo { class bar; }
namespace baz { struct quaz; }
...

Unfortunately, clang-format dislikes braces which are arranged this way and 
always wants to break after them, and after the forward declaration, no matter 
what configuration options are used (as far as I can tell).

I wrote this small patch so that short namespaces like these can be set on a 
single line regardless of chosen brace-style if a boolean option 
"AllowShortNamespacesOnASingleLine" is enabled.

Repository:
  rL LLVM

http://reviews.llvm.org/D11851

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10433,6 +10433,17 @@
   verifyNoCrash("#define a\\\n /**/}");
 }
 
+TEST_F(FormatTest, ShortNamespacesOption) {
+  FormatStyle style = getLLVMStyle();
+  style.AllowShortNamespacesOnASingleLine = true;
+
+  verifyFormat("namespace foo { class bar; }", style);
+  verifyFormat("namespace foo {\nclass bar;\nclass baz;\n}", style);
+  verifyFormat("namespace foo {\nint f() { return 5; }\n}", style);
+  verifyFormat("namespace foo { template  struct bar; }", style);
+  verifyFormat("namespace foo { constexpr int num = 42; }", style);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -199,6 +199,12 @@
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
 if (TheLine->Last->is(tok::l_brace)) {
+  if (Style.AllowShortNamespacesOnASingleLine &&
+ TheLine->First->is(tok::kw_namespace)) {
+if (unsigned result = tryMergeNamespace(I, E, Limit)) {
+  return result;
+}
+  }
   return Style.BreakBeforeBraces == FormatStyle::BS_Attach
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
@@ -258,6 +264,35 @@
 return 1;
   }
 
+  unsigned tryMergeNamespace(
+  SmallVectorImpl::const_iterator I,
+  SmallVectorImpl::const_iterator E, unsigned Limit) {
+if (Limit == 0)
+  return 0;
+if (I[1]->InPPDirective != (*I)->InPPDirective ||
+(I[1]->InPPDirective && I[1]->First->HasUnescapedNewline))
+  return 0;
+
+Limit = limitConsideringMacros(I + 1, E, Limit);
+
+if (1 + I[1]->Last->TotalLength > Limit)
+  return 0;
+
+// An assumption of the function is that the first line begins with
+// keyword namespace (i.e. I[0]->First->is(tok::kw_namespace))
+
+// The line which is in the namespace should end with semicolon
+if (I[1]->Last->isNot(tok::semi))
+  return 0;
+
+// Last, check that the third line starts with a closing brace.
+if (I[2]->First->isNot(tok::r_brace))
+  return 0;
+
+// If so, merge all three lines.
+return 2;
+  }
+
   unsigned tryMergeSimpleControlStatement(
   SmallVectorImpl::const_iterator I,
   SmallVectorImpl::const_iterator E, unsigned Limit) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -212,6 +212,8 @@
Style.AllowShortIfStatementsOnASingleLine);
 IO.mapOptional("AllowShortLoopsOnASingleLine",
Style.AllowShortLoopsOnASingleLine);
+IO.mapOptional("AllowShortNamespacesOnASingleLine",
+   Style.AllowShortNamespacesOnASingleLine);
 IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
Style.AlwaysBreakAfterDefinitionReturnType);
 IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
@@ -354,6 +356,7 @@
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
+  LLVMStyle.AllowShortNamespacesOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;
Index: include/clang/Format/Format.h
==

Re: [PATCH] clang-format: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Daniel Jasper via cfe-commits
If possible, could you send to patch through reviews.llvm.org?

On Tue, Aug 11, 2015 at 1:37 PM, Jon Chesterfield via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi,
>
> I believe this is being sent to the correct list. Please let me know if
> there is a better choice.
>
> The clang-format option SpaceBeforeParens "Always" does not insert a space
> before the opening parenthesis of an overloaded operator function. The
> attached patch against trunk resolves this.
>
> Kind regards,
>
> Jon Chesterfield
> SN Systems - Sony Computer Entertainment Group.
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r244488 - [dllimport] A non-imported class with an imported key can't have a key

2015-08-11 Thread Hans Wennborg via cfe-commits
On Mon, Aug 10, 2015 at 12:39 PM, Reid Kleckner via cfe-commits
 wrote:
> Author: rnk
> Date: Mon Aug 10 14:39:01 2015
> New Revision: 244488
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244488&view=rev
> Log:
> [dllimport] A non-imported class with an imported key can't have a key
>
> Summary:
> The vtable takes its DLL storage class from the class, not the key
> function. When they disagree, the vtable won't be exported by the DLL
> that defines the key function. The easiest way to ensure that importers
> of the class emit their own vtable is to say that the class has no key
> function.
>
> Reviewers: hans, majnemer
>
> Subscribers: cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D11913

Should we merge this and r244266 to 3.7?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r244488 - [dllimport] A non-imported class with an imported key can't have a key

2015-08-11 Thread Reid Kleckner via cfe-commits
On Mon, Aug 10, 2015 at 6:40 PM, Richard Smith 
wrote:

> On Mon, Aug 10, 2015 at 5:43 PM, Reid Kleckner  wrote:
>
>> On Mon, Aug 10, 2015 at 5:00 PM, Richard Smith 
>> wrote:
>>
>>> On Mon, Aug 10, 2015 at 12:39 PM, Reid Kleckner via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:

 +// If the key function is dllimport but the class isn't, then the
 class has
 +// no key function. The DLL that exports the key function won't
 export the
 +// vtable in this case.
 +if (MD->hasAttr() && !RD->hasAttr())
 +  return nullptr;

>>>
>>> Does the same apply if the key function is DLLExport and the class is
>>> not? (Presumably this would just lead us to export a vtable that we don't
>>> need to, which is presumably harmless?)
>>>
>>
>> No, there's no issue with dllexport. If the key function is dllexport,
>> then it must be part of the same DLL as the current TU, and we can rely on
>> it to provide (potentially non-exported) vtable and RTTI symbols.
>>
>
> Even if a different compiler is used to compile the other TUs in this DLL?
> It seems to me that if the rule really is "the vtable takes its DLL storage
> class from the class, not the key function", then the TU with the key
> function need not actually provide a vtable symbol if the class is not
> dllexport but the key function is.
>

GCC (and presumably other Itanium compilers that support dllexport) still
emit the vtable with the key function when the key is exported. It's just
not necessarily exported. All the TUs that are statically linked into the
DLL containing an exported key function can rely on the vtable to be there,
so I don't see any reason to add logic around dllexport here. Make sense?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r244488 - [dllimport] A non-imported class with an imported key can't have a key

2015-08-11 Thread Reid Kleckner via cfe-commits
Yeah, let's do that.

On Tue, Aug 11, 2015 at 9:40 AM, Hans Wennborg  wrote:

> On Mon, Aug 10, 2015 at 12:39 PM, Reid Kleckner via cfe-commits
>  wrote:
> > Author: rnk
> > Date: Mon Aug 10 14:39:01 2015
> > New Revision: 244488
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=244488&view=rev
> > Log:
> > [dllimport] A non-imported class with an imported key can't have a key
> >
> > Summary:
> > The vtable takes its DLL storage class from the class, not the key
> > function. When they disagree, the vtable won't be exported by the DLL
> > that defines the key function. The easiest way to ensure that importers
> > of the class emit their own vtable is to say that the class has no key
> > function.
> >
> > Reviewers: hans, majnemer
> >
> > Subscribers: cfe-commits
> >
> > Differential Revision: http://reviews.llvm.org/D11913
>
> Should we merge this and r244266 to 3.7?
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D10365: Add cmd to compilation database file format

2015-08-11 Thread Daniel Dilts via cfe-commits
diltsman updated this revision to Diff 31824.

http://reviews.llvm.org/D10365

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

Index: ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -36,8 +36,13 @@
   expectFailure("[{[]:\"\"}]", "Incorrectly typed entry");
   expectFailure("[{}]", "Empty entry");
   expectFailure("[{\"directory\":\"\",\"command\":\"\"}]", "Missing file");
-  expectFailure("[{\"directory\":\"\",\"file\":\"\"}]", "Missing command");
+  expectFailure("[{\"directory\":\"\",\"file\":\"\"}]", "Missing command or arguments");
   expectFailure("[{\"command\":\"\",\"file\":\"\"}]", "Missing directory");
+  expectFailure("[{\"directory\":\"\",\"arguments\":[]}]", "Missing file");
+  expectFailure("[{\"arguments\":\"\",\"file\":\"\"}]", "Missing directory");
+  expectFailure("[{\"command\":\"\",\"arguments\":[],\"file\":\"\"}]", "Command and arguments");
+  expectFailure("[{\"directory\":\"\",\"arguments\":\"\",\"file\":\"\"}]", "Arguments not array");
+  expectFailure("[{\"directory\":\"\",\"command\":[],\"file\":\"\"}]", "Command not string");
 }
 
 static std::vector getAllFiles(StringRef JSONDatabase,
Index: ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -221,9 +221,8 @@
 SmallString<8> DirectoryStorage;
 SmallString<1024> CommandStorage;
 Commands.emplace_back(
-// FIXME: Escape correctly:
-CommandsRef[I].first->getValue(DirectoryStorage),
-unescapeCommandLine(CommandsRef[I].second->getValue(CommandStorage)));
+  CommandsRef[I].first->getValue(DirectoryStorage),
+  CommandsRef[I].second);
   }
 }
 
@@ -252,35 +251,63 @@
   return false;
 }
 llvm::yaml::ScalarNode *Directory = nullptr;
-llvm::yaml::ScalarNode *Command = nullptr;
+std::vector Args;
+bool CommandFound = false;
 llvm::yaml::ScalarNode *File = nullptr;
 for (llvm::yaml::MappingNode::iterator KVI = Object->begin(),
KVE = Object->end();
  KVI != KVE; ++KVI) {
+  llvm::yaml::ScalarNode *KeyString =
+  dyn_cast((*KVI).getKey());
+  if (!KeyString) {
+ErrorMessage = "Expected strings as key.";
+return false;
+  }
+  SmallString<8> KeyStorage;
+  StringRef KeyValue = KeyString->getValue(KeyStorage);
   llvm::yaml::Node *Value = (*KVI).getValue();
   if (!Value) {
 ErrorMessage = "Expected value.";
 return false;
   }
   llvm::yaml::ScalarNode *ValueString =
   dyn_cast(Value);
-  if (!ValueString) {
-ErrorMessage = "Expected string as value.";
+  llvm::yaml::SequenceNode *SequenceString =
+  dyn_cast(Value);
+  if (KeyValue == "arguments" && !SequenceString) {
+ErrorMessage = "Expected sequence as value.";
 return false;
-  }
-  llvm::yaml::ScalarNode *KeyString =
-  dyn_cast((*KVI).getKey());
-  if (!KeyString) {
-ErrorMessage = "Expected strings as key.";
+  } else if (KeyValue != "arguments" && !ValueString) {
+ErrorMessage = "Expected string as value.";
 return false;
   }
-  SmallString<8> KeyStorage;
-  if (KeyString->getValue(KeyStorage) == "directory") {
+  if (KeyValue == "directory") {
 Directory = ValueString;
-  } else if (KeyString->getValue(KeyStorage) == "command") {
-Command = ValueString;
-  } else if (KeyString->getValue(KeyStorage) == "file") {
+  } else if (KeyValue == "command") {
+if (CommandFound) {
+  ErrorMessage = "Multiple command and arguments found";
+  return false;
+}
+SmallString<1024> CommandStorage;
+// FIXME: Escape correctly:
+Args = unescapeCommandLine(ValueString->getValue(CommandStorage));
+CommandFound = true;
+  } else if (KeyValue == "file") {
 File = ValueString;
+  } else if (KeyValue == "arguments") {
+if (CommandFound) {
+  ErrorMessage = "Multiple command and arguments found";
+  return false;
+}
+for (llvm::yaml::SequenceNode::iterator CI = SequenceString->begin(),
+  CE = SequenceString->end();
+  CI != CE; ++CI) {
+  SmallString<128> CommandStorage;
+  auto ValueString = dyn_cast(&*CI);
+
+  Args.push_back(ValueString->getValue(CommandStorage));
+}
+CommandFound = true;
   } else {

Re: [PATCH] D10586: Add HSAIL support

2015-08-11 Thread Matt Arsenault via cfe-commits
arsenm updated this revision to Diff 31825.
arsenm added a comment.

Update for trunk


http://reviews.llvm.org/D10586

Files:
  include/clang/Basic/BuiltinsHSAIL.def
  include/clang/Basic/TargetBuiltins.h
  include/clang/module.modulemap
  lib/Basic/Targets.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/Tools.cpp
  test/CodeGen/target-data.c
  test/CodeGenOpenCL/builtins-hsail.cl
  test/Driver/hsail-mcpu.cl
  test/Preprocessor/init.c
  test/SemaOpenCL/builtins-hsail-restrictions.cl

Index: test/SemaOpenCL/builtins-hsail-restrictions.cl
===
--- /dev/null
+++ test/SemaOpenCL/builtins-hsail-restrictions.cl
@@ -0,0 +1,82 @@
+// REQUIRES: hsail-registered-target
+// RUN: %clang_cc1 -triple hsail-unknown-unknown -verify -pedantic -fsyntax-only %s
+
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
+typedef __attribute__((ext_vector_type(4))) long long4;
+
+
+void test_sqrt_builtin(volatile global double* out,
+   volatile float* outf,
+   double x,
+   float xf,
+   int var)
+{
+  *out = __builtin_hsail_fsqrt(var, 0, x); // expected-error {{argument to '__builtin_hsail_fsqrt' must be a constant integer}}
+  *outf = __builtin_hsail_fsqrtf(var, 0, xf); // expected-error {{argument to '__builtin_hsail_fsqrtf' must be a constant integer}}
+
+  *out = __builtin_hsail_fsqrt(0, var, x); // expected-error {{argument to '__builtin_hsail_fsqrt' must be a constant integer}}
+  *outf = __builtin_hsail_fsqrtf(0, var, xf); // expected-error {{argument to '__builtin_hsail_fsqrtf' must be a constant integer}}
+}
+
+void test_unpackcvt(volatile global float* out, int x, int y)
+{
+  *out = __builtin_hsail_unpackcvt(x, y); // expected-error {{argument to '__builtin_hsail_unpackcvt' must be a constant integer}}
+}
+
+void test_segmentp(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_segmentp(x, false, (__attribute__((address_space(4))) char*)0); // expected-error {{argument to '__builtin_hsail_segmentp' must be a constant integer}}
+  *out = __builtin_hsail_segmentp(0, x, (__attribute__((address_space(4))) char*)0); // expected-error {{argument to '__builtin_hsail_segmentp' must be a constant integer}}
+}
+
+void test_memfence(int x)
+{
+  __builtin_hsail_memfence(x, 0); // expected-error {{argument to '__builtin_hsail_memfence' must be a constant integer}}
+  __builtin_hsail_memfence(0, x); // expected-error {{argument to '__builtin_hsail_memfence' must be a constant integer}}
+}
+
+void test_barrier_builtin(int x)
+{
+  __builtin_hsail_barrier(x); // expected-error {{argument to '__builtin_hsail_barrier' must be a constant integer}}
+}
+
+void test_activelanecount(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_activelanecount(x, 0); // expected-error {{argument to '__builtin_hsail_activelanecount' must be a constant integer}}
+}
+
+void test_activelaneid(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_activelaneid(x); // expected-error {{argument to '__builtin_hsail_activelaneid' must be a constant integer}}
+}
+
+void test_activelanemask(volatile global long4* out, int x)
+{
+  *out = __builtin_hsail_activelanemask(x, false); // expected-error {{argument to '__builtin_hsail_activelanemask' must be a constant integer}}
+}
+
+void test_activelanepermute(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_activelanepermute(x, 0, 0, 0, 0); // expected-error {{argument to '__builtin_hsail_activelanepermute' must be a constant integer}}
+}
+
+void test_activelanepermutel(volatile global long* out, int x)
+{
+  *out = __builtin_hsail_activelanepermutel(x, 0, 0, 0, 0); // expected-error {{argument to '__builtin_hsail_activelanepermutel' must be a constant integer}}
+}
+
+void test_currentworkgroupsize(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_currentworkgroupsize(x); // expected-error {{argument to '__builtin_hsail_currentworkgroupsize' must be a constant integer}}
+}
+
+void test_workitemabsid(volatile global int* out, int x)
+{
+  *out = __builtin_hsail_workitemabsid(x); // expected-error {{argument to '__builtin_hsail_workitemabsid' must be a constant integer}}
+}
+
+void test_workitemabsidl(volatile global long* out, int x)
+{
+  *out = __builtin_hsail_workitemabsidl(x); // expected-error {{argument to '__builtin_hsail_workitemabsidl' must be a constant integer}}
+}
Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -6615,6 +6615,10 @@
 // AMDGPU:#define cl_khr_local_int32_base_atomics 1
 // AMDGPU:#define cl_khr_local_int32_extended_atomics 1
 
+// RUN: %clang_cc1 -x cl -E -dM -ffreestanding -triple=hsail < /dev/null | FileCheck -check-prefix=HSAIL %s
+// RUN: %clang_cc1 -x cl -E -dM -ffreestanding -triple=hsail64 < /dev/null | FileCheck -check-prefix=HSAIL %s
+// HSAIL:#define cl

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-11 Thread pierre gousseau via cfe-commits
pgousseau added inline comments.


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1098
@@ +1097,3 @@
+  if (!NumElements)
+return;
+  QualType ElementTy = AT->getElementType();

zaks.anna wrote:
> What happens on early returns? Here and the one below. Are there tests for 
> these cases?
Oops yes returning here is wrong, if we return here we skip the code conjuring 
the default value at line 1122.
I will change it to a goto conjure_default for !NumElements == true.
Also a value of 0 for ElemSize is ok so no need to return actually.
I will add a test for 0 sized elements' array and 0 elements array and update 
the patch.
What do you think ?
Thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2359
@@ -2314,1 +2358,3 @@
+RegionAndSymbolInvalidationTraits ITraits;
+W.AddToWorkList(*I, ITraits);
   }

ayartsev wrote:
> Too much unnecessary passing around of RegionAndSymbolInvalidationTraits 
> parameter. What about moving "RegionAndSymbolInvalidationTraits" member from 
> "invalidateRegionsWorker" class to the base class "ClusterAnalysis"?
Makes sense, I will update the patch.
Thanks !


http://reviews.llvm.org/D11832



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


Re: [PATCH] [Rewriter] Fix RemoveText when IncludeInsertsAtBeginOfRange=true

2015-08-11 Thread Justin Bogner via cfe-commits
Alex Corrado  writes:
> Hello,
>
> I’m new to Clang, and I’d like to start off by saying that it's been a
> real treat to work with so far.
>
> I’ve been putting the Rewriter through its paces and found a bug:
> RemoveText does not use the correct starting offset when
> IncludeInsertsAtBeginOfRange=true in the RewriteOptions and some text
> has been inserted at the beginning of the range.
>
> I’ve attached a patch. Apologies if it isn’t in the correct format, as
> this is my first one.

This looks pretty reasonable, thanks for working on it! We like to
accompany changes with tests, so could you please write a test for this?

I'm guessing the test belongs in test/Rewriter. There are examples of
how our tests work there that should get you on the right track, but
feel free to ask questions if you have any trouble.

> Thanks again for all your great work!
>
> -Alex
>
> From 1006ff28881f3df293e1f1b587fda686b624910a Mon Sep 17 00:00:00 2001
> From: Alex Corrado 
> Date: Sun, 9 Aug 2015 15:09:15 -0400
> Subject: [PATCH] [Rewriter] Fix RemoveText when
>  IncludeInsertsAtBeginOfRange=true
>
> ---
>  include/clang/Rewrite/Core/RewriteBuffer.h | 3 ++-
>  lib/Rewrite/Rewriter.cpp   | 7 ---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/clang/Rewrite/Core/RewriteBuffer.h 
> b/include/clang/Rewrite/Core/RewriteBuffer.h
> index d69c69b..6259918 100644
> --- a/include/clang/Rewrite/Core/RewriteBuffer.h
> +++ b/include/clang/Rewrite/Core/RewriteBuffer.h
> @@ -56,7 +56,8 @@ public:
>  
>/// RemoveText - Remove the specified text.
>void RemoveText(unsigned OrigOffset, unsigned Size,
> -  bool removeLineIfEmpty = false);
> +  bool removeLineIfEmpty = false,
> +  bool afterInserts = true);
>  
>/// InsertText - Insert some text at the specified point, where the offset 
> in
>/// the buffer is specified relative to the original SourceBuffer.  The
> diff --git a/lib/Rewrite/Rewriter.cpp b/lib/Rewrite/Rewriter.cpp
> index be09a36..cc7b24c 100644
> --- a/lib/Rewrite/Rewriter.cpp
> +++ b/lib/Rewrite/Rewriter.cpp
> @@ -49,11 +49,11 @@ static inline bool isWhitespace(unsigned char c) {
>  }
>  
>  void RewriteBuffer::RemoveText(unsigned OrigOffset, unsigned Size,
> -   bool removeLineIfEmpty) {
> +   bool removeLineIfEmpty, bool afterInserts) {
>// Nothing to remove, exit early.
>if (Size == 0) return;
>  
> -  unsigned RealOffset = getMappedOffset(OrigOffset, true);
> +  unsigned RealOffset = getMappedOffset(OrigOffset, afterInserts);
>assert(RealOffset+Size <= Buffer.size() && "Invalid location");
>  
>// Remove the dead characters.
> @@ -295,7 +295,8 @@ bool Rewriter::RemoveText(SourceLocation Start, unsigned 
> Length,
>if (!isRewritable(Start)) return true;
>FileID FID;
>unsigned StartOffs = getLocationOffsetAndFileID(Start, FID);
> -  getEditBuffer(FID).RemoveText(StartOffs, Length, opts.RemoveLineIfEmpty);
> +  getEditBuffer(FID).RemoveText(StartOffs, Length, opts.RemoveLineIfEmpty,
> +!opts.IncludeInsertsAtBeginOfRange);
>return false;
>  }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r244488 - [dllimport] A non-imported class with an imported key can't have a key

2015-08-11 Thread Richard Smith via cfe-commits
On Aug 11, 2015 9:45 AM, "Reid Kleckner"  wrote:
>
> On Mon, Aug 10, 2015 at 6:40 PM, Richard Smith 
wrote:
>>
>> On Mon, Aug 10, 2015 at 5:43 PM, Reid Kleckner  wrote:
>>>
>>> On Mon, Aug 10, 2015 at 5:00 PM, Richard Smith 
wrote:

 On Mon, Aug 10, 2015 at 12:39 PM, Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
>
> +// If the key function is dllimport but the class isn't, then
the class has
> +// no key function. The DLL that exports the key function won't
export the
> +// vtable in this case.
> +if (MD->hasAttr() &&
!RD->hasAttr())
> +  return nullptr;


 Does the same apply if the key function is DLLExport and the class is
not? (Presumably this would just lead us to export a vtable that we don't
need to, which is presumably harmless?)
>>>
>>>
>>> No, there's no issue with dllexport. If the key function is dllexport,
then it must be part of the same DLL as the current TU, and we can rely on
it to provide (potentially non-exported) vtable and RTTI symbols.
>>
>>
>> Even if a different compiler is used to compile the other TUs in this
DLL? It seems to me that if the rule really is "the vtable takes its DLL
storage class from the class, not the key function", then the TU with the
key function need not actually provide a vtable symbol if the class is not
dllexport but the key function is.
>
>
> GCC (and presumably other Itanium compilers that support dllexport) still
emit the vtable with the key function when the key is exported. It's just
not necessarily exported. All the TUs that are statically linked into the
DLL containing an exported key function can rely on the vtable to be there,
so I don't see any reason to add logic around dllexport here. Make sense?

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


Re: [clang-tools-extra] r244596 - Default initialize from explicitly constructed object.

2015-08-11 Thread David Blaikie via cfe-commits
Rightio - good to know. Thanks!

On Tue, Aug 11, 2015 at 8:08 AM, Manuel Klimek  wrote:

> Don't remember which bot it was; the message said something about an
> explicit constructor being called for std::map
>
> On Tue, Aug 11, 2015 at 5:00 PM David Blaikie  wrote:
>
>> Which compiler do we support that didn't accept this? I thought we'd
>> already grown a few uses of initializer lists like this...
>> On Aug 11, 2015 5:13 AM, "Manuel Klimek via cfe-commits" <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: klimek
>>> Date: Tue Aug 11 07:13:15 2015
>>> New Revision: 244596
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=244596&view=rev
>>> Log:
>>> Default initialize from explicitly constructed object.
>>>
>>> Modified:
>>> clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>>>
>>> Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244596&r1=244595&r2=244596&view=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
>>> (original)
>>> +++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug
>>> 11 07:13:15 2015
>>> @@ -48,7 +48,8 @@ runCheckOnCode(StringRef Code, std::vect
>>> const Twine &Filename = "input.cc",
>>> ArrayRef ExtraArgs = None,
>>> const ClangTidyOptions &ExtraOptions =
>>> ClangTidyOptions(),
>>> -   std::map PathsToContent = {}) {
>>> +   std::map PathsToContent =
>>> +   std::map()) {
>>>ClangTidyOptions Options = ExtraOptions;
>>>Options.Checks = "*";
>>>ClangTidyContext Context(llvm::make_unique(
>>> @@ -70,7 +71,7 @@ runCheckOnCode(StringRef Code, std::vect
>>>tooling::ToolInvocation Invocation(
>>>ArgCXX11, new TestClangTidyAction(Check, Finder, Context),
>>> Files.get());
>>>Invocation.mapVirtualFile(Filename.str(), Code);
>>> -  for (const auto & FileContent : PathsToContent) {
>>> +  for (const auto &FileContent : PathsToContent) {
>>>  Invocation.mapVirtualFile(Twine("include/" +
>>> FileContent.first).str(),
>>>FileContent.second);
>>>}
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Marshall Clow via cfe-commits
mclow.lists created this revision.
mclow.lists added reviewers: chandlerc, rsmith, EricWF.
mclow.lists added a subscriber: cfe-commits.
Herald added subscribers: danalbert, tberghammer.

The C standard says that calling `memcpy`, etc with null parameters is 
undefined behavior.
GCC (and clang) have attributes that allow us to mark the parameters of 
functions as "must not be null".

Define a mechanism to do this, and use it (for the first time) to mark the 
parameters of `memcpy`, `memmove`, `memcmp` and `strncmp` as "must not be null".

This gives us compile time checking for constant pointers, and hints to the 
code generator.

Note: This will not be a big win on systems that use glibc, because it marks 
the global functions `::memcpy`, etc the same way. On Mac OS X, iOS, Android, 
FreeBSD, etc, this will make a bigger difference.

I will be adding tests as well; this post is to gather consensus that this is 
the right way to go.


http://reviews.llvm.org/D11948

Files:
  include/__config
  include/cstring

Index: include/cstring
===
--- include/cstring
+++ include/cstring
@@ -67,15 +67,34 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::size_t;
-using ::memcpy;
-using ::memmove;
+
+// using ::memcpy;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+void* memcpy(void* __s1, const void* __s2, size_t __n) 
+{ return ::memcpy(__s1, __s2, __n); }
+
+// using ::memmove;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+void* memmove(void* __s1, const void* __s2, size_t __n)
+{ return ::memmove(__s1, __s2, __n); }
+
 using ::strcpy;
 using ::strncpy;
 using ::strcat;
 using ::strncat;
-using ::memcmp;
+
+// using ::memcmp;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+int memcmp(const void* __s1, const void* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+
 using ::strcmp;
-using ::strncmp;
+
+// using ::strncmp;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+int strncmp(const char* __s1, const char* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+
 using ::strcoll;
 using ::strxfrm;
 
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -273,6 +273,11 @@
 
 #define _LIBCPP_UNUSED __attribute__((__unused__))
 
+#define_LIBCPP_NON_NULL
__attribute__((__nonnull__))
+#define_LIBCPP_NON_NULL1(x)__attribute__((__nonnull__(x)))
+#define_LIBCPP_NON_NULL2(x,y)  
__attribute__((__nonnull__(x,y)))
+#define_LIBCPP_NON_NULL3(x,y,z)
__attribute__((__nonnull__(x,y,z)))
+
 #if !(__has_feature(cxx_defaulted_functions))
 #define _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
 #endif  // !(__has_feature(cxx_defaulted_functions))
@@ -405,6 +410,11 @@
 
 #define _LIBCPP_UNUSED __attribute__((__unused__))
 
+#define_LIBCPP_NON_NULL
__attribute__((__nonnull__))
+#define_LIBCPP_NON_NULL1(x)__attribute__((__nonnull__(x)))
+#define_LIBCPP_NON_NULL2(x,y)  
__attribute__((__nonnull__(x,y)))
+#define_LIBCPP_NON_NULL3(x,y,z)
__attribute__((__nonnull__(x,y,z)))
+
 #if _GNUC_VER >= 407
 #define _LIBCPP_UNDERLYING_TYPE(T) __underlying_type(T)
 #define _LIBCPP_IS_LITERAL(T) __is_literal_type(T)


Index: include/cstring
===
--- include/cstring
+++ include/cstring
@@ -67,15 +67,34 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::size_t;
-using ::memcpy;
-using ::memmove;
+
+// using ::memcpy;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+void* memcpy(void* __s1, const void* __s2, size_t __n) 
+{ return ::memcpy(__s1, __s2, __n); }
+
+// using ::memmove;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+void* memmove(void* __s1, const void* __s2, size_t __n)
+{ return ::memmove(__s1, __s2, __n); }
+
 using ::strcpy;
 using ::strncpy;
 using ::strcat;
 using ::strncat;
-using ::memcmp;
+
+// using ::memcmp;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+int memcmp(const void* __s1, const void* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+
 using ::strcmp;
-using ::strncmp;
+
+// using ::strncmp;
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_NON_NULL2(1, 2)
+int strncmp(const char* __s1, const char* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+
 using ::strcoll;
 using ::strxfrm;
 
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -273,6 +273,11 @@
 
 #define _LIBCPP_UNUSED __attribute__((__unused__))
 
+#define	_LIBCPP_NON_NULL			__attribute__((__nonnull__))
+#define	_LIBCPP_NON_NULL1(x)		__attribute__((__nonnull__(x)))
+#define	_LIBCPP_NON_NULL2(x,y)		__attribute__((__nonnull__(x,y)))
+#define	_LIBCPP_NON_NULL3(x,y,z)	__attribute__((__nonnull__(x,y,z)))
+
 #if !(__has_feature(cxx_defaulted_fun

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

2015-08-11 Thread Serge Pavlov via cfe-commits
sepavloff updated this revision to Diff 31831.
sepavloff added a comment.

Fixed module boundary treatment.

This version must fix problems in the treatment of  annot_modulbegin and
annot_module_end. Error recover for the token annot_module_include found
in wrong context can be made as previously, just by ignoring it. To handle
token 'annot_module_end' parser bails out to upper level, where the module
end can be processed, it will complain on missing '}' etc. As for
'annot_module_start', now parser tries to recover by skipping all content
till matching 'annot_module_end'.

Also made changes according to reviewer's notes.


http://reviews.llvm.org/D11844

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/Modules/auto-module-import.m
  test/Modules/malformed.cpp
  test/Modules/misplaced.cpp

Index: test/Modules/misplaced.cpp
===
--- /dev/null
+++ test/Modules/misplaced.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs %s -verify
+
+namespace N1 {  // expected-note{{namespace 'N1' begins here}}
+#include "dummy.h"  // expected-error{{import of module 'dummy' appears within namespace 'N1'}}
+}
+
+void func1() {  // expected-note{{function 'func1' begins here}}
+#include "dummy.h"  // expected-error{{import of module 'dummy' appears within function 'func1'}}
+}
+
+struct S1 {  // expected-note{{'S1' begins here}}
+#include "dummy.h"  // expected-error{{import of module 'dummy' appears within 'S1'}}
+};
Index: test/Modules/malformed.cpp
===
--- test/Modules/malformed.cpp
+++ test/Modules/malformed.cpp
@@ -12,20 +12,14 @@
 #include STR(HEADER)
 
 // CHECK-A: While building module 'malformed_a'
+// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: unexpected module end
 // CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: expected '}'
 // CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} note: to match this '{'
 //
 // CHECK-A: While building module 'malformed_a'
-// CHECK-A: {{^}}Inputs/malformed/a2.h:1:{{.*}} error: extraneous closing brace
 
 // CHECK-B: While building module 'malformed_b'
-// CHECK-B: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: expected '}'
-// CHECK-B: {{^}}Inputs/malformed/b1.h:1:{{.*}} note: to match this '{'
-// CHECK-B: {{^}}Inputs/malformed/b1.h:3:{{.*}} error: extraneous closing brace ('}')
-//
-// CHECK-B: While building module 'malformed_b'
-// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} error: redefinition of 'g'
-// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} note: previous definition is here
+// CHECK-B: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: submodule cannot be started here
 
 void test() { f(); }
 // Test that we use relative paths to name files within an imported module.
Index: test/Modules/auto-module-import.m
===
--- test/Modules/auto-module-import.m
+++ test/Modules/auto-module-import.m
@@ -83,6 +83,6 @@
   return not_in_module;
 }
 
-void includeNotAtTopLevel() { // expected-note {{to match this '{'}}
-  #include  // expected-warning {{treating #include as an import}} expected-error {{expected '}'}}
-} // expected-error {{extraneous closing brace}}
+void includeNotAtTopLevel() { // expected-note {{function 'includeNotAtTopLevel' begins here}}
+  #include  // expected-warning {{treating #include as an import}} expected-error {{import of module 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}}
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14274,6 +14274,10 @@
   }
 }
 
+void Sema::diagnoseMisplacedModuleImport(Module *M, SourceLocation ImportLoc) {
+  return checkModuleImportContext(*this, M, ImportLoc, CurContext);
+}
+
 DeclResult Sema::ActOnModuleImport(SourceLocation AtLoc, 
SourceLocation ImportLoc, 
ModuleIdPath Path) {
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -1989,6 +1989,43 @@
   return Actions.ConvertDeclToDeclGroup(Import.get());
 }
 
+/// \brief Try recover parser when module annotation appears where it must not
+/// be found.
+/// \returns true if the recover was successful and parsing may be continued, or
+/// false if parser must bail out to top level and handle the token there.
+bool Parser::tryParseMisplacedModuleImport() {
+  switch (Tok.getKind()) {
+  case tok::annot_module_end:
+Diag(Tok, diag::err_unexpected_module_end);
+break;
+  case tok::annot_module_begin: {
+Diag(Tok, diag::err_unexpected_module_start

[PATCH] D11949: [MIPS] Use arch values for lock-free atomic operations

2015-08-11 Thread Petar Jovanovic via cfe-commits
petarj created this revision.
petarj added a reviewer: dschuff.
petarj added a subscriber: cfe-commits.
Herald added subscribers: dschuff, jfb.

Let NaClMips32ELTargetInfo inherit arch values for maximum width lock-free
atomic operations.


http://reviews.llvm.org/D11949

Files:
  lib/Basic/Targets.cpp

Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -6842,8 +6842,7 @@
 class NaClMips32ELTargetInfo : public Mips32ELTargetInfo {
 public:
   NaClMips32ELTargetInfo(const llvm::Triple &Triple) :
-Mips32ELTargetInfo(Triple)  {
-  MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
+Mips32ELTargetInfo(Triple) {
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {


Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -6842,8 +6842,7 @@
 class NaClMips32ELTargetInfo : public Mips32ELTargetInfo {
 public:
   NaClMips32ELTargetInfo(const llvm::Triple &Triple) :
-Mips32ELTargetInfo(Triple)  {
-  MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
+Mips32ELTargetInfo(Triple) {
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11781: Refactored pthread usage in libcxx

2015-08-11 Thread Marshall Clow via cfe-commits
mclow.lists added a subscriber: mclow.lists.
mclow.lists added a reviewer: mclow.lists.
mclow.lists added a comment.

I agree with @jroelofs that we don't want to `#include ` in 
`<__config>`


Repository:
  rL LLVM

http://reviews.llvm.org/D11781



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


[PATCH] D11950: [CUDA] Check register names on appropriate side of cuda compilation only.

2015-08-11 Thread Artem Belevich via cfe-commits
tra created this revision.
tra added reviewers: eliben, echristo.
tra added a subscriber: cfe-commits.

The patch makes sure that register names in named register variables and inline 
assembly are checked only on appropriate side of CUDA compilation.

http://reviews.llvm.org/D11950

Files:
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/SemaCUDA/asm-constraints-mixed.cu

Index: test/SemaCUDA/asm-constraints-mixed.cu
===
--- test/SemaCUDA/asm-constraints-mixed.cu
+++ test/SemaCUDA/asm-constraints-mixed.cu
@@ -1,15 +1,39 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fsyntax-only -fcuda-is-device -verify %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
-// expected-no-diagnostics
+
+__attribute__((device)) register long global_dev_reg asm("r0");
+__attribute__((device)) register long
+global_dev_hreg asm("rax"); // device-side error
+
+register long global_host_reg asm("rax");
+register long global_host_dreg asm("r0"); // host-side error
 
 __attribute__((device)) void df() {
+  register long local_dev_reg asm("r0");
+  register long local_host_reg asm("rax"); // device-side error
   short h;
   // asm with PTX constraints. Some of them are PTX-specific.
-  __asm__("dont care" : "=h"(h): "f"(0.0), "d"(0.0), "h"(0), "r"(0), "l"(0));
+  __asm__("dont care" : "=h"(h) : "f"(0.0), "d"(0.0), "h"(0), "r"(0), "l"(0));
 }
 
 void hf() {
+  register long local_dev_reg asm("r0"); // host-side error
+  register long local_host_reg asm("rax");
   int a;
-  // Asm with x86 constraints that are not supported by PTX.
-  __asm__("dont care" : "=a"(a): "a"(0), "b"(0), "c"(0));
+  // Asm with x86 constraints and registers that are not supported by PTX.
+  __asm__("dont care" : "=a"(a) : "a"(0), "b"(0), "c"(0) : "flags");
 }
+
+// Check errors in named register variables.
+// We should only see errors relevant to current compilation mode.
+#if defined(__CUDA_ARCH__)
+// Device-side compilation:
+// expected-error@8 {{unknown register name 'rax' in asm}}
+// expected-error@15 {{unknown register name 'rax' in asm}}
+#else
+// Host-side compilation:
+// expected-error@11 {{unknown register name 'r0' in asm}}
+// expected-error@22 {{unknown register name 'r0' in asm}}
+#endif
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -155,8 +155,14 @@
   // The parser verifies that there is a string literal here.
   assert(AsmString->isAscii());
 
-  bool ValidateConstraints =
-  DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl());
+  // If we're compiling CUDA file and function attributes indicate that it's not
+  // for this compilation side, skip all the checks.
+  if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
+GCCAsmStmt *NS = new (Context) GCCAsmStmt(
+Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
+Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, RParenLoc);
+return NS;
+  }
 
   for (unsigned i = 0; i != NumOutputs; i++) {
 StringLiteral *Literal = Constraints[i];
@@ -167,8 +173,7 @@
   OutputName = Names[i]->getName();
 
 TargetInfo::ConstraintInfo Info(Literal->getString(), OutputName);
-if (ValidateConstraints &&
-!Context.getTargetInfo().validateOutputConstraint(Info))
+if (!Context.getTargetInfo().validateOutputConstraint(Info))
   return StmtError(Diag(Literal->getLocStart(),
 diag::err_asm_invalid_output_constraint)
<< Info.getConstraintStr());
@@ -247,8 +252,7 @@
   InputName = Names[i]->getName();
 
 TargetInfo::ConstraintInfo Info(Literal->getString(), InputName);
-if (ValidateConstraints &&
-!Context.getTargetInfo().validateInputConstraint(
+if (!Context.getTargetInfo().validateInputConstraint(
 OutputConstraintInfos.data(), NumOutputs, Info)) {
   return StmtError(Diag(Literal->getLocStart(),
 diag::err_asm_invalid_input_constraint)
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5941,9 +5941,9 @@
 
   // Handle attributes prior to checking for duplicates in MergeVarDecl
   ProcessDeclAttributes(S, NewVD, D);
-
+  bool ShouldHandleTargetErrors = DeclAttrsMatchCUDAMode(getLangOpts(), NewVD);
   if (getLangOpts().CUDA) {
-if (EmitTLSUnsupportedError && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD))
+if (EmitTLSUnsupportedError && ShouldHandleTargetErrors)
   Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
diag::err_thread_unsupported);
 // CUDA B.2.5: "__shared__ and __constant__ variables have implied static
@@ -5980,7 +5980,8 @@
 break;
   ca

Re: r244413 - [modules] When building a dependency file, include module maps parsed in the

2015-08-11 Thread Richard Smith via cfe-commits
On Tue, Aug 11, 2015 at 4:57 AM, Manuel Klimek  wrote:

> I believe this breaks -fno-module-file-deps.
>

I don't think so; this affects which module map files end up in the .d
file, not which .pcm files do.


> On Sun, Aug 9, 2015 at 6:47 AM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Sat Aug  8 23:46:57 2015
>> New Revision: 244413
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=244413&view=rev
>> Log:
>> [modules] When building a dependency file, include module maps parsed in
>> the
>> current compilation, not just those from imported modules.
>>
>> Modified:
>> cfe/trunk/include/clang/Lex/ModuleMap.h
>> cfe/trunk/lib/Frontend/DependencyFile.cpp
>> cfe/trunk/lib/Lex/ModuleMap.cpp
>> cfe/trunk/test/Modules/dependency-gen-pch.m
>> cfe/trunk/test/Modules/dependency-gen.m
>> cfe/trunk/test/Modules/dependency-gen.modulemap
>> cfe/trunk/test/Modules/relative-dep-gen.cpp
>>
>> Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=244413&r1=244412&r2=244413&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
>> +++ cfe/trunk/include/clang/Lex/ModuleMap.h Sat Aug  8 23:46:57 2015
>> @@ -35,6 +35,22 @@ class DiagnosticConsumer;
>>  class DiagnosticsEngine;
>>  class HeaderSearch;
>>  class ModuleMapParser;
>> +
>> +/// \brief A mechanism to observe the actions of the module map parser
>> as it
>> +/// reads module map files.
>> +class ModuleMapCallbacks {
>> +public:
>> +  virtual ~ModuleMapCallbacks() {}
>> +
>> +  /// \brief Called when a module map file has been read.
>> +  ///
>> +  /// \param FileStart A SourceLocation referring to the start of the
>> file's
>> +  /// contents.
>> +  /// \param File The file itself.
>> +  /// \param IsSystem Whether this is a module map from a system include
>> path.
>> +  virtual void moduleMapFileRead(SourceLocation FileStart,
>> + const FileEntry &File, bool IsSystem) {}
>> +};
>>
>>  class ModuleMap {
>>SourceManager &SourceMgr;
>> @@ -42,6 +58,8 @@ class ModuleMap {
>>const LangOptions &LangOpts;
>>const TargetInfo *Target;
>>HeaderSearch &HeaderInfo;
>> +
>> +  llvm::SmallVector, 1> Callbacks;
>>
>>/// \brief The directory used for Clang-supplied, builtin include
>> headers,
>>/// such as "stdint.h".
>> @@ -263,6 +281,11 @@ public:
>>  BuiltinIncludeDir = Dir;
>>}
>>
>> +  /// \brief Add a module map callback.
>> +  void addModuleMapCallbacks(std::unique_ptr
>> Callback) {
>> +Callbacks.push_back(std::move(Callback));
>> +  }
>> +
>>/// \brief Retrieve the module that owns the given header file, if any.
>>///
>>/// \param File The header file that is likely to be included.
>>
>> Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=244413&r1=244412&r2=244413&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
>> +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Sat Aug  8 23:46:57 2015
>> @@ -18,6 +18,7 @@
>>  #include "clang/Frontend/FrontendDiagnostic.h"
>>  #include "clang/Lex/DirectoryLookup.h"
>>  #include "clang/Lex/LexDiagnostic.h"
>> +#include "clang/Lex/ModuleMap.h"
>>  #include "clang/Lex/PPCallbacks.h"
>>  #include "clang/Lex/Preprocessor.h"
>>  #include "clang/Serialization/ASTReader.h"
>> @@ -82,6 +83,20 @@ struct DepCollectorPPCallbacks : public
>>}
>>  };
>>
>> +struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
>> +  DependencyCollector &DepCollector;
>> +  DepCollectorMMCallbacks(DependencyCollector &DC) : DepCollector(DC) {}
>> +
>> +  void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
>> + bool IsSystem) override {
>> +StringRef Filename = Entry.getName();
>> +DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
>> +/*IsSystem*/IsSystem,
>> +/*IsModuleFile*/false,
>> +/*IsMissing*/false);
>> +  }
>> +};
>> +
>>  struct DepCollectorASTListener : public ASTReaderListener {
>>DependencyCollector &DepCollector;
>>DepCollectorASTListener(DependencyCollector &L) : DepCollector(L) { }
>> @@ -132,6 +147,8 @@ DependencyCollector::~DependencyCollecto
>>  void DependencyCollector::attachToPreprocessor(Preprocessor &PP) {
>>PP.addPPCallbacks(
>>llvm::make_unique(*this,
>> PP.getSourceManager()));
>> +  PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks(
>> +  llvm::make_unique(*this));
>>  }
>>  void DependencyCollector::attachToASTReader(ASTReader &R) {
>>R.addListener(llvm::make_unique(*this));
>> @@ -185,6 +202,

Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki abandoned this revision.
danielmarjamaki added a comment.

In http://reviews.llvm.org/D11940#221718, @thakis wrote:

> Can't you just change your signature to
>
>   virtual void a(int /* x */) {}
>   
>
> in these cases?


hmm.. yes I agree.

You are right, so I change my mind about this patch.


http://reviews.llvm.org/D11940



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Nuno Lopes via cfe-commits
nlopes added a subscriber: nlopes.


Comment at: include/cstring:96
@@ +95,3 @@
+int strncmp(const char* __s1, const char* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+

typo here.


http://reviews.llvm.org/D11948



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


Re: r244413 - [modules] When building a dependency file, include module maps parsed in the

2015-08-11 Thread Manuel Klimek via cfe-commits
Ah, so we need  a flag -fno-module-map-file-deps, I assume...

On Tue, Aug 11, 2015 at 8:12 PM Richard Smith  wrote:

> On Tue, Aug 11, 2015 at 4:57 AM, Manuel Klimek  wrote:
>
>> I believe this breaks -fno-module-file-deps.
>>
>
> I don't think so; this affects which module map files end up in the .d
> file, not which .pcm files do.
>
>
>> On Sun, Aug 9, 2015 at 6:47 AM Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: rsmith
>>> Date: Sat Aug  8 23:46:57 2015
>>> New Revision: 244413
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=244413&view=rev
>>> Log:
>>> [modules] When building a dependency file, include module maps parsed in
>>> the
>>> current compilation, not just those from imported modules.
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Lex/ModuleMap.h
>>> cfe/trunk/lib/Frontend/DependencyFile.cpp
>>> cfe/trunk/lib/Lex/ModuleMap.cpp
>>> cfe/trunk/test/Modules/dependency-gen-pch.m
>>> cfe/trunk/test/Modules/dependency-gen.m
>>> cfe/trunk/test/Modules/dependency-gen.modulemap
>>> cfe/trunk/test/Modules/relative-dep-gen.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=244413&r1=244412&r2=244413&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
>>> +++ cfe/trunk/include/clang/Lex/ModuleMap.h Sat Aug  8 23:46:57 2015
>>> @@ -35,6 +35,22 @@ class DiagnosticConsumer;
>>>  class DiagnosticsEngine;
>>>  class HeaderSearch;
>>>  class ModuleMapParser;
>>> +
>>> +/// \brief A mechanism to observe the actions of the module map parser
>>> as it
>>> +/// reads module map files.
>>> +class ModuleMapCallbacks {
>>> +public:
>>> +  virtual ~ModuleMapCallbacks() {}
>>> +
>>> +  /// \brief Called when a module map file has been read.
>>> +  ///
>>> +  /// \param FileStart A SourceLocation referring to the start of the
>>> file's
>>> +  /// contents.
>>> +  /// \param File The file itself.
>>> +  /// \param IsSystem Whether this is a module map from a system
>>> include path.
>>> +  virtual void moduleMapFileRead(SourceLocation FileStart,
>>> + const FileEntry &File, bool IsSystem)
>>> {}
>>> +};
>>>
>>>  class ModuleMap {
>>>SourceManager &SourceMgr;
>>> @@ -42,6 +58,8 @@ class ModuleMap {
>>>const LangOptions &LangOpts;
>>>const TargetInfo *Target;
>>>HeaderSearch &HeaderInfo;
>>> +
>>> +  llvm::SmallVector, 1> Callbacks;
>>>
>>>/// \brief The directory used for Clang-supplied, builtin include
>>> headers,
>>>/// such as "stdint.h".
>>> @@ -263,6 +281,11 @@ public:
>>>  BuiltinIncludeDir = Dir;
>>>}
>>>
>>> +  /// \brief Add a module map callback.
>>> +  void addModuleMapCallbacks(std::unique_ptr
>>> Callback) {
>>> +Callbacks.push_back(std::move(Callback));
>>> +  }
>>> +
>>>/// \brief Retrieve the module that owns the given header file, if
>>> any.
>>>///
>>>/// \param File The header file that is likely to be included.
>>>
>>> Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=244413&r1=244412&r2=244413&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
>>> +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Sat Aug  8 23:46:57 2015
>>> @@ -18,6 +18,7 @@
>>>  #include "clang/Frontend/FrontendDiagnostic.h"
>>>  #include "clang/Lex/DirectoryLookup.h"
>>>  #include "clang/Lex/LexDiagnostic.h"
>>> +#include "clang/Lex/ModuleMap.h"
>>>  #include "clang/Lex/PPCallbacks.h"
>>>  #include "clang/Lex/Preprocessor.h"
>>>  #include "clang/Serialization/ASTReader.h"
>>> @@ -82,6 +83,20 @@ struct DepCollectorPPCallbacks : public
>>>}
>>>  };
>>>
>>> +struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
>>> +  DependencyCollector &DepCollector;
>>> +  DepCollectorMMCallbacks(DependencyCollector &DC) : DepCollector(DC) {}
>>> +
>>> +  void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
>>> + bool IsSystem) override {
>>> +StringRef Filename = Entry.getName();
>>> +DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
>>> +/*IsSystem*/IsSystem,
>>> +/*IsModuleFile*/false,
>>> +/*IsMissing*/false);
>>> +  }
>>> +};
>>> +
>>>  struct DepCollectorASTListener : public ASTReaderListener {
>>>DependencyCollector &DepCollector;
>>>DepCollectorASTListener(DependencyCollector &L) : DepCollector(L) { }
>>> @@ -132,6 +147,8 @@ DependencyCollector::~DependencyCollecto
>>>  void DependencyCollector::attachToPreprocessor(Preprocessor &PP) {
>>>PP.addPPCallbacks(
>>>llvm::make_unique(*this,
>>> PP.getSo

Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread David Blaikie via cfe-commits
On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Can't you just change your signature to
>
>   virtual void a(int /* x */) {}
>
> in these cases?
>

You could - does it add much value to do that, though? (perhaps it does -
it means you express the intent that the parameter is not used and the
compiler helps you check that (so that for the parameters you think
/should/ be used (you haven't commented out their name but accidentally
shadow or otherwise fail to reference, you still get a warning))

- David


>
> On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki <
> cfe-commits@lists.llvm.org> wrote:
>
>> danielmarjamaki created this revision.
>> danielmarjamaki added a reviewer: krememek.
>> danielmarjamaki added a subscriber: cfe-commits.
>>
>> Don't diagnose -Wunused-parameter in methods that override other methods
>> because the overridden methods might use the parameter
>>
>> Don't diagnose -Wunused-parameter in virtual methods because these might
>> be overriden by other methods that use the parameter.
>>
>> Such diagnostics could be more accurately written if they are based on
>> whole-program-analysis that establish if such parameter is unused in all
>> methods.
>>
>>
>>
>> http://reviews.llvm.org/D11940
>>
>> Files:
>>   lib/Sema/SemaDecl.cpp
>>   test/SemaCXX/warn-unused-parameters.cpp
>>
>> Index: test/SemaCXX/warn-unused-parameters.cpp
>> ===
>> --- test/SemaCXX/warn-unused-parameters.cpp
>> +++ test/SemaCXX/warn-unused-parameters.cpp
>> @@ -32,3 +32,20 @@
>>auto l = [&t...]() { return sizeof...(s); };
>>return l();
>>  }
>> +
>> +// Don't diagnose virtual methods or methods that override base class
>> +// methods.
>> +class Base {
>> +public:
>> +  virtual void f(int x);
>> +};
>> +
>> +class Derived : public Base {
>> +public:
>> +  // Don't warn in overridden methods.
>> +  virtual void f(int x) {}
>> +
>> +  // Don't warn in virtual methods.
>> +  virtual void a(int x) {}
>> +};
>> +
>> Index: lib/Sema/SemaDecl.cpp
>> ===
>> --- lib/Sema/SemaDecl.cpp
>> +++ lib/Sema/SemaDecl.cpp
>> @@ -10797,8 +10797,13 @@
>>
>>  if (!FD->isInvalidDecl()) {
>>// Don't diagnose unused parameters of defaulted or deleted
>> functions.
>> -  if (!FD->isDeleted() && !FD->isDefaulted())
>> -DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>> +  if (!FD->isDeleted() && !FD->isDefaulted()) {
>> +// Don't diagnose unused parameters in virtual methods or
>> +// in methods that override base class methods.
>> +const auto MD = dyn_cast(FD);
>> +if (!MD || (MD->size_overridden_methods() == 0U &&
>> !MD->isVirtual()))
>> +  DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>> +  }
>>DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(),
>> FD->param_end(),
>>   FD->getReturnType(), FD);
>>
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D9741: Reject multiplication by zero cases in MallocOverflowSecurityChecker

2015-08-11 Thread Aditya Kumar via cfe-commits
hiraditya abandoned this revision.
hiraditya added a comment.

Duplicate of: 
http://reviews.llvm.org/D9924


http://reviews.llvm.org/D9741



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


Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

2015-08-11 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: include/clang/Basic/DiagnosticParseKinds.td:1018-1019
@@ -1017,2 +1017,4 @@
   "expected ';' after module name">;
+def err_unexpected_module_end : Error<"unexpected module end">;
+def err_unexpected_module_start : Error<"submodule cannot be started here">;
 }

These diagnostics are phrased from the point of view of the implementation, and 
should instead be phrased from the point of view of the user.

The first diagnostic would be more useful if it said something like "missing 
'}' at end of module". That's pretty close to the diagnostic we already 
produce; maybe instead of changing the behavior of this case you could improve 
`BalancedDelimiterTracker` to use a custom diagnostic for the case where the 
missing closing delimiter is instead an end-of-module token?

The second diagnostic seems like an improvement, but from the user's 
perspective, this wasn't the start of a submodule, it was a module import / 
`#include`. Maybe just use `diagnoseMisplacedModuleImport` here?


Comment at: lib/Parse/ParseDeclCXX.cpp:3075
@@ +3074,3 @@
+  tok::annot_module_end)) {
+if (tryParseMisplacedModuleImport())
+  continue;

This shouldn't be named `try*` if it has a precondition that the current token 
is an EoM token. (The `try` form, if there is one, should check for that token 
itself.)


Comment at: lib/Parse/Parser.cpp:1999
@@ +1998,3 @@
+  case tok::annot_module_end:
+Diag(Tok, diag::err_unexpected_module_end);
+break;

This will result in you producing two errors "unexpected end of module" and 
"missing }" for each construct that is still open at the end of the module. I 
don't think that's what we want.


Comment at: lib/Parse/Parser.cpp:2003
@@ +2002,3 @@
+Diag(Tok, diag::err_unexpected_module_start);
+// Recover by skipping content of the included submodule.
+unsigned ModuleNesting = 1;

This is liable to produce bad follow-on diagnostics; I don't think this is a 
reasonable way to recover. I can see a few feasible options here:

1) Emit a fatal error when this happens (suppressing further diagnostics)
2) Break out to the top level of parsing and resume from there (that is, assume 
that a modular header expects to be included at the top level and that the user 
didn't screw up their module map)
3) Enter the module and carry on parsing from here

My preference would be either (1) or (2). But either way, we should diagnose 
the still-open bracketing constructs, because the problem is likely to be a 
missing close brace at the end of an unrelated header file.


Comment at: lib/Parse/Parser.cpp:2018
@@ +2017,3 @@
+// Module import found where it should not be, for instance, inside a
+// namespace. Recover by ignoring the import.
+Actions.diagnoseMisplacedModuleImport(reinterpret_cast(

Again, this will recover very poorly. In this case, our best bet seems to be to 
recover by importing the module.


http://reviews.llvm.org/D11844



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


Re: r244413 - [modules] When building a dependency file, include module maps parsed in the

2015-08-11 Thread Richard Smith via cfe-commits
Those files were parsed as part of building the output, and are legitimate
dependencies of the compilation process; why do you want to suppress them
from the .d file? (I think adding a whole bunch of -fno-*-deps flags is
going in the wrong direction, and would like to understand if there's some
higher-level notion that we should instead be capturing here.)

On Tue, Aug 11, 2015 at 11:27 AM, Manuel Klimek  wrote:

> Ah, so we need  a flag -fno-module-map-file-deps, I assume...
>
> On Tue, Aug 11, 2015 at 8:12 PM Richard Smith 
> wrote:
>
>> On Tue, Aug 11, 2015 at 4:57 AM, Manuel Klimek  wrote:
>>
>>> I believe this breaks -fno-module-file-deps.
>>>
>>
>> I don't think so; this affects which module map files end up in the .d
>> file, not which .pcm files do.
>>
>>
>>> On Sun, Aug 9, 2015 at 6:47 AM Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: rsmith
 Date: Sat Aug  8 23:46:57 2015
 New Revision: 244413

 URL: http://llvm.org/viewvc/llvm-project?rev=244413&view=rev
 Log:
 [modules] When building a dependency file, include module maps parsed
 in the
 current compilation, not just those from imported modules.

 Modified:
 cfe/trunk/include/clang/Lex/ModuleMap.h
 cfe/trunk/lib/Frontend/DependencyFile.cpp
 cfe/trunk/lib/Lex/ModuleMap.cpp
 cfe/trunk/test/Modules/dependency-gen-pch.m
 cfe/trunk/test/Modules/dependency-gen.m
 cfe/trunk/test/Modules/dependency-gen.modulemap
 cfe/trunk/test/Modules/relative-dep-gen.cpp

 Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=244413&r1=244412&r2=244413&view=diff

 ==
 --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
 +++ cfe/trunk/include/clang/Lex/ModuleMap.h Sat Aug  8 23:46:57 2015
 @@ -35,6 +35,22 @@ class DiagnosticConsumer;
  class DiagnosticsEngine;
  class HeaderSearch;
  class ModuleMapParser;
 +
 +/// \brief A mechanism to observe the actions of the module map parser
 as it
 +/// reads module map files.
 +class ModuleMapCallbacks {
 +public:
 +  virtual ~ModuleMapCallbacks() {}
 +
 +  /// \brief Called when a module map file has been read.
 +  ///
 +  /// \param FileStart A SourceLocation referring to the start of the
 file's
 +  /// contents.
 +  /// \param File The file itself.
 +  /// \param IsSystem Whether this is a module map from a system
 include path.
 +  virtual void moduleMapFileRead(SourceLocation FileStart,
 + const FileEntry &File, bool IsSystem)
 {}
 +};

  class ModuleMap {
SourceManager &SourceMgr;
 @@ -42,6 +58,8 @@ class ModuleMap {
const LangOptions &LangOpts;
const TargetInfo *Target;
HeaderSearch &HeaderInfo;
 +
 +  llvm::SmallVector, 1> Callbacks;

/// \brief The directory used for Clang-supplied, builtin include
 headers,
/// such as "stdint.h".
 @@ -263,6 +281,11 @@ public:
  BuiltinIncludeDir = Dir;
}

 +  /// \brief Add a module map callback.
 +  void addModuleMapCallbacks(std::unique_ptr
 Callback) {
 +Callbacks.push_back(std::move(Callback));
 +  }
 +
/// \brief Retrieve the module that owns the given header file, if
 any.
///
/// \param File The header file that is likely to be included.

 Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=244413&r1=244412&r2=244413&view=diff

 ==
 --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
 +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Sat Aug  8 23:46:57 2015
 @@ -18,6 +18,7 @@
  #include "clang/Frontend/FrontendDiagnostic.h"
  #include "clang/Lex/DirectoryLookup.h"
  #include "clang/Lex/LexDiagnostic.h"
 +#include "clang/Lex/ModuleMap.h"
  #include "clang/Lex/PPCallbacks.h"
  #include "clang/Lex/Preprocessor.h"
  #include "clang/Serialization/ASTReader.h"
 @@ -82,6 +83,20 @@ struct DepCollectorPPCallbacks : public
}
  };

 +struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
 +  DependencyCollector &DepCollector;
 +  DepCollectorMMCallbacks(DependencyCollector &DC) : DepCollector(DC)
 {}
 +
 +  void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
 + bool IsSystem) override {
 +StringRef Filename = Entry.getName();
 +DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
 +/*IsSystem*/IsSyst

Re: r244413 - [modules] When building a dependency file, include module maps parsed in the

2015-08-11 Thread Manuel Klimek via cfe-commits
On Tue, Aug 11, 2015 at 8:38 PM Richard Smith  wrote:

> Those files were parsed as part of building the output, and are legitimate
> dependencies of the compilation process; why do you want to suppress them
> from the .d file? (I think adding a whole bunch of -fno-*-deps flags is
> going in the wrong direction, and would like to understand if there's some
> higher-level notion that we should instead be capturing here.)
>

You're right, that makes sense. We'll just need to adapt our build system
to work with that.


>
>
> On Tue, Aug 11, 2015 at 11:27 AM, Manuel Klimek  wrote:
>
>> Ah, so we need  a flag -fno-module-map-file-deps, I assume...
>>
>> On Tue, Aug 11, 2015 at 8:12 PM Richard Smith 
>> wrote:
>>
>>> On Tue, Aug 11, 2015 at 4:57 AM, Manuel Klimek 
>>> wrote:
>>>
 I believe this breaks -fno-module-file-deps.

>>>
>>> I don't think so; this affects which module map files end up in the .d
>>> file, not which .pcm files do.
>>>
>>>
 On Sun, Aug 9, 2015 at 6:47 AM Richard Smith via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Sat Aug  8 23:46:57 2015
> New Revision: 244413
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244413&view=rev
> Log:
> [modules] When building a dependency file, include module maps parsed
> in the
> current compilation, not just those from imported modules.
>
> Modified:
> cfe/trunk/include/clang/Lex/ModuleMap.h
> cfe/trunk/lib/Frontend/DependencyFile.cpp
> cfe/trunk/lib/Lex/ModuleMap.cpp
> cfe/trunk/test/Modules/dependency-gen-pch.m
> cfe/trunk/test/Modules/dependency-gen.m
> cfe/trunk/test/Modules/dependency-gen.modulemap
> cfe/trunk/test/Modules/relative-dep-gen.cpp
>
> Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=244413&r1=244412&r2=244413&view=diff
>
> ==
> --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
> +++ cfe/trunk/include/clang/Lex/ModuleMap.h Sat Aug  8 23:46:57 2015
> @@ -35,6 +35,22 @@ class DiagnosticConsumer;
>  class DiagnosticsEngine;
>  class HeaderSearch;
>  class ModuleMapParser;
> +
> +/// \brief A mechanism to observe the actions of the module map
> parser as it
> +/// reads module map files.
> +class ModuleMapCallbacks {
> +public:
> +  virtual ~ModuleMapCallbacks() {}
> +
> +  /// \brief Called when a module map file has been read.
> +  ///
> +  /// \param FileStart A SourceLocation referring to the start of the
> file's
> +  /// contents.
> +  /// \param File The file itself.
> +  /// \param IsSystem Whether this is a module map from a system
> include path.
> +  virtual void moduleMapFileRead(SourceLocation FileStart,
> + const FileEntry &File, bool
> IsSystem) {}
> +};
>
>  class ModuleMap {
>SourceManager &SourceMgr;
> @@ -42,6 +58,8 @@ class ModuleMap {
>const LangOptions &LangOpts;
>const TargetInfo *Target;
>HeaderSearch &HeaderInfo;
> +
> +  llvm::SmallVector, 1> Callbacks;
>
>/// \brief The directory used for Clang-supplied, builtin include
> headers,
>/// such as "stdint.h".
> @@ -263,6 +281,11 @@ public:
>  BuiltinIncludeDir = Dir;
>}
>
> +  /// \brief Add a module map callback.
> +  void addModuleMapCallbacks(std::unique_ptr
> Callback) {
> +Callbacks.push_back(std::move(Callback));
> +  }
> +
>/// \brief Retrieve the module that owns the given header file, if
> any.
>///
>/// \param File The header file that is likely to be included.
>
> Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=244413&r1=244412&r2=244413&view=diff
>
> ==
> --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
> +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Sat Aug  8 23:46:57 2015
> @@ -18,6 +18,7 @@
>  #include "clang/Frontend/FrontendDiagnostic.h"
>  #include "clang/Lex/DirectoryLookup.h"
>  #include "clang/Lex/LexDiagnostic.h"
> +#include "clang/Lex/ModuleMap.h"
>  #include "clang/Lex/PPCallbacks.h"
>  #include "clang/Lex/Preprocessor.h"
>  #include "clang/Serialization/ASTReader.h"
> @@ -82,6 +83,20 @@ struct DepCollectorPPCallbacks : public
>}
>  };
>
> +struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
> +  DependencyCollector &DepCollector;
> +  DepCollectorMMCallbacks(DependencyCollector &DC) : DepCollector(DC)
> {}
> +
> +  void moduleM

Fwd: r232721 - Add option to switch off putting header modules into the dependency file.

2015-08-11 Thread Richard Smith via cfe-commits
On Thu, Mar 19, 2015 at 5:00 AM, Manuel Klimek  wrote:

> Author: klimek
> Date: Thu Mar 19 07:00:22 2015
> New Revision: 232721
>
> URL: http://llvm.org/viewvc/llvm-project?rev=232721&view=rev
> Log:
> Add option to switch off putting header modules into the dependency file.
>

Why? It is not correct to use a .pch file after the corresponding .pcm file
has been rebuilt. And why do you want a switch to turn on adding implicit
module files to the .d file? (I think the right behavior here is that
explicit modules are always listed, and implicit modules are never listed
except when building a .pch file.)


> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/test/Driver/pch-deps.c
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=232721&r1=232720&r2=232721&view=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Thu Mar 19 07:00:22 2015
> @@ -772,6 +772,10 @@ def fno_modules_strict_decluse : Flag <[
>Flags<[DriverOption]>;
>  def fimplicit_modules : Flag <["-"], "fimplicit-modules">, Group,
>Flags<[DriverOption]>;
> +def fmodule_file_deps : Flag <["-"], "fmodule-file-deps">, Group,
> +  Flags<[DriverOption]>;
> +def fno_module_file_deps : Flag <["-"], "fno-module-file-deps">,
> Group,
> +  Flags<[DriverOption]>;
>  def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">, Group;
>  def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">,
> Group;
>  def fno_delayed_template_parsing : Flag<["-"],
> "fno-delayed-template-parsing">, Group;
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=232721&r1=232720&r2=232721&view=diff
>
> ==
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Mar 19 07:00:22 2015
> @@ -324,8 +324,9 @@ void Clang::AddPreprocessingOptions(Comp
>  if (A->getOption().matches(options::OPT_M) ||
>  A->getOption().matches(options::OPT_MD))
>CmdArgs.push_back("-sys-header-deps");
> -
> -if (isa(JA))
> +if ((isa(JA) &&
> + !Args.hasArg(options::OPT_fno_module_file_deps)) ||
> +Args.hasArg(options::OPT_fmodule_file_deps))
>CmdArgs.push_back("-module-file-deps");
>}
>
>
> Modified: cfe/trunk/test/Driver/pch-deps.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pch-deps.c?rev=232721&r1=232720&r2=232721&view=diff
>
> ==
> --- cfe/trunk/test/Driver/pch-deps.c (original)
> +++ cfe/trunk/test/Driver/pch-deps.c Thu Mar 19 07:00:22 2015
> @@ -8,3 +8,14 @@
>  // RUN: FileCheck %s -check-prefix=CHECK-NOPCH -input-file=%t
>  // CHECK-NOPCH: -dependency-file
>  // CHECK-NOPCH-NOT: -module-file-deps
> +
> +// RUN: %clang -x c-header %s -o %t.pch -MMD -MT dependencies -MF %t.d \
> +// RUN: -fno-module-file-deps -### 2> %t
> +// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT -input-file=%t
> +// CHECK-EXPLICIT: -dependency-file
> +// CHECK-EXPLICIT-NOT: -module-file-deps
> +
> +// RUN: %clang -x c++ %s -o %t.o -MMD -MT dependencies -MF %t.d
> -fmodule-file-deps -### 2> %t
> +// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT-NOPCH -input-file=%t
> +// CHECK-EXPLICIT-NOPCH: -dependency-file
> +// CHECK-EXPLICIT-NOPCH: -module-file-deps
>
>
> ___
> cfe-commits mailing list
> cfe-comm...@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Joerg Sonnenberger via cfe-commits
joerg added a subscriber: joerg.
joerg added a comment.

I'm against doing this unconditionally. IMO it creates bugs without reasonable 
compensation. Just because glibc wants to hurt people doesn't mean anyone 
should get hurt.


http://reviews.llvm.org/D11948



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Dan Albert via cfe-commits
danalbert added a comment.

In http://reviews.llvm.org/D11948#221936, @joerg wrote:

> I'm against doing this unconditionally. IMO it creates bugs without 
> reasonable compensation. Just because glibc wants to hurt people doesn't mean 
> anyone should get hurt.


+1. We don't want this in Android.


http://reviews.llvm.org/D11948



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


Re: r232721 - Add option to switch off putting header modules into the dependency file.

2015-08-11 Thread Manuel Klimek via cfe-commits
Back in the day I'm pretty sure you agreed with the general concept :)
http://reviews.llvm.org/D8378
I remember that we had problems with absolute paths ending up in the .d
file (which should not happen if none of the paths provided are absolute),
but I don't remember why we went for not writing them to the .d file
instead of fixing the underlying problem.

On Tue, Aug 11, 2015 at 8:41 PM Richard Smith  wrote:

> On Thu, Mar 19, 2015 at 5:00 AM, Manuel Klimek  wrote:
>
>> Author: klimek
>> Date: Thu Mar 19 07:00:22 2015
>> New Revision: 232721
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=232721&view=rev
>> Log:
>> Add option to switch off putting header modules into the dependency file.
>>
>
> Why? It is not correct to use a .pch file after the corresponding .pcm
> file has been rebuilt. And why do you want a switch to turn on adding
> implicit module files to the .d file? (I think the right behavior here is
> that explicit modules are always listed, and implicit modules are never
> listed except when building a .pch file.)
>
>
>> Modified:
>> cfe/trunk/include/clang/Driver/Options.td
>> cfe/trunk/lib/Driver/Tools.cpp
>> cfe/trunk/test/Driver/pch-deps.c
>>
>> Modified: cfe/trunk/include/clang/Driver/Options.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=232721&r1=232720&r2=232721&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/Options.td Thu Mar 19 07:00:22 2015
>> @@ -772,6 +772,10 @@ def fno_modules_strict_decluse : Flag <[
>>Flags<[DriverOption]>;
>>  def fimplicit_modules : Flag <["-"], "fimplicit-modules">,
>> Group,
>>Flags<[DriverOption]>;
>> +def fmodule_file_deps : Flag <["-"], "fmodule-file-deps">,
>> Group,
>> +  Flags<[DriverOption]>;
>> +def fno_module_file_deps : Flag <["-"], "fno-module-file-deps">,
>> Group,
>> +  Flags<[DriverOption]>;
>>  def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">, Group;
>>  def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">,
>> Group;
>>  def fno_delayed_template_parsing : Flag<["-"],
>> "fno-delayed-template-parsing">, Group;
>>
>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=232721&r1=232720&r2=232721&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Mar 19 07:00:22 2015
>> @@ -324,8 +324,9 @@ void Clang::AddPreprocessingOptions(Comp
>>  if (A->getOption().matches(options::OPT_M) ||
>>  A->getOption().matches(options::OPT_MD))
>>CmdArgs.push_back("-sys-header-deps");
>> -
>> -if (isa(JA))
>> +if ((isa(JA) &&
>> + !Args.hasArg(options::OPT_fno_module_file_deps)) ||
>> +Args.hasArg(options::OPT_fmodule_file_deps))
>>CmdArgs.push_back("-module-file-deps");
>>}
>>
>>
>> Modified: cfe/trunk/test/Driver/pch-deps.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pch-deps.c?rev=232721&r1=232720&r2=232721&view=diff
>>
>> ==
>> --- cfe/trunk/test/Driver/pch-deps.c (original)
>> +++ cfe/trunk/test/Driver/pch-deps.c Thu Mar 19 07:00:22 2015
>> @@ -8,3 +8,14 @@
>>  // RUN: FileCheck %s -check-prefix=CHECK-NOPCH -input-file=%t
>>  // CHECK-NOPCH: -dependency-file
>>  // CHECK-NOPCH-NOT: -module-file-deps
>> +
>> +// RUN: %clang -x c-header %s -o %t.pch -MMD -MT dependencies -MF %t.d \
>> +// RUN: -fno-module-file-deps -### 2> %t
>> +// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT -input-file=%t
>> +// CHECK-EXPLICIT: -dependency-file
>> +// CHECK-EXPLICIT-NOT: -module-file-deps
>> +
>> +// RUN: %clang -x c++ %s -o %t.o -MMD -MT dependencies -MF %t.d
>> -fmodule-file-deps -### 2> %t
>> +// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT-NOPCH -input-file=%t
>> +// CHECK-EXPLICIT-NOPCH: -dependency-file
>> +// CHECK-EXPLICIT-NOPCH: -module-file-deps
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-comm...@cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

2015-08-11 Thread Nico Weber via cfe-commits
On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie  wrote:

>
>
> On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Can't you just change your signature to
>>
>>   virtual void a(int /* x */) {}
>>
>> in these cases?
>>
>
> You could - does it add much value to do that, though?
>

If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?


> (perhaps it does - it means you express the intent that the parameter is
> not used and the compiler helps you check that (so that for the parameters
> you think /should/ be used (you haven't commented out their name but
> accidentally shadow or otherwise fail to reference, you still get a
> warning))
>
> - David
>
>
>>
>> On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> danielmarjamaki created this revision.
>>> danielmarjamaki added a reviewer: krememek.
>>> danielmarjamaki added a subscriber: cfe-commits.
>>>
>>> Don't diagnose -Wunused-parameter in methods that override other methods
>>> because the overridden methods might use the parameter
>>>
>>> Don't diagnose -Wunused-parameter in virtual methods because these might
>>> be overriden by other methods that use the parameter.
>>>
>>> Such diagnostics could be more accurately written if they are based on
>>> whole-program-analysis that establish if such parameter is unused in all
>>> methods.
>>>
>>>
>>>
>>> http://reviews.llvm.org/D11940
>>>
>>> Files:
>>>   lib/Sema/SemaDecl.cpp
>>>   test/SemaCXX/warn-unused-parameters.cpp
>>>
>>> Index: test/SemaCXX/warn-unused-parameters.cpp
>>> ===
>>> --- test/SemaCXX/warn-unused-parameters.cpp
>>> +++ test/SemaCXX/warn-unused-parameters.cpp
>>> @@ -32,3 +32,20 @@
>>>auto l = [&t...]() { return sizeof...(s); };
>>>return l();
>>>  }
>>> +
>>> +// Don't diagnose virtual methods or methods that override base class
>>> +// methods.
>>> +class Base {
>>> +public:
>>> +  virtual void f(int x);
>>> +};
>>> +
>>> +class Derived : public Base {
>>> +public:
>>> +  // Don't warn in overridden methods.
>>> +  virtual void f(int x) {}
>>> +
>>> +  // Don't warn in virtual methods.
>>> +  virtual void a(int x) {}
>>> +};
>>> +
>>> Index: lib/Sema/SemaDecl.cpp
>>> ===
>>> --- lib/Sema/SemaDecl.cpp
>>> +++ lib/Sema/SemaDecl.cpp
>>> @@ -10797,8 +10797,13 @@
>>>
>>>  if (!FD->isInvalidDecl()) {
>>>// Don't diagnose unused parameters of defaulted or deleted
>>> functions.
>>> -  if (!FD->isDeleted() && !FD->isDefaulted())
>>> -DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>>> +  if (!FD->isDeleted() && !FD->isDefaulted()) {
>>> +// Don't diagnose unused parameters in virtual methods or
>>> +// in methods that override base class methods.
>>> +const auto MD = dyn_cast(FD);
>>> +if (!MD || (MD->size_overridden_methods() == 0U &&
>>> !MD->isVirtual()))
>>> +  DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>>> +  }
>>>DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(),
>>> FD->param_end(),
>>>   FD->getReturnType(), FD);
>>>
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11859: Generating vptr assume loads

2015-08-11 Thread Piotr Padlewski via cfe-commits
Prazek marked 4 inline comments as done.


Comment at: lib/CodeGen/ItaniumCXXABI.cpp:193-194
@@ -192,1 +192,4 @@
 
+  bool isVirtualOffsetNeeded(CodeGenFunction &CGF,
+ const CXXRecordDecl *NearestVBase) override;
+

isVirtualOffsetNeededForVtableField good enough?


http://reviews.llvm.org/D11859



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


Re: [PATCH] D11859: Generating vptr assume loads

2015-08-11 Thread Piotr Padlewski via cfe-commits
Prazek marked 3 inline comments as done.
Prazek added a comment.

http://reviews.llvm.org/D11859



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


Re: [PATCH] D11859: Generating vptr assume loads

2015-08-11 Thread Piotr Padlewski via cfe-commits
Prazek updated this revision to Diff 31846.

http://reviews.llvm.org/D11859

Files:
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGen/available-externally-hidden.cpp
  test/CodeGenCXX/ctor-globalopt.cpp
  test/CodeGenCXX/template-instantiation.cpp
  test/CodeGenCXX/thunks.cpp
  test/CodeGenCXX/virtual-base-ctor.cpp
  test/CodeGenCXX/vtable-assume-load.cpp
  test/CodeGenCXX/vtable-available-externally.cpp

Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -182,8 +182,8 @@
 namespace Test9 {
 // all virtual functions are outline, so we can assume that it will
 // be generated in translation unit where foo is defined
-// CHECK-TEST9: @_ZTVN5Test91AE = available_externally unnamed_addr constant
-// CHECK-TEST9: @_ZTVN5Test91BE = available_externally unnamed_addr constant
+// CHECK-TEST9-DAG: @_ZTVN5Test91AE = available_externally unnamed_addr constant
+// CHECK-TEST9-DAG: @_ZTVN5Test91BE = available_externally unnamed_addr constant
 struct A {
   virtual void foo();
   virtual void bar();
@@ -206,39 +206,39 @@
 namespace Test10 {
 
 // because A's key function is defined here, vtable is generated in this TU
-// CHECK-TEST10: @_ZTVN6Test101AE = unnamed_addr constant
+// CHECK-TEST10-DAG: @_ZTVN6Test101AE = unnamed_addr constant
 struct A {
   virtual void foo();
   virtual void bar();
 };
 void A::foo() {}
 
 // Because key function is inline we will generate vtable as linkonce_odr
-// CHECK-TEST10: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
+// CHECK-TEST10-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
 struct D : A {
   void bar();
 };
 inline void D::bar() {}
 
 // because B has outline key function then we can refer to
-// CHECK-TEST10: @_ZTVN6Test101BE = available_externally unnamed_addr constant
+// CHECK-TEST10-DAG: @_ZTVN6Test101BE = available_externally unnamed_addr constant
 struct B : A {
   void foo();
   void bar();
 };
 
 // C's key function (car) is outline, but C has inline virtual function so we
 // can't guarantee that we will be able to refer to bar from name
 // so (at the moment) we can't emit vtable available_externally
-// CHECK-TEST10: @_ZTVN6Test101CE = external unnamed_addr constant
+// CHECK-TEST10-DAG: @_ZTVN6Test101CE = external unnamed_addr constant
 struct C : A {
   void bar() {}   // defined in body - not key function
   virtual inline void gar();  // inline in body - not key function
   virtual void car();
 };
 
 // no key function, vtable will be generated everywhere it will be used
-// CHECK-TEST10: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
+// CHECK-TEST10-DAG: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
 struct E : A {};
 
 void g(A& a) {
Index: test/CodeGenCXX/vtable-assume-load.cpp
===
--- /dev/null
+++ test/CodeGenCXX/vtable-assume-load.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -emit-llvm -o %t.ll -O1 -disable-llvm-optzns -fms-extensions
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -emit-llvm -o %t.ms.ll -O1 -disable-llvm-optzns -fms-extensions
+
+// RUN: FileCheck --check-prefix=CHECK1 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK2 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK3 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK4 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK-MS --input-file=%t.ms.ll %s
+
+namespace test1 {
+
+struct A {
+  A();
+  virtual void foo();
+};
+
+struct B : A {
+  virtual void foo();
+};
+
+void g(A *a) { a->foo(); }
+
+void fooA() {
+  A a;
+  g(&a);
+}
+void fooB() {
+  B b;
+  g(&b);
+}
+// CHECK1-LABEL: define void @_ZN5test14fooAEv()
+// CHECK1: %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([3 x i8*], [3 x i8*]* @_ZTVN5test11AE, i64 0, i64 2)
+// CHECK1: call void @llvm.assume(i1 %cmp.vtables)
+// CHECK1-LABEL: }
+
+// CHECK1-LABEL: define void @_ZN5test14fooBEv()
+// CHECK1: call void @_ZN5test11BC1Ev(%"struct.test1::B"* %b)
+// CHECK1: %vtable = load i8**, i8*** %1, !tbaa !5
+// CHECK1: %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([3 x i8*], [3 x i8*]* @_ZTVN5test11BE, i64 0, i64 2)
+// CHECK1: call void @llvm.assume(i1 %cmp.vtables)
+// CHECK1-LABLE: }
+
+// there should not be any assumes in the ctor that calls base ctor
+// CHECK1-LABEL: define linkonce_odr void @_ZN5test11BC2Ev(%"struct.test1::B"* %this)
+// CHECK1-NOT: @llvm.assume(
+// CHECK1-LABEL: }
+}
+namespace test2 {
+struct A {
+  A();
+  virtual void foo();
+};
+
+struct B {
+  B();
+  virtual void bar();
+};
+
+struct C : A, B {
+  C();
+  virtual void foo();
+};
+void g(A *a) { a->foo(); }
+void h(B *b) { b->bar(); }
+
+// CHECK2-LABEL: defin

Re: r232721 - Add option to switch off putting header modules into the dependency file.

2015-08-11 Thread Richard Smith via cfe-commits
On Tue, Aug 11, 2015 at 11:49 AM, Manuel Klimek  wrote:

> Back in the day I'm pretty sure you agreed with the general concept :)
>

Yeah, I don't think I fully understood the reason for having a cc1
-module-file-deps flag at the time.

http://reviews.llvm.org/D8378
> I remember that we had problems with absolute paths ending up in the .d
> file (which should not happen if none of the paths provided are absolute),
> but I don't remember why we went for not writing them to the .d file
> instead of fixing the underlying problem.
>

Here's the situation from before your change:

 * Names of (implicit) module files were not written to the .pcm file;
they're an implementation detail of the compilation process and not an
actual input, so this is correct behavior (the input files used to *build*
the implicit modules do get written to the .pcm file, which again seems
right)
 * As a special case, when creating a .d file for a .pch file, implicit
module files *were* written to the .d file. This is unfortunate but
necessary, because rebuilding a .pcm file in the module cache does not give
a bit-for-bit identical result (sometimes just the signature changes,
sometimes the .pcm is built with a different configuration in a way that
the module cache hash doesn't detect, and very occasionally there's a hash
collision).

The -module-file-deps flag was an implementation detail of the .pch
building process, as a way of the driver telling the frontend to include
.pcm files in the produced .d file. The only effect of the flag added here
was to turn off this bugfix for PCH files; I don't think this is a useful
flag.

On Tue, Aug 11, 2015 at 8:41 PM Richard Smith  wrote:
>
>> On Thu, Mar 19, 2015 at 5:00 AM, Manuel Klimek  wrote:
>>
>>> Author: klimek
>>> Date: Thu Mar 19 07:00:22 2015
>>> New Revision: 232721
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=232721&view=rev
>>> Log:
>>> Add option to switch off putting header modules into the dependency file.
>>>
>>
>> Why? It is not correct to use a .pch file after the corresponding .pcm
>> file has been rebuilt. And why do you want a switch to turn on adding
>> implicit module files to the .d file? (I think the right behavior here is
>> that explicit modules are always listed, and implicit modules are never
>> listed except when building a .pch file.)
>>
>>
>>> Modified:
>>> cfe/trunk/include/clang/Driver/Options.td
>>> cfe/trunk/lib/Driver/Tools.cpp
>>> cfe/trunk/test/Driver/pch-deps.c
>>>
>>> Modified: cfe/trunk/include/clang/Driver/Options.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=232721&r1=232720&r2=232721&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>>> +++ cfe/trunk/include/clang/Driver/Options.td Thu Mar 19 07:00:22 2015
>>> @@ -772,6 +772,10 @@ def fno_modules_strict_decluse : Flag <[
>>>Flags<[DriverOption]>;
>>>  def fimplicit_modules : Flag <["-"], "fimplicit-modules">,
>>> Group,
>>>Flags<[DriverOption]>;
>>> +def fmodule_file_deps : Flag <["-"], "fmodule-file-deps">,
>>> Group,
>>> +  Flags<[DriverOption]>;
>>> +def fno_module_file_deps : Flag <["-"], "fno-module-file-deps">,
>>> Group,
>>> +  Flags<[DriverOption]>;
>>>  def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">,
>>> Group;
>>>  def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">,
>>> Group;
>>>  def fno_delayed_template_parsing : Flag<["-"],
>>> "fno-delayed-template-parsing">, Group;
>>>
>>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=232721&r1=232720&r2=232721&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>>> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Mar 19 07:00:22 2015
>>> @@ -324,8 +324,9 @@ void Clang::AddPreprocessingOptions(Comp
>>>  if (A->getOption().matches(options::OPT_M) ||
>>>  A->getOption().matches(options::OPT_MD))
>>>CmdArgs.push_back("-sys-header-deps");
>>> -
>>> -if (isa(JA))
>>> +if ((isa(JA) &&
>>> + !Args.hasArg(options::OPT_fno_module_file_deps)) ||
>>> +Args.hasArg(options::OPT_fmodule_file_deps))
>>>CmdArgs.push_back("-module-file-deps");
>>>}
>>>
>>>
>>> Modified: cfe/trunk/test/Driver/pch-deps.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pch-deps.c?rev=232721&r1=232720&r2=232721&view=diff
>>>
>>> ==
>>> --- cfe/trunk/test/Driver/pch-deps.c (original)
>>> +++ cfe/trunk/test/Driver/pch-deps.c Thu Mar 19 07:00:22 2015
>>> @@ -8,3 +8,14 @@
>>>  // RUN: FileCheck %s -check-prefix=CHECK-NOPCH -input-file=%t
>>>  // CHECK-NOPCH: -dependency-file
>>>  // CHECK-NOPCH-NOT: -module-file-deps
>>> +
>>> +// RUN: %clang -x c-header %s -o %t.pch -MMD -

Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Marshall Clow via cfe-commits
mclow.lists added inline comments.


Comment at: include/cstring:96
@@ +95,3 @@
+int strncmp(const char* __s1, const char* __s2, size_t __n)
+{ return ::memcmp(__s1, __s2, __n); }
+

nlopes wrote:
> typo here.
Oops. Thanks!


http://reviews.llvm.org/D11948



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Marshall Clow via cfe-commits
mclow.lists added a comment.

In http://reviews.llvm.org/D11948#221936, @joerg wrote:

> I'm against doing this unconditionally. IMO it creates bugs without 
> reasonable compensation. Just because glibc wants to hurt people doesn't mean 
> anyone should get hurt.


Really? I see it as:

  It tells people who have UB in their code that they have a bug.

There's nothing here that "creates bugs".


http://reviews.llvm.org/D11948



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


r244650 - [MSVC Compatibility] Classify ext_ms_cast_fn_obj as DefaultError

2015-08-11 Thread David Majnemer via cfe-commits
Author: majnemer
Date: Tue Aug 11 14:25:13 2015
New Revision: 244650

URL: http://llvm.org/viewvc/llvm-project?rev=244650&view=rev
Log:
[MSVC Compatibility] Classify ext_ms_cast_fn_obj as DefaultError

This non-conforming extension was introduced to make it possible for us
to correctly compile  in VS 2013 and 2015.  Let's limit its
impact to system headers to encourage portable code.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/SemaCXX/MicrosoftCompatibility-cxx98.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=244650&r1=244649&r2=244650&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 11 14:25:13 
2015
@@ -5436,7 +5436,7 @@ def ext_cast_fn_obj : Extension<
 def ext_ms_cast_fn_obj : ExtWarn<
   "static_cast between pointer-to-function and pointer-to-object is a "
   "Microsoft extension">,
-  InGroup;
+  InGroup, DefaultError, SFINAEFailure;
 def warn_cxx98_compat_cast_fn_obj : Warning<
   "cast between pointer-to-function and pointer-to-object is incompatible with 
C++98">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/test/SemaCXX/MicrosoftCompatibility-cxx98.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftCompatibility-cxx98.cpp?rev=244650&r1=244649&r2=244650&view=diff
==
--- cfe/trunk/test/SemaCXX/MicrosoftCompatibility-cxx98.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftCompatibility-cxx98.cpp Tue Aug 11 14:25:13 
2015
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++98 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++98 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-Wno-error=microsoft-cast
 
 
 //MSVC allows forward enum declaration


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


Re: [PATCH] D11950: [CUDA] Check register names on appropriate side of cuda compilation only.

2015-08-11 Thread Eli Bendersky via cfe-commits
eliben added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:5944
@@ -5943,3 +5943,3 @@
   ProcessDeclAttributes(S, NewVD, D);
-
+  bool ShouldHandleTargetErrors = DeclAttrsMatchCUDAMode(getLangOpts(), NewVD);
   if (getLangOpts().CUDA) {

Since this is a CUDA-only thing, ShouldHandleTargetErrors can perhaps be named 
better. The checks you added are very selective now - some diags are disabled, 
some are not. It's not clear why some fall under the "should handle target 
error" umbrella. A clearer name like MatchingCUDAMode or something of the sort, 
may help? 


Comment at: lib/Sema/SemaDecl.cpp:5971
@@ -5970,3 +5970,3 @@
   // Handle GNU asm-label extension (encoded as an attribute).
   if (Expr *E = (Expr*)D.getAsmLabel()) {
 // The parser guarantees this is a string.

Do we plan to support this for CUDA at all? Why not disable here on top?


http://reviews.llvm.org/D11950



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Joerg Sonnenberger via cfe-commits
joerg added a comment.

No, it doesn't. It tells the compiler that it is free to make such assumptions. 
Take a step back from the standard. Can you think of any reasonable and 
efficient implementation of memcpy and friends, which fails for size 0? Adding 
the annotations (whether here or in string.h) effectively changes the behavior 
of the program. It is behavior people have been expecting for two decades, even 
when C90 said something else. This is completely different from the warning 
annotations. I'm just waiting for some of the bigger projects like PostgreSQL 
to start getting annoyed enough to introduce sane_memcpy for this.
I can't speak for Linux distributions using glibc, but I find this kind of 
smoking gun completely unacceptable to force unconditionally on everyone.


http://reviews.llvm.org/D11948



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


Re: [PATCH] D11859: Generating vptr assume loads

2015-08-11 Thread Piotr Padlewski via cfe-commits
Prazek marked 4 inline comments as done.
Prazek added a comment.

http://reviews.llvm.org/D11859



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


Re: [PATCH] D11916: [CONCEPTS] Add diagnostic; invalid tag when concept specified

2015-08-11 Thread Nathan Wilson via cfe-commits
On Tue, Aug 11, 2015 at 8:54 AM, Aaron Ballman 
wrote:

> On Mon, Aug 10, 2015 at 3:00 PM, Nathan Wilson 
> wrote:
> > nwilson created this revision.
> > nwilson added reviewers: rsmith, hubert.reinterpretcast, fraggamuffin,
> faisalv, aaron.ballman.
> > nwilson added a subscriber: cfe-commits.
> >
> > Adding check to emit diagnostic for invalid tag when concept is
> specified and associated tests.
> >
> > http://reviews.llvm.org/D11916
> >
> > Files:
> >   include/clang/Basic/DiagnosticSemaKinds.td
> >   lib/Sema/SemaDecl.cpp
> >   test/SemaCXX/cxx-concept-declaration.cpp
> >
> > Index: test/SemaCXX/cxx-concept-declaration.cpp
> > ===
> > --- test/SemaCXX/cxx-concept-declaration.cpp
> > +++ test/SemaCXX/cxx-concept-declaration.cpp
> > @@ -23,3 +23,13 @@
> >  template
> >  concept bool D6; // expected-error {{variable concept declaration must
> be initialized}}
> >
> > +// Tag
> > +concept class CC1 {}; // expected-error {{class cannot be marked
> concept}}
> > +concept struct CS1 {}; // expected-error {{struct cannot be marked
> concept}}
> > +concept union CU1 {}; // expected-error {{union cannot be marked
> concept}}
> > +concept enum CE1 {}; // expected-error {{enum cannot be marked concept}}
> > +template  concept class TCC1 {}; // expected-error {{class
> cannot be marked concept}}
> > +template  concept struct TCS1 {}; // expected-error
> {{struct cannot be marked concept}}
> > +template  concept union TCU1 {}; // expected-error {{union
> cannot be marked concept}}
> > +
> > +extern concept bool ExtC; // expected-error {{'concept' can only appear
> on the definition of a function template or variable template}}
> > Index: lib/Sema/SemaDecl.cpp
> > ===
> > --- lib/Sema/SemaDecl.cpp
> > +++ lib/Sema/SemaDecl.cpp
> > @@ -3662,6 +3662,19 @@
> >  return TagD;
> >}
> >
> > +  if (DS.isConceptSpecified()) {
> > +// C++ Concepts TS [dcl.spec.concept]p1: A concept definition
> refers to
> > +// either a function concept and its definition or a variable
> concept and
> > +// its initializer.
> > +if (Tag)
> > +  Diag(DS.getConceptSpecLoc(), diag::err_concept_tag)
> > +  << GetDiagnosticTypeSpecifierID(DS.getTypeSpecType());
> > +else
> > +  Diag(DS.getConceptSpecLoc(), diag::err_concept_decl_non_template);
> > +// Don't emit warnings after this error.
> > +return TagD;
> > +  }
>
> I'm not certain I understand why we need two different diagnostics for
> this case. I think err_concept_decl_non_template is sufficiently clear
> for both.
>
> ~Aaron
>

This was based on how constexpr handles these checks.

Perhaps someone can correct me if I'm wrong, but I believe the idea is that
when the `struct` tag exists, for example, we think the user meant to
declare a `struct` and not the start of a `concept` declaration. So, the
`concept` specifier would be erroneous and we try give a more helpful
diagnostic.

I would need to add/fix the test case for this, but I tend to think the
declaration such as `concept bool;` could be the users intention to try to
create a `concept` declaration which is where
the err_concept_decl_non_template comes in.


>
> > +
> >DiagnoseFunctionSpecifiers(DS);
> >
> >if (DS.isFriendSpecified()) {
> > Index: include/clang/Basic/DiagnosticSemaKinds.td
> > ===
> > --- include/clang/Basic/DiagnosticSemaKinds.td
> > +++ include/clang/Basic/DiagnosticSemaKinds.td
> > @@ -1973,6 +1973,8 @@
> >"function concept declaration must be a definition">;
> >  def err_var_concept_not_initialized : Error<
> >"variable concept declaration must be initialized">;
> > +def err_concept_tag : Error<
> > +  "%select{class|struct|interface|union|enum}0 cannot be marked
> concept">;
> >
> >  // C++11 char16_t/char32_t
> >  def warn_cxx98_compat_unicode_type : Warning<
> >
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Aaron Ballman via cfe-commits
On Tue, Aug 11, 2015 at 3:32 PM, Joerg Sonnenberger via cfe-commits
 wrote:
> joerg added a comment.
>
> No, it doesn't. It tells the compiler that it is free to make such 
> assumptions. Take a step back from the standard. Can you think of any 
> reasonable and efficient implementation of memcpy and friends, which fails 
> for size 0? Adding the annotations (whether here or in string.h) effectively 
> changes the behavior of the program. It is behavior people have been 
> expecting for two decades, even when C90 said something else. This is 
> completely different from the warning annotations. I'm just waiting for some 
> of the bigger projects like PostgreSQL to start getting annoyed enough to 
> introduce sane_memcpy for this.
> I can't speak for Linux distributions using glibc, but I find this kind of 
> smoking gun completely unacceptable to force unconditionally on everyone.

Would you be opposed to annotations that tell the programmer they have
UB in their code, but *do not* effect the code generation?

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


Re: [PATCH] D11916: [CONCEPTS] Add diagnostic; invalid tag when concept specified

2015-08-11 Thread Aaron Ballman via cfe-commits
On Tue, Aug 11, 2015 at 3:36 PM, Nathan Wilson  wrote:
>
>
> On Tue, Aug 11, 2015 at 8:54 AM, Aaron Ballman 
> wrote:
>>
>> On Mon, Aug 10, 2015 at 3:00 PM, Nathan Wilson 
>> wrote:
>> > nwilson created this revision.
>> > nwilson added reviewers: rsmith, hubert.reinterpretcast, fraggamuffin,
>> > faisalv, aaron.ballman.
>> > nwilson added a subscriber: cfe-commits.
>> >
>> > Adding check to emit diagnostic for invalid tag when concept is
>> > specified and associated tests.
>> >
>> > http://reviews.llvm.org/D11916
>> >
>> > Files:
>> >   include/clang/Basic/DiagnosticSemaKinds.td
>> >   lib/Sema/SemaDecl.cpp
>> >   test/SemaCXX/cxx-concept-declaration.cpp
>> >
>> > Index: test/SemaCXX/cxx-concept-declaration.cpp
>> > ===
>> > --- test/SemaCXX/cxx-concept-declaration.cpp
>> > +++ test/SemaCXX/cxx-concept-declaration.cpp
>> > @@ -23,3 +23,13 @@
>> >  template
>> >  concept bool D6; // expected-error {{variable concept declaration must
>> > be initialized}}
>> >
>> > +// Tag
>> > +concept class CC1 {}; // expected-error {{class cannot be marked
>> > concept}}
>> > +concept struct CS1 {}; // expected-error {{struct cannot be marked
>> > concept}}
>> > +concept union CU1 {}; // expected-error {{union cannot be marked
>> > concept}}
>> > +concept enum CE1 {}; // expected-error {{enum cannot be marked
>> > concept}}
>> > +template  concept class TCC1 {}; // expected-error {{class
>> > cannot be marked concept}}
>> > +template  concept struct TCS1 {}; // expected-error
>> > {{struct cannot be marked concept}}
>> > +template  concept union TCU1 {}; // expected-error {{union
>> > cannot be marked concept}}
>> > +
>> > +extern concept bool ExtC; // expected-error {{'concept' can only appear
>> > on the definition of a function template or variable template}}
>> > Index: lib/Sema/SemaDecl.cpp
>> > ===
>> > --- lib/Sema/SemaDecl.cpp
>> > +++ lib/Sema/SemaDecl.cpp
>> > @@ -3662,6 +3662,19 @@
>> >  return TagD;
>> >}
>> >
>> > +  if (DS.isConceptSpecified()) {
>> > +// C++ Concepts TS [dcl.spec.concept]p1: A concept definition
>> > refers to
>> > +// either a function concept and its definition or a variable
>> > concept and
>> > +// its initializer.
>> > +if (Tag)
>> > +  Diag(DS.getConceptSpecLoc(), diag::err_concept_tag)
>> > +  << GetDiagnosticTypeSpecifierID(DS.getTypeSpecType());
>> > +else
>> > +  Diag(DS.getConceptSpecLoc(),
>> > diag::err_concept_decl_non_template);
>> > +// Don't emit warnings after this error.
>> > +return TagD;
>> > +  }
>>
>> I'm not certain I understand why we need two different diagnostics for
>> this case. I think err_concept_decl_non_template is sufficiently clear
>> for both.
>>
>> ~Aaron
>
>
> This was based on how constexpr handles these checks.
>
> Perhaps someone can correct me if I'm wrong, but I believe the idea is that
> when the `struct` tag exists, for example, we think the user meant to
> declare a `struct` and not the start of a `concept` declaration. So, the
> `concept` specifier would be erroneous and we try give a more helpful
> diagnostic.

It could just be me, but I don't find the new diagnostic particularly
helpful. "foo cannot be marked concept" tells me why I have an error
but does not tell me what to do about it. "'concept' can only appear
on the definition of a function template or variable template" tells
me what I need to do to not have the error in the first place, as well
as why I have the error.

However, others may have differing opinions on the subject.

~Aaron

>
> I would need to add/fix the test case for this, but I tend to think the
> declaration such as `concept bool;` could be the users intention to try to
> create a `concept` declaration which is where the
> err_concept_decl_non_template comes in.
>
>>
>>
>> > +
>> >DiagnoseFunctionSpecifiers(DS);
>> >
>> >if (DS.isFriendSpecified()) {
>> > Index: include/clang/Basic/DiagnosticSemaKinds.td
>> > ===
>> > --- include/clang/Basic/DiagnosticSemaKinds.td
>> > +++ include/clang/Basic/DiagnosticSemaKinds.td
>> > @@ -1973,6 +1973,8 @@
>> >"function concept declaration must be a definition">;
>> >  def err_var_concept_not_initialized : Error<
>> >"variable concept declaration must be initialized">;
>> > +def err_concept_tag : Error<
>> > +  "%select{class|struct|interface|union|enum}0 cannot be marked
>> > concept">;
>> >
>> >  // C++11 char16_t/char32_t
>> >  def warn_cxx98_compat_unicode_type : Warning<
>> >
>> >
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Dan Albert via cfe-commits
>
> Would you be opposed to annotations that tell the programmer they have
> UB in their code, but *do not* effect the code generation?


Not on our end. This would be great.

On Tue, Aug 11, 2015 at 12:56 PM, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Aug 11, 2015 at 3:32 PM, Joerg Sonnenberger via cfe-commits
>  wrote:
> > joerg added a comment.
> >
> > No, it doesn't. It tells the compiler that it is free to make such
> assumptions. Take a step back from the standard. Can you think of any
> reasonable and efficient implementation of memcpy and friends, which fails
> for size 0? Adding the annotations (whether here or in string.h)
> effectively changes the behavior of the program. It is behavior people have
> been expecting for two decades, even when C90 said something else. This is
> completely different from the warning annotations. I'm just waiting for
> some of the bigger projects like PostgreSQL to start getting annoyed enough
> to introduce sane_memcpy for this.
> > I can't speak for Linux distributions using glibc, but I find this kind
> of smoking gun completely unacceptable to force unconditionally on everyone.
>
> Would you be opposed to annotations that tell the programmer they have
> UB in their code, but *do not* effect the code generation?
>
> ~Aaron
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Aaron Ballman via cfe-commits
On Tue, Aug 11, 2015 at 4:10 PM, Dan Albert  wrote:
>> Would you be opposed to annotations that tell the programmer they have
>> UB in their code, but *do not* effect the code generation?
>
>
> Not on our end. This would be great.

I ask because the new nullability attributes do not affect codegen
(that I'm aware of), and so they might be a reasonable tradeoff
between warning users that they've done something with UB, without
triggering aggressive optimizations, if we didn't want to have some
sort of flag for this.

That being said, the user's code does have UB if it passes in a null
pointer for these APIs and relying on the compiler to not optimize
that is asking for trouble. Warning them about the UB is very
important. Not optimizing on the UB is far less so, IMO.

~Aaron

>
> On Tue, Aug 11, 2015 at 12:56 PM, Aaron Ballman via cfe-commits
>  wrote:
>>
>> On Tue, Aug 11, 2015 at 3:32 PM, Joerg Sonnenberger via cfe-commits
>>  wrote:
>> > joerg added a comment.
>> >
>> > No, it doesn't. It tells the compiler that it is free to make such
>> > assumptions. Take a step back from the standard. Can you think of any
>> > reasonable and efficient implementation of memcpy and friends, which fails
>> > for size 0? Adding the annotations (whether here or in string.h) 
>> > effectively
>> > changes the behavior of the program. It is behavior people have been
>> > expecting for two decades, even when C90 said something else. This is
>> > completely different from the warning annotations. I'm just waiting for 
>> > some
>> > of the bigger projects like PostgreSQL to start getting annoyed enough to
>> > introduce sane_memcpy for this.
>> > I can't speak for Linux distributions using glibc, but I find this kind
>> > of smoking gun completely unacceptable to force unconditionally on 
>> > everyone.
>>
>> Would you be opposed to annotations that tell the programmer they have
>> UB in their code, but *do not* effect the code generation?
>>
>> ~Aaron
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D11957: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Jon Chesterfield via cfe-commits
JonChesterfield created this revision.
JonChesterfield added a subscriber: cfe-commits.
JonChesterfield set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.

The clang-format option SpaceBeforeParens "Always" does not insert a space 
before the opening parenthesis of an overloaded operator function. The attached 
patch against trunk resolves this.

Repository:
  rL LLVM

http://reviews.llvm.org/D11957

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8276,6 +8276,8 @@
   verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");", NoSpace);
   verifyFormat("int f() throw(Deprecated);", NoSpace);
   verifyFormat("typedef void (*cb)(int);", NoSpace);
+  verifyFormat("T A::operator()();", NoSpace);
+  verifyFormat("X A::operator++(T);", NoSpace);
 
   FormatStyle Space = getLLVMStyle();
   Space.SpaceBeforeParens = FormatStyle::SBPO_Always;
@@ -8321,6 +8323,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1997,7 +1997,7 @@
   if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
 return true;
   if (Right.is(TT_OverloadedOperatorLParen))
-return false;
+return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
   if (Right.is(tok::colon)) {
 if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
 !Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8276,6 +8276,8 @@
   verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");", NoSpace);
   verifyFormat("int f() throw(Deprecated);", NoSpace);
   verifyFormat("typedef void (*cb)(int);", NoSpace);
+  verifyFormat("T A::operator()();", NoSpace);
+  verifyFormat("X A::operator++(T);", NoSpace);
 
   FormatStyle Space = getLLVMStyle();
   Space.SpaceBeforeParens = FormatStyle::SBPO_Always;
@@ -8321,6 +8323,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1997,7 +1997,7 @@
   if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
 return true;
   if (Right.is(TT_OverloadedOperatorLParen))
-return false;
+return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
   if (Right.is(tok::colon)) {
 if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
 !Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r244658 - Revert the diagnostic improvements in r244602 as they introduced a problematic dependency

2015-08-11 Thread David Blaikie via cfe-commits
Author: dblaikie
Date: Tue Aug 11 15:21:45 2015
New Revision: 244658

URL: http://llvm.org/viewvc/llvm-project?rev=244658&view=rev
Log:
Revert the diagnostic improvements in r244602 as they introduced a problematic 
dependency

Seems we had some internal uses that include ClangTidyTest.h and weren't
ready for a gtest dependency. Reverting to give Manuel some time to look
into it.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=244658&r1=244657&r2=244658&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug 11 
15:21:45 2015
@@ -17,7 +17,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
-#include "gtest/gtest.h"
 #include 
 
 namespace clang {
@@ -77,15 +76,8 @@ runCheckOnCode(StringRef Code, std::vect
   FileContent.second);
   }
   Invocation.setDiagnosticConsumer(&DiagConsumer);
-  bool Result = Invocation.run();
-  if (!Result) {
-std::string ErrorText;
-for (const auto &Error : Context.getErrors()) {
-  ErrorText += Error.Message.Message + "\n";
-}
-ADD_FAILURE() << ErrorText;
+  if (!Invocation.run())
 return "";
-  }
 
   DiagConsumer.finish();
   tooling::Replacements Fixes;


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


Re: [PATCH] D11957: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Daniel Jasper via cfe-commits
djasper added a subscriber: djasper.
djasper accepted this revision.
djasper added a reviewer: djasper.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good. Do you have commit access?


Repository:
  rL LLVM

http://reviews.llvm.org/D11957



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


Re: [PATCH] D11957: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Jon Chesterfield via cfe-commits
JonChesterfield added a comment.

Thanks. I have no commit access.


Repository:
  rL LLVM

http://reviews.llvm.org/D11957



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Marshall Clow via cfe-commits
mclow.lists added a comment.

In http://reviews.llvm.org/D11948#221991, @joerg wrote:

> No, it doesn't. It tells the compiler that it is free to make such 
> assumptions.


Again, I disagree. The compiler already knows it is free to make such 
assumptions.
(LLVM has an entire optimizer pass devoted to `memcpy` and friends)


http://reviews.llvm.org/D11948



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


Re: [PATCH] D3583: Sema: Implement DR244

2015-08-11 Thread Richard Smith via cfe-commits
rsmith added a comment.

This will be in 3.7.


Repository:
  rL LLVM

http://reviews.llvm.org/D3583



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


Re: [PATCH] D11845: Properly pass through the PIC mode to the integrated assembler when doing assembly-only, and unify the Driver's PIC argument parsing.

2015-08-11 Thread Joerg Sonnenberger via cfe-commits
joerg added a subscriber: joerg.
joerg added a comment.

I agree in principle. The "-static" thing is already in the initial code, but 
doesn't make sense to me.



Comment at: lib/Driver/Tools.cpp:2928
@@ +2927,3 @@
+/// Parses the various -fpic/-fPIC/-fpie/-fPIE arguments.  Then,
+/// smooshes them together with platform defaults, to decide with
+/// whether this compile should be using PIC mode or not. Returns the

Drop the last with?


Comment at: lib/Driver/Tools.cpp:2930
@@ +2929,3 @@
+/// whether this compile should be using PIC mode or not. Returns the
+/// answer in a tuple of (RelocationModel, PICLevel, IsPIE).
+static std::tuple

Not a native speaker, but "returns as tuple" sounds more natural?

Why is the PICLevel signed?


Comment at: lib/Driver/Tools.cpp:2998
@@ +2997,3 @@
+  // both PIC and PIE are disabled. Any PIE option implicitly enables PIC
+  // at the same level.
+  Arg *LastPICArg = Args.getLastArg(options::OPT_fPIC, options::OPT_fno_PIC,

No need to rant about GCC behavior that much. Just state that older GCC 
versions provided different (inconsistent behavior).


Comment at: lib/Driver/Tools.cpp:3022
@@ +3021,3 @@
+  // Introduce a Darwin-specific hack. If the default is PIC but the flags
+  // specified while enabling PIC enabled level 1 PIC, just force it back to
+  // level 2 PIC instead. This matches the behavior of Darwin GCC (based on my

Difficult to parse. Please rewrite the second sentence.


Comment at: lib/Driver/Tools.cpp:3032
@@ +3031,3 @@
+PIC = PIE = false;
+  if (Args.hasArg(options::OPT_static))
+PIC = PIE = false;

This check doesn't make sense to me. When using just -S, -static is completely 
ignored. So why should it be relevant here?


Comment at: lib/Driver/Tools.cpp:3087
@@ -2943,1 +3086,3 @@
  const ArgList &Args, const char *LinkingOutput) const 
{
+  std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
+  const llvm::Triple Triple(TripleStr);

This part looks like a clean up that can be applied separately first?


http://reviews.llvm.org/D11845



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


r244660 - clang-format: Make SpaceBeforeParens work with overloaded operators.

2015-08-11 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Tue Aug 11 15:32:24 2015
New Revision: 244660

URL: http://llvm.org/viewvc/llvm-project?rev=244660&view=rev
Log:
clang-format: Make SpaceBeforeParens work with overloaded operators.

Patch by Jon Chesterfield, thank you!

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=244660&r1=244659&r2=244660&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Aug 11 15:32:24 2015
@@ -1997,7 +1997,7 @@ bool TokenAnnotator::spaceRequiredBefore
   if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
 return true;
   if (Right.is(TT_OverloadedOperatorLParen))
-return false;
+return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
   if (Right.is(tok::colon)) {
 if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
 !Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=244660&r1=244659&r2=244660&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Aug 11 15:32:24 2015
@@ -8279,6 +8279,8 @@ TEST_F(FormatTest, ConfigurableSpaceBefo
   verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");", NoSpace);
   verifyFormat("int f() throw(Deprecated);", NoSpace);
   verifyFormat("typedef void (*cb)(int);", NoSpace);
+  verifyFormat("T A::operator()();", NoSpace);
+  verifyFormat("X A::operator++(T);", NoSpace);
 
   FormatStyle Space = getLLVMStyle();
   Space.SpaceBeforeParens = FormatStyle::SBPO_Always;
@@ -8324,6 +8326,8 @@ TEST_F(FormatTest, ConfigurableSpaceBefo
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {


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


Re: [PATCH][Solaris] Clang/Driver, stop hardcoding GCC paths in crt/ld.so lookup

2015-08-11 Thread Xan López via cfe-commits
Hi,

thanks for the review, I was not even aware that this could be
tested. Adding a test helped to fix me a couple extra issues (plus the
one you already mentioned). New patch attached.

Xan

On Wed, Aug 05, 2015 at 09:14:30AM -0400, Rafael Espíndola wrote:
> Please git-clang-format this patch.
> 
> +  // /usr/gcc/./lib/gcc/../,
> 
> The code appends a triple after the "/lib/gcc". Is the comment missing it?
> 
> The inner loop has no version comparison. Are you depending on the
> directory iteration order?
> 
> Can you add a testcase?
> 
> 
> On 28 July 2015 at 12:35, Xan López  wrote:
> > Here it is.
> >
> > On Tue, Jul 28, 2015 at 01:21:06PM +0200, Xan López wrote:
> >> On Tue, Jul 28, 2015 at 01:55:23PM +0300, Yaron Keren wrote:
> >> > +cfe-commits
> >> >
> >> > This is a very large Solaris special case in ScanLibDirForGCCTriple which
> >> > shares almost no code with the function.
> >> > How about splitting it out to a helper function or
> >> > making ScanLibDirForGCCTriple virtual and overriding on Solaris?
> >>
> >> Yep, at least a helper function makes sense, you are right. I'll send
> >> another patch with either of those suggestions later today.
> >>
> >>
> >> Xan
> >> ___
> >> llvm-commits mailing list
> >> llvm-comm...@cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> > ___
> > llvm-commits mailing list
> > llvm-comm...@cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>From 27215294c9a4aeaf25b2d618839bf413c137f8e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Xan=20L=C3=B3pez?= 
Date: Fri, 24 Jul 2015 19:17:51 +0200
Subject: [PATCH] [Solaris] Stop hardcoding the location of the c runtime files

This refactors the GCC installation detection to work properly on
Solaris (which is a bit of a special case compared to Linux
distributions), and adds the proper /usr/lib/... logic in the
Driver. That way we can just use ToolChain::GetFilePath to locate all
crt*.o files instead of hardcoding their location.
---
 lib/Driver/ToolChains.cpp  | 107 +++--
 lib/Driver/ToolChains.h|   6 ++
 lib/Driver/Tools.cpp   |  54 ---
 .../4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o |   0
 .../lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o |   0
 .../lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o   |   0
 .../Inputs/sparc-sun-solaris2.11/usr/lib/crti.o|   0
 .../Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o|   0
 .../Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1   |   0
 .../sparc-sun-solaris2.11/usr/lib/values-Xa.o  |   0
 test/Driver/solaris-ld.c   |  16 +++
 11 files changed, 140 insertions(+), 43 deletions(-)
 create mode 100644 
test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o
 create mode 100644 
test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o
 create mode 100644 
test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o
 create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crti.o
 create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o
 create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1
 create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/values-Xa.o
 create mode 100644 test/Driver/solaris-ld.c

diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
index 59e6a2e..8e49c5a 100644
--- a/lib/Driver/ToolChains.cpp
+++ b/lib/Driver/ToolChains.cpp
@@ -1340,9 +1340,21 @@ bool 
Generic_GCC::GCCInstallationDetector::getBiarchSibling(Multilib &M) const {
   "s390x-linux-gnu", "s390x-unknown-linux-gnu", "s390x-ibm-linux-gnu",
   "s390x-suse-linux", "s390x-redhat-linux"};
 
+  // Solaris.
+  static const char *const SolarisSPARCLibDirs[] = {"/gcc"};
+  static const char *const SolarisSPARCTriples[] = {"sparc-sun-solaris2.11",
+"i386-pc-solaris2.11"};
+
   using std::begin;
   using std::end;
 
+  if (TargetTriple.getOS() == llvm::Triple::Solaris) {
+LibDirs.append(begin(SolarisSPARCLibDirs), end(SolarisSPARCLibDirs));
+TripleAliases.append(begin(SolarisSPARCTriples), end(SolarisSPARCTriples));
+
+return;
+  }
+
   switch (TargetTriple.getArch()) {
   case llvm::Triple::aarch64:
 LibDirs.append(begin(AArch64LibDirs), end(AArch64LibDirs));
@@ -1907,6 +1919,54 @@ static bool findBiarchMultilibs(const llvm::Triple 
&TargetTriple,
   return true;
 }
 
+void Generic_GCC::GCCInstallationDetector::SolarisScanLibDirForGCCTriple(
+const llvm::Triple &TargetArch, const llvm::opt::ArgList &Args,
+const std::string &LibDir, StringRef CandidateTriple,
+bool NeedsBiarchSuffix) {
+  // Solaris is a special case. The GCC installation is under
+  // /usr/gcc/./lib/gcc

Re: [PATCH] D11957: SpaceBeforeParens (Always) with overloaded operators

2015-08-11 Thread Daniel Jasper via cfe-commits
djasper closed this revision.
djasper added a comment.

Submitted as r244660. Thank you.


Repository:
  rL LLVM

http://reviews.llvm.org/D11957



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Dan Albert via cfe-commits
Yeah, those sound like exactly what we want. Helping people find UB is
good, but optimizing assuming we've fixed all of the UB isn't something we
can do. Our bugs end up being rather permanent so we need to be defensive.

On Tue, Aug 11, 2015 at 1:18 PM, Aaron Ballman 
wrote:

> On Tue, Aug 11, 2015 at 4:10 PM, Dan Albert  wrote:
> >> Would you be opposed to annotations that tell the programmer they have
> >> UB in their code, but *do not* effect the code generation?
> >
> >
> > Not on our end. This would be great.
>
> I ask because the new nullability attributes do not affect codegen
> (that I'm aware of), and so they might be a reasonable tradeoff
> between warning users that they've done something with UB, without
> triggering aggressive optimizations, if we didn't want to have some
> sort of flag for this.
>
> That being said, the user's code does have UB if it passes in a null
> pointer for these APIs and relying on the compiler to not optimize
> that is asking for trouble. Warning them about the UB is very
> important. Not optimizing on the UB is far less so, IMO.
>
> ~Aaron
>
> >
> > On Tue, Aug 11, 2015 at 12:56 PM, Aaron Ballman via cfe-commits
> >  wrote:
> >>
> >> On Tue, Aug 11, 2015 at 3:32 PM, Joerg Sonnenberger via cfe-commits
> >>  wrote:
> >> > joerg added a comment.
> >> >
> >> > No, it doesn't. It tells the compiler that it is free to make such
> >> > assumptions. Take a step back from the standard. Can you think of any
> >> > reasonable and efficient implementation of memcpy and friends, which
> fails
> >> > for size 0? Adding the annotations (whether here or in string.h)
> effectively
> >> > changes the behavior of the program. It is behavior people have been
> >> > expecting for two decades, even when C90 said something else. This is
> >> > completely different from the warning annotations. I'm just waiting
> for some
> >> > of the bigger projects like PostgreSQL to start getting annoyed
> enough to
> >> > introduce sane_memcpy for this.
> >> > I can't speak for Linux distributions using glibc, but I find this
> kind
> >> > of smoking gun completely unacceptable to force unconditionally on
> everyone.
> >>
> >> Would you be opposed to annotations that tell the programmer they have
> >> UB in their code, but *do not* effect the code generation?
> >>
> >> ~Aaron
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r244266 - [ItaniumCXXABI] Don't import RTTI data for classes with key functions

2015-08-11 Thread Hans Wennborg via cfe-commits
On Thu, Aug 6, 2015 at 1:56 PM, David Majnemer via cfe-commits
 wrote:
> Author: majnemer
> Date: Thu Aug  6 15:56:55 2015
> New Revision: 244266
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244266&view=rev
> Log:
> [ItaniumCXXABI] Don't import RTTI data for classes with key functions
>
> MinGW has some pretty strange behvaior around RTTI and
> dllimport/dllexport:
> - RTTI data is never imported
> - RTTI data is only exported if the class has no key function.
>
> Modified:
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp

I tried to merge this to 3.7 (as discussed on the commit thread for
r244488), but the test doesn't pass:

--
/usr/local/google/work/llvm-3.7/cfe.src/test/CodeGenCXX/dllimport-rtti.cpp:22:13:
error: expected string not found in input
// GNU-DAG: @_ZTV1V = available_externally dllimport
^
:1:1: note: scanning from here
; ModuleID = 
'/usr/local/google/work/llvm-3.7/cfe.src/test/CodeGenCXX/dllimport-rtti.cpp'
^
:15:1: note: possible intended match here
@_ZTV1S = available_externally dllimport unnamed_addr constant [3 x
i8*] [i8* null, i8* bitcast (i8** @_ZTI1S to i8*), i8* bitcast (void
(%struct.S*)* @_ZN1S1fEv to i8*)], align 4
^
--

I assume there's some other commit this depends on, but I haven't been
able to figure out which. Can you take a look?

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


r244662 - Add an AST matcher to match member intializers of a CXXCtorInitializer.

2015-08-11 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Tue Aug 11 15:42:00 2015
New Revision: 244662

URL: http://llvm.org/viewvc/llvm-project?rev=244662&view=rev
Log:
Add an AST matcher to match member intializers of a CXXCtorInitializer.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=244662&r1=244661&r2=244662&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Tue Aug 11 15:42:00 2015
@@ -1512,6 +1512,23 @@ constructorDecl(hasAnyConstructorInitial
 
 
 
+MatcherCXXCtorInitializer>isMemberInitializer
+Matches a 
constructor initializer if it is initializing a direct member, as opposed to a 
base.
+
+Given
+  struct B {};
+  struct D : B {
+int I;
+D(int i) : I(i) {}
+  };
+  struct E : B {
+E() : B() {}
+  };
+constructorDecl(hasAnyConstructorInitializer(isMemberInitializer()))
+  will match D(int), but not match E().
+
+
+
 MatcherCXXCtorInitializer>isWritten
 Matches a constructor 
initializer if it is explicitly written in
 code (as opposed to implicitly added by the compiler).

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=244662&r1=244661&r2=244662&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Tue Aug 11 15:42:00 2015
@@ -2652,6 +2652,26 @@ AST_MATCHER(CXXCtorInitializer, isBaseIn
   return Node.isBaseInitializer();
 }
 
+/// \brief Matches a constructor initializer if it is initializing a member, as
+/// opposed to a base.
+///
+/// Given
+/// \code
+///   struct B {};
+///   struct D : B {
+/// int I;
+/// D(int i) : I(i) {}
+///   };
+///   struct E : B {
+/// E() : B() {}
+///   };
+/// \endcode
+/// constructorDecl(hasAnyConstructorInitializer(isMemberInitializer()))
+///   will match D(int), but not match E().
+AST_MATCHER(CXXCtorInitializer, isMemberInitializer) {
+  return Node.isMemberInitializer();
+}
+
 /// \brief Matches any argument of a call expression or a constructor call
 /// expression.
 ///

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244662&r1=244661&r2=244662&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Tue Aug 11 15:42:00 2015
@@ -262,6 +262,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isIntegral);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isListInitialization);
+  REGISTER_MATCHER(isMemberInitializer);
   REGISTER_MATCHER(isMoveConstructor);
   REGISTER_MATCHER(isOverride);
   REGISTER_MATCHER(isPrivate);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=244662&r1=244661&r2=244662&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Tue Aug 11 15:42:00 2015
@@ -2097,6 +2097,12 @@ TEST(HasAnyConstructorInitializer, IsBas
   EXPECT_TRUE(notMatches(Code, constructorDecl(allOf(
 hasAnyConstructorInitializer(allOf(isBaseInitializer(), isWritten())),
 hasName("D");
+  EXPECT_TRUE(matches(Code, constructorDecl(allOf(
+hasAnyConstructorInitializer(allOf(isMemberInitializer(), isWritten())),
+hasName("D");
+  EXPECT_TRUE(notMatches(Code, constructorDecl(allOf(
+hasAnyConstructorInitializer(allOf(isMemberInitializer(), isWritten())),
+hasName("E");
 }
 
 TEST(Matcher, NewExpression) {


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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Joerg Sonnenberger via cfe-commits
joerg added a comment.

LLVM makes the assumptions only if the prototype has them too? Attribute 
nonnull is certainly changing optimiser behavior with GCC...


http://reviews.llvm.org/D11948



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


[PATCH] D11958: Add a -gmodules option to the clang driver.

2015-08-11 Thread Adrian Prantl via cfe-commits
aprantl created this revision.
aprantl added reviewers: dblaikie, echristo.
aprantl added a subscriber: cfe-commits.
aprantl set the repository for this revision to rL LLVM.

This patch adds a -gmodules option to the driver and a -dwarf-ext-refs to cc1 
to enable the use of external type references in the debug info (a.k.a. module 
debugging).

The driver expands -gmodules to "-g -fmodule-format=obj -dwarf-ext-refs" and 
passes that to cc1.
Most options that start with -g (e.g., -gdwarf-2) also turn on -g, and module 
requires object-container-wrapped modules, "-dwarf-ext-refs" been the actual 
low-level option for turning on external type references.

Rationale for the choice of names (and this is really all there is to review in 
this patch):
"-gmodules": is meant to pair nicely with "-fmodules"
"-dwarf-ext-refs": Fits into the naming scheme of similar options like 
"-dwarf-column-info" and "-dwarf-debug-flags". Spelling out the option 
"-dwarf-external-type-references" seemed to be overkill.

All this does at the moment is set a flag codegenopts. Having this flag in 
place is a prerequisite for emitting debug info into modules: The debug info 
for a module needs to use external type references for types defined in (other) 
modules or we would violate the minimal deserialization requirements (cf. 
test/PCH/check-deserializations.cpp).

Repository:
  rL LLVM

http://reviews.llvm.org/D11958

Files:
  docs/CommandGuide/clang.rst
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/debug-options.c

Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -77,6 +77,9 @@
 //
 // RUN: %clang -### -g %s 2>&1 | FileCheck -check-prefix=CI %s
 //
+// RUN: %clang -### -gmodules %s 2>&1 \
+// RUN:| FileCheck -check-prefix=GEXTREFS %s
+//
 // G: "-cc1"
 // G: "-g"
 //
@@ -126,3 +129,5 @@
 // CI: "-dwarf-column-info"
 //
 // NOCI-NOT: "-dwarf-column-info"
+//
+// GEXTREFS: "-g" "-dwarf-ext-refs" "-fmodule-format=obj"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -424,6 +424,7 @@
 Opts.DwarfVersion = 3;
   else if (Args.hasArg(OPT_gdwarf_4))
 Opts.DwarfVersion = 4;
+  Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
 
   if (const Arg *A =
   Args.getLastArg(OPT_emit_llvm_uselists, OPT_no_emit_llvm_uselists))
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3702,6 +3702,12 @@
 CmdArgs.push_back("-dwarf-column-info");
 
   // FIXME: Move backend command line options to the module.
+  if (Args.hasArg(options::OPT_gmodules)) {
+CmdArgs.push_back("-g");
+CmdArgs.push_back("-dwarf-ext-refs");
+CmdArgs.push_back("-fmodule-format=obj");
+  }
+
   // -gsplit-dwarf should turn on -g and enable the backend dwarf
   // splitting and extraction.
   // FIXME: Currently only works on Linux.
Index: lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -64,6 +64,7 @@
 // ThreadModel, but the backend expects them to be nonempty.
 CodeGenOpts.CodeModel = "default";
 CodeGenOpts.ThreadModel = "single";
+CodeGenOpts.DebugTypeExtRefs = true;
 CodeGenOpts.setDebugInfo(CodeGenOptions::FullDebugInfo);
 CodeGenOpts.SplitDwarfFile = OutputFileName;
   }
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -52,6 +52,7 @@
   friend class SaveAndRestoreLocation;
   CodeGenModule &CGM;
   const CodeGenOptions::DebugInfoKind DebugKind;
+  bool DebugTypeExtRefs;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   SourceLocation CurLoc;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -45,6 +45,7 @@
 
 CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
 : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
+  DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
   DBuilder(CGM.getModule()) {
   CreateCompileUnit();
 }
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -161,6 +161,9 @@
 CODEGENOP

Re: [PATCH] D10371: clang-format: Support @synchronized.

2015-08-11 Thread strager via cfe-commits
strager added a comment.

ping


http://reviews.llvm.org/D10371



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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Marshall Clow via cfe-commits
On Tue, Aug 11, 2015 at 1:34 PM, Dan Albert  wrote:

> Yeah, those sound like exactly what we want. Helping people find UB is
> good, but optimizing assuming we've fixed all of the UB isn't something we
> can do.
>

Dan -- that's the situation you're in today.
GCC has done that kind of optimization for *years*.

Consider the following code (simplified to the point that it's a toy,
but...)

void * doFoo ( void *p, size_t sz ) {
std::memcpy(buf, p, sz);
if (p == nullptr)
p = malloc (10);
return p;
}

int main() {
void * q = doFoo(nullptr, 0);
std::cout << q << std::endl;
}

You see the UB there - right?  memcpy(buf, null, 0);

Built that with gcc (I used 4.9) - and see what it prints.
At -O3 (on my machine - Mac OS X) it prints "0".
It has removed the check for nullptr, and the malloc.

Interestingly enough, clang completely elides the call to doFoo, and just
calls malloc(10).

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


Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Hal Finkel via cfe-commits
- Original Message -
> From: "Marshall Clow via cfe-commits" 
> To: "mclow lists" , chandl...@gmail.com, 
> rich...@metafoo.co.uk, e...@efcs.ca
> Cc: jo...@netbsd.org, cfe-commits@lists.llvm.org
> Sent: Tuesday, August 11, 2015 3:30:10 PM
> Subject: Re: [PATCH] D11948: Add some macros to abstract marking of 
> parameters as "not null", and use them in
> 
> 
> mclow.lists added a comment.
> 
> In http://reviews.llvm.org/D11948#221991, @joerg wrote:
> 
> > No, it doesn't. It tells the compiler that it is free to make such
> > assumptions.
> 
> 
> Again, I disagree. The compiler already knows it is free to make such
> assumptions.
> (LLVM has an entire optimizer pass devoted to `memcpy` and friends)

I agree with Marshall. This ship has already sailed - the standard says what is 
says, and GCC and other compilers already take advantage of it, and thus 
portable code already cannot depend on the behavior of passing null pointers. 
Even non-portable code will be locked to specific compiler/library versions 
(which is already true with GCC) because, annotations aside, compilers are free 
to assume the standard semantics regardless. The annotations at least give 
users some more information on what the compiler is assuming, and so having 
them is better than not.

 -Hal

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

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D11928: Small fixup

2015-08-11 Thread Richard Smith via cfe-commits
rsmith added a comment.

Reid, do you remember what this FIXME was for? (Added in r198462.)


http://reviews.llvm.org/D11928



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


r244666 - Add a polymorphic AST matcher for testing whether a constructor or a conversion declaration is marked as explicit or not.

2015-08-11 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Tue Aug 11 16:09:52 2015
New Revision: 244666

URL: http://llvm.org/viewvc/llvm-project?rev=244666&view=rev
Log:
Add a polymorphic AST matcher for testing whether a constructor or a conversion 
declaration is marked as explicit or not.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=244666&r1=244665&r2=244666&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Tue Aug 11 16:09:52 2015
@@ -1483,6 +1483,19 @@ Example matches #1, but not #2 or #3 (ma
 
 
 
+MatcherCXXConstructorDecl>isExplicit
+Matches a constructor 
declaration if it is marked explicit.
+
+Given
+  struct S {
+S(int); // #1
+explicit S(double); // #2
+  };
+constructorDecl(isExplicit())
+  will match #2, but not #1.
+
+
+
 MatcherCXXConstructorDecl>isMoveConstructor
 Matches 
constructor declarations that are move constructors.
 
@@ -1495,6 +1508,19 @@ Example matches #3, but not #1 or #2 (ma
 
 
 
+MatcherCXXConversionDecl>isExplicit
+Matches a conversion 
declaration if it is marked explicit.
+
+Given
+  struct S {
+operator int(); // #1
+explicit operator bool(); // #2
+  };
+conversionDecl(isExplicit())
+  will match #2, but not #1.
+
+
+
 MatcherCXXCtorInitializer>isBaseInitializer
 Matches a 
constructor initializer if it is initializing a base, as opposed to a member.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=244666&r1=244665&r2=244666&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Tue Aug 11 16:09:52 2015
@@ -4189,6 +4189,27 @@ AST_MATCHER(CXXConstructorDecl, isDefaul
   return Node.isDefaultConstructor();
 }
 
+/// \brief Matches constructor and conversion declarations that are marked with
+/// the explicit keyword.
+///
+/// Given
+/// \code
+///   struct S {
+/// S(int); // #1
+/// explicit S(double); // #2
+/// operator int(); // #3
+/// explicit operator bool(); // #4
+///   };
+/// \endcode
+/// constructorDecl(isExplicit()) will match #2, but not #1.
+/// conversionDecl(isExplicit()) will match #4, but not #3.
+AST_POLYMORPHIC_MATCHER(isExplicit,
+AST_POLYMORPHIC_SUPPORTED_TYPES(CXXConstructorDecl,
+CXXConversionDecl)) {
+  return Node.isExplicit();
+
+}
+
 /// \brief If the given case statement does not use the GNU case range
 /// extension, matches the constant given in the statement.
 ///

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244666&r1=244665&r2=244666&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Tue Aug 11 16:09:52 2015
@@ -249,6 +249,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isDefinition);
   REGISTER_MATCHER(isDeleted);
   REGISTER_MATCHER(isExceptionVariable);
+  REGISTER_MATCHER(isExplicit);
   REGISTER_MATCHER(isExplicitTemplateSpecialization);
   REGISTER_MATCHER(isExpr);
   REGISTER_MATCHER(isExternC);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=244666&r1=244665&r2=244666&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Tue Aug 11 16:09:52 2015
@@ -1420,6 +1420,13 @@ TEST(Callee, MatchesDeclarations) {
  CallMethodX));
 }
 
+TEST(ConversionDeclaration, IsExplicit) {
+  EXPECT_TRUE(matches("struct S { explicit operator int(); };",
+  conversionDecl(isExplicit(;
+  EXPECT_TRUE(notMatches("struct S { operator int(); };",
+ conversionDecl(isExplicit(;
+}
+
 TEST(Callee, MatchesMemberExpressions) {
   EXPECT_TRUE(matches("class Y { void x() { this->x(); }

  1   2   >