Re: Submodule semantics with macro guard

[Richard Smith via cfe-commits]

On 19 Sep 2016 6:53 pm, "Manman via cfe-commits" 
wrote:



On Sep 19, 2016, at 5:55 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

Your c.h is not correct. It would introduce a definition of c in every file
where it's included, so it's not a modular header.

Hi Richard,

What do you mean by c.h is not correct? It is guarded by a macro, so if we
are not using modules, it will work.


It will not work (due to multiple definition errors) if it's included into
two source files anywhere in the entire program.

If it's only intended to be included into a single source file in the
entire program, there's no point in putting it in a module.

Also I thought the definition of a non-modular header is that it is not
mapped to any module. Is that my misunderstanding? Is there a formal
definition for modular header?


I'm not sure if we have this written down anywhere, but it's something
like: a header that is self contained, does not intended to depend on the
compilation state at its point of inclusion, and has no effect other than
making some set of declarations and macros visible.

Thanks,
Manman


On 19 Sep 2016 5:21 pm, "Manman via cfe-commits" 
wrote:

>
> Hi Richard & Ben,
>
> Given a simple testing case, where we have two submodules X.A (A.h) and
> X.B (B.h, it includes C.h, and C.h is guarded with a macro), when we import
> X.A and then textually include a header C.h, we get redefinition error.
> This is because the macro guard is owned by module X.B that is not yet
> visible. I wonder what the fix is if this is considered a compiler issue.
> We can:
> 1> do not emit the redefinition error if the earlier definition is not
> visible, or
> 2> emit diagnostics to suggest user to import the whole module, when
> redefinition happens because the module includes a non-modular header and
> the non-modular header is also textually included, or
> 3> other suggestions?
>
> Note that if we textually include C.h, then import X.A, there are no
> compiler errors.
>
> The testing case here:
> clang -cc1 repro.c -fsyntax-only -I Inputs4/ -fmodules
> -fimplicit-module-maps -fmodules-cache-path=tmp
> cat repro.c
> #include "A.h" //modular header
> #include "C.h" //non-modular header
>
> cat Inputs4/module.map
> module X {
>   module A {
> header "A.h"
> export *
>   }
>   module B {
> header "B.h"
> export *
>   }
>   export *
> }
> cat Inputs4/A.h
> #ifndef __A_h__
> #define __A_h__
> #endif
> cat Inputs4/B.h
> #ifndef __B_h__
> #define __B_h__
> #include "C.h"
> #endif
> cat Inputs4/C.h
> #ifndef __C_h__
> #define __C_h__
> int c = 1;
> const int d = 2;
> #endif
>
> Thanks,
> Manman
> ___
> 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r281954 - clang-change-namespace: Update libdeps.

[Eric Liu via cfe-commits]

Thanks for the fix!

On Tue, Sep 20, 2016, 02:53 NAKAMURA Takumi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: chapuni
> Date: Mon Sep 19 19:44:45 2016
> New Revision: 281954
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281954&view=rev
> Log:
> clang-change-namespace: Update libdeps.
>
> FIXME: It seems clangFormat is not referred.
>
> Modified:
> clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
>
> Modified: clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt?rev=281954&r1=281953&r2=281954&view=diff
>
> ==
> --- clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt Mon Sep
> 19 19:44:45 2016
> @@ -1,9 +1,14 @@
>  include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
>
> +set(LLVM_LINK_COMPONENTS
> +  Support
> +  )
> +
>  add_clang_executable(clang-change-namespace
>ClangChangeNamespace.cpp
>)
>  target_link_libraries(clang-change-namespace
> +  clangAST
>clangASTMatchers
>clangBasic
>clangChangeNamespace
>
>
> ___
> 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] D24752: [Modules] Add missing dependencies to clang builtins modulemap

[Elad Cohen via cfe-commits]

eladcohen created this revision.
eladcohen added a reviewer: rsmith.
eladcohen added a subscriber: cfe-commits.

X86 intrinsic header files mm3dnow.h and wmmintrin.h include and depend on 
mmintrin.h and emmintrin.h respectively.
This patch adds these missing dependencies to the corresponding submodules in 
the clang builtins modulemap.

https://reviews.llvm.org/D24752

Files:
  lib/Headers/module.modulemap
  test/Modules/compiler_builtins_x86_submodules.c

Index: test/Modules/compiler_builtins_x86_submodules.c
===
--- test/Modules/compiler_builtins_x86_submodules.c
+++ test/Modules/compiler_builtins_x86_submodules.c
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -triple i686-unknown-unknown -target-feature +3dnowa 
-fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s 
-verify -ffreestanding
+
+// expected-no-diagnostics
+
+#include
+
+__m64 x; // Verify that types which are used by mm3dnow.h
+ // but declared in the mmx submodule get imported
+
+#include
+
+__m128i y; // Verify that types which are used by wmmintrin.h
+   // but declared in the sse2 submodule get imported
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -119,6 +119,7 @@
 }
 
 explicit module mm3dnow {
+  export mmx
   header "mm3dnow.h"
 }
 
@@ -129,6 +130,7 @@
 }
 
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"
 }
 


Index: test/Modules/compiler_builtins_x86_submodules.c
===
--- test/Modules/compiler_builtins_x86_submodules.c
+++ test/Modules/compiler_builtins_x86_submodules.c
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -triple i686-unknown-unknown -target-feature +3dnowa -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -verify -ffreestanding
+
+// expected-no-diagnostics
+
+#include
+
+__m64 x; // Verify that types which are used by mm3dnow.h
+ // but declared in the mmx submodule get imported
+
+#include
+
+__m128i y; // Verify that types which are used by wmmintrin.h
+   // but declared in the sse2 submodule get imported
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -119,6 +119,7 @@
 }
 
 explicit module mm3dnow {
+  export mmx
   header "mm3dnow.h"
 }
 
@@ -129,6 +130,7 @@
 }
 
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics

[Saleem Abdulrasool via cfe-commits]

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

That sounds reasonable.


https://reviews.llvm.org/D24609



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

Re: r281277 - [Sema] Fix PR30346: relax __builtin_object_size checks.

[George Burgess IV via cfe-commits]

WFM; I'll put together a patch that only allows this under
-fno-strict-aliasing.

I'm entirely unfamiliar with struct-path-tbaa, so Hal, do you see a reason
why struct-path-tbaa wouldn't play nicely with flexible arrays at the end
of types? Glancing at it, I don't think it should cause problems, but a
more authoritative answer would really be appreciated. :) If it might cause
issues now or in the future, I'm happy to be conservative here if
-fno-strict-path-tbaa is given, too.

On Tue, Sep 13, 2016 at 2:00 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Sep 13, 2016 at 12:51:52PM -0700, Richard Smith wrote:
> > On Tue, Sep 13, 2016 at 10:44 AM, Joerg Sonnenberger via cfe-commits <
> > cfe-commits@lists.llvm.org> wrote:
> >
> > > IMO this should be restricted to code that explicitly disables C/C++
> > > aliasing rules.
> >
> >
> > Do you mean -fno-strict-aliasing or -fno-struct-path-tbaa or something
> else
> > here? (I think we're not doing anyone any favours by making
> _FORTIFY_SOURCE
> > say that a pattern is OK in cases when LLVM will in fact optimize on the
> > assumption that it's UB, but I don't recall how aggressive
> > -fstruct-path-tbaa is for trailing array members.)
>
> The former immediately, the latter potentially as well. I can't think of
> many use cases for this kind of idiom that don't involve type prunning
> and socket code is notoriously bad in that regard by necessity.
>
> Joerg
> ___
> 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] D24751: [cleanup] Remove excessive padding from TextTokenRetokenizer::Position

[David Majnemer via cfe-commits]

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

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D24751



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

Re: [PATCH] D24751: [cleanup] Remove excessive padding from TextTokenRetokenizer::Position

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

F2434937: Screen Shot 2016-09-19 at 7.33.38 PM.png 

the order BufferStart, BufferEnd,BufferPtr,BufferStartLoc,CurToken works as well


Repository:
  rL LLVM

https://reviews.llvm.org/D24751



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

[PATCH] D24751: [cleanup] Remove excessive padding from TextTokenRetokenizer::Position

[Alexander Shaposhnikov via cfe-commits]

alexshap created this revision.
alexshap added reviewers: gribozavr, craig.topper.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

Reorder the fields of the struct TextTokenRetokenizer::Position to remove 
excessive padding.
Test plan: make -j8 check-clang

Repository:
  rL LLVM

https://reviews.llvm.org/D24751

Files:
  lib/AST/CommentParser.cpp

Index: lib/AST/CommentParser.cpp
===
--- lib/AST/CommentParser.cpp
+++ lib/AST/CommentParser.cpp
@@ -40,11 +40,11 @@
 
   /// A position in \c Toks.
   struct Position {
-unsigned CurToken;
 const char *BufferStart;
 const char *BufferEnd;
 const char *BufferPtr;
 SourceLocation BufferStartLoc;
+unsigned CurToken;
   };
 
   /// Current position in Toks.


Index: lib/AST/CommentParser.cpp
===
--- lib/AST/CommentParser.cpp
+++ lib/AST/CommentParser.cpp
@@ -40,11 +40,11 @@
 
   /// A position in \c Toks.
   struct Position {
-unsigned CurToken;
 const char *BufferStart;
 const char *BufferEnd;
 const char *BufferPtr;
 SourceLocation BufferStartLoc;
+unsigned CurToken;
   };
 
   /// Current position in Toks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Submodule semantics with macro guard

[Manman via cfe-commits]

> On Sep 19, 2016, at 5:55 PM, Richard Smith via cfe-commits 
>  wrote:
> 
> Your c.h is not correct. It would introduce a definition of c in every file 
> where it's included, so it's not a modular header.
Hi Richard,

What do you mean by c.h is not correct? It is guarded by a macro, so if we are 
not using modules, it will work.

Also I thought the definition of a non-modular header is that it is not mapped 
to any module. Is that my misunderstanding? Is there a formal definition for 
modular header?

Thanks,
Manman
> 
>> On 19 Sep 2016 5:21 pm, "Manman via cfe-commits" 
>>  wrote:
>> 
>> Hi Richard & Ben,
>> 
>> Given a simple testing case, where we have two submodules X.A (A.h) and X.B 
>> (B.h, it includes C.h, and C.h is guarded with a macro), when we import X.A 
>> and then textually include a header C.h, we get redefinition error. This is 
>> because the macro guard is owned by module X.B that is not yet visible. I 
>> wonder what the fix is if this is considered a compiler issue. We can:
>> 1> do not emit the redefinition error if the earlier definition is not 
>> visible, or
>> 2> emit diagnostics to suggest user to import the whole module, when 
>> redefinition happens because the module includes a non-modular header and 
>> the non-modular header is also textually included, or
>> 3> other suggestions?
>> 
>> Note that if we textually include C.h, then import X.A, there are no 
>> compiler errors.
>> 
>> The testing case here:
>> clang -cc1 repro.c -fsyntax-only -I Inputs4/ -fmodules 
>> -fimplicit-module-maps -fmodules-cache-path=tmp
>> cat repro.c
>> #include "A.h" //modular header
>> #include "C.h" //non-modular header
>> 
>> cat Inputs4/module.map
>> module X {
>>   module A {
>> header "A.h"
>> export *
>>   }
>>   module B {
>> header "B.h"
>> export *
>>   }
>>   export *
>> }
>> cat Inputs4/A.h
>> #ifndef __A_h__
>> #define __A_h__
>> #endif
>> cat Inputs4/B.h
>> #ifndef __B_h__
>> #define __B_h__
>> #include "C.h"
>> #endif
>> cat Inputs4/C.h
>> #ifndef __C_h__
>> #define __C_h__
>> int c = 1;
>> const int d = 2;
>> #endif
>> 
>> Thanks,
>> Manman
>> ___
>> 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

Buildbot numbers for the week of 9/11/2016 - 9/17/2016

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 9/11/2016 - 9/17/2016.

Please see the same data in attached csv files:

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

Thanks

Galina


The longest time each builder was red during the last week:

 buildername|  was_red
+---
 sanitizer-x86_64-linux-bootstrap   | 90:23:46
 sanitizer-x86_64-linux-fast| 51:27:30
 sanitizer-x86_64-linux | 34:46:47
 clang-cmake-mipsel | 30:58:03
 clang-with-lto-ubuntu  | 30:33:33
 clang-x86-win2008-selfhost | 27:13:22
 clang-x64-ninja-win7   | 23:42:33
 sanitizer-ppc64le-linux| 20:19:40
 sanitizer-windows  | 18:24:47
 clang-cmake-thumbv7-a15-full-sh| 17:55:50
 perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only  | 17:52:21
 perf-x86_64-penryn-O3-polly-before-vectorizer  | 17:39:45
 perf-x86_64-penryn-O3  | 15:38:52
 clang-3stage-ubuntu| 14:55:01
 sanitizer-x86_64-linux-fuzzer  | 13:39:51
 perf-x86_64-penryn-O3-polly| 13:27:32
 lldb-windows7-android  | 12:14:17
 clang-native-aarch64-full  | 11:27:17
 lldb-x86_64-ubuntu-14.04-buildserver   | 11:11:23
 polly-amd64-linux  | 11:10:13
 clang-ppc64be-linux-multistage | 11:07:03
 perf-x86_64-penryn-O3-polly-parallel-fast  | 10:53:44
 perf-x86_64-penryn-O3-polly-unprofitable   | 10:53:03
 sanitizer-ppc64be-linux| 10:48:16
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 10:40:27
 lldb-x86_64-ubuntu-14.04-cmake | 10:35:07
 clang-ppc64le-linux-lnt| 10:13:21
 clang-ppc64le-linux| 10:11:45
 clang-ppc64be-linux-lnt| 09:53:59
 clang-ppc64be-linux| 09:53:34
 clang-ppc64le-linux-multistage | 09:52:18
 clang-cmake-armv7-a15-selfhost-neon| 09:45:15
 clang-x86_64-linux-selfhost-modules| 09:19:24
 clang-cmake-armv7-a15-selfhost | 07:05:34
 perf-x86_64-penryn-O3-polly-fast   | 05:47:48
 clang-x86-windows-msvc2015 | 04:34:53
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions  | 04:24:06
 lldb-amd64-ninja-netbsd7   | 04:21:29
 clang-cmake-armv7-a15-full | 04:01:50
 clang-cmake-armv7-a15  | 03:47:55
 clang-cmake-thumbv7-a15| 03:47:24
 clang-cmake-mips   | 03:46:28
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 03:29:31
 clang-bpf-build| 03:19:29
 clang-cmake-aarch64-full   | 03:12:06
 llvm-sphinx-docs   | 03:10:17
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 02:51:02
 llvm-mips-linux| 02:43:07
 clang-native-arm-lnt   | 02:42:00
 clang-x86_64-debian-fast   | 02:26:27
 clang-cmake-aarch64-quick  | 01:52:25
 clang-sphinx-docs  | 01:41:27
 clang-cuda-build   | 01:33:27
 clang-s390x-linux  | 01:33:06
 llvm-hexagon-elf   | 01:32:57
 clang-hexagon-elf  | 01:32:38
 libcxx-libcxxabi-x86_64-linux-debian   | 01:28:40
 lldb-x86_64-ubuntu-14.04-android   | 01:28:02
 clang-atom-d525-fedora-rel | 01:16:38
 lldb-x86_64-darwin-13.4

Buildbot numbers for the week of 9/04/2016 - 9/10/201

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 9/04/2016 - 9/10/2016.

Please see the same data in attached csv files:

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

Thanks

Galina


The longest time each builder was red during the last week:

 buildername|  was_red
+---
 sanitizer-x86_64-linux-autoconf| 63:10:34
 clang-x64-ninja-win7   | 59:00:46
 clang-ppc64le-linux-multistage | 56:49:27
 clang-hexagon-elf  | 56:32:00
 clang-x86-win2008-selfhost | 56:13:02
 clang-x86-windows-msvc2015 | 42:34:12
 llvm-mips-linux| 39:25:19
 sanitizer-x86_64-linux-bootstrap   | 38:35:50
 clang-cmake-thumbv7-a15| 37:55:23
 sanitizer-x86_64-linux-fast| 37:45:14
 clang-cmake-armv7-a15-selfhost-neon| 34:39:46
 clang-cmake-armv7-a15-selfhost | 31:47:04
 sanitizer-ppc64be-linux| 27:13:18
 clang-native-arm-lnt   | 26:48:38
 clang-ppc64le-linux-lnt| 26:24:42
 lldb-x86_64-darwin-13.4| 26:23:31
 clang-cmake-armv7-a15-full | 26:18:16
 clang-ppc64be-linux-lnt| 26:07:43
 clang-cmake-armv7-a15  | 26:06:27
 lldb-x86_64-ubuntu-14.04-android   | 25:56:42
 clang-cmake-mipsel | 25:27:09
 lldb-windows7-android  | 25:23:08
 clang-cmake-thumbv7-a15-full-sh| 24:42:18
 sanitizer-ppc64le-linux| 24:11:00
 clang-s390x-linux  | 23:19:49
 clang-3stage-ubuntu| 21:11:37
 clang-cmake-aarch64-full   | 19:47:59
 clang-cuda-build   | 19:31:48
 clang-cmake-aarch64-quick  | 18:21:31
 clang-with-lto-ubuntu  | 17:53:41
 lldb-x86_64-ubuntu-14.04-cmake | 17:19:15
 clang-native-aarch64-full  | 11:55:16
 lldb-amd64-ninja-netbsd7   | 09:03:13
 lldb-x86_64-ubuntu-14.04-buildserver   | 07:25:11
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan | 05:30:18
 lldb-amd64-ninja-freebsd11 | 05:02:41
 clang-ppc64be-linux| 05:00:40
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 | 04:56:22
 llvm-hexagon-elf   | 04:55:47
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions  | 04:48:42
 libcxx-libcxxabi-libunwind-arm-linux   | 04:43:26
 clang-cmake-mips   | 04:18:34
 clang-x86_64-debian-fast   | 03:39:44
 clang-ppc64be-linux-multistage | 02:59:40
 clang-ppc64le-linux| 02:33:50
 sanitizer-x86_64-linux-fuzzer  | 02:19:10
 clang-x86_64-linux-selfhost-modules| 02:10:07
 sanitizer-x86_64-linux | 02:09:23
 lld-x86_64-darwin13| 02:05:51
 lld-x86_64-win7| 01:57:02
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 01:55:39
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 01:52:12
 perf-x86_64-penryn-O3-polly-parallel-fast  | 01:49:43
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 01:48:19
 llvm-sphinx-docs   | 01:48:00
 perf-x86_64-penryn-O3-polly-unprofitable   | 01:47:49
 clang-cmake-aarch64-42vma  | 01:46:20
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan  | 01:39:04
 sanitizer-windows  | 01:28:02
 libcxx-libcxxabi-x86_64-linu

Re: [PATCH] D24411: [Analyzer] Suppress false positives on the clang static analyzer

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

The thread from cfe-dev is called "Clang Static Analyzer: False Positive 
Suppression Support":
http://clang-developers.42468.n3.nabble.com/Clang-Static-Analyzer-False-Positive-Suppression-Support-tt4053071.html


https://reviews.llvm.org/D24411



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

Re: [PATCH] D21506: [analyzer] Block in critical section

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

This checker is now in alpha.unix, because it is new and is in active 
development. However, alpha checkers are not supported and are not turned on by 
default, so we should move it into unix package once we think it is ready to be 
used.

Evaluation on a large real codebase (or several) would give us a higher 
confidence in the checker, so it will be valuable to perform that before we 
move the package out of alpha. However, clang is probably not a good codebase 
to test this on because it's not heavily multithreaded.


Repository:
  rL LLVM

https://reviews.llvm.org/D21506



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

Re: Submodule semantics with macro guard

[Richard Smith via cfe-commits]

Your c.h is not correct. It would introduce a definition of c in every file
where it's included, so it's not a modular header.

On 19 Sep 2016 5:21 pm, "Manman via cfe-commits" 
wrote:

>
> Hi Richard & Ben,
>
> Given a simple testing case, where we have two submodules X.A (A.h) and
> X.B (B.h, it includes C.h, and C.h is guarded with a macro), when we import
> X.A and then textually include a header C.h, we get redefinition error.
> This is because the macro guard is owned by module X.B that is not yet
> visible. I wonder what the fix is if this is considered a compiler issue.
> We can:
> 1> do not emit the redefinition error if the earlier definition is not
> visible, or
> 2> emit diagnostics to suggest user to import the whole module, when
> redefinition happens because the module includes a non-modular header and
> the non-modular header is also textually included, or
> 3> other suggestions?
>
> Note that if we textually include C.h, then import X.A, there are no
> compiler errors.
>
> The testing case here:
> clang -cc1 repro.c -fsyntax-only -I Inputs4/ -fmodules
> -fimplicit-module-maps -fmodules-cache-path=tmp
> cat repro.c
> #include "A.h" //modular header
> #include "C.h" //non-modular header
>
> cat Inputs4/module.map
> module X {
>   module A {
> header "A.h"
> export *
>   }
>   module B {
> header "B.h"
> export *
>   }
>   export *
> }
> cat Inputs4/A.h
> #ifndef __A_h__
> #define __A_h__
> #endif
> cat Inputs4/B.h
> #ifndef __B_h__
> #define __B_h__
> #include "C.h"
> #endif
> cat Inputs4/C.h
> #ifndef __C_h__
> #define __C_h__
> int c = 1;
> const int d = 2;
> #endif
>
> Thanks,
> Manman
> ___
> 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

[clang-tools-extra] r281954 - clang-change-namespace: Update libdeps.

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Mon Sep 19 19:44:45 2016
New Revision: 281954

URL: http://llvm.org/viewvc/llvm-project?rev=281954&view=rev
Log:
clang-change-namespace: Update libdeps.

FIXME: It seems clangFormat is not referred.

Modified:
clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt

Modified: clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt?rev=281954&r1=281953&r2=281954&view=diff
==
--- clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt Mon Sep 19 
19:44:45 2016
@@ -1,9 +1,14 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
 add_clang_executable(clang-change-namespace
   ClangChangeNamespace.cpp
   )
 target_link_libraries(clang-change-namespace
+  clangAST
   clangASTMatchers
   clangBasic
   clangChangeNamespace


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

Re: [PATCH] D9168: [NVPTX] Check if callsite is defined when computing argument allignment

[Jacques Pienaar via cfe-commits]

@Valentin: It would be great if you could give it a go. I tried
resurrecting it earlier today but noticed getDataLayout() had been removed
and then I got tied up with other pending work.

On Mon, Sep 19, 2016 at 5:11 PM, Valentin Churavy 
wrote:

> vchuravy added a comment.
>
> @jpienaar are you planning to work on this again? Or should I give it a go?
>
>
> https://reviews.llvm.org/D9168
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20392: [CodeView] Modify emitTypeInformation to use MemoryTypeTableBuilder

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r270106.


https://reviews.llvm.org/D20392



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

Re: [PATCH] D22577: Include unreferenced nested types in member list only for CodeView

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r276271. Please specify Differential revision in commit message.


https://reviews.llvm.org/D22577



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

Re: [PATCH] D9168: [NVPTX] Check if callsite is defined when computing argument allignment

[Valentin Churavy via cfe-commits]

vchuravy added a comment.

@jpienaar are you planning to work on this again? Or should I give it a go?


https://reviews.llvm.org/D9168



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

Submodule semantics with macro guard

[Manman via cfe-commits]

Hi Richard & Ben,

Given a simple testing case, where we have two submodules X.A (A.h) and X.B 
(B.h, it includes C.h, and C.h is guarded with a macro), when we import X.A and 
then textually include a header C.h, we get redefinition error. This is because 
the macro guard is owned by module X.B that is not yet visible. I wonder what 
the fix is if this is considered a compiler issue. We can:
1> do not emit the redefinition error if the earlier definition is not visible, 
or
2> emit diagnostics to suggest user to import the whole module, when 
redefinition happens because the module includes a non-modular header and the 
non-modular header is also textually included, or
3> other suggestions?

Note that if we textually include C.h, then import X.A, there are no compiler 
errors.

The testing case here:
clang -cc1 repro.c -fsyntax-only -I Inputs4/ -fmodules -fimplicit-module-maps 
-fmodules-cache-path=tmp
cat repro.c
#include "A.h" //modular header
#include "C.h" //non-modular header

cat Inputs4/module.map 
module X {
  module A {
header "A.h"
export *
  }
  module B {
header "B.h"
export *
  }
  export *
}
cat Inputs4/A.h
#ifndef __A_h__
#define __A_h__
#endif
cat Inputs4/B.h
#ifndef __B_h__
#define __B_h__
#include "C.h"
#endif
cat Inputs4/C.h
#ifndef __C_h__
#define __C_h__
int c = 1;
const int d = 2;
#endif

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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[Nick Lewycky via cfe-commits]

On 19 September 2016 at 15:58, John McCall  wrote:

> rjmccall added a comment.
>
> Actually, that should demonstrate the difference, assuming the LLVM
> function looks through selects, since IRGen should generate that as a
> select.
>

It doesn't look past the Value* you hand it. As well as isa, it
returns true on Arguments marked nonnull, byval, inalloca or
dereferenceable, global variables in addrspace 0 and not extern_weak, load
instructions marked with !nonnull metadata, or a call/invoke with the
nonnull parameter attribute on the return. It doesn't look at operands.


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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

Oh.  One danger with invoking a generic LLVM routine is that they often expect 
a well-formed function, not something that is plausibly still being emitted.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

Actually, that should demonstrate the difference, assuming the LLVM function 
looks through selects, since IRGen should generate that as a select.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

The formation restrictions on ARC writeback conversions probably make this 
more-or-less impossible to test, but I can try to explain when they happen.  
They happen when you have an argument of type "id __strong *" and pass it as a 
parameter of type "id __autoreleasing *".  __strong is the default for 
variables, and __autoreleasing is the default for pointer-to-id parameters, so 
basically you need something like:

  void test(int opaque) {
extern void foo(id*);
id x;
id y;
foo(opaque ? &x : &y);
  }

or any other expression that forms a sufficiently complex argument of type "id 
__strong *" prior to writeback conversion.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D21863: Fix typo in atomic macros

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r280607.


https://reviews.llvm.org/D21863



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

Re: [PATCH] D24319: clang-format: Add an option to git-clang-format to diff between to commits

[Mark Lodato via cfe-commits]

lodato added a comment.

Sorry for the delay - I haven't had a chance to review. I'll be sure to review 
by tomorrow. Thanks for the updates!


https://reviews.llvm.org/D24319



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

Re: [PATCH] D24319: clang-format: Add an option to git-clang-format to diff between to commits

[Luis Héctor Chávez via cfe-commits]

lhchavez added a comment.

Gentle ping. Is there anything else that needs addressing? Did I miss anything?


https://reviews.llvm.org/D24319



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

Re: [PATCH] D24743: Fix signatures of fallback tow(upper|lower)_l.

[Dan Albert via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281936: Fix signatures of fallback tow(upper|lower)_l. 
(authored by danalbert).

Changed prior to commit:
  https://reviews.llvm.org/D24743?vs=71871&id=71876#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24743

Files:
  libcxx/trunk/include/support/xlocale/__posix_l_fallback.h

Index: libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
===
--- libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
+++ libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
@@ -124,11 +124,11 @@
   return ::tolower(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towupper_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towupper_l(wint_t c, locale_t) {
   return ::towupper(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towlower_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towlower_l(wint_t c, locale_t) {
   return ::towlower(c);
 }
 


Index: libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
===
--- libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
+++ libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
@@ -124,11 +124,11 @@
   return ::tolower(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towupper_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towupper_l(wint_t c, locale_t) {
   return ::towupper(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towlower_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towlower_l(wint_t c, locale_t) {
   return ::towlower(c);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r281936 - Fix signatures of fallback tow(upper|lower)_l.

[Dan Albert via cfe-commits]

Author: danalbert
Date: Mon Sep 19 15:42:57 2016
New Revision: 281936

URL: http://llvm.org/viewvc/llvm-project?rev=281936&view=rev
Log:
Fix signatures of fallback tow(upper|lower)_l.

Summary:
These functions take and return wint_t, not int:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/towupper.html

Reviewers: mclow.lists, EricWF

Subscribers: cfe-commits

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

Modified:
libcxx/trunk/include/support/xlocale/__posix_l_fallback.h

Modified: libcxx/trunk/include/support/xlocale/__posix_l_fallback.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/support/xlocale/__posix_l_fallback.h?rev=281936&r1=281935&r2=281936&view=diff
==
--- libcxx/trunk/include/support/xlocale/__posix_l_fallback.h (original)
+++ libcxx/trunk/include/support/xlocale/__posix_l_fallback.h Mon Sep 19 
15:42:57 2016
@@ -124,11 +124,11 @@ inline _LIBCPP_ALWAYS_INLINE int tolower
   return ::tolower(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towupper_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towupper_l(wint_t c, locale_t) {
   return ::towupper(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towlower_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towlower_l(wint_t c, locale_t) {
   return ::towlower(c);
 }
 


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

[clang-tools-extra] r281935 - Add missing dependency to change-namespace.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Sep 19 15:41:39 2016
New Revision: 281935

URL: http://llvm.org/viewvc/llvm-project?rev=281935&view=rev
Log:
Add missing dependency to change-namespace.

Modified:
clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp

Modified: clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt?rev=281935&r1=281934&r2=281935&view=diff
==
--- clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt Mon Sep 19 
15:41:39 2016
@@ -4,6 +4,7 @@ add_clang_executable(clang-change-namesp
   ClangChangeNamespace.cpp
   )
 target_link_libraries(clang-change-namespace
+  clangASTMatchers
   clangBasic
   clangChangeNamespace
   clangFormat

Modified: clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp?rev=281935&r1=281934&r2=281935&view=diff
==
--- clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp 
(original)
+++ clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp Mon 
Sep 19 15:41:39 2016
@@ -30,6 +30,7 @@
 //} // namespace x
 
 #include "ChangeNamespace.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"


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

Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281934: [analyzer] Calculate extent size for memory regions 
allocated by new expression. (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D24307?vs=70821&id=71874#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24307

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/out-of-bounds-new.cpp

Index: cfe/trunk/test/Analysis/out-of-bounds-new.cpp
===
--- cfe/trunk/test/Analysis/out-of-bounds-new.cpp
+++ cfe/trunk/test/Analysis/out-of-bounds-new.cpp
@@ -0,0 +1,150 @@
+// RUN: %clang_cc1 -std=c++11 -Wno-array-bounds -analyze -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
+
+// Tests doing an out-of-bounds access after the end of an array using:
+// - constant integer index
+// - constant integer size for buffer
+void test1(int x) {
+  int *buf = new int[100];
+  buf[100] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ok(int x) {
+  int *buf = new int[100];
+  buf[99] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access after the end of an array using:
+// - indirect pointer to buffer
+// - constant integer index
+// - constant integer size for buffer
+void test1_ptr(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[101] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_ok(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[99] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer, manipulated using simple pointer arithmetic
+// - constant integer index
+// - constant integer size for buffer
+void test1_ptr_arith(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 100;
+  p[0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_arith_ok(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[0] = 1; // no-warning
+}
+
+void test1_ptr_arith_bad(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_arith_ok2(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[-1] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - constant integer index
+// - constant integer size for buffer
+void test2(int x) {
+  int *buf = new int[100];
+  buf[-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer
+// - constant integer index
+// - constant integer size for buffer
+void test2_ptr(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer, manipulated using simple pointer arithmetic
+// - constant integer index
+// - constant integer size for buffer
+void test2_ptr_arith(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  --p;
+  p[0] = 1; // expected-warning {{Out of bound memory access (accessed memory precedes memory block)}}
+}
+
+// Tests under-indexing
+// of a multi-dimensional array
+void test2_multi(int x) {
+  auto buf = new int[100][100];
+  buf[0][-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests under-indexing
+// of a multi-dimensional array
+void test2_multi_b(int x) {
+  auto buf = new int[100][100];
+  buf[-1][0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests over-indexing
+// of a multi-dimensional array
+void test2_multi_c(int x) {
+  auto buf = new int[100][100];
+  buf[100][0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests over-indexing
+// of a multi-dimensional array
+void test2_multi_2(int x) {
+  auto buf = new int[100][100];
+  buf[99][100] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests normal access of
+// a multi-dimensional array
+void test2_multi_ok(int x) {
+  auto buf = new int[100][100];
+  buf[0][0] = 1; // no-warning
+}
+
+// Tests over-indexing using different types
+// array
+void test_diff_types(int x) {
+  int *buf = new int[10]; //10*sizeof(int) Bytes allocated
+  char *cptr = (char *)buf;
+  cptr[sizeof(int) * 9] = 1;  // no-warning
+  cptr[sizeof(int) * 10] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests over-indexing
+//if the allocated area is non-array
+void test_non_array(int x) {
+  int *ip = new int;
+  ip[0] = 1; // no-warning
+  ip[1] = 2; // expected-warning{{Out of bound memory access}}
+}
+
+//Tests over-indexing
+//if the allocated area size is a runtime parameter
+void test_dynamic_size

r281934 - [analyzer] Calculate extent size for memory regions allocated by new expression.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Sep 19 15:39:52 2016
New Revision: 281934

URL: http://llvm.org/viewvc/llvm-project?rev=281934&view=rev
Log:
[analyzer] Calculate extent size for memory regions allocated by new expression.

ArrayBoundChecker did not detect out of bounds memory access errors in case an
array was allocated by the new expression. This patch resolves this issue.

Patch by Daniel Krupp!

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


Added:
cfe/trunk/test/Analysis/out-of-bounds-new.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp?rev=281934&r1=281933&r2=281934&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp Mon Sep 19 
15:39:52 2016
@@ -68,18 +68,11 @@ public:
 
 static SVal computeExtentBegin(SValBuilder &svalBuilder,
const MemRegion *region) {
-  while (true)
-switch (region->getKind()) {
-  default:
-return svalBuilder.makeZeroArrayIndex();
-  case MemRegion::SymbolicRegionKind:
-// FIXME: improve this later by tracking symbolic lower bounds
-// for symbolic regions.
-return UnknownVal();
-  case MemRegion::ElementRegionKind:
-region = cast(region)->getSuperRegion();
-continue;
-}
+  const MemSpaceRegion *SR = region->getMemorySpace();
+  if (SR->getKind() == MemRegion::UnknownSpaceRegionKind)
+return UnknownVal();
+  else
+return svalBuilder.makeZeroArrayIndex();
 }
 
 // TODO: once the constraint manager is smart enough to handle non simplified

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=281934&r1=281933&r2=281934&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Mon Sep 19 15:39:52 
2016
@@ -287,6 +287,9 @@ private:
   ProgramStateRef State,
   AllocationFamily Family = AF_Malloc);
 
+  static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE,
+   ProgramStateRef State);
+
   // Check if this malloc() for special flags. At present that means M_ZERO or
   // __GFP_ZERO (in which case, treat it like calloc).
   llvm::Optional
@@ -981,10 +984,59 @@ void MallocChecker::checkPostStmt(const
   // existing binding.
   State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray
: AF_CXXNew);
+  State = addExtentSize(C, NE, State);
   State = ProcessZeroAllocation(C, NE, 0, State);
   C.addTransition(State);
 }
 
+// Sets the extent value of the MemRegion allocated by
+// new expression NE to its size in Bytes.
+//
+ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C,
+ const CXXNewExpr *NE,
+ ProgramStateRef State) {
+  if (!State)
+return nullptr;
+  SValBuilder &svalBuilder = C.getSValBuilder();
+  SVal ElementCount;
+  const LocationContext *LCtx = C.getLocationContext();
+  const SubRegion *Region;
+  if (NE->isArray()) {
+const Expr *SizeExpr = NE->getArraySize();
+ElementCount = State->getSVal(SizeExpr, C.getLocationContext());
+// Store the extent size for the (symbolic)region
+// containing the elements.
+Region = (State->getSVal(NE, LCtx))
+ .getAsRegion()
+ ->getAs()
+ ->getSuperRegion()
+ ->getAs();
+  } else {
+ElementCount = svalBuilder.makeIntVal(1, true);
+Region = (State->getSVal(NE, LCtx)).getAsRegion()->getAs();
+  }
+  assert(Region);
+
+  // Set the region's extent equal to the Size in Bytes.
+  QualType ElementType = NE->getAllocatedType();
+  ASTContext &AstContext = C.getASTContext();
+  CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
+
+  if (Optional DefinedSize =
+  ElementCount.getAs()) {
+DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder);
+// size in Bytes = ElementCount*TypeSize
+SVal SizeInBytes = svalBuilder.evalBinOpNN(
+State, BO_Mul, ElementCount.castAs(),
+svalBuilder.makeArrayIndex(TypeSize.getQuantity()),
+svalBuilder.getArrayIndexType());
+DefinedOrUnknownSVal extentMatchesSize = svalBuilder.evalEQ(
+State, Extent, SizeInBytes.castAs());
+S

Re: [PATCH] D24411: [Analyzer] Suppress false positives on the clang static analyzer

[Nikhil Gupta via cfe-commits]

nikhgupt added a comment.

In https://reviews.llvm.org/D24411#545381, @zaks.anna wrote:

> It is not clear to me that we've reached a consensus on cfe-dev list that 
> suppressing with comments and printing the checker name is the way to go.


I'm new to the LLVM upstreaming process and have not been a part of the 
previous threads discussing this. It is my understanding that false positive 
suppression is of importance to the community. What is the common consensus on 
implementing Analyzer suppressions?

While suppressing with the use of comments is debatable, my findings indicate 
that a blind suppression statement for a line of code (ie: without the use of a 
checker name) can lead to some confusion with developers. For instance, the 
(simplified) code example below emits two analyzer warnings on the last line: A 
dead-code warning for 'y' as well as a division-by-zero warning for the 
arithmetic operation. A blind suppression by a developer who assumes that this 
would only emit a false positive deadcode warning, will unintentionally 
suppress the crucial division by zero warning.

  void dummyFunc(){
int a=5;
int b=0;
volatile int c = a/b;
  }

By annotating the warnings they intend on suppressing we can ensure that 
developers are aware of any other bugs that can emerge from that line.

ie:

  void dummyFunc(){
int a=5;
int b=0;
volatile int c = a/b; //clang_sa_ignore[deadcode,core.DivideZero]
  }

In order to do so, we will have to make the specific warning checker name 
visible to the user.


https://reviews.llvm.org/D24411



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

Re: [PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D21698#540237, @Anastasia wrote:

> I have made an experiment with a simple kernel:
>
>   void foo1(void);
>   void foo2(void);
>   void foo3(void);
>   void foo4(void);
>   void foo5(void);
>   void foo6(void);
>   void foo7(void);
>   void foo8(void);
>   void foo9(void);
>   void foo10(void);
>  
>   void test(){
> foo1();
> foo2();
> foo3();
> foo4();
> foo5();
> foo6();
> foo7();
> foo8();
> foo9();
> foo10();
>   }
>   
>
> I am using time utility of linux to measure the compile time running Clang in 
> CL2.0 mode and average over 100 samples. It shows me around 7% overhead with 
> your approach.


Since the program is very small, it is difficult to measure the compilation 
time accurately. I compile the program 1000 times and measure its time in a 
script:

   $ cat run.sh
   run() {
   i=1
   while [[ $i -le 1000 ]]; do
 ./$1 -cc1 -emit-llvm tmp.cl
 i=$((i+1))
   done
   }
   
  time -p run $1


Even so, I found large variations in the measured compilation time. I ran the 
script 10 times for clang before my change and I got the real time

  real 8.96
  real 9.01
  real 8.99
  real 9.07
  real 9.03
  real 8.99
  real 9.03
  real 9.01
  real 8.99
  real 9.01

and the average time is 9.009s.

For clang after my change, I got

  real 9.06
  real 9.09
  real 9.10
  real 9.03
  real 9.05
  real 9.17
  real 9.08
  real 9.08
  real 9.07
  real 9.08

And the average time is 9.081s.

The increase of compilation time is 0.8%. Considering this program consists 
mostly of function declarations, which emphasized the cost of evaluating 
disabled function declarations unrealistically. In real programs this increment 
in compilation time should be even smaller.

Since the increment of compilation time is less than 1% even for exaggerated 
cases, I think it is reasonable to accept the cost for the improved diagnostics 
for extensions.

If there are concerns that the newly added diagnostics may break applications 
which use builtin functions associated with an extension without enabling it, 
we can make the diagnostic msg a warning which can be turned off.


https://reviews.llvm.org/D21698



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

[PATCH] D24743: Fix signatures of fallback tow(upper|lower)_l.

[Dan Albert via cfe-commits]

danalbert created this revision.
danalbert added reviewers: EricWF, mclow.lists.
danalbert added a subscriber: cfe-commits.
danalbert set the repository for this revision to rL LLVM.

These functions take and return wint_t, not int:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/towupper.html

Repository:
  rL LLVM

https://reviews.llvm.org/D24743

Files:
  include/support/xlocale/__posix_l_fallback.h

Index: include/support/xlocale/__posix_l_fallback.h
===
--- include/support/xlocale/__posix_l_fallback.h
+++ include/support/xlocale/__posix_l_fallback.h
@@ -124,11 +124,11 @@
   return ::tolower(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towupper_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towupper_l(wint_t c, locale_t) {
   return ::towupper(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towlower_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towlower_l(wint_t c, locale_t) {
   return ::towlower(c);
 }
 


Index: include/support/xlocale/__posix_l_fallback.h
===
--- include/support/xlocale/__posix_l_fallback.h
+++ include/support/xlocale/__posix_l_fallback.h
@@ -124,11 +124,11 @@
   return ::tolower(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towupper_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towupper_l(wint_t c, locale_t) {
   return ::towupper(c);
 }
 
-inline _LIBCPP_ALWAYS_INLINE int towlower_l(int c, locale_t) {
+inline _LIBCPP_ALWAYS_INLINE wint_t towlower_l(wint_t c, locale_t) {
   return ::towlower(c);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments

[Bruno Cardoso Lopes via cfe-commits]

bruno updated this revision to Diff 71870.
bruno added a comment.

Update again (now with the right patch) after Akira's comment


https://reviews.llvm.org/D24472

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c

Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   float64x2_t v = {0.0, 1.0};
-  f2 += d;
+  // FIXME: These diagnostics are inaccurate: should complain that 'double' to 
vector 'float2' involves truncation
+  f2 += d; // expected-error {{cannot convert between vector values of 
different size ('float2' (vector of 2 'float' values) and 'double')}}
+  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
   a = 3.0 + vget_low_f64(v);
   b = vget_low_f64(v) + 3.0;
   c = vget_low_f64(v);
-  // LAX conversions within compound assignments are not supported.
-  // FIXME: This diagnostic is inaccurate.
-  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
-  c -= vget_low_f64(v); // expected-error {{cannot convert between vector 
values of different size}}
+  c -= vget_low_f64(v);
   // LAX conversions between scalar and vector types require same size and one 
element sized vectors.
   d = f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
   d = d + f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8051,6 +8051,7 @@
 
   // If there's an ext-vector type and a scalar, try to convert the scalar to
   // the vector element type and splat.
+  // FIXME: this should also work for regular vector types as supported in GCC.
   if (!RHSVecType && isa(LHSVecType)) {
 if (!tryVectorConvertAndSplat(*this, &RHS, RHSType,
   LHSVecType->getElementType(), LHSType))
@@ -8063,16 +8064,31 @@
   return RHSType;
   }
 
-  // If we're allowing lax vector conversions, only the total (data) size needs
-  // to be the same. If one of the types is scalar, the result is always the
-  // vector type. Don't allow this if the scalar operand is an lvalue.
+  // FIXME: The code below also handles convertion between vectors and
+  // non-scalars, we should break this down into fine grained specific checks
+  // and emit proper diagnostics.
   QualType VecType = LHSVecType ? LHSType : RHSType;
-  QualType ScalarType = LHSVecType ? RHSType : LHSType;
-  ExprResult *ScalarExpr = LHSVecType ? &RHS : &LHS;
-  if (isLaxVectorConversion(ScalarType, VecType) &&
-  !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+  const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+  QualType OtherType = LHSVecType ? RHSType : LHSType;
+  ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
+  if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types 
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+  *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+  return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only biscast for
+// <1 x T> -> T. The result is also a vector type.
+} else if (OtherType->isExtVectorType() ||
+   (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+  ExprResult *RHSExpr = &RHS;
+  *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+  return VecType;
+}
   }
 
   // Okay, the expression is invalid.


Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   float64x2_t v = {0.0, 1.0};
-  f2 += d;
+  // FIXME: These diagnostics are inaccurate: should complain that 'double' to vector 'float2' involves truncation
+  f2 += d; // expected-error {{cannot convert between vector values of different size ('float2' (vector of 2 'float' values) and 'double')}}
+  d += f2; // expected-error {{cannot convert between vector values of different size}}
   a = 3.0 + vget_low_f64(v);
   b = vget_low_f64(v) + 3.0;
   c = vget_low_f64(v);
-  // LAX conversions within compound assignments are not supported.
-  // FIXME: This diagnostic is inaccurate.
-  d += f2; // expected-error {{cannot convert between ve

Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments

[Bruno Cardoso Lopes via cfe-commits]

bruno updated this revision to Diff 71869.
bruno marked an inline comment as done.
bruno added a comment.

Update after Akira's comment


https://reviews.llvm.org/D24472

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c

Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   float64x2_t v = {0.0, 1.0};
-  f2 += d;
+  // FIXME: These diagnostics are inaccurate: should complain that 'double' to 
vector 'float2' involves truncation
+  f2 += d; // expected-error {{cannot convert between vector values of 
different size ('float2' (vector of 2 'float' values) and 'double')}}
+  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
   a = 3.0 + vget_low_f64(v);
   b = vget_low_f64(v) + 3.0;
   c = vget_low_f64(v);
-  // LAX conversions within compound assignments are not supported.
-  // FIXME: This diagnostic is inaccurate.
-  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
-  c -= vget_low_f64(v); // expected-error {{cannot convert between vector 
values of different size}}
+  c -= vget_low_f64(v);
   // LAX conversions between scalar and vector types require same size and one 
element sized vectors.
   d = f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
   d = d + f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8051,6 +8051,7 @@
 
   // If there's an ext-vector type and a scalar, try to convert the scalar to
   // the vector element type and splat.
+  // FIXME: this should also work for regular vector types as supported in GCC.
   if (!RHSVecType && isa(LHSVecType)) {
 if (!tryVectorConvertAndSplat(*this, &RHS, RHSType,
   LHSVecType->getElementType(), LHSType))
@@ -8063,16 +8064,31 @@
   return RHSType;
   }
 
-  // If we're allowing lax vector conversions, only the total (data) size needs
-  // to be the same. If one of the types is scalar, the result is always the
-  // vector type. Don't allow this if the scalar operand is an lvalue.
+  // FIXME: The code below also handles convertion between vectors and
+  // non-scalars, we should break this down into fine grained specific checks
+  // and emit proper diagnostics.
   QualType VecType = LHSVecType ? LHSType : RHSType;
-  QualType ScalarType = LHSVecType ? RHSType : LHSType;
-  ExprResult *ScalarExpr = LHSVecType ? &RHS : &LHS;
-  if (isLaxVectorConversion(ScalarType, VecType) &&
-  !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+  const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+  QualType OtherType = LHSVecType ? RHSType : LHSType;
+  ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
+  if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types 
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+  *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+  return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only biscast for
+// <1 x T> -> T.
+} else if (OtherType->isExtVectorType() ||
+   (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+  ExprResult *RHSExpr = &RHS;
+  *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+  return LHSType;
+}
   }
 
   // Okay, the expression is invalid.


Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   float64x2_t v = {0.0, 1.0};
-  f2 += d;
+  // FIXME: These diagnostics are inaccurate: should complain that 'double' to vector 'float2' involves truncation
+  f2 += d; // expected-error {{cannot convert between vector values of different size ('float2' (vector of 2 'float' values) and 'double')}}
+  d += f2; // expected-error {{cannot convert between vector values of different size}}
   a = 3.0 + vget_low_f64(v);
   b = vget_low_f64(v) + 3.0;
   c = vget_low_f64(v);
-  // LAX conversions within compound assignments are not supported.
-  // FIXME: This diagnostic is inaccurate.
-  d += f2; // expected-error {{cannot convert between vector values of different si

Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments

[Bruno Cardoso Lopes via cfe-commits]

bruno marked an inline comment as done.


Comment at: lib/Sema/SemaExpr.cpp:8084
@@ +8083,3 @@
+  *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+  return LHSType;
+}

ahatanak wrote:
> My understanding is that, when we have a compound assign like "LHS += RHS", 
> this function (CheckVectorOperands) is supposed to return the result type 
> (LHS + RHS). However, it is returning different types for "<1 x T> += T"  and 
> "T += <1 x T>" (the former returns <1 x T> and the latter returns T). Would 
> CheckAssignmentOperands reject the compound statement if you returned the 
> vector type here?
> 
> Also, are you planning to allow the same kind of conversions done above for 
> non-compound assignment statements (e.g., <4 x short> += <2 x int>) in the 
> future?
1) CheckAssignmentOperands doesn't reject the compound statement if we return 
the vector type instead, because it already supports vector to scalar cast 
idiom. It makes more sense to return a vector indeed, gonna change that.

2) It would be nice to catch up with GCC with the idioms supported for regular 
(non-ext) vectors (like splatting scalar operands to vectors in a arith 
binops), and those, AFAIK, don't include support for truncation as in the 
example you suggested. I guess this is supported with ext-vectors, but I don't 
see any reason to support it for "regular" vectors.


https://reviews.llvm.org/D24472



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

Re: [PATCH] D24679: [libc++] Fix extern template visibility for Windows

[Shoaib Meenai via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281925: [libc++] Fix extern template visibility for Windows 
(authored by smeenai).

Changed prior to commit:
  https://reviews.llvm.org/D24679?vs=71685&id=71861#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24679

Files:
  libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
  libcxx/trunk/include/__config
  libcxx/trunk/src/ios.cpp
  libcxx/trunk/src/locale.cpp
  libcxx/trunk/src/string.cpp

Index: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
===
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
@@ -71,6 +71,26 @@
   However since `_LIBCPP_TYPE_VIS_ONLY` is the same as `_LIBCPP_TYPE_VIS` the
   visibility is already correct. The macro has an empty definition with GCC.
 
+  **Windows Behavior**: `extern template` and `dllexport` are fundamentally
+  incompatible *on a template class* on Windows; the former suppresses
+  instantiation, while the latter forces it. Specifying both on the same
+  declaration makes the template class be instantiated, which is not desirable
+  inside headers. This macro therefore expands to `dllimport` outside of libc++
+  but nothing inside of it (rather than expanding to `dllexport`); instead, the
+  explicit instantiations themselves are marked as exported. Note that this
+  applies *only* to extern template *classes*. Extern template *functions* obey
+  regular import/export semantics, and applying `dllexport` directly to the
+  extern template declaration is the correct thing to do for them.
+
+**_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS**
+  Mark the member functions, typeinfo, and vtable of an explicit instantiation
+  of a class template as being exported by the libc++ library. This attribute
+  must be specified on all template class explicit instantiations.
+
+  It is only necessary to mark the explicit instantiation itself (as opposed to
+  the extern template declaration) as exported on Windows, as discussed above.
+  On all other platforms, this macro has an empty definition.
+
 **_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY**
   Mark a member function of a class template as hidden and inline except when
   building the libc++ library where it marks the symbol as being exported by
Index: libcxx/trunk/src/string.cpp
===
--- libcxx/trunk/src/string.cpp
+++ libcxx/trunk/src/string.cpp
@@ -20,10 +20,10 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template class __basic_string_common;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __basic_string_common;
 
-template class basic_string;
-template class basic_string;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_string;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_string;
 
 template
 string
Index: libcxx/trunk/src/locale.cpp
===
--- libcxx/trunk/src/locale.cpp
+++ libcxx/trunk/src/locale.cpp
@@ -6031,66 +6031,66 @@
 #endif
 }
 
-template class collate;
-template class collate;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS collate;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS collate;
 
-template class num_get;
-template class num_get;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS num_get;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS num_get;
 
-template struct __num_get;
-template struct __num_get;
+template struct _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __num_get;
+template struct _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __num_get;
 
-template class num_put;
-template class num_put;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS num_put;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS num_put;
 
-template struct __num_put;
-template struct __num_put;
+template struct _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __num_put;
+template struct _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __num_put;
 
-template class time_get;
-template class time_get;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_get;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_get;
 
-template class time_get_byname;
-template class time_get_byname;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_get_byname;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_get_byname;
 
-template class time_put;
-template class time_put;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_put;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_put;
 
-template class time_put_byname;
-template class time_put_byname;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_put_byname;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS time_put_byname;
 
-template class moneypunct;
-template class moneypunct;
-template class moneypunct;
-template c

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Vitaly Buka via cfe-commits]

vitalybuka updated this revision to Diff 71862.
vitalybuka added a comment.

Test


https://reviews.llvm.org/D24693

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/VarBypassDetector.cpp
  lib/CodeGen/VarBypassDetector.h
  test/CodeGen/lifetime2.c

Index: test/CodeGen/lifetime2.c
===
--- test/CodeGen/lifetime2.c
+++ test/CodeGen/lifetime2.c
@@ -1,8 +1,9 @@
-// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefix=O2
-// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefix=O0
+// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefixes=CHECK,O2
+// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0
 
 extern int bar(char *A, int n);
 
+// CHECK-LABEL: @foo
 // O0-NOT: @llvm.lifetime.start
 int foo (int n) {
   if (n) {
@@ -15,3 +16,66 @@
 return bar(A, 2);
   }
 }
+
+// CHECK-LABEL: @no_goto_bypass
+void no_goto_bypass() {
+  // O2: @llvm.lifetime.start(i64 1
+  char x;
+l1:
+  bar(&x, 1);
+  // O2: @llvm.lifetime.start(i64 5
+  // O2: @llvm.lifetime.end(i64 5
+  char y[5];
+  bar(y, 5);
+  goto l1;
+  // Infinite loop
+  // O2-NOT: @llvm.lifetime.end(i64 1
+}
+
+// CHECK-LABEL: @goto_bypass
+void goto_bypass() {
+  {
+// O2-NOT: @llvm.lifetime.start(i64 1
+// O2-NOT: @llvm.lifetime.end(i64 1
+char x;
+  l1:
+bar(&x, 1);
+  }
+  goto l1;
+}
+
+// CHECK-LABEL: @no_switch_bypass
+void no_switch_bypass(int n) {
+  switch (n) {
+  case 1: {
+// O2: @llvm.lifetime.start(i64 1
+// O2: @llvm.lifetime.end(i64 1
+char x;
+bar(&x, 1);
+break;
+  }
+  case 2:
+n = n;
+// O2: @llvm.lifetime.start(i64 5
+// O2: @llvm.lifetime.end(i64 5
+char y[5];
+bar(y, 5);
+break;
+  }
+}
+
+// CHECK-LABEL: @switch_bypass
+void switch_bypass(int n) {
+  switch (n) {
+  case 1:
+n = n;
+// O2-NOT: @llvm.lifetime.start(i64 1
+// O2-NOT: @llvm.lifetime.end(i64 1
+char x;
+bar(&x, 1);
+break;
+  case 2:
+bar(&x, 1);
+break;
+  }
+}
Index: lib/CodeGen/VarBypassDetector.h
===
--- /dev/null
+++ lib/CodeGen/VarBypassDetector.h
@@ -0,0 +1,67 @@
+//===--- VarBypassDetector.cpp - Bypass jumps detector *- C++ -*-=//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains VarBypassDetector class, which is used to detect
+// local variable declarations which can be bypassed by jumps.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+
+class Decl;
+class Stmt;
+class VarDecl;
+
+namespace CodeGen {
+
+/// VarBypassDetector - This class detects jumps which bypass local variables
+/// declaration:
+///goto L;
+///int a;
+///  L:
+/// This is simplified version of JumpScopeChecker. Primary differences:
+///  * Detects only jumps into the scope local variables.
+///  * Does not detect jumps out of the scope of local variables.
+///  * Not limited to variables with initializers, JumpScopeChecker is limited.
+///  * FIXME: Does not support indirect jumps.
+class VarBypassDetector {
+  // Scope information. Contains a parent scope and related variable
+  // declaration.
+  llvm::SmallVector, 48> Scopes;
+  // Lookup map to file scope for jumps and its destinations.
+  llvm::DenseMap LabelAndGotoScopes;
+  // Set of variables which were bypassed by some jump.
+  llvm::DenseSet Bypasses;
+
+public:
+  void Init(const Stmt *Body);
+
+  /// IsBypassed - Returns true if the variable declaration was by bypassed by
+  /// any goto or switch statement.
+  bool IsBypassed(const VarDecl *D) const {
+return Bypasses.find(D) != Bypasses.end();
+  }
+
+private:
+  void BuildScopeInformation(const Decl *D, unsigned &ParentScope);
+  void BuildScopeInformation(const Stmt *S, unsigned &origParentScope);
+  void Detect();
+  void Detect(const Stmt *FromStmt, const Stmt *ToStmt);
+};
+}
+}
+
+#endif
Index: lib/CodeGen/VarBypassDetector.cpp
===
--- /dev/null
+++ lib/CodeGen/VarBypassDetector.cpp
@@ -0,0 +1,154 @@
+//===--- VarBypassDetector.h - Bypass jumps detector --*- C++ -*-=//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--

[libcxx] r281925 - [libc++] Fix extern template visibility for Windows

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Sep 19 13:29:07 2016
New Revision: 281925

URL: http://llvm.org/viewvc/llvm-project?rev=281925&view=rev
Log:
[libc++] Fix extern template visibility for Windows

On Windows, marking an `extern template class` declaration as exported
actually forces an instantiation, which is not the desired behavior.
Instead, the actual explicit instantiations need to be exported.

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

Modified:
libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
libcxx/trunk/include/__config
libcxx/trunk/src/ios.cpp
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/string.cpp

Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=281925&r1=281924&r2=281925&view=diff
==
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Mon Sep 19 13:29:07 2016
@@ -71,6 +71,26 @@ Visibility Macros
   However since `_LIBCPP_TYPE_VIS_ONLY` is the same as `_LIBCPP_TYPE_VIS` the
   visibility is already correct. The macro has an empty definition with GCC.
 
+  **Windows Behavior**: `extern template` and `dllexport` are fundamentally
+  incompatible *on a template class* on Windows; the former suppresses
+  instantiation, while the latter forces it. Specifying both on the same
+  declaration makes the template class be instantiated, which is not desirable
+  inside headers. This macro therefore expands to `dllimport` outside of libc++
+  but nothing inside of it (rather than expanding to `dllexport`); instead, the
+  explicit instantiations themselves are marked as exported. Note that this
+  applies *only* to extern template *classes*. Extern template *functions* obey
+  regular import/export semantics, and applying `dllexport` directly to the
+  extern template declaration is the correct thing to do for them.
+
+**_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS**
+  Mark the member functions, typeinfo, and vtable of an explicit instantiation
+  of a class template as being exported by the libc++ library. This attribute
+  must be specified on all template class explicit instantiations.
+
+  It is only necessary to mark the explicit instantiation itself (as opposed to
+  the extern template declaration) as exported on Windows, as discussed above.
+  On all other platforms, this macro has an empty definition.
+
 **_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY**
   Mark a member function of a class template as hidden and inline except when
   building the libc++ library where it marks the symbol as being exported by

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=281925&r1=281924&r2=281925&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Mon Sep 19 13:29:07 2016
@@ -551,15 +551,20 @@ namespace std {
 // only really useful for a DLL. _LIBCPP_DLL should be a compiler builtin 
define ideally...
 #if defined(_LIBCPP_DLL) && defined(cxx_EXPORTS)
 # define _LIBCPP_DLL_VIS __declspec(dllexport)
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS _LIBCPP_DLL_VIS
 #elif defined(_LIBCPP_DLL)
 # define _LIBCPP_DLL_VIS __declspec(dllimport)
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS _LIBCPP_DLL_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
 #else
 # define _LIBCPP_DLL_VIS
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
 #endif
 #define _LIBCPP_TYPE_VIS_LIBCPP_DLL_VIS
 #define _LIBCPP_FUNC_VIS_LIBCPP_DLL_VIS
 #define _LIBCPP_EXCEPTION_ABI   _LIBCPP_DLL_VIS
-#define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS _LIBCPP_DLL_VIS
 #define _LIBCPP_HIDDEN
 #define _LIBCPP_TYPE_VIS_ONLY
 #define _LIBCPP_FUNC_VIS_ONLY
@@ -619,6 +624,10 @@ namespace std {
 #  endif
 #endif
 
+#ifndef _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
+#  define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
+#endif
+
 #ifndef _LIBCPP_INLINE_VISIBILITY
 #define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), 
__always_inline__))
 #endif

Modified: libcxx/trunk/src/ios.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ios.cpp?rev=281925&r1=281924&r2=281925&view=diff
==
--- libcxx/trunk/src/ios.cpp (original)
+++ libcxx/trunk/src/ios.cpp Mon Sep 19 13:29:07 2016
@@ -25,19 +25,19 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template class basic_ios;
-template class basic_ios;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_ios;
+template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_ios;
 
-template class basic_streambuf;
-template class basic_streambuf;
+template class _LIBCPP_CLASS_T

Re: [PATCH] D24695: [CodeGen] Move shouldEmitLifetimeMarkers into more convenient place

[Vitaly Buka via cfe-commits]

vitalybuka updated this revision to Diff 71860.
vitalybuka added a comment.

Fixed accidentally changed test


https://reviews.llvm.org/D24695

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h

Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1175,6 +1175,9 @@
   llvm::BasicBlock *TerminateHandler;
   llvm::BasicBlock *TrapBB;
 
+  /// True if we need emit the life-time markers.
+  const bool ShouldEmitLifetimeMarkers;
+
   /// Add a kernel metadata node to the named metadata node 'opencl.kernels'.
   /// In the kernel metadata node, reference the kernel function and metadata 
   /// nodes for its optional attribute qualifiers (OpenCL 1.1 6.7.2):
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -37,29 +37,46 @@
 using namespace clang;
 using namespace CodeGen;
 
+/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
+/// markers.
+static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
+  const LangOptions &LangOpts) {
+  // Asan uses markers for use-after-scope checks.
+  if (CGOpts.SanitizeAddressUseAfterScope)
+return true;
+
+  // Disable lifetime markers in msan builds.
+  // FIXME: Remove this when msan works with lifetime markers.
+  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+return false;
+
+  // For now, only in optimized builds.
+  return CGOpts.OptimizationLevel != 0;
+}
+
 CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext)
 : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
   Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
   CGBuilderInserterTy(this)),
   CurFn(nullptr), ReturnValue(Address::invalid()),
-  CapturedStmtInfo(nullptr),
-  SanOpts(CGM.getLangOpts().Sanitize), IsSanitizerScope(false),
-  CurFuncIsThunk(false), AutoreleaseResult(false), SawAsmBlock(false),
-  IsOutlinedSEHHelper(false),
-  BlockInfo(nullptr), BlockPointer(nullptr),
-  LambdaThisCaptureField(nullptr), NormalCleanupDest(nullptr),
-  NextCleanupDestIndex(1), FirstBlockInfo(nullptr), EHResumeBlock(nullptr),
-  ExceptionSlot(nullptr), EHSelectorSlot(nullptr),
-  DebugInfo(CGM.getModuleDebugInfo()),
+  CapturedStmtInfo(nullptr), SanOpts(CGM.getLangOpts().Sanitize),
+  IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
+  SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
+  BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
+  NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+  FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
+  EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
   DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
   PGO(cgm), SwitchInsn(nullptr), SwitchWeights(nullptr),
   CaseRangeBlock(nullptr), UnreachableBlock(nullptr), NumReturnExprs(0),
   NumSimpleReturnExprs(0), CXXABIThisDecl(nullptr),
   CXXABIThisValue(nullptr), CXXThisValue(nullptr),
   CXXStructorImplicitParamDecl(nullptr),
   CXXStructorImplicitParamValue(nullptr), OutermostConditional(nullptr),
   CurLexicalScope(nullptr), TerminateLandingPad(nullptr),
-  TerminateHandler(nullptr), TrapBB(nullptr) {
+  TerminateHandler(nullptr), TrapBB(nullptr),
+  ShouldEmitLifetimeMarkers(
+  shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
   if (!suppressNewContext)
 CGM.getCXXABI().getMangleContext().startNewFunction();
 
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -916,29 +916,12 @@
   EmitAutoVarCleanups(emission);
 }
 
-/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
-/// markers.
-static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
-  const LangOptions &LangOpts) {
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
-return true;
-
-  // Disable lifetime markers in msan builds.
-  // FIXME: Remove this when msan works with lifetime markers.
-  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
-return false;
-
-  // For now, only in optimized builds.
-  return CGOpts.OptimizationLevel != 0;
-}
-
 /// Emit a lifetime.begin marker if some criteria are satisfied.
 /// \return a pointer to the temporary size Value if a marker was emitted, null
 /// otherwise
 llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
 llvm::Value *Addr) {
-  if (!shouldEm

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Vitaly Buka via cfe-commits]

vitalybuka updated this revision to Diff 71858.
vitalybuka added a comment.

recovered test


https://reviews.llvm.org/D24693

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/VarBypassDetector.cpp
  lib/CodeGen/VarBypassDetector.h
  test/CodeGen/lifetime2.c

Index: test/CodeGen/lifetime2.c
===
--- /dev/null
+++ test/CodeGen/lifetime2.c
@@ -0,0 +1,81 @@
+// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefixes=CHECK,O2
+// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0
+
+extern int bar(char *A, int n);
+
+// CHECK-LABEL: @foo
+// O0-NOT: @llvm.lifetime.start
+int foo (int n) {
+  if (n) {
+// O2: @llvm.lifetime.start
+char A[100];
+return bar(A, 1);
+  } else {
+// O2: @llvm.lifetime.start
+char A[100];
+return bar(A, 2);
+  }
+}
+
+// CHECK-LABEL: @no_goto_bypass
+void no_goto_bypass() {
+  // O2: @llvm.lifetime.start(i64 1
+  char x;
+l1:
+  bar(&x, 1);
+  // O2: @llvm.lifetime.start(i64 5
+  // O2: @llvm.lifetime.end(i64 5
+  char y[5];
+  bar(y, 5);
+  goto l1;
+  // Infinite loop
+  // O2-NOT: @llvm.lifetime.end(i64 1
+}
+
+// CHECK-LABEL: @goto_bypass
+void goto_bypass() {
+  {
+// O2-NOT: @llvm.lifetime.start(i64 1
+// O2-NOT: @llvm.lifetime.end(i64 1
+char x;
+  l1:
+bar(&x, 1);
+  }
+  goto l1;
+}
+
+// CHECK-LABEL: @no_switch_bypass
+void no_switch_bypass(int n) {
+  switch (n) {
+  case 1: {
+// O2: @llvm.lifetime.start(i64 1
+// O2: @llvm.lifetime.end(i64 1
+char x;
+bar(&x, 1);
+break;
+  }
+  case 2:
+n = n;
+// O2: @llvm.lifetime.start(i64 5
+// O2: @llvm.lifetime.end(i64 5
+char y[5];
+bar(y, 5);
+break;
+  }
+}
+
+// CHECK-LABEL: @switch_bypass
+void switch_bypass(int n) {
+  switch (n) {
+  case 1:
+n = n;
+// O2-NOT: @llvm.lifetime.start(i64 1
+// O2-NOT: @llvm.lifetime.end(i64 1
+char x;
+bar(&x, 1);
+break;
+  case 2:
+bar(&x, 1);
+break;
+  }
+}
Index: lib/CodeGen/VarBypassDetector.h
===
--- /dev/null
+++ lib/CodeGen/VarBypassDetector.h
@@ -0,0 +1,67 @@
+//===--- VarBypassDetector.cpp - Bypass jumps detector *- C++ -*-=//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains VarBypassDetector class, which is used to detect
+// local variable declarations which can be bypassed by jumps.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+
+class Decl;
+class Stmt;
+class VarDecl;
+
+namespace CodeGen {
+
+/// VarBypassDetector - This class detects jumps which bypass local variables
+/// declaration:
+///goto L;
+///int a;
+///  L:
+/// This is simplified version of JumpScopeChecker. Primary differences:
+///  * Detects only jumps into the scope local variables.
+///  * Does not detect jumps out of the scope of local variables.
+///  * Not limited to variables with initializers, JumpScopeChecker is limited.
+///  * FIXME: Does not support indirect jumps.
+class VarBypassDetector {
+  // Scope information. Contains a parent scope and related variable
+  // declaration.
+  llvm::SmallVector, 48> Scopes;
+  // Lookup map to file scope for jumps and its destinations.
+  llvm::DenseMap LabelAndGotoScopes;
+  // Set of variables which were bypassed by some jump.
+  llvm::DenseSet Bypasses;
+
+public:
+  void Init(const Stmt *Body);
+
+  /// IsBypassed - Returns true if the variable declaration was by bypassed by
+  /// any goto or switch statement.
+  bool IsBypassed(const VarDecl *D) const {
+return Bypasses.find(D) != Bypasses.end();
+  }
+
+private:
+  void BuildScopeInformation(const Decl *D, unsigned &ParentScope);
+  void BuildScopeInformation(const Stmt *S, unsigned &origParentScope);
+  void Detect();
+  void Detect(const Stmt *FromStmt, const Stmt *ToStmt);
+};
+}
+}
+
+#endif
Index: lib/CodeGen/VarBypassDetector.cpp
===
--- /dev/null
+++ lib/CodeGen/VarBypassDetector.cpp
@@ -0,0 +1,154 @@
+//===--- VarBypassDetector.h - Bypass jumps detector --*- C++ -*-=//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#

Re: [PATCH] D24679: [libc++] Fix extern template visibility for Windows

[Saleem Abdulrasool via cfe-commits]

compnerd accepted this revision.
This revision is now accepted and ready to land.


Comment at: include/__config:559
@@ -554,1 +558,3 @@
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
 #endif

smeenai wrote:
> compnerd wrote:
> > Does it make sense for `_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS` to ever 
> > be marked as `__declspec(dllimport)`?  The macro is applied on 
> > instantiations in the implementation, not the header, so this shouldn't be 
> > visible to users.
> It doesn't, which is why I always made it expand to either 
> `__declspec(dllexport)` or empty.
Ugh, I can't match the lines.  I misread the two lines and saw the class 
template instantiation as being defined.


https://reviews.llvm.org/D24679



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

Re: [PATCH] D24695: [CodeGen] Move shouldEmitLifetimeMarkers into more convenient place

[Vitaly Buka via cfe-commits]

vitalybuka updated this revision to Diff 71856.
vitalybuka added a comment.

Removed unrelated test


https://reviews.llvm.org/D24695

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/lifetime2.c

Index: test/CodeGen/lifetime2.c
===
--- test/CodeGen/lifetime2.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefix=O2
-// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefix=O0
-
-extern int bar(char *A, int n);
-
-// O0-NOT: @llvm.lifetime.start
-int foo (int n) {
-  if (n) {
-// O2: @llvm.lifetime.start
-char A[100];
-return bar(A, 1);
-  } else {
-// O2: @llvm.lifetime.start
-char A[100];
-return bar(A, 2);
-  }
-}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1175,6 +1175,9 @@
   llvm::BasicBlock *TerminateHandler;
   llvm::BasicBlock *TrapBB;
 
+  /// True if we need emit the life-time markers.
+  const bool ShouldEmitLifetimeMarkers;
+
   /// Add a kernel metadata node to the named metadata node 'opencl.kernels'.
   /// In the kernel metadata node, reference the kernel function and metadata 
   /// nodes for its optional attribute qualifiers (OpenCL 1.1 6.7.2):
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -37,29 +37,46 @@
 using namespace clang;
 using namespace CodeGen;
 
+/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
+/// markers.
+static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
+  const LangOptions &LangOpts) {
+  // Asan uses markers for use-after-scope checks.
+  if (CGOpts.SanitizeAddressUseAfterScope)
+return true;
+
+  // Disable lifetime markers in msan builds.
+  // FIXME: Remove this when msan works with lifetime markers.
+  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+return false;
+
+  // For now, only in optimized builds.
+  return CGOpts.OptimizationLevel != 0;
+}
+
 CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext)
 : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
   Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
   CGBuilderInserterTy(this)),
   CurFn(nullptr), ReturnValue(Address::invalid()),
-  CapturedStmtInfo(nullptr),
-  SanOpts(CGM.getLangOpts().Sanitize), IsSanitizerScope(false),
-  CurFuncIsThunk(false), AutoreleaseResult(false), SawAsmBlock(false),
-  IsOutlinedSEHHelper(false),
-  BlockInfo(nullptr), BlockPointer(nullptr),
-  LambdaThisCaptureField(nullptr), NormalCleanupDest(nullptr),
-  NextCleanupDestIndex(1), FirstBlockInfo(nullptr), EHResumeBlock(nullptr),
-  ExceptionSlot(nullptr), EHSelectorSlot(nullptr),
-  DebugInfo(CGM.getModuleDebugInfo()),
+  CapturedStmtInfo(nullptr), SanOpts(CGM.getLangOpts().Sanitize),
+  IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
+  SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
+  BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
+  NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+  FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
+  EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
   DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
   PGO(cgm), SwitchInsn(nullptr), SwitchWeights(nullptr),
   CaseRangeBlock(nullptr), UnreachableBlock(nullptr), NumReturnExprs(0),
   NumSimpleReturnExprs(0), CXXABIThisDecl(nullptr),
   CXXABIThisValue(nullptr), CXXThisValue(nullptr),
   CXXStructorImplicitParamDecl(nullptr),
   CXXStructorImplicitParamValue(nullptr), OutermostConditional(nullptr),
   CurLexicalScope(nullptr), TerminateLandingPad(nullptr),
-  TerminateHandler(nullptr), TrapBB(nullptr) {
+  TerminateHandler(nullptr), TrapBB(nullptr),
+  ShouldEmitLifetimeMarkers(
+  shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
   if (!suppressNewContext)
 CGM.getCXXABI().getMangleContext().startNewFunction();
 
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -916,29 +916,12 @@
   EmitAutoVarCleanups(emission);
 }
 
-/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
-/// markers.
-static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
-  const LangOptions &LangOpts) {
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
-return true;

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Vitaly Buka via cfe-commits]

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

The patch was split in two and I moved the test into the wrong one. I'll fix 
this.


https://reviews.llvm.org/D24693



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

r281923 - Reorder initializers in CallStackFrame so that we don't get a warning.

[Samuel Antao via cfe-commits]

Author: sfantao
Date: Mon Sep 19 13:13:13 2016
New Revision: 281923

URL: http://llvm.org/viewvc/llvm-project?rev=281923&view=rev
Log:
Reorder initializers in CallStackFrame so that we don't get a warning.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=281923&r1=281922&r2=281923&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 19 13:13:13 2016
@@ -961,8 +961,8 @@ void SubobjectDesignator::diagnosePointe
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments)
-: Info(Info), Caller(Info.CurrentCall), CallLoc(CallLoc), Callee(Callee),
-  Index(Info.NextCallIndex++), This(This), Arguments(Arguments) {
+: Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
+  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }


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

Re: [PATCH] D24679: [libc++] Fix extern template visibility for Windows

[Shoaib Meenai via cfe-commits]

smeenai added inline comments.


Comment at: include/__config:559
@@ -554,1 +558,3 @@
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
 #endif

compnerd wrote:
> Does it make sense for `_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS` to ever be 
> marked as `__declspec(dllimport)`?  The macro is applied on instantiations in 
> the implementation, not the header, so this shouldn't be visible to users.
It doesn't, which is why I always made it expand to either 
`__declspec(dllexport)` or empty.


https://reviews.llvm.org/D24679



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

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

It looks like the test case was removed when this patch we rebased.


https://reviews.llvm.org/D24693



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

[clang-tools-extra] r281920 - Trying to fix name conflict in change-namespace tool.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Sep 19 12:58:59 2016
New Revision: 281920

URL: http://llvm.org/viewvc/llvm-project?rev=281920&view=rev
Log:
Trying to fix name conflict in change-namespace tool.

Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/change-namespace/ChangeNamespace.h

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=281920&r1=281919&r2=281920&view=diff
==
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Mon Sep 19 
12:58:59 2016
@@ -327,8 +327,8 @@ void ChangeNamespaceTool::moveOldNamespa
   MoveNs.InsertionOffset = Result.SourceManager->getFileOffset(
   Result.SourceManager->getSpellingLoc(LocAfterNs));
 
-  MoveNs.FileID = Result.SourceManager->getFileID(Start);
-  MoveNs.SourceManager = Result.SourceManager;
+  MoveNs.FID = Result.SourceManager->getFileID(Start);
+  MoveNs.SourceMgr = Result.SourceManager;
   MoveNamespaces[R.getFilePath()].push_back(MoveNs);
 }
 
@@ -447,8 +447,8 @@ void ChangeNamespaceTool::onEndOfTransla
   continue;
 const std::string &FilePath = FileAndNsMoves.first;
 auto &Replaces = FileToReplacements[FilePath];
-auto &SM = *NsMoves.begin()->SourceManager;
-llvm::StringRef Code = SM.getBufferData(NsMoves.begin()->FileID);
+auto &SM = *NsMoves.begin()->SourceMgr;
+llvm::StringRef Code = SM.getBufferData(NsMoves.begin()->FID);
 auto ChangedCode = tooling::applyAllReplacements(Code, Replaces);
 if (!ChangedCode) {
   llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.h?rev=281920&r1=281919&r2=281920&view=diff
==
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h Mon Sep 19 
12:58:59 2016
@@ -84,8 +84,8 @@ private:
 // original code.
 unsigned InsertionOffset;
 // The file in which the namespace is declared.
-FileID FileID;
-SourceManager *SourceManager;
+FileID FID;
+SourceManager *SourceMgr;
   };
 
   // Information about inserting a class forward declaration.


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

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Akira Hatanaka via cfe-commits]

ahatanak added a subscriber: ahatanak.
ahatanak added a comment.

Can you add some test cases?


https://reviews.llvm.org/D24693



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

Re: [PATCH] D24679: [libc++] Fix extern template visibility for Windows

[Saleem Abdulrasool via cfe-commits]

compnerd added inline comments.


Comment at: include/__config:559
@@ -554,1 +558,3 @@
+# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
+# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
 #endif

Does it make sense for `_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS` to ever be 
marked as `__declspec(dllimport)`?  The macro is applied on instantiations in 
the implementation, not the header, so this shouldn't be visible to users.


https://reviews.llvm.org/D24679



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

Re: [PATCH] D24690: Replace __ANDROID__ with __BIONIC__.

[Dan Albert via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281921: Replace __ANDROID__ with __BIONIC__. (authored by 
danalbert).

Changed prior to commit:
  https://reviews.llvm.org/D24690?vs=71714&id=71852#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24690

Files:
  libcxx/trunk/include/__config
  libcxx/trunk/include/support/android/locale_bionic.h
  libcxx/trunk/src/locale.cpp

Index: libcxx/trunk/src/locale.cpp
===
--- libcxx/trunk/src/locale.cpp
+++ libcxx/trunk/src/locale.cpp
@@ -28,7 +28,7 @@
 #include "__sso_allocator"
 #if defined(_LIBCPP_MSVCRT) || defined(__MINGW32__)
 #include "support/win32/locale_win32.h"
-#elif !defined(__ANDROID__)
+#elif !defined(__BIONIC__)
 #include 
 #endif
 #include 
Index: libcxx/trunk/include/support/android/locale_bionic.h
===
--- libcxx/trunk/include/support/android/locale_bionic.h
+++ libcxx/trunk/include/support/android/locale_bionic.h
@@ -11,7 +11,7 @@
 #ifndef _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 #define _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 
-#if defined(__ANDROID__)
+#if defined(__BIONIC__)
 
 #ifdef __cplusplus
 extern "C" {
@@ -27,5 +27,5 @@
 #include 
 #include 
 
-#endif // defined(__ANDROID__)
+#endif // defined(__BIONIC__)
 #endif // _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
Index: libcxx/trunk/include/__config
===
--- libcxx/trunk/include/__config
+++ libcxx/trunk/include/__config
@@ -85,6 +85,13 @@
 #define __is_identifier(__x) 1
 #endif
 
+// Need to detect which libc we're using if we're on Linux.
+#if defined(__linux__)
+#include 
+#if !defined(__GLIBC_PREREQ)
+#define __GLIBC_PREREQ(a, b) 0
+#endif // !defined(__GLIBC_PREREQ)
+#endif // defined(__linux__)
 
 #ifdef __LITTLE_ENDIAN__
 #if __LITTLE_ENDIAN__
@@ -333,12 +340,9 @@
 #if defined(__FreeBSD__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #define _LIBCPP_HAS_C11_FEATURES
-#elif defined(__ANDROID__)
-#define _LIBCPP_HAS_QUICK_EXIT
 #elif defined(__linux__)
 #if !defined(_LIBCPP_HAS_MUSL_LIBC)
-# include 
-#if __GLIBC_PREREQ(2, 15)
+#if __GLIBC_PREREQ(2, 15) || defined(__BIONIC__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #endif
 #if __GLIBC_PREREQ(2, 17)
@@ -759,7 +763,7 @@
 
 #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
 // Most unix variants have catopen.  These are the specific ones that don't.
-#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION)
+#if !defined(_WIN32) && !defined(__BIONIC__) && !defined(_NEWLIB_VERSION)
 #define _LIBCPP_HAS_CATOPEN 1
 #endif
 #endif
@@ -885,7 +889,8 @@
 #define _LIBCPP_HAS_NO_STDOUT
 #endif
 
-#if defined(__ANDROID__) || defined(__CloudABI__) || 
defined(_LIBCPP_HAS_MUSL_LIBC)
+#if defined(__BIONIC__) || defined(__CloudABI__) ||
\
+defined(_LIBCPP_HAS_MUSL_LIBC)
 #define _LIBCPP_PROVIDES_DEFAULT_RUNE_TABLE
 #endif
 


Index: libcxx/trunk/src/locale.cpp
===
--- libcxx/trunk/src/locale.cpp
+++ libcxx/trunk/src/locale.cpp
@@ -28,7 +28,7 @@
 #include "__sso_allocator"
 #if defined(_LIBCPP_MSVCRT) || defined(__MINGW32__)
 #include "support/win32/locale_win32.h"
-#elif !defined(__ANDROID__)
+#elif !defined(__BIONIC__)
 #include 
 #endif
 #include 
Index: libcxx/trunk/include/support/android/locale_bionic.h
===
--- libcxx/trunk/include/support/android/locale_bionic.h
+++ libcxx/trunk/include/support/android/locale_bionic.h
@@ -11,7 +11,7 @@
 #ifndef _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 #define _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 
-#if defined(__ANDROID__)
+#if defined(__BIONIC__)
 
 #ifdef __cplusplus
 extern "C" {
@@ -27,5 +27,5 @@
 #include 
 #include 
 
-#endif // defined(__ANDROID__)
+#endif // defined(__BIONIC__)
 #endif // _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
Index: libcxx/trunk/include/__config
===
--- libcxx/trunk/include/__config
+++ libcxx/trunk/include/__config
@@ -85,6 +85,13 @@
 #define __is_identifier(__x) 1
 #endif
 
+// Need to detect which libc we're using if we're on Linux.
+#if defined(__linux__)
+#include 
+#if !defined(__GLIBC_PREREQ)
+#define __GLIBC_PREREQ(a, b) 0
+#endif // !defined(__GLIBC_PREREQ)
+#endif // defined(__linux__)
 
 #ifdef __LITTLE_ENDIAN__
 #if __LITTLE_ENDIAN__
@@ -333,12 +340,9 @@
 #if defined(__FreeBSD__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #define _LIBCPP_HAS_C11_FEATURES
-#elif defined(__ANDROID__)
-#define _LIBCPP_HAS_QUICK_EXIT
 #elif defined(__linux__)
 #if !defined(_LIBCPP_HAS_MUSL_LIBC)
-# include 
-#if __GLIBC_PREREQ(2, 15)
+#if __GLIBC_PREREQ(2, 15) || defined(__BIONIC__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #endif
 #if __GLIBC_PREREQ(2, 17)
@@ -759,7 +763,7 @@
 
 #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH_

[libcxx] r281921 - Replace __ANDROID__ with __BIONIC__.

[Dan Albert via cfe-commits]

Author: danalbert
Date: Mon Sep 19 13:00:45 2016
New Revision: 281921

URL: http://llvm.org/viewvc/llvm-project?rev=281921&view=rev
Log:
Replace __ANDROID__ with __BIONIC__.

Summary:
None of these checks are specific to Android devices. If libc++ was
used with Bionic on a normal Linux system these checks would still be
needed.

Reviewers: mclow.lists, EricWF

Subscribers: compnerd, tberghammer, danalbert, srhines, cfe-commits

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

Modified:
libcxx/trunk/include/__config
libcxx/trunk/include/support/android/locale_bionic.h
libcxx/trunk/src/locale.cpp

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=281921&r1=281920&r2=281921&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Mon Sep 19 13:00:45 2016
@@ -85,6 +85,13 @@
 #define __is_identifier(__x) 1
 #endif
 
+// Need to detect which libc we're using if we're on Linux.
+#if defined(__linux__)
+#include 
+#if !defined(__GLIBC_PREREQ)
+#define __GLIBC_PREREQ(a, b) 0
+#endif // !defined(__GLIBC_PREREQ)
+#endif // defined(__linux__)
 
 #ifdef __LITTLE_ENDIAN__
 #if __LITTLE_ENDIAN__
@@ -333,12 +340,9 @@ typedef __char32_t char32_t;
 #if defined(__FreeBSD__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #define _LIBCPP_HAS_C11_FEATURES
-#elif defined(__ANDROID__)
-#define _LIBCPP_HAS_QUICK_EXIT
 #elif defined(__linux__)
 #if !defined(_LIBCPP_HAS_MUSL_LIBC)
-# include 
-#if __GLIBC_PREREQ(2, 15)
+#if __GLIBC_PREREQ(2, 15) || defined(__BIONIC__)
 #define _LIBCPP_HAS_QUICK_EXIT
 #endif
 #if __GLIBC_PREREQ(2, 17)
@@ -759,7 +763,7 @@ template  struct __static_asse
 
 #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
 // Most unix variants have catopen.  These are the specific ones that don't.
-#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION)
+#if !defined(_WIN32) && !defined(__BIONIC__) && !defined(_NEWLIB_VERSION)
 #define _LIBCPP_HAS_CATOPEN 1
 #endif
 #endif
@@ -885,7 +889,8 @@ extern "C" void __sanitizer_annotate_con
 #define _LIBCPP_HAS_NO_STDOUT
 #endif
 
-#if defined(__ANDROID__) || defined(__CloudABI__) || 
defined(_LIBCPP_HAS_MUSL_LIBC)
+#if defined(__BIONIC__) || defined(__CloudABI__) ||
\
+defined(_LIBCPP_HAS_MUSL_LIBC)
 #define _LIBCPP_PROVIDES_DEFAULT_RUNE_TABLE
 #endif
 

Modified: libcxx/trunk/include/support/android/locale_bionic.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/support/android/locale_bionic.h?rev=281921&r1=281920&r2=281921&view=diff
==
--- libcxx/trunk/include/support/android/locale_bionic.h (original)
+++ libcxx/trunk/include/support/android/locale_bionic.h Mon Sep 19 13:00:45 
2016
@@ -11,7 +11,7 @@
 #ifndef _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 #define _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H
 
-#if defined(__ANDROID__)
+#if defined(__BIONIC__)
 
 #ifdef __cplusplus
 extern "C" {
@@ -27,5 +27,5 @@ extern "C" {
 #include 
 #include 
 
-#endif // defined(__ANDROID__)
+#endif // defined(__BIONIC__)
 #endif // _LIBCPP_SUPPORT_ANDROID_LOCALE_BIONIC_H

Modified: libcxx/trunk/src/locale.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/locale.cpp?rev=281921&r1=281920&r2=281921&view=diff
==
--- libcxx/trunk/src/locale.cpp (original)
+++ libcxx/trunk/src/locale.cpp Mon Sep 19 13:00:45 2016
@@ -28,7 +28,7 @@
 #include "__sso_allocator"
 #if defined(_LIBCPP_MSVCRT) || defined(__MINGW32__)
 #include "support/win32/locale_win32.h"
-#elif !defined(__ANDROID__)
+#elif !defined(__BIONIC__)
 #include 
 #endif
 #include 


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

Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

[Eric Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281918: A clang tool for changing surrouding namespaces of 
class/function definitions. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D24183?vs=71689&id=71848#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24183

Files:
  clang-tools-extra/trunk/CMakeLists.txt
  clang-tools-extra/trunk/change-namespace/CMakeLists.txt
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
  clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
  clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/trunk/test/CMakeLists.txt
  clang-tools-extra/trunk/test/change-namespace/simple-move.cpp
  clang-tools-extra/trunk/unittests/CMakeLists.txt
  clang-tools-extra/trunk/unittests/change-namespace/CMakeLists.txt
  clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

Index: clang-tools-extra/trunk/unittests/change-namespace/CMakeLists.txt
===
--- clang-tools-extra/trunk/unittests/change-namespace/CMakeLists.txt
+++ clang-tools-extra/trunk/unittests/change-namespace/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+  support
+  )
+
+get_filename_component(CHANGE_NAMESPACE_SOURCE_DIR
+  ${CMAKE_CURRENT_SOURCE_DIR}/../../change-namespace REALPATH)
+include_directories(
+  ${CHANGE_NAMESPACE_SOURCE_DIR}
+  )
+
+# We'd like clang/unittests/Tooling/RewriterTestContext.h in the test.
+include_directories(${CLANG_SOURCE_DIR})
+
+add_extra_unittest(ChangeNamespaceTests
+  ChangeNamespaceTests.cpp
+  )
+
+target_link_libraries(ChangeNamespaceTests
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangChangeNamespace
+  clangFormat
+  clangFrontend
+  clangRewrite
+  clangTooling
+  clangToolingCore
+  )
Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -0,0 +1,234 @@
+//===-- ChangeNamespaceTests.cpp - Change namespace unit tests ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ChangeNamespace.h"
+#include "unittests/Tooling/RewriterTestContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace change_namespace {
+namespace {
+
+class ChangeNamespaceTest : public ::testing::Test {
+public:
+  std::string runChangeNamespaceOnCode(llvm::StringRef Code) {
+clang::RewriterTestContext Context;
+clang::FileID ID = Context.createInMemoryFile(FileName, Code);
+
+std::map FileToReplacements;
+change_namespace::ChangeNamespaceTool NamespaceTool(
+OldNamespace, NewNamespace, FilePattern, &FileToReplacements);
+ast_matchers::MatchFinder Finder;
+NamespaceTool.registerMatchers(&Finder);
+std::unique_ptr Factory =
+tooling::newFrontendActionFactory(&Finder);
+tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
+   FileName);
+formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite);
+return format(Context.getRewrittenText(ID));
+  }
+
+  std::string format(llvm::StringRef Code) {
+tooling::Replacements Replaces = format::reformat(
+format::getLLVMStyle(), Code, {tooling::Range(0, Code.size())});
+auto ChangedCode = tooling::applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(ChangedCode));
+if (!ChangedCode) {
+  llvm::errs() << llvm::toString(ChangedCode.takeError());
+  return "";
+}
+return *ChangedCode;
+  }
+
+protected:
+  std::string FileName = "input.cc";
+  std::string OldNamespace = "na::nb";
+  std::string NewNamespace = "x::y";
+  std::string FilePattern = "input.cc";
+};
+
+TEST_F(ChangeNamespaceTest, NoMatchingNamespace) {
+  std::string Code = "namespace na {\n"
+ "namespace nx {\n"
+ "class A {};\n"
+ "} // namespace nx\n"
+ "} // n

[clang-tools-extra] r281918 - A clang tool for changing surrouding namespaces of class/function definitions.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Sep 19 12:40:32 2016
New Revision: 281918

URL: http://llvm.org/viewvc/llvm-project?rev=281918&view=rev
Log:
A clang tool for changing surrouding namespaces of class/function definitions.

Summary:
A tool for changing surrouding namespaces of class/function definitions while 
keeping
references to types in the changed namespace correctly qualified by prepending
namespace specifiers before them.

Example: test.cc
   namespace na {
   class X {};
   namespace nb {
   class Y { X x; };
   } // namespace nb
   } // namespace na

To move the definition of class Y from namespace "na::nb" to "x::y", run:
   clang-change-namespace --old_namespace "na::nb" \
 --new_namespace "x::y" --file_pattern "test.cc" test.cc --

Output:
   namespace na {
   class X {};
   } // namespace na
   namespace x {
   namespace y {
   class Y { na::X x; };
   } // namespace y
   } // namespace x

Reviewers: alexfh, omtcyfz, hokein

Subscribers: mgorny, klimek, djasper, beanz, alexshap, Eugene.Zelenko, 
cfe-commits

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

Added:
clang-tools-extra/trunk/change-namespace/
clang-tools-extra/trunk/change-namespace/CMakeLists.txt
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
clang-tools-extra/trunk/change-namespace/tool/
clang-tools-extra/trunk/change-namespace/tool/CMakeLists.txt
clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp
clang-tools-extra/trunk/test/change-namespace/
clang-tools-extra/trunk/test/change-namespace/simple-move.cpp
clang-tools-extra/trunk/unittests/change-namespace/
clang-tools-extra/trunk/unittests/change-namespace/CMakeLists.txt
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
Modified:
clang-tools-extra/trunk/CMakeLists.txt
clang-tools-extra/trunk/test/CMakeLists.txt
clang-tools-extra/trunk/unittests/CMakeLists.txt

Modified: clang-tools-extra/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CMakeLists.txt?rev=281918&r1=281917&r2=281918&view=diff
==
--- clang-tools-extra/trunk/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/CMakeLists.txt Mon Sep 19 12:40:32 2016
@@ -7,6 +7,7 @@ add_subdirectory(clang-tidy)
 add_subdirectory(clang-tidy-vs)
 endif()
 
+add_subdirectory(change-namespace)
 add_subdirectory(clang-query)
 add_subdirectory(include-fixer)
 add_subdirectory(pp-trace)

Added: clang-tools-extra/trunk/change-namespace/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/CMakeLists.txt?rev=281918&view=auto
==
--- clang-tools-extra/trunk/change-namespace/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/change-namespace/CMakeLists.txt Mon Sep 19 12:40:32 
2016
@@ -0,0 +1,19 @@
+set(LLVM_LINK_COMPONENTS
+  support
+  )
+
+add_clang_library(clangChangeNamespace
+  ChangeNamespace.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangTooling
+  clangToolingCore
+  )
+
+add_subdirectory(tool)

Added: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=281918&view=auto
==
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (added)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Mon Sep 19 
12:40:32 2016
@@ -0,0 +1,509 @@
+//===-- ChangeNamespace.cpp - Change namespace implementation 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "ChangeNamespace.h"
+#include "clang/Format/Format.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace change_namespace {
+
+namespace {
+
+inline std::string
+joinNamespaces(const llvm::SmallVectorImpl &Namespaces) {
+  if (Namespaces.empty())
+return "";
+  std::string Result = Namespaces.front();
+  for (auto I = Namespaces.begin() + 1, E = Namespaces.end(); I != E; ++I)
+Result += ("::" + *I).str();
+  return Result;
+}
+
+SourceLocation startLocationForType(TypeLoc TLoc) {
+  // For elaborated types (e.g. `struct a::A`) we want the portion after the
+  // `struct` but including the namespace qualifier, `a::`.
+  if (TLoc.getTypeLocClass() == TypeLoc::Elaborated) {
+NestedNameSpecifierLoc NestedNameSpecifier =
+TLoc.castAs().getQualifierLoc();
+if (NestedNameSpecifier.getNestedNameSpecifier())
+

Re: [PATCH] D24235: [OpenCL] Improve double literal handling

[Yaxun Liu via cfe-commits]

yaxunl added inline comments.


Comment at: lib/Sema/SemaExpr.cpp:3431
@@ -3410,1 +3430,3 @@
+.getSupportedOpenCLOpts()
+.cl_khr_fp64) ||
getOpenCLOptions().cl_khr_fp64)) {

This check 
  (getLangOpts().OpenCLVersion >= 120 &&
Context.getTargetInfo()
.getSupportedOpenCLOpts()
.cl_khr_fp64)

is redundant since for CL 1.2 and above getOpenCLOptions().cl_khr_fp64 is set 
to be true by default.


https://reviews.llvm.org/D24235



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

Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: lib/Sema/SemaExpr.cpp:8084
@@ +8083,3 @@
+  *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+  return LHSType;
+}

My understanding is that, when we have a compound assign like "LHS += RHS", 
this function (CheckVectorOperands) is supposed to return the result type (LHS 
+ RHS). However, it is returning different types for "<1 x T> += T"  and "T += 
<1 x T>" (the former returns <1 x T> and the latter returns T). Would 
CheckAssignmentOperands reject the compound statement if you returned the 
vector type here?

Also, are you planning to allow the same kind of conversions done above for 
non-compound assignment statements (e.g., <4 x short> += <2 x int>) in the 
future?


https://reviews.llvm.org/D24472



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

Re: [PATCH] D24380: [migrate-tool] Framework for a codebase-dependent migration tool.

[Eric Liu via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D24380#544720, @hokein wrote:

> Sorry for the delay. 
>  The patch only contains an unittest for `HeaderGenerato`r, which is not 
> quite enough. Should we create a fake migrate-tool binary to illustrate APIs 
> usage?


Done.

Good idea. I wanted to add a dummy tool once the interfaces are stable. But I 
guess now is the right time.



Comment at: migrate-tool/MigrateTool.h:50
@@ +49,3 @@
+  public:
+enum class MigrateType {
+  Class,

hokein wrote:
> What is the 'MigrateType' used for? I can't find any usage in the patch.
Deleted it for now. Will add it when it is actually used in the future.


https://reviews.llvm.org/D24380



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

Re: [PATCH] D24380: [migrate-tool] Framework for a codebase-dependent migration tool.

[Eric Liu via cfe-commits]

ioeric updated this revision to Diff 71845.
ioeric marked 4 inline comments as done.
ioeric added a comment.

- Added MigrationEnvironment class and a dummy migrate-tool binary.


https://reviews.llvm.org/D24380

Files:
  CMakeLists.txt
  migrate-tool/AffectedFilesFinder.h
  migrate-tool/BuildManager.h
  migrate-tool/CMakeLists.txt
  migrate-tool/DummyMigrationEnvironment.cpp
  migrate-tool/DummyMigrationEnvironment.h
  migrate-tool/HeaderGenerator.cpp
  migrate-tool/HeaderGenerator.h
  migrate-tool/MigrateTool.cpp
  migrate-tool/MigrateTool.h
  migrate-tool/MigrationEnvironment.h
  migrate-tool/RefactoringManager.h
  migrate-tool/tool/CMakeLists.txt
  migrate-tool/tool/ClangMigrateTool.cpp
  test/CMakeLists.txt
  test/migrate-tool/Inputs/old.h
  test/migrate-tool/old.cpp
  unittests/CMakeLists.txt
  unittests/migrate-tool/CMakeLists.txt
  unittests/migrate-tool/HeaderBuildTest.cpp

Index: unittests/migrate-tool/HeaderBuildTest.cpp
===
--- /dev/null
+++ unittests/migrate-tool/HeaderBuildTest.cpp
@@ -0,0 +1,100 @@
+//===-- HeaderGeneratorTest.cpp - HeaderGenerator unit tests --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "HeaderGenerator.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace migrate_tool {
+
+TEST(HeaderGenerator, Empty) {
+  HeaderGenerator Hdr("a.h");
+  std::string Expected = "#ifndef A_H\n"
+ "#define A_H\n"
+ "\n"
+ "#endif // A_H";
+  EXPECT_EQ(Expected, Hdr.generateContent());
+}
+
+TEST(HeaderGenerator, SingleAlias) {
+  HeaderGenerator Hdr("a/b/c.h");
+  Hdr.addAlias("na::nb::C", "x::y::Z");
+  std::string Expected = "#ifndef A_B_C_H\n"
+ "#define A_B_C_H\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "using C = ::x::y::Z;\n"
+ "} // namespace nb\n"
+ "} // namespace na\n"
+ "#endif // A_B_C_H";
+  EXPECT_EQ(Expected, Hdr.generateContent());
+}
+
+TEST(HeaderGenerator, SingleAliasWithIncludes) {
+  HeaderGenerator Hdr("a/b/c.h");
+  Hdr.addInclude("x/y/z.h");
+  Hdr.addInclude("x/y/zz.h");
+  Hdr.addAlias("na::nb::C", "x::y::Z");
+  std::string Expected = "#ifndef A_B_C_H\n"
+ "#define A_B_C_H\n"
+ "#include \"x/y/z.h\"\n"
+ "#include \"x/y/zz.h\"\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "using C = ::x::y::Z;\n"
+ "} // namespace nb\n"
+ "} // namespace na\n"
+ "#endif // A_B_C_H";
+  EXPECT_EQ(Expected, Hdr.generateContent());
+}
+
+TEST(HeaderGenerator, MultipleAliasInOneNamespace) {
+  HeaderGenerator Hdr("a/b/c.h");
+  Hdr.addAlias("na::nb::C", "x::y::Z");
+  Hdr.addAlias("na::nb::D", "x::y::D");
+  Hdr.addAlias("na::nb::Q", "x::y::Q");
+  std::string Expected = "#ifndef A_B_C_H\n"
+ "#define A_B_C_H\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "using C = ::x::y::Z;\n"
+ "using D = ::x::y::D;\n"
+ "using Q = ::x::y::Q;\n"
+ "} // namespace nb\n"
+ "} // namespace na\n"
+ "#endif // A_B_C_H";
+  EXPECT_EQ(Expected, Hdr.generateContent());
+}
+
+TEST(HeaderGenerator, AliasesInMultipleNamespace) {
+  HeaderGenerator Hdr("a/b/c.h");
+  Hdr.addAlias("nb::Q", "x::Q");
+  Hdr.addAlias("na::nb::C", "x::y::Z");
+  Hdr.addAlias("na::nc::D", "x::y::D");
+  Hdr.addAlias("na::nb::Q", "x::y::Q");
+  std::string Expected = "#ifndef A_B_C_H\n"
+ "#define A_B_C_H\n"
+ "namespace nb {\n"
+ "using Q = ::x::Q;\n"
+ "} // namespace nb\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "using C = ::x::y::Z;\n"
+ "using Q = ::x::y::Q;\n"
+ "} // namespace nb\n"
+ "namespace nc {\n"
+ "using D = ::x::y::D;\n"
+ "} // namespace nc\n"
+ "} // namespace na\n"
+ "#endif // A_B_C_H";
+  EXPECT_EQ(Expected, Hdr.generateContent());
+}
+
+} // namespace migrate_tool
+} // namespace clang
Index: unittests/migrate-tool/CMakeLists.txt
==

Re: r281278 - [DebugInfo] Deduplicate debug info limiting logic

[Reid Kleckner via cfe-commits]

On Mon, Sep 12, 2016 at 5:20 PM, David Blaikie  wrote:

> On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rnk
>> Date: Mon Sep 12 19:01:23 2016
>> New Revision: 281278
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281278&view=rev
>> Log:
>> [DebugInfo] Deduplicate debug info limiting logic
>>
>> We should be doing the same checks when a type is completed as we do
>> when a complete type is used during emission. Previously, we duplicated
>> the logic, and it got out of sync. This could be observed with
>> dllimported classes.
>>
>
> Thanks for having a go at this!
>
> The logic in completeType (& maybe also completeClassData) still seems to
> duplicate some of the logic in shouldOmitDefinition - I think the original
> idea I had was a series of cascading entry points (completeType checking
> the "oh, the type now has a definition", then falling into
> completeRequiredType, etc)
>
> In theory the "shouldOmitDefinition" could be split into the extra
> conditions for each of the parts of that cascade, so the checks wouldn't be
> redundantly executed when we were already being called about the completion
> or required completion of a type, etc. But I'm not sure the performance
> gain would be measurable, etc.
>
> If not, we might as well just have one entry point that's something like
> "revisit type because something interesting happened to it" (it became
> complete, it became required to be complete, etc)... but that seems a bit
> weird too.
>

Seems reasonable, but I didn't see a nice, clear, obvious refactoring to
do, so I left it here. Seems better than it was before.


>
>> Also reduce a test case for this slightly.
>>
>
> The UseCompleteType is still there/unnecessary (as is the 'private' in
> UnicodeString) - and I'd probably rename it to foo/bar and throw it in a
> namespace like the rest for consistency - having semantic names in a test
> when they're just remnants from whatever code it was reduced from seems
> confusing to me. But I realize I'm a bit more pedantic about that than most.
>

The private was necessary at some point during the reduction process for
some reason.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24693: [CodeGen] Don't emit lifetime intrinsics for some local variables

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/CodeGen/VarBypassDetector.h:50
@@ +49,3 @@
+public:
+  void Reset(const Stmt *Body);
+

rename to smth like StartFunction()?
add some API documentation.


https://reviews.llvm.org/D24693



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

r281915 - [OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Mon Sep 19 12:11:22 2016
New Revision: 281915

URL: http://llvm.org/viewvc/llvm-project?rev=281915&view=rev
Log:
[OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaOpenCL/half.cl
cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=281915&r1=281914&r2=281915&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 19 12:11:22 2016
@@ -7526,7 +7526,7 @@ enum OpenCLParamType {
   RecordKernelParam
 };
 
-static OpenCLParamType getOpenCLKernelParameterType(QualType PT) {
+static OpenCLParamType getOpenCLKernelParameterType(Sema &S, QualType PT) {
   if (PT->isPointerType()) {
 QualType PointeeType = PT->getPointeeType();
 if (PointeeType->isPointerType())
@@ -7547,7 +7547,10 @@ static OpenCLParamType getOpenCLKernelPa
   if (PT->isEventT())
 return InvalidKernelParam;
 
-  if (PT->isHalfType())
+  // OpenCL extension spec v1.2 s9.5:
+  // This extension adds support for half scalar and vector types as built-in
+  // types that can be used for arithmetic operations, conversions etc.
+  if (!S.getOpenCLOptions().cl_khr_fp16 && PT->isHalfType())
 return InvalidKernelParam;
 
   if (PT->isRecordType())
@@ -7568,7 +7571,7 @@ static void checkIsValidOpenCLKernelPara
   if (ValidTypes.count(PT.getTypePtr()))
 return;
 
-  switch (getOpenCLKernelParameterType(PT)) {
+  switch (getOpenCLKernelParameterType(S, PT)) {
   case PtrPtrKernelParam:
 // OpenCL v1.2 s6.9.a:
 // A kernel function argument cannot be declared as a
@@ -7595,7 +7598,10 @@ static void checkIsValidOpenCLKernelPara
 // OpenCL v1.2 s6.8 n:
 // A kernel function argument cannot be declared
 // of event_t type.
-S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
+// Do not diagnose half type since it is diagnosed as invalid argument
+// type for any function elsewhere.
+if (!PT->isHalfType())
+  S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
 D.setInvalidType();
 return;
 
@@ -7651,7 +7657,7 @@ static void checkIsValidOpenCLKernelPara
   if (ValidTypes.count(QT.getTypePtr()))
 continue;
 
-  OpenCLParamType ParamType = getOpenCLKernelParameterType(QT);
+  OpenCLParamType ParamType = getOpenCLKernelParameterType(S, QT);
   if (ParamType == ValidKernelParam)
 continue;
 

Modified: cfe/trunk/test/SemaOpenCL/half.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/half.cl?rev=281915&r1=281914&r2=281915&view=diff
==
--- cfe/trunk/test/SemaOpenCL/half.cl (original)
+++ cfe/trunk/test/SemaOpenCL/half.cl Mon Sep 19 12:11:22 2016
@@ -25,6 +25,9 @@ half half_disabled(half *p, // expected-
   return h;
 }
 
+kernel void half_disabled_kernel(global half *p,
+ half h);  // expected-error{{declaring 
function parameter of type 'half' is not allowed; did you forget * ?}}
+
 // Exactly the same as above but with the cl_khr_fp16 extension enabled.
 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
 constant half a = 1.0h;
@@ -48,3 +51,7 @@ half half_enabled(half *p, half h)
 
   return h;
 }
+
+kernel void half_enabled_kernel(global half *p,
+half h);
+

Modified: cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl?rev=281915&r1=281914&r2=281915&view=diff
==
--- cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl (original)
+++ cfe/trunk/test/SemaOpenCL/invalid-kernel-parameters.cl Mon Sep 19 12:11:22 
2016
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple spir-unknown-unknown
 
+kernel void half_arg(half x) { } // expected-error{{declaring function 
parameter of type 'half' is not allowed; did you forget * ?}}
+
 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
 
@@ -11,7 +13,8 @@
 
 kernel void bool_arg(bool x) { } // expected-error{{'bool' cannot be used as 
the type of a kernel parameter}}
 
-kernel void half_arg(half x) { } // expected-error{{'half' cannot be used as 
the type of a kernel parameter}}
+// half kernel argument is allowed when cl_khr_fp16 is enabled.
+kernel void half_arg(half x) { }
 
 typedef struct ContainsBool // expected-note{{within field of type 
'ContainsBool' declared here}}
 {


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

Re: [PATCH] D24666: [OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled

[Yaxun Liu via cfe-commits]

yaxunl marked 6 inline comments as done.


Comment at: lib/Sema/SemaDecl.cpp:7600
@@ +7599,3 @@
+// Do not diagnose half type since it is diagnosed as invalid argument
+// type for any function eleswhere.
+if (!PT->isHalfType())

Anastasia wrote:
> -> elsewhere
I will fix it in commit.


https://reviews.llvm.org/D24666



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

Re: r281278 - [DebugInfo] Deduplicate debug info limiting logic

[David Blaikie via cfe-commits]

ping

On Mon, Sep 12, 2016 at 5:20 PM David Blaikie  wrote:

> On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: rnk
> Date: Mon Sep 12 19:01:23 2016
> New Revision: 281278
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281278&view=rev
> Log:
> [DebugInfo] Deduplicate debug info limiting logic
>
> We should be doing the same checks when a type is completed as we do
> when a complete type is used during emission. Previously, we duplicated
> the logic, and it got out of sync. This could be observed with
> dllimported classes.
>
>
> Thanks for having a go at this!
>
> The logic in completeType (& maybe also completeClassData) still seems to
> duplicate some of the logic in shouldOmitDefinition - I think the original
> idea I had was a series of cascading entry points (completeType checking
> the "oh, the type now has a definition", then falling into
> completeRequiredType, etc)
>
> In theory the "shouldOmitDefinition" could be split into the extra
> conditions for each of the parts of that cascade, so the checks wouldn't be
> redundantly executed when we were already being called about the completion
> or required completion of a type, etc. But I'm not sure the performance
> gain would be measurable, etc.
>
> If not, we might as well just have one entry point that's something like
> "revisit type because something interesting happened to it" (it became
> complete, it became required to be complete, etc)... but that seems a bit
> weird too.
>
>
>
> Also reduce a test case for this slightly.
>
>
> The UseCompleteType is still there/unnecessary (as is the 'private' in
> UnicodeString) - and I'd probably rename it to foo/bar and throw it in a
> namespace like the rest for consistency - having semantic names in a test
> when they're just remnants from whatever code it was reduced from seems
> confusing to me. But I realize I'm a bit more pedantic about that than most.
>
> - Dave
>
>
>
> Implementing review feedback from David Blaikie on r281057.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281278&r1=281277&r2=281278&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Sep 12 19:01:23 2016
> @@ -1644,27 +1644,6 @@ void CGDebugInfo::completeType(const Rec
>  completeRequiredType(RD);
>  }
>
> -void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
> -  if (DebugKind <= codegenoptions::DebugLineTablesOnly)
> -return;
> -
> -  // If this is a dynamic class and we're emitting limited debug info,
> wait
> -  // until the vtable is emitted to complete the class debug info.
> -  if (DebugKind <= codegenoptions::LimitedDebugInfo) {
> -if (const auto *CXXDecl = dyn_cast(RD))
> -  if (CXXDecl->isDynamicClass())
> -return;
> -  }
> -
> -  if (DebugTypeExtRefs && RD->isFromASTFile())
> -return;
> -
> -  QualType Ty = CGM.getContext().getRecordType(RD);
> -  llvm::DIType *T = getTypeOrNull(Ty);
> -  if (T && T->isForwardDecl())
> -completeClassData(RD);
> -}
> -
>  void CGDebugInfo::completeClassData(const RecordDecl *RD) {
>if (DebugKind <= codegenoptions::DebugLineTablesOnly)
>  return;
> @@ -1763,6 +1742,16 @@ static bool shouldOmitDefinition(codegen
>return false;
>  }
>
> +void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
> +  if (shouldOmitDefinition(DebugKind, DebugTypeExtRefs, RD,
> CGM.getLangOpts()))
> +return;
> +
> +  QualType Ty = CGM.getContext().getRecordType(RD);
> +  llvm::DIType *T = getTypeOrNull(Ty);
> +  if (T && T->isForwardDecl())
> +completeClassData(RD);
> +}
> +
>  llvm::DIType *CGDebugInfo::CreateType(const RecordType *Ty) {
>RecordDecl *RD = Ty->getDecl();
>llvm::DIType *T = cast_or_null(getTypeOrNull(QualType(Ty,
> 0)));
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281278&r1=281277&r2=281278&view=diff
>
> ==
> --- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Mon Sep 12
> 19:01:23 2016
> @@ -6,10 +6,7 @@
>  // more general than that.
>
>  struct UnicodeString;
> -struct GetFwdDecl {
> -  static UnicodeString format;
> -};
> -GetFwdDecl force_fwd_decl;
> +UnicodeString *force_fwd_decl;
>  struct UnicodeString {
>  private:
>virtual ~UnicodeString();
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
> URL:
> http://llvm.org/viewvc/ll

Re: [PATCH] D9168: [NVPTX] Check if callsite is defined when computing argument allignment

[Valentin Churavy via cfe-commits]

vchuravy added a comment.

I have a potential test-case for this, but the patch doesn't apply cleanly to 
master so I was unable to test if this solves the problem.

F2433038: strip.ll 


https://reviews.llvm.org/D9168



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

Re: [PATCH] D9168: [NVPTX] Check if callsite is defined when computing argument allignment

[Jacques Pienaar via cfe-commits]

jpienaar added a comment.

Oh, sorry, I didn't see your response before I clicked abandoned. It has been a 
while, so this patch is pretty stale.


https://reviews.llvm.org/D9168



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

Re: [PATCH] D24666: [OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled

[Yaxun Liu via cfe-commits]

yaxunl added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:7599-7602
@@ -7595,3 +7598,6 @@
 // of event_t type.
-S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
+// Do not diagnose half type since it is diagnosed as invalid argument
+// type for any function eleswhere.
+if (!PT->isHalfType())
+  S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
 D.setInvalidType();

bader wrote:
> yaxunl wrote:
> > bader wrote:
> > > It looks strange to me. First we check if parameter type is half - we set 
> > > InvalidKernelParam status, later we check again and do not report an 
> > > error.
> > > I think it would be simpler just return ValidKernelParam for half data 
> > > type from getOpenCLKernelParameterType,
> > > 
> > > I think the whole patch should two deleted lines from 
> > > getOpenCLKernelParameterType function + test case.
> > getOpenCLKernelParameterType should report half type as InvalidKernelParam 
> > since it really is an invalid kernel argument type. We do not emit 
> > diagnostic msg because the msg is redundant, not because half type is a 
> > valid kernel argument type. 
> > 
> > getOpenCLKernelParameterType may be used for other purpose. Reporting half 
> > type as a valid kernel argument violates the semantics of 
> > getOpenCLKernelParameterType and can cause confusion and potential error.
> Maybe we should the other way.
> Leave half parameter check here only and remove duplicate check that reports 
> "declaring function parameter of type 'half' is not allowed; did you forget * 
> ?" message.
Clang can emit two error msgs in this case since it does two independent checks:

1. half type cannot be used as function argument when cl_khr_fp16 is disabled

2. half type cannot be used as kernel function argument when cl_khr_fp16 is 
disabled

I think error msg 1 is better than error msg 2 since error msg 1 provides more 
information, i.e., when cl_khr_fp16 is disabled, half type cannot be used as 
either kernel function argument or non-kernel function argument, whereas error 
msg 2 may give user the wrong impression that half type can not be used for 
kernel function argument only.


https://reviews.llvm.org/D24666



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

r281910 - Remove InstructionCombining and its related pass from sample pgo passes as we can handle "invoke" correctly.

[Dehao Chen via cfe-commits]

Author: dehao
Date: Mon Sep 19 11:02:52 2016
New Revision: 281910

URL: http://llvm.org/viewvc/llvm-project?rev=281910&view=rev
Log:
Remove InstructionCombining and its related pass from sample pgo passes as we 
can handle "invoke" correctly.

Summary: We previously relies on InstructionCombining pass to remove invoke 
instructions. Now that we can inline invoke instructions correctly, we do not 
need these passes any more.

Reviewers: dnovillo

Subscribers: llvm-commits

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

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/pgo-sample.c

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=281910&r1=281909&r2=281910&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Sep 19 11:02:52 2016
@@ -149,17 +149,6 @@ static void addAddDiscriminatorsPass(con
   PM.add(createAddDiscriminatorsPass());
 }
 
-static void addCleanupPassesForSampleProfiler(
-const PassManagerBuilder &Builder, legacy::PassManagerBase &PM) {
-  // instcombine is needed before sample profile annotation because it converts
-  // certain function calls to be inlinable. simplifycfg and sroa are needed
-  // before instcombine for necessary preparation. E.g. load store is 
eliminated
-  // properly so that instcombine will not introduce unecessary liverange.
-  PM.add(createCFGSimplificationPass());
-  PM.add(createSROAPass());
-  PM.add(createInstructionCombiningPass());
-}
-
 static void addBoundsCheckingPass(const PassManagerBuilder &Builder,
   legacy::PassManagerBase &PM) {
   PM.add(createBoundsCheckingPass());
@@ -464,8 +453,6 @@ void EmitAssemblyHelper::CreatePasses(le
   if (!CodeGenOpts.SampleProfileFile.empty()) {
 MPM.add(createPruneEHPass());
 MPM.add(createSampleProfileLoaderPass(CodeGenOpts.SampleProfileFile));
-PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
-   addCleanupPassesForSampleProfiler);
   }
 
   PMBuilder.populateFunctionPassManager(FPM);

Modified: cfe/trunk/test/CodeGen/pgo-sample.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pgo-sample.c?rev=281910&r1=281909&r2=281910&view=diff
==
--- cfe/trunk/test/CodeGen/pgo-sample.c (original)
+++ cfe/trunk/test/CodeGen/pgo-sample.c Mon Sep 19 11:02:52 2016
@@ -2,8 +2,5 @@
 //
 // Ensure Pass PGOInstrumentationGenPass is invoked.
 // RUN: %clang_cc1 -O2 -fprofile-sample-use=%S/Inputs/pgo-sample.prof %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s
-// CHECK: Simplify the CFG
-// CHECK: SROA
-// CHECK: Combine redundant instructions
 // CHECK: Remove unused exception handling info
 // CHECK: Sample profile pass


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

r281907 - Remove excessive padding from the struct CallStackFrame

[Alexander Shaposhnikov via cfe-commits]

Author: alexshap
Date: Mon Sep 19 10:57:29 2016
New Revision: 281907

URL: http://llvm.org/viewvc/llvm-project?rev=281907&view=rev
Log:
Remove excessive padding from the struct CallStackFrame

The struct CallStackFrame is in lib/AST/ExprConstant.cpp
inside anonymous namespace.
This diff reorders the fields and removes excessive padding.
Test plan: make -j8 check-clang

Differential revision: https://reviews.llvm.org/D23901

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=281907&r1=281906&r2=281907&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 19 10:57:29 2016
@@ -310,15 +310,9 @@ namespace {
 /// Parent - The caller of this stack frame.
 CallStackFrame *Caller;
 
-/// CallLoc - The location of the call expression for this call.
-SourceLocation CallLoc;
-
 /// Callee - The function which was called.
 const FunctionDecl *Callee;
 
-/// Index - The call index of this call.
-unsigned Index;
-
 /// This - The binding for the this pointer in this call, if any.
 const LValue *This;
 
@@ -333,6 +327,12 @@ namespace {
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
+/// CallLoc - The location of the call expression for this call.
+SourceLocation CallLoc;
+
+/// Index - The call index of this call.
+unsigned Index;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);


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

Re: [PATCH] D23901: Minor cleanup of the class CallStackFrame

[Alexander Shaposhnikov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281907: Remove excessive padding from the struct 
CallStackFrame (authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D23901?vs=69310&id=71838#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23901

Files:
  cfe/trunk/lib/AST/ExprConstant.cpp

Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -310,15 +310,9 @@
 /// Parent - The caller of this stack frame.
 CallStackFrame *Caller;
 
-/// CallLoc - The location of the call expression for this call.
-SourceLocation CallLoc;
-
 /// Callee - The function which was called.
 const FunctionDecl *Callee;
 
-/// Index - The call index of this call.
-unsigned Index;
-
 /// This - The binding for the this pointer in this call, if any.
 const LValue *This;
 
@@ -333,6 +327,12 @@
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
+/// CallLoc - The location of the call expression for this call.
+SourceLocation CallLoc;
+
+/// Index - The call index of this call.
+unsigned Index;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -310,15 +310,9 @@
 /// Parent - The caller of this stack frame.
 CallStackFrame *Caller;
 
-/// CallLoc - The location of the call expression for this call.
-SourceLocation CallLoc;
-
 /// Callee - The function which was called.
 const FunctionDecl *Callee;
 
-/// Index - The call index of this call.
-unsigned Index;
-
 /// This - The binding for the this pointer in this call, if any.
 const LValue *This;
 
@@ -333,6 +327,12 @@
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
+/// CallLoc - The location of the call expression for this call.
+SourceLocation CallLoc;
+
+/// Index - The call index of this call.
+unsigned Index;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24235: [OpenCL] Improve double literal handling

[Neil Hickey via cfe-commits]

neil.hickey added a reviewer: tstellarAMD.
neil.hickey updated this revision to Diff 71837.
neil.hickey added a comment.
Herald added subscribers: yaxunl, wdng.

There was a bug whereby an implicitcast was being applied from float to float, 
this caused issues later on in builin processing which caused an assertion. 
This changed patch removes the duplicate, superfluous, cast.


https://reviews.llvm.org/D24235

Files:
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/fpmath.cl
  test/SemaOpenCL/extensions.cl

Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -1,13 +1,16 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.1
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.2 -DFP64
 
 // Test with a target not supporting fp64.
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64
 
+#if __OPENCL_C_VERSION__ < 120
 void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
 }
+#endif
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 #ifdef NOFP64
@@ -21,16 +24,22 @@
 #endif
 
   (void) 1.0;
+#ifdef FP64
+// expected-no-diagnostics
+#endif
+
 #ifdef NOFP64
-// expected-warning@-2{{double precision constant requires cl_khr_fp64, casting to single precision}}
+// expected-warning@-6{{double precision constant requires cl_khr_fp64, casting to single precision}}
 #endif
 }
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : disable
 #ifdef NOFP64
 // expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
 #endif
 
+#if __OPENCL_C_VERSION__ < 120
 void f3(void) {
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
 }
+#endif
Index: test/CodeGenOpenCL/fpmath.cl
===
--- test/CodeGenOpenCL/fpmath.cl
+++ test/CodeGenOpenCL/fpmath.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown | FileCheck --check-prefix=CHECK --check-prefix=NODIVOPT %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown -cl-fp32-correctly-rounded-divide-sqrt | FileCheck --check-prefix=CHECK --check-prefix=DIVOPT %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -DNOFP64 -cl-std=CL1.1 -triple r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck --check-prefix=CHECK-DBL %s
 
 typedef __attribute__(( ext_vector_type(4) )) float float4;
 
@@ -21,14 +22,26 @@
   return a / b;
 }
 
+void printf(constant char* fmt, ...);
+
+#ifndef NOFP64
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
+#endif
+void testdbllit(long *val) {
+  // CHECK-DBL: float 2.00e+01
+  // CHECK: double 2.00e+01
+  printf("%f", 20.0);
+}
 
+#ifndef NOFP64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
   // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
+#endif
 
 // CHECK: attributes #[[ATTR]] = {
 // NODIVOPT: "correctly-rounded-divide-sqrt-fp-math"="false"
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1401,7 +1401,8 @@
   Result = Context.DoubleTy;
 
 if (S.getLangOpts().OpenCL &&
-!((S.getLangOpts().OpenCLVersion >= 120) ||
+!((S.getLangOpts().OpenCLVersion >= 120 
+   && S.Context.getTargetInfo().getSupportedOpenCLOpts().cl_khr_fp64) ||
   S.getOpenCLOptions().cl_khr_fp64)) {
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_requires_extension)
   << Result << "cl_khr_fp64";
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -716,9 +716,13 @@
   if (getLangOpts().ObjCAutoRefCount &&
   E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
 Cleanup.setExprNeedsCleanups(true);
+  
+  ExprResult Res = E;
 
-  ExprResult Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
-nullptr, VK_RValue);
+  if ( T != E->getType()) {
+Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
+   nullptr, VK_RValue);
+  }
 
   // C11 6.3.2.1p2:
   //   ... if the lvalue has atomic type, the value has the non-atomic version 
@@ -828,8 +832,20 @@
   // double.
   const BuiltinType *BTy = Ty->getAs();
   if (BTy && (BTy->getKind() == BuiltinType::Half ||
-  BTy->getKind() == BuiltinType::Float))
-E = ImpCastExprToType(

Re: [PATCH] D24507: Add attribute for return values that shouldn't be cast to bool

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24507#546241, @urusant wrote:

> Thank you for the feedback.
>
> > The patch is missing Sema tests for the attribute (that it only applies to 
> > declarations you expect, accepts no args, etc).
>
>
> There is one test case for that in test/ReturnNonBoolTestCompileTime.cpp. 
> I've added another one for attribute accepting no args, so now the last two 
> test cases in this file are those you were asking about. Can you think of any 
> other cases of invalid attribute usage?


We try to keep our tests segregated by functionality. e.g., tests relating to 
the way the attribute is handled (what it appertains to, args, etc) should live 
in Sema, tests relating to the static analyzer behavior should live in 
test/Analysis, etc.

Tests that are still missing are: applying to a non-function type, applying to 
a member function, applying to an Obj-C method. For member functions, what 
should happen if the function is virtual? What if the overriders do not specify 
the attribute? What if an override specifies the attribute but the base does 
not?

> > Have you considered making this a type attribute on the return type of the 
> > function rather than a declaration attribute on the function declaration?

> 

> 

> No, I hadn't. On a quick look though, I couldn't find a way to simplify my 
> solution using this idea, because as far as I understand, the type attribute 
> isn't inherited, so, for example, if I have something like `int r = 
> X509_verify_cert(...)` and the function `X509_verify_cert` has a return type 
> with attribute, `r` won't have the attribute. If that is correct, we still 
> need to backtrace the value to the function declaration. Is there something I 
> am missing?


I was thinking it would be diagnosed if you attempted to assign from your 
attributed type to a type that is not compatible. However, that may still be 
problematic because it raises other questions (can you SFINAE on it? Overload? 
etc).


Repository:
  rL LLVM

https://reviews.llvm.org/D24507



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

Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

[Jonas Devlieghere via cfe-commits]

JDevlieghere updated this revision to Diff 71835.
JDevlieghere added a comment.
Herald added subscribers: mgorny, beanz.

Still working on comment #2 from Alex but wanted to update my diff since it's 
been a while and I haven't gotten around to looking into it further. So no need 
to review yet.


Repository:
  rL LLVM

https://reviews.llvm.org/D22725

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseAlgorithmCheck.cpp
  clang-tidy/modernize/UseAlgorithmCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-algorithm.rst
  test/clang-tidy/modernize-use-algorithm.cpp

Index: test/clang-tidy/modernize-use-algorithm.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-algorithm.cpp
@@ -0,0 +1,156 @@
+// RUN: %check_clang_tidy %s modernize-use-algorithm %t
+// CHECK-FIXES: #include 
+
+namespace std {
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *src, std::size_t count);
+void *memset(void *dest, int ch, std::size_t count);
+
+template 
+OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
+
+template 
+OutputIt move(InputIt first, InputIt last, OutputIt d_first);
+
+template 
+void fill(ForwardIt first, ForwardIt last, const T &value);
+}
+
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *source, size_t count);
+void *memset(void *dest, int ch, size_t count);
+
+namespace alternative_std {
+using ::memcpy;
+using ::memset;
+}
+
+namespace awful {
+void memcpy(int, int, int);
+}
+
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *source, size_t size);
+
+void a() {
+  char foo[] = "foo", bar[4];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+  alternative_std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+  void *baz = bar;
+  std::memcpy(baz, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+
+  memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+  std::copy(foo, foo + sizeof bar, bar);
+}
+
+void b() {
+  char foo[] = "foobar";
+  std::memset(foo, 'a', 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::fill(foo, foo + 3, 'a');
+
+  alternative_std::memset(foo, 'a', 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::fill(foo, foo + 3, 'a');
+
+  std::memset(foo, 1, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+
+  std::fill(foo, foo + 2, 'a');
+}
+
+#define MEMCPY(dest, src) std::memcpy((dest), (src), sizeof(dest))
+void c() {
+  char foo[] = "foo", bar[3];
+  MEMCPY(bar, foo);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+}
+
+void d() {
+  typedef char foo_t;
+  typedef char bar_t;
+  foo_t foo[] = "foo";
+  bar_t bar[4];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void e() {
+  typedef const char *foo_t;
+  typedef const char *bar_t;
+  foo_t foo[] = {"foo", "bar"};
+  bar_t bar[2];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void f() {
+  typedef int some_type;
+  typedef some_type *const *volatile *foo_ptr;
+
+  typedef int *const some_other_type;
+  typedef some_other_type *volatile *bar_ptr;
+
+  foo_ptr foo[4];
+  bar_ptr bar[3];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void g() {
+  int foo[3][4] = {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 10, 11}};
+  int bar[2][4];
+
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MES

Re: [PATCH] D24682: [PR30341] Alias must point to a definition

[Sebastian Pop via cfe-commits]

sebpop added a comment.

The change looks good to me.  Somebody else should approve.


https://reviews.llvm.org/D24682



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

Re: [PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304

[Daphne Pfister via cfe-commits]

daphnediane added a comment.

In https://reviews.llvm.org/D24703#545917, @djasper wrote:

> Looks good. Thank you!


Thanks. Are there additional steps I need to get it commited or do I just wait 
for a commiter to notice?


https://reviews.llvm.org/D24703



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

Re: [PATCH] D24666: [OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled

[Alexey Bader via cfe-commits]

bader added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:7599-7602
@@ -7595,3 +7598,6 @@
 // of event_t type.
-S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
+// Do not diagnose half type since it is diagnosed as invalid argument
+// type for any function eleswhere.
+if (!PT->isHalfType())
+  S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT;
 D.setInvalidType();

yaxunl wrote:
> bader wrote:
> > It looks strange to me. First we check if parameter type is half - we set 
> > InvalidKernelParam status, later we check again and do not report an error.
> > I think it would be simpler just return ValidKernelParam for half data type 
> > from getOpenCLKernelParameterType,
> > 
> > I think the whole patch should two deleted lines from 
> > getOpenCLKernelParameterType function + test case.
> getOpenCLKernelParameterType should report half type as InvalidKernelParam 
> since it really is an invalid kernel argument type. We do not emit diagnostic 
> msg because the msg is redundant, not because half type is a valid kernel 
> argument type. 
> 
> getOpenCLKernelParameterType may be used for other purpose. Reporting half 
> type as a valid kernel argument violates the semantics of 
> getOpenCLKernelParameterType and can cause confusion and potential error.
Maybe we should the other way.
Leave half parameter check here only and remove duplicate check that reports 
"declaring function parameter of type 'half' is not allowed; did you forget * 
?" message.


https://reviews.llvm.org/D24666



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

Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.

[Eric Liu via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D24717#546279, @djasper wrote:

> I actually think this is a good example. So lets assume we'd write a tool to 
> fully quote binary expressions, e.g. that turns
>
>   if (a * b + c * d == 10) ...
>   
>
> into
>
>   if (((a * b) + (c * d)) == 10) ...
>   
>
> So, here, we would be inserting two "(" and two ")" at the same locations. 
> And, as you correctly mention, the order doesn't matter because we are 
> inserting the same string twice. I think this is actually good behavior.


I agree that this is good behavior.

> Deduplication is an interesting concern, but I think we probably want to 
> handle that at a different layer. E.g. in the use case above, deduplicating 
> would be quite fatal :).


Okay, it does make more sense to handle deduplication in a different layer.

So, with this assumption, the implementation should be much easier now: when 
there is conflict found in `add`, check this condition. If `A` and `B` are 
`order-dependent` as defined above, we then 
`merge(getReplacementInChangedCode(B))` into the set.


https://reviews.llvm.org/D24717



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

Re: [PATCH] D24682: [PR30341] Alias must point to a definition

[Aditya Kumar via cfe-commits]

hiraditya marked 2 inline comments as done.
hiraditya added a comment.

https://reviews.llvm.org/D24682



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

Re: [PATCH] D24682: [PR30341] Alias must point to a definition

[Aditya Kumar via cfe-commits]

hiraditya updated this revision to Diff 71827.
hiraditya added a comment.

Added a test case (contributed by Eric).


https://reviews.llvm.org/D24682

Files:
  clang/lib/CodeGen/CGCXX.cpp
  clang/test/CodeGenCXX/alias-available-externally.cpp

Index: clang/test/CodeGenCXX/alias-available-externally.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/alias-available-externally.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -O1 -std=c++11 -emit-llvm -disable-llvm-passes -o - %s | 
FileCheck %s
+// Clang should not generate alias to available_externally definitions.
+// Check that the destructor of Foo is defined.
+// CHECK: define linkonce_odr void @_ZN3FooD2Ev
+template 
+struct String {
+  String() {}
+  ~String();
+};
+
+template 
+inline __attribute__((visibility("hidden"), always_inline))
+String::~String() {}
+
+extern template struct String;
+
+struct Foo : public String { Foo() { String s; } };
+
+Foo f;
Index: clang/lib/CodeGen/CGCXX.cpp
===
--- clang/lib/CodeGen/CGCXX.cpp
+++ clang/lib/CodeGen/CGCXX.cpp
@@ -134,6 +134,16 @@
   llvm::GlobalValue::LinkageTypes TargetLinkage =
   getFunctionLinkage(TargetDecl);
 
+  // Disallow aliases to available_externally because available_externally
+  // will not be there in the end to allow the creation of the alias (PR30341).
+  // FIXME: An extern template instantiation will create functions with
+  // linkage "AvailableExternally". In libc++, some classes also define
+  // members with attribute "AlwaysInline" and expect no reference to
+  // be generated. It is desirable to reenable this optimisation after
+  // corresponding LLVM changes.
+  if (TargetLinkage == llvm::GlobalValue::AvailableExternallyLinkage)
+return true;
+
   // Check if we have it already.
   StringRef MangledName = getMangledName(AliasDecl);
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
@@ -157,13 +167,7 @@
   // Instead of creating as alias to a linkonce_odr, replace all of the uses
   // of the aliasee.
   if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) &&
- (TargetLinkage != llvm::GlobalValue::AvailableExternallyLinkage ||
-  !TargetDecl.getDecl()->hasAttr())) {
-// FIXME: An extern template instantiation will create functions with
-// linkage "AvailableExternally". In libc++, some classes also define
-// members with attribute "AlwaysInline" and expect no reference to
-// be generated. It is desirable to reenable this optimisation after
-// corresponding LLVM changes.
+  !TargetDecl.getDecl()->hasAttr()) {
 addReplacement(MangledName, Aliasee);
 return false;
   }


Index: clang/test/CodeGenCXX/alias-available-externally.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/alias-available-externally.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -O1 -std=c++11 -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+// Clang should not generate alias to available_externally definitions.
+// Check that the destructor of Foo is defined.
+// CHECK: define linkonce_odr void @_ZN3FooD2Ev
+template 
+struct String {
+  String() {}
+  ~String();
+};
+
+template 
+inline __attribute__((visibility("hidden"), always_inline))
+String::~String() {}
+
+extern template struct String;
+
+struct Foo : public String { Foo() { String s; } };
+
+Foo f;
Index: clang/lib/CodeGen/CGCXX.cpp
===
--- clang/lib/CodeGen/CGCXX.cpp
+++ clang/lib/CodeGen/CGCXX.cpp
@@ -134,6 +134,16 @@
   llvm::GlobalValue::LinkageTypes TargetLinkage =
   getFunctionLinkage(TargetDecl);
 
+  // Disallow aliases to available_externally because available_externally
+  // will not be there in the end to allow the creation of the alias (PR30341).
+  // FIXME: An extern template instantiation will create functions with
+  // linkage "AvailableExternally". In libc++, some classes also define
+  // members with attribute "AlwaysInline" and expect no reference to
+  // be generated. It is desirable to reenable this optimisation after
+  // corresponding LLVM changes.
+  if (TargetLinkage == llvm::GlobalValue::AvailableExternallyLinkage)
+return true;
+
   // Check if we have it already.
   StringRef MangledName = getMangledName(AliasDecl);
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
@@ -157,13 +167,7 @@
   // Instead of creating as alias to a linkonce_odr, replace all of the uses
   // of the aliasee.
   if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) &&
- (TargetLinkage != llvm::GlobalValue::AvailableExternallyLinkage ||
-  !TargetDecl.getDecl()->hasAttr())) {
-// FIXME: An extern template instantiation will create functions with
-// linkage "AvailableExternally". In libc++, some classes also define
-// members with attribute "AlwaysInline" and expect no reference to
-// be generated. It i

Re: [PATCH] D24626: [OpenCL] Diagnose assignment to dereference of half type pointer

[Yaxun Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281904: [OpenCL] Diagnose assignment to dereference of half 
type pointer (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D24626?vs=71548&id=71823#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24626

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/SemaOpenCL/half.cl

Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -10100,6 +10100,16 @@
   QualType LHSType = LHSExpr->getType();
   QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() :
  CompoundType;
+  // OpenCL v1.2 s6.1.1.1 p2:
+  // The half data type can only be used to declare a pointer to a buffer that
+  // contains half values
+  if (getLangOpts().OpenCL && !getOpenCLOptions().cl_khr_fp16 &&
+LHSType->isHalfType()) {
+Diag(Loc, diag::err_opencl_half_load_store) << 1
+<< LHSType.getUnqualifiedType();
+return QualType();
+  }
+
   AssignConvertType ConvTy;
   if (CompoundType.isNull()) {
 Expr *RHSCheck = RHS.get();
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -659,7 +659,8 @@
 def err_parameters_retval_cannot_have_fp16_type : Error<
   "%select{parameters|function return value}0 cannot have __fp16 type; did you 
forget * ?">;
 def err_opencl_half_load_store : Error<
-  "%select{loading directly from|assigning directly to}0 pointer to type %1 is 
not allowed">;
+  "%select{loading directly from|assigning directly to}0 pointer to type %1 
requires "
+  "cl_khr_fp16. Use vector data %select{load|store}0 builtin functions 
instead">;
 def err_opencl_cast_to_half : Error<"casting to type %0 is not allowed">;
 def err_opencl_half_declaration : Error<
   "declaring variable of type %0 is not allowed">;
Index: cfe/trunk/test/SemaOpenCL/half.cl
===
--- cfe/trunk/test/SemaOpenCL/half.cl
+++ cfe/trunk/test/SemaOpenCL/half.cl
@@ -8,8 +8,10 @@
 {
   half a[2]; // expected-error{{declaring variable of type 'half [2]' is not 
allowed}}
   half b;// expected-error{{declaring variable of type 'half' is not 
allowed}}
-  *p; // expected-error{{loading directly from pointer to type 'half' is not 
allowed}}
-  p[1]; // expected-error{{loading directly from pointer to type 'half' is not 
allowed}}
+  *p; // expected-error{{loading directly from pointer to type 'half' requires 
cl_khr_fp16. Use vector data load builtin functions instead}}
+  *p = 0; // expected-error{{assigning directly to pointer to type 'half' 
requires cl_khr_fp16. Use vector data store builtin functions instead}}
+  p[1]; // expected-error{{loading directly from pointer to type 'half' 
requires cl_khr_fp16. Use vector data load builtin functions instead}}
+  p[1] = 0; // expected-error{{assigning directly to pointer to type 'half' 
requires cl_khr_fp16. Use vector data store builtin functions instead}}
 
   float c = 1.0f;
   b = (half) c;  // expected-error{{casting to type 'half' is not allowed}}
@@ -31,7 +33,9 @@
   half a[2];
   half b;
   *p;
+  *p = 0;
   p[1];
+  p[1] = 0;
 
   float c = 1.0f;
   b = (half) c;


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -10100,6 +10100,16 @@
   QualType LHSType = LHSExpr->getType();
   QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() :
  CompoundType;
+  // OpenCL v1.2 s6.1.1.1 p2:
+  // The half data type can only be used to declare a pointer to a buffer that
+  // contains half values
+  if (getLangOpts().OpenCL && !getOpenCLOptions().cl_khr_fp16 &&
+LHSType->isHalfType()) {
+Diag(Loc, diag::err_opencl_half_load_store) << 1
+<< LHSType.getUnqualifiedType();
+return QualType();
+  }
+
   AssignConvertType ConvTy;
   if (CompoundType.isNull()) {
 Expr *RHSCheck = RHS.get();
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -659,7 +659,8 @@
 def err_parameters_retval_cannot_have_fp16_type : Error<
   "%select{parameters|function return value}0 cannot have __fp16 type; did you forget * ?">;
 def err_opencl_half_load_store : Error<
-  "%select{loading directly from|assigning directly to}0 pointer to type %1 is not allowed">;
+  "%select{loading directly from|assigning directly to}0 po

r281904 - [OpenCL] Diagnose assignment to dereference of half type pointer

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Mon Sep 19 09:54:41 2016
New Revision: 281904

URL: http://llvm.org/viewvc/llvm-project?rev=281904&view=rev
Log:
[OpenCL] Diagnose assignment to dereference of half type pointer

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaOpenCL/half.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=281904&r1=281903&r2=281904&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 19 09:54:41 
2016
@@ -659,7 +659,8 @@ def err_object_cannot_be_passed_returned
 def err_parameters_retval_cannot_have_fp16_type : Error<
   "%select{parameters|function return value}0 cannot have __fp16 type; did you 
forget * ?">;
 def err_opencl_half_load_store : Error<
-  "%select{loading directly from|assigning directly to}0 pointer to type %1 is 
not allowed">;
+  "%select{loading directly from|assigning directly to}0 pointer to type %1 
requires "
+  "cl_khr_fp16. Use vector data %select{load|store}0 builtin functions 
instead">;
 def err_opencl_cast_to_half : Error<"casting to type %0 is not allowed">;
 def err_opencl_half_declaration : Error<
   "declaring variable of type %0 is not allowed">;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=281904&r1=281903&r2=281904&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Sep 19 09:54:41 2016
@@ -10100,6 +10100,16 @@ QualType Sema::CheckAssignmentOperands(E
   QualType LHSType = LHSExpr->getType();
   QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() :
  CompoundType;
+  // OpenCL v1.2 s6.1.1.1 p2:
+  // The half data type can only be used to declare a pointer to a buffer that
+  // contains half values
+  if (getLangOpts().OpenCL && !getOpenCLOptions().cl_khr_fp16 &&
+LHSType->isHalfType()) {
+Diag(Loc, diag::err_opencl_half_load_store) << 1
+<< LHSType.getUnqualifiedType();
+return QualType();
+  }
+
   AssignConvertType ConvTy;
   if (CompoundType.isNull()) {
 Expr *RHSCheck = RHS.get();

Modified: cfe/trunk/test/SemaOpenCL/half.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/half.cl?rev=281904&r1=281903&r2=281904&view=diff
==
--- cfe/trunk/test/SemaOpenCL/half.cl (original)
+++ cfe/trunk/test/SemaOpenCL/half.cl Mon Sep 19 09:54:41 2016
@@ -8,8 +8,10 @@ half half_disabled(half *p, // expected-
 {
   half a[2]; // expected-error{{declaring variable of type 'half [2]' is not 
allowed}}
   half b;// expected-error{{declaring variable of type 'half' is not 
allowed}}
-  *p; // expected-error{{loading directly from pointer to type 'half' is not 
allowed}}
-  p[1]; // expected-error{{loading directly from pointer to type 'half' is not 
allowed}}
+  *p; // expected-error{{loading directly from pointer to type 'half' requires 
cl_khr_fp16. Use vector data load builtin functions instead}}
+  *p = 0; // expected-error{{assigning directly to pointer to type 'half' 
requires cl_khr_fp16. Use vector data store builtin functions instead}}
+  p[1]; // expected-error{{loading directly from pointer to type 'half' 
requires cl_khr_fp16. Use vector data load builtin functions instead}}
+  p[1] = 0; // expected-error{{assigning directly to pointer to type 'half' 
requires cl_khr_fp16. Use vector data store builtin functions instead}}
 
   float c = 1.0f;
   b = (half) c;  // expected-error{{casting to type 'half' is not allowed}}
@@ -31,7 +33,9 @@ half half_enabled(half *p, half h)
   half a[2];
   half b;
   *p;
+  *p = 0;
   p[1];
+  p[1] = 0;
 
   float c = 1.0f;
   b = (half) c;


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

Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.

[Daniel Jasper via cfe-commits]

djasper added a comment.

I actually think this is a good example. So lets assume we'd write a tool to 
fully quote binary expressions, e.g. that turns

  if (a * b + c * d == 10) ...

into

  if (((a * b) + (c * d)) == 10) ...

So, here, we would be inserting two "(" and two ")" at the same locations. And, 
as you correctly mention, the order doesn't matter because we are inserting the 
same string twice. I think this is actually good behavior.

Deduplication is an interesting concern, but I think we probably want to handle 
that at a different layer. E.g. in the use case above, deduplicating would be 
quite fatal :).


https://reviews.llvm.org/D24717



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

Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.

[Eric Liu via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D24717#546096, @djasper wrote:

> Thinking about this some more, starting to merge deletions now, but only some 
> of them is a bit suspect. I think we either want to allow even more or 
> continue to be restrictive for now.
>
> I think fundamentally, there are two questions that we need to answer:
>
> 1. Is this something that the user/tool author would likely want to do?
> 2. Is add the replacement order-dependent in any way?
>
>   I have no clue about #1, I'd have to see use cases. E.g. what use case are 
> you trying to solve here?


Cong has this problem with dead code deletion where one dead code block is 
contained in another dead code block. Removing both dead entities will cause 
conflict now, so I figure maybe this is something we can also support because 
they are also order-independent and safe to deduplicate.

> But lets look at #2: I think I have come up with an easy definition of what 
> makes something order-dependent. Lets assume we have two replacements A and B 
> that both refer to the same original code (I am using A and B as single 
> replacements or as sets of a single replacement for simplicity). The question 
> is whether A.add(B) is order-dependent. I think we should define this as 
> (assuming we have a function that shifts a replacement by another replacement 
> like your getReplacementInChangedCode from https://reviews.llvm.org/D24383):

> 

> A.add(B) is order-dependent (and thus should conflict, if 
> A.merge(getReplacementInChangedCode(B)) != 
> B.merge(getReplacementInChangedCode(A)).

> 

> I think, this enables exactly the kinds of additions that we have so far 
> enabled, which seems good. It also enables overlapping deletions, e.g. 
> deleting range [0-2] and [1-3] will result in deleting [0-3], not matter in 
> which order.


This seems to be a nice definition for `order-dependent`. Just one caveat: with 
this condition, `A=(0,0,"a")` and `B=(0,0,"a")` are now also order-independent. 
Although the result for applying A and B in either order would be the same, I 
feel this is somehow less safe than merging deletions. And I guess the question 
here is whether users want to deduplicate. But for deletions, duplication 
doesn't matter.


https://reviews.llvm.org/D24717



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

Re: r281785 - CodeGen: Add more checks to nobuiltin.c test, add a negative test.

[Mikael Holmén via cfe-commits]

Hi,

On 09/17/2016 12:05 AM, Peter Collingbourne via cfe-commits wrote:

Author: pcc
Date: Fri Sep 16 17:05:53 2016
New Revision: 281785

URL: http://llvm.org/viewvc/llvm-project?rev=281785&view=rev
Log:
CodeGen: Add more checks to nobuiltin.c test, add a negative test.

Modified:
cfe/trunk/test/CodeGen/nobuiltin.c

Modified: cfe/trunk/test/CodeGen/nobuiltin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/nobuiltin.c?rev=281785&r1=281784&r2=281785&view=diff
==
--- cfe/trunk/test/CodeGen/nobuiltin.c (original)
+++ cfe/trunk/test/CodeGen/nobuiltin.c Fri Sep 16 17:05:53 2016
@@ -1,17 +1,19 @@
-// RUN: %clang_cc1 -fno-builtin -O1 -S -o - %s | FileCheck %s
-// RUN: %clang_cc1 -fno-builtin-memset -O1 -S -o - %s | FileCheck 
-check-prefix=MEMSET %s
+// RUN: %clang_cc1 -O1 -S -o - %s | FileCheck -check-prefix=STRCPY 
-check-prefix=MEMSET %s
+// RUN: %clang_cc1 -fno-builtin -O1 -S -o - %s | FileCheck 
-check-prefix=NOSTRCPY -check-prefix=NOMEMSET %s
+// RUN: %clang_cc1 -fno-builtin-memset -O1 -S -o - %s | FileCheck 
-check-prefix=STRCPY -check-prefix=NOMEMSET %s

 void PR13497() {
   char content[2];
   // make sure we don't optimize this call to strcpy()
-  // CHECK: __strcpy_chk
+  // STRCPY-NOT: __strcpy_chk
+  // NOSTRCPY: __strcpy_chk
   __builtin___strcpy_chk(content, "", 1);
 }

 void PR4941(char *s) {
   // Make sure we don't optimize this loop to a memset().
-  // MEMSET-LABEL: PR4941:
-  // MEMSET-NOT: memset
+  // NOMEMSET-NOT: memset
+  // MEMSET: memset


I suppose this is a general problem for CHECK-NOT:s, but I happened to 
have a repo with "memset" in the name, and then this NOMEMSET-NOT failed 
because it found "memset" in the file path.


So maybe change so it looks for "memset@" or something that might at 
least be less likely to appear in the path? Ideally the test should ofc 
be changed so it only looks for "memset" inside the PR4941 function but 
I'm too noobish on FileCheck to know if that's possible,


Regards,
Mikael


   for (unsigned i = 0; i < 8192; ++i)
 s[i] = 0;
 }


___
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] D16309: Use getCharWidth() instead of magic number

[Daniel Marjamäki via cfe-commits]

danielmarjamaki added a comment.

ping.


https://reviews.llvm.org/D16309



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

Re: [PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304

[Daphne Pfister via cfe-commits]

daphnediane updated this revision to Diff 71767.
daphnediane added a comment.

Adds test case, changes to suggested fix allowing break after template opener 
that is not immediately followed by template closer.


https://reviews.llvm.org/D24703

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5403,6 +5403,44 @@
"};");
 }
 
+TEST_F(FormatTest, WrapsTemplateParameters) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
+  verifyFormat(
+  "template  struct q {};\n"
+  "extern q\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "template  struct r {};\n"
+  "extern r\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
+  verifyFormat(
+  "template  struct s {};\n"
+  "extern s<\n"
+  "aa, aa, 
aa,\n"
+  "aa, aa, 
aa>\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "template  struct t {};\n"
+  "extern t<\n"
+  "aa, aa, 
aa,\n"
+  "aa, aa, 
aa>\n"
+  "y;",
+  Style);
+}
+
 TEST_F(FormatTest, WrapsAtNestedNameSpecifiers) {
   verifyFormat(
   
"::\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2524,7 +2524,8 @@
tok::colon, tok::l_square, tok::at) ||
  (Left.is(tok::r_paren) &&
   Right.isOneOf(tok::identifier, tok::kw_const)) ||
- (Left.is(tok::l_paren) && !Right.is(tok::r_paren));
+ (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
+ (Left.is(TT_TemplateOpener) && !Right.is(TT_TemplateCloser));
 }
 
 void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5403,6 +5403,44 @@
"};");
 }
 
+TEST_F(FormatTest, WrapsTemplateParameters) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
+  verifyFormat(
+  "template  struct q {};\n"
+  "extern q\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "template  struct r {};\n"
+  "extern r\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
+  verifyFormat(
+  "template  struct s {};\n"
+  "extern s<\n"
+  "aa, aa, aa,\n"
+  "aa, aa, aa>\n"
+  "y;",
+  Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "template  struct t {};\n"
+  "extern t<\n"
+  "aa, aa, aa,\n"
+  "aa, aa, aa>\n"
+  "y;",
+  Style);
+}
+
 TEST_F(FormatTest, WrapsAtNestedNameSpecifiers) {
   verifyFormat(
   "::\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2524,7 +2524,8 @@
tok::colon, tok::l_square, tok::at) ||
  (Left.is(tok::r_paren) &&
   Right.isOneOf(tok::identifier, tok::kw_const)) ||
- (Left.is(tok::l_paren) && !Right.is(tok::r_paren));
+ (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
+ (Left.is(TT_TemplateOpener) && !Right.is(TT_TemplateCloser));
 }
 
 void TokenAnnotator::printDebugInfo(const A

Re: [PATCH] D24704: PR30401: Fix substitutions for functions with abi_tag

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 71778.
DmitryPolukhin marked an inline comment as done.

https://reviews.llvm.org/D24704

Files:
  lib/AST/ItaniumMangle.cpp
  test/CodeGenCXX/mangle-abi-tag.cpp

Index: test/CodeGenCXX/mangle-abi-tag.cpp
===
--- test/CodeGenCXX/mangle-abi-tag.cpp
+++ test/CodeGenCXX/mangle-abi-tag.cpp
@@ -203,3 +203,19 @@
 }
 // A18::operator A[abi:A][abi:B]() but GCC adds the same tags twice!
 // CHECK-DAG: define linkonce_odr {{.+}} @_ZN3A18cv1AB1AB1BEv(
+
+namespace N19 {
+  class A {};
+  class __attribute__((abi_tag("B"))) B {};
+  class D {};
+  class F {};
+
+  template
+  class C {};
+
+  B foo(A, D);
+}
+void f19_test(N19::C, N19::F, N19::D) {
+}
+// f19_test(N19::C, N19::F, N19::D)
+// CHECK-DAG: define void 
@_Z8f19_testN3N191CINS_1AEXadL_ZNS_3fooB1BES1_NS_1DENS_1FES2_(
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -405,12 +405,14 @@
   CXXNameMangler(CXXNameMangler &Outer, raw_ostream &Out_)
   : Context(Outer.Context), Out(Out_), NullOut(false),
 Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), AbiTagsRoot(AbiTags) {}
+SeqID(Outer.SeqID), AbiTagsRoot(AbiTags),
+Substitutions(Outer.Substitutions) {}
 
   CXXNameMangler(CXXNameMangler &Outer, llvm::raw_null_ostream &Out_)
   : Context(Outer.Context), Out(Out_), NullOut(true),
 Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), AbiTagsRoot(AbiTags) {}
+SeqID(Outer.SeqID), AbiTagsRoot(AbiTags),
+Substitutions(Outer.Substitutions) {}
 
 #if MANGLE_CHECKER
   ~CXXNameMangler() {
@@ -458,6 +460,8 @@
   void addSubstitution(QualType T);
   void addSubstitution(TemplateName Template);
   void addSubstitution(uintptr_t Ptr);
+  // Destructive copy substitutions from other mangler.
+  void extendSubstitutions(CXXNameMangler* Other);
 
   void mangleUnresolvedPrefix(NestedNameSpecifier *qualifier,
   bool recursive = false);
@@ -685,6 +689,10 @@
   // Output name with implicit tags and function encoding from temporary 
buffer.
   mangleNameWithAbiTags(FD, &AdditionalAbiTags);
   Out << FunctionEncodingStream.str().substr(EncodingPositionStart);
+
+  // Function encoding could create new substitutions so we have to add
+  // temp mangled substitutions to main mangler.
+  extendSubstitutions(&FunctionEncodingMangler);
 }
 
 void CXXNameMangler::mangleFunctionEncodingBareType(const FunctionDecl *FD) {
@@ -4426,6 +4434,14 @@
   Substitutions[Ptr] = SeqID++;
 }
 
+void CXXNameMangler::extendSubstitutions(CXXNameMangler* Other) {
+  assert(Other->SeqID >= SeqID && "Must be superset of substitutions!");
+  if (Other->SeqID > SeqID) {
+Substitutions.swap(Other->Substitutions);
+SeqID = Other->SeqID;
+  }
+}
+
 CXXNameMangler::AbiTagList
 CXXNameMangler::makeFunctionReturnTypeTags(const FunctionDecl *FD) {
   // When derived abi tags are disabled there is no need to make any list.


Index: test/CodeGenCXX/mangle-abi-tag.cpp
===
--- test/CodeGenCXX/mangle-abi-tag.cpp
+++ test/CodeGenCXX/mangle-abi-tag.cpp
@@ -203,3 +203,19 @@
 }
 // A18::operator A[abi:A][abi:B]() but GCC adds the same tags twice!
 // CHECK-DAG: define linkonce_odr {{.+}} @_ZN3A18cv1AB1AB1BEv(
+
+namespace N19 {
+  class A {};
+  class __attribute__((abi_tag("B"))) B {};
+  class D {};
+  class F {};
+
+  template
+  class C {};
+
+  B foo(A, D);
+}
+void f19_test(N19::C, N19::F, N19::D) {
+}
+// f19_test(N19::C, N19::F, N19::D)
+// CHECK-DAG: define void @_Z8f19_testN3N191CINS_1AEXadL_ZNS_3fooB1BES1_NS_1DENS_1FES2_(
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -405,12 +405,14 @@
   CXXNameMangler(CXXNameMangler &Outer, raw_ostream &Out_)
   : Context(Outer.Context), Out(Out_), NullOut(false),
 Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), AbiTagsRoot(AbiTags) {}
+SeqID(Outer.SeqID), AbiTagsRoot(AbiTags),
+Substitutions(Outer.Substitutions) {}
 
   CXXNameMangler(CXXNameMangler &Outer, llvm::raw_null_ostream &Out_)
   : Context(Outer.Context), Out(Out_), NullOut(true),
 Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), AbiTagsRoot(AbiTags) {}
+SeqID(Outer.SeqID), AbiTagsRoot(AbiTags),
+Substitutions(Outer.Substitutions) {}
 
 #if MANGLE_CHECKER
   ~CXXNameMangler() {
@@ -458,6 +460,8 @@
   void addSubstitution(QualType T);
   void addSubstitution(TemplateName Template);
   void addSubstitution(uintptr_t Ptr);
+  // Destructive copy substitutions from other mangler.
+  void extendSubstitutions(CXXNameMa

Re: [PATCH] D24704: PR30401: Fix substitutions for functions with abi_tag

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added inline comments.


Comment at: lib/AST/ItaniumMangle.cpp:668
@@ -664,3 +667,3 @@
   llvm::raw_svector_ostream FunctionEncodingStream(FunctionEncodingBuf);
   CXXNameMangler FunctionEncodingMangler(*this, FunctionEncodingStream);
   // Output name of the function.

rsmith wrote:
> DmitryPolukhin wrote:
> > rsmith wrote:
> > > Maybe it'd be simpler to just override the output stream here rather than 
> > > creating a new mangler?
> > I'm not sure that it is simpler because it will also require substitutions 
> > save/restore for name mangling (mangleNameWithAbiTags) to produce the same 
> > substitutions twice (without implicate abi_tags and later with implicit 
> > abi_tags). IMHO, it is almost identical approaches but current one is a bit 
> > cleaner because it copies explicitly everything required from temp to outer 
> > mangler.
> I think we'd want to override the output stream to write to a temporary 
> buffer at the point when we would otherwise write out the ABI tags, to avoid 
> needing to save/restore any substitutions. But I agree, that seems more 
> invasive than what you're doing here. I'll leave this up to you.
It is significant redesign and additional complexity to remember which exactly 
ABI tags we would like to rewrite later (it may not be the first call of 
writeAbiTags for current mangler). I would keep it as is.


https://reviews.llvm.org/D24704



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

Re: [PATCH] D24507: Add attribute for return values that shouldn't be cast to bool

[Anton Urusov via cfe-commits]

urusant added a comment.

Thank you for the feedback.

> The patch is missing Sema tests for the attribute (that it only applies to 
> declarations you expect, accepts no args, etc).


There is one test case for that in test/ReturnNonBoolTestCompileTime.cpp. I've 
added another one for attribute accepting no args, so now the last two test 
cases in this file are those you were asking about. Can you think of any other 
cases of invalid attribute usage?

> Have you considered making this a type attribute on the return type of the 
> function rather than a declaration attribute on the function declaration?


No, I hadn't. On a quick look though, I couldn't find a way to simplify my 
solution using this idea, because as far as I understand, the type attribute 
isn't inherited, so, for example, if I have something like `int r = 
X509_verify_cert(...)` and the function `X509_verify_cert` has a return type 
with attribute, `r` won't have the attribute. If that is correct, we still need 
to backtrace the value to the function declaration. Is there something I am 
missing?



Comment at: include/clang/Basic/Attr.td:1138
@@ +1137,3 @@
+def WarnImpcastToBool : InheritableAttr {
+  let Spellings = [GCC<"warn_impcast_to_bool">];
+  let Subjects = SubjectList<[ObjCMethod, Function], WarnDiag,

aaron.ballman wrote:
> This should not use a GCC spelling because it's not an attribute that GCC 
> supports. You should probably use GNU instead, since I suspect this attribute 
> will be useful in C as well as C++.
Yeah, makes sense.


Comment at: include/clang/Basic/Attr.td:1140
@@ +1139,3 @@
+  let Subjects = SubjectList<[ObjCMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod">;
+  let Documentation = [WarnImpcastToBoolDocs];

aaron.ballman wrote:
> No need to specify the WarnDiag or ExpectedFunctionOrMethod arguments; they 
> will be handled automatically.
I didn't know that, thanks.


Comment at: include/clang/Basic/AttrDocs.td:2055
@@ -2054,1 +2054,3 @@
 }
+def WarnImpcastToBoolDocs : Documentation {
+  let Category = DocCatFunction;

zaks.anna wrote:
> You probably need to "propose" the attribute to the clang community. I'd send 
> an email to the cfe-dev as it might not have enough attention if it's just 
> the patch.  
OK, will do.


Comment at: include/clang/Basic/AttrDocs.td:2058
@@ +2057,3 @@
+  let Content = [{
+The ``warn_impcast_to_bool`` attribute is used to indicate that the return 
value of a function with integral return type cannot be used as a boolean 
value. For example, if a function returns -1 if it couldn't efficiently read 
the data, 0 if the data is invalid and 1 for success, it might be dangerous to 
implicitly cast the return value to bool, e.g. to indicate success. Therefore, 
it is a good idea to trigger a warning about such cases. However, in case a 
programmer uses an explicit cast to bool, that probably means that he knows 
what he is doing, therefore a warning should be triggered only for implicit 
casts.
+

aaron.ballman wrote:
> You should manually wrap this to roughly the 80 col limit.
> 
> Instead of "he", can you use "they" please?
OK, I did that. However, 80 col limit in this case feels a bit inconsistent 
with the rest of the file to me because most of other similar descriptions 
don't follow it.


Comment at: include/clang/Basic/DiagnosticGroups.td:57
@@ -56,2 +56,3 @@
 def DoublePromotion : DiagGroup<"double-promotion">;
+def UnsafeBoolConversion : DiagGroup<"unsafe-bool-conversion">;
 def EnumTooLarge : DiagGroup<"enum-too-large">;

aaron.ballman wrote:
> I'm not certain this requires its own diagnostic group. This can probably be 
> handled under `BoolConversion`
OK.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2259
@@ +2258,3 @@
+def warn_attribute_return_int_only : Warning<
+  "%0 attribute only applies to return values that are integers">,
+  InGroup;

aaron.ballman wrote:
> How about: ...only applies to integer return types?
Yeah, that sounds better.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2883
@@ +2882,3 @@
+  "implicit conversion turns non-bool into bool: %0 to %1">,
+  InGroup, DefaultIgnore;
+

aaron.ballman wrote:
> I don't think this should be a DefaultIgnore diagnostic -- if the user wrote 
> the attribute, they should get the diagnostic when appropriate.
Makes sense.


Comment at: lib/Sema/SemaChecking.cpp:8262
@@ +8261,3 @@
+/// e.g. (x ? f : g)(y)
+if (isa(E)) {
+  FunctionDecl* fn = cast(E)->getDirectCallee();

aaron.ballman wrote:
> Should use `if (const auto *CE = dyn_cast(E)) {`
Done.


Comment at: lib/Sema/SemaChecking.cpp:8263-8264
@@ +8262,4 @@
+if (isa(E)) {
+  FunctionDecl* fn = cast(E)->getDirectCallee();
+  

Re: [PATCH] D24507: Add attribute for return values that shouldn't be cast to bool

[Anton Urusov via cfe-commits]

urusant updated this revision to Diff 71807.
urusant added a comment.

Made some changes based on the comments. Please refer to the replies below.


Repository:
  rL LLVM

https://reviews.llvm.org/D24507

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/ReturnNonBoolChecker.cpp
  test/ReturnNonBoolTest.c
  test/ReturnNonBoolTest.cpp
  test/ReturnNonBoolTestCompileTime.cpp

Index: test/ReturnNonBoolTestCompileTime.cpp
===
--- /dev/null
+++ test/ReturnNonBoolTestCompileTime.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wbool-conversion -verify %s
+#ifdef __clang__
+#define RETURNS_NON_BOOL __attribute__((warn_impcast_to_bool))
+#else
+#define RETURNS_NON_BOOL
+#endif
+
+int NoAttributes() { return 2; }
+
+int NonBool(int x) RETURNS_NON_BOOL;
+int NonBool(int x) {  // expected-note{{'NonBool' declared here}}
+  return x * 2;
+}
+
+int good(int x) { return x * 2; }
+
+void test1() {
+  if (NonBool(2)) {  // expected-warning{{implicit conversion turns non-bool into bool: 'int' to 'bool'}}
+return;
+  }
+}
+
+void test3() {
+  if ((bool)NonBool(2)) {  // no warning, explicit cast
+return;
+  }
+}
+
+void test5() {
+  if (good(2)) {  // no warning, return value isn't marked as dangerous
+return;
+  }
+}
+
+double InvalidReturnType() RETURNS_NON_BOOL;  // expected-warning{{'warn_impcast_to_bool' attribute only applies to integer return types}}
+
+int AttributeWithArguments() __attribute__((warn_impcast_to_bool(2)));  // expected-error {{'warn_impcast_to_bool' attribute takes no arguments}}
Index: test/ReturnNonBoolTest.cpp
===
--- /dev/null
+++ test/ReturnNonBoolTest.cpp
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=alpha.core.ReturnNonBool -Wno-bool-conversion -verify %s
+#ifdef __clang__
+#define RETURNS_NON_BOOL __attribute__((warn_impcast_to_bool))
+#else
+#define RETURNS_NON_BOOL
+#endif
+
+int NoAttributes() { return 2; }
+
+int NonBool(int x) RETURNS_NON_BOOL;
+
+int good(int x);
+
+int wrap(int x) {
+  int r = NonBool(x);
+  return r;
+}
+
+void test1() {
+  if (NonBool(1)) {  // expected-warning{{implicit cast to bool is dangerous for this value}}
+return;
+  }
+}
+
+void test2() {
+  if (wrap(2)) {  // expected-warning{{implicit cast to bool is dangerous for this value}}
+return;
+  }
+}
+
+void test3() {
+  if ((bool)NonBool(3)) {  // no warning, explicit cast
+return;
+  }
+}
+
+void test4(int x) {
+  if (bool(wrap(2 * x))) {  // no warning, explicit cast
+return;
+  }
+}
+
+void test5() {
+  if (good(5)) {  // no warning, return value isn't marked as dangerous
+return;
+  }
+}
+
+void test6() {
+  if (good(wrap(2))) {  // no warning, wrap is treated as int, not as bool
+return;
+  }
+}
+
+double InvalidAttributeUsage()
+RETURNS_NON_BOOL;  // expected-warning{{'warn_impcast_to_bool' attribute only applies to integer return types}}
+
+void test_function_pointer(void (*f)()) {
+  // This is to test the case when Call.getDecl() returns NULL, because f()
+  // doesn't have a declaration
+  f();
+}
+
+bool universal_bool_wrapper(int (*f)(int), int x) {
+  // When we call universal_bool_wrapper from test_universal_bool_wrapper, the
+  // analyzer follows the path and detects that in this line we are doing
+  // something wrong (assuming that f is actually NonBool). So if we didn't call
+  // universal_bool_wrapper with any dangerous function, there would be no
+  // warning.
+  return f(x);  // expected-warning {{implicit cast to bool is dangerous for this value}}
+}
+
+int universal_int_wrapper(int (*f)(int), int x) { return f(x); }
+
+void test_universal_bool_wrapper(int x) {
+  if (universal_bool_wrapper(NonBool, x)) return;
+}
+
+void test_universal_int_wrapper(int x) {
+  if (universal_int_wrapper(NonBool, x))  // expected-warning{{implicit cast to bool is dangerous for this value}}
+return;
+}
+
+void test_lambdas(int x) {
+  if ([](int a) __attribute__((warn_impcast_to_bool))-> int{ return a; }(x)) { // expected-warning{{implicit cast to bool is dangerous for this value}}
+  }
+}
Index: test/ReturnNonBoolTest.c
===
--- /dev/null
+++ test/ReturnNonBoolTest.c
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.ReturnNonBool -Wno-bool-conversion -verify %s
+
+/// C is checked slightly differently than C++, in particular, C doesn't have
+/// implicit casts to bool, so we need to test different branching situations,
+/// like if, for, while, etc.
+
+#ifdef __clang__
+#define RETURNS_NON_BOOL __attribute__((warn_impcast_to_bool))
+#else
+#define RETURNS_NON_B

[PATCH] [CMake] Pass NO_CMAKE_SYSTEM_PATH to find_library

[Akihiko Odaki via cfe-commits]

Searching system path causes to find the host library and could lead to
failure when cross-compiling.
---
 tools/libclang/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libclang/CMakeLists.txt b/tools/libclang/CMakeLists.txt
index 630be12..7e5906a 100644
--- a/tools/libclang/CMakeLists.txt
+++ b/tools/libclang/CMakeLists.txt
@@ -52,7 +52,7 @@ if (TARGET clangTidyPlugin)
   list(APPEND LIBS clangTidyPlugin)
 endif ()
 
-find_library(DL_LIBRARY_PATH dl)
+find_library(DL_LIBRARY_PATH dl NO_CMAKE_SYSTEM_PATH)
 if (DL_LIBRARY_PATH)
   list(APPEND LIBS dl)
 endif()
-- 
2.9.3

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

Re: [PATCH] D24628: [ASAN] Pass previous stack information through __sanitizer_finish_switch_fiber

[Dmitry Vyukov via cfe-commits]

dvyukov added a comment.

We need a test that passes non-NULL to these arguments and shows how to use the 
returned values.



Comment at: test/asan/TestCases/Linux/swapcontext_annotation.cc:176-199
@@ -164,7 +175,26 @@
 ret += Run(argc - 1, 0, stack);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: Main context from: [[CHILD_STACK]] 524288
 ret += Run(argc - 1, 1, stack);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: Main context from: [[CHILD_STACK]] 524288
 ret += Run(argc - 1, 2, stack);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: NextChild stack: [[NEXT_CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: NextChild from: [[CHILD_STACK]] 524288
+// CHECK: Main context from: [[NEXT_CHILD_STACK]] 524288
 ret += Run(argc - 1, 0, heap);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: Main context from: [[CHILD_STACK]] 524288
 ret += Run(argc - 1, 1, heap);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: Main context from: [[CHILD_STACK]] 524288
 ret += Run(argc - 1, 2, heap);
+// CHECK: Child stack: [[CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: NextChild stack: [[NEXT_CHILD_STACK:0x[0-9a-f]*]]
+// CHECK: NextChild from: [[CHILD_STACK]] 524288
+// CHECK: Main context from: [[NEXT_CHILD_STACK]] 524288
+
+// CHECK: Iteration 0 passed
+printf("Iteration %d passed\n", i);
   }

andriigrynenko wrote:
> This only checks the first iteration of the loop. Can I do it better with 
> FileCheck ? 
Yes, you can add CHECKs for the second iteration as well.



https://reviews.llvm.org/D24628



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

r281899 - Reverting r281714 due to causing an assert when calling builtins that expect a double, from CL

[Neil Hickey via cfe-commits]

Author: neil.hickey
Date: Mon Sep 19 06:42:14 2016
New Revision: 281899

URL: http://llvm.org/viewvc/llvm-project?rev=281899&view=rev
Log:
Reverting r281714 due to causing an assert when calling builtins that expect a 
double, from CL

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/CodeGenOpenCL/fpmath.cl
cfe/trunk/test/SemaOpenCL/extensions.cl

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=281899&r1=281898&r2=281899&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Sep 19 06:42:14 2016
@@ -828,18 +828,8 @@ ExprResult Sema::DefaultArgumentPromotio
   // double.
   const BuiltinType *BTy = Ty->getAs();
   if (BTy && (BTy->getKind() == BuiltinType::Half ||
-  BTy->getKind() == BuiltinType::Float)) {
-if (getLangOpts().OpenCL &&
-!((getLangOpts().OpenCLVersion >= 120 &&
-   Context.getTargetInfo()
-   .getSupportedOpenCLOpts()
-   .cl_khr_fp64) ||
-  getOpenCLOptions().cl_khr_fp64)) {
-E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
-} else {
-  E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
-}
-  }
+  BTy->getKind() == BuiltinType::Float))
+E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
 
   // C++ performs lvalue-to-rvalue conversion as a default argument
   // promotion, even on class types, but note:
@@ -3416,14 +3406,8 @@ ExprResult Sema::ActOnNumericConstant(co
   if (getLangOpts().SinglePrecisionConstants) {
 Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
   } else if (getLangOpts().OpenCL &&
- !((getLangOpts().OpenCLVersion >= 120 &&
-Context.getTargetInfo()
-.getSupportedOpenCLOpts()
-.cl_khr_fp64) ||
+ !((getLangOpts().OpenCLVersion >= 120) ||
getOpenCLOptions().cl_khr_fp64)) {
-// Impose single-precision float type when:
-//  - in CL 1.2 or above and cl_khr_fp64 is not supported, or
-//  - in CL 1.1 or below and cl_khr_fp64 is not enabled.
 Diag(Tok.getLocation(), diag::warn_double_const_requires_fp64);
 Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
   }

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=281899&r1=281898&r2=281899&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Sep 19 06:42:14 2016
@@ -1401,8 +1401,7 @@ static QualType ConvertDeclSpecToType(Ty
   Result = Context.DoubleTy;
 
 if (S.getLangOpts().OpenCL &&
-!((S.getLangOpts().OpenCLVersion >= 120 
-   && S.Context.getTargetInfo().getSupportedOpenCLOpts().cl_khr_fp64) 
||
+!((S.getLangOpts().OpenCLVersion >= 120) ||
   S.getOpenCLOptions().cl_khr_fp64)) {
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_requires_extension)
   << Result << "cl_khr_fp64";

Modified: cfe/trunk/test/CodeGenOpenCL/fpmath.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/fpmath.cl?rev=281899&r1=281898&r2=281899&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/fpmath.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/fpmath.cl Mon Sep 19 06:42:14 2016
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown | FileCheck 
--check-prefix=CHECK --check-prefix=NODIVOPT %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown 
-cl-fp32-correctly-rounded-divide-sqrt | FileCheck --check-prefix=CHECK 
--check-prefix=DIVOPT %s
-// RUN: %clang_cc1 %s -emit-llvm -o - -DNOFP64 -cl-std=CL1.1 -triple 
r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck 
--check-prefix=CHECK-DBL %s
 
 typedef __attribute__(( ext_vector_type(4) )) float float4;
 
@@ -22,26 +21,14 @@ float4 spvectordiv(float4 a, float4 b) {
   return a / b;
 }
 
-void printf(constant char* fmt, ...);
-
-#ifndef NOFP64
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
-#endif
-void testdbllit(long *val) {
-  // CHECK-DBL: float 2.00e+01
-  // CHECK: double 2.00e+01
-  printf("%f", 20.0);
-}
 
-#ifndef NOFP64
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
   // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
-#endif
 
 // CHECK: attributes #[[ATTR]] = {
 // NODIVOPT: "correctly-rounded-divide-sqrt-fp-math"="false"

Modified: cfe/trunk/test/SemaOpenCL/extensions.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Malcolm Parsons via cfe-commits]

malcolm.parsons updated this revision to Diff 71802.
malcolm.parsons added a comment.

Handle delegating and base class constructors


https://reviews.llvm.org/D24339

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tidy/readability/RedundantMemberInitCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-member-init.rst
  test/clang-tidy/readability-redundant-member-init.cpp

Index: test/clang-tidy/readability-redundant-member-init.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-member-init.cpp
@@ -0,0 +1,134 @@
+// RUN: %check_clang_tidy %s readability-redundant-member-init %t
+
+struct S {
+  S() = default;
+  S(int i) : i(i) {}
+  int i = 1;
+};
+
+struct T {
+  T(int i = 1) : i(i) {}
+  int i;
+};
+
+struct U {
+  int i;
+};
+
+// Initializer calls default constructor
+struct F1 {
+  F1() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  // CHECK-FIXES:  F1()  {}
+  S f;
+};
+
+// Initializer calls default constructor with default argument
+struct F2 {
+  F2() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  // CHECK-FIXES:  F2()  {}
+  T f;
+};
+
+// Multiple redundant initializers for same constructor
+struct F3 {
+  F3() : f(), g(1), h() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: initializer for 'h' is redundant [readability-redundant-member-init]
+  // CHECK-FIXES:  F3() : g(1) {}
+  S f, g, h;
+};
+
+// Templated class independent type
+template 
+struct F4 {
+  F4() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  // CHECK-FIXES:  F4()  {}
+  S f;
+};
+F4 f4i;
+F4 f4s;
+
+// Base class
+struct F5 : S {
+  F5() : S() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'S' is redundant [readability-redundant-member-init]
+  // CHECK-FIXES:  F5()  {}
+};
+
+// Initializer not written
+struct NF1 {
+  NF1() {}
+  S f;
+};
+
+// Initializer doesn't call default constructor
+struct NF2 {
+  NF2() : f(1) {}
+  S f;
+};
+
+// Initializer calls default constructor without using default argument
+struct NF3 {
+  NF3() : f(1) {}
+  T f;
+};
+
+// Initializer calls default constructor without using default argument
+struct NF4 {
+  NF4() : f(2) {}
+  T f;
+};
+
+// Initializer is zero-initialization
+struct NF5 {
+  NF5() : i() {}
+  int i;
+};
+
+// Initializer is direct-initialization
+struct NF6 {
+  NF6() : i(1) {}
+  int i;
+};
+
+// Initializer is aggregate initialization of struct
+struct NF7 {
+  NF7() : f{} {}
+  U f;
+};
+
+// Initializer is aggregate initialization of array
+struct NF8 {
+  NF8() : f{} {}
+  int f[2];
+};
+
+struct NF9 {
+  NF9() : f{} {}
+  S f[2];
+};
+
+// Initializing member of union
+union NF10 {
+  NF10() : s() {}
+  int i;
+  S s;
+};
+
+// Templated class dependent type
+template 
+struct NF11 {
+  NF11() : f() {}
+  V f;
+};
+NF11 nf11i;
+NF11 nf11s;
+
+// Delegating constructor
+class NF12 {
+  NF12() = default;
+  NF12(int) : NF12() {}
+};
Index: docs/clang-tidy/checks/readability-redundant-member-init.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-member-init.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - readability-redundant-member-init
+
+readability-redundant-member-init
+=
+
+Finds member initializations that are unnecessary because the same default
+constructor would be called if they were not present.
+
+Example:
+
+.. code-block:: c++
+
+  // Explicitly initializing the member s is unnecessary.  
+  class Foo {
+  public:
+Foo() : s() {}
+
+  private:
+std::string s;
+  };
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -133,6 +133,7 @@
readability-named-parameter
readability-non-const-parameter
readability-redundant-control-flow
+   readability-redundant-member-init
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@
   Flags function parameters of a pointer type that could be 

Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Daniel Krupp via cfe-commits]

dkrupp added a comment.

Thanks. Gabor, could you please merge this? I don't have commit right.


https://reviews.llvm.org/D24307



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

Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.

[Daniel Jasper via cfe-commits]

djasper added a comment.

Thinking about this some more, starting to merge deletions now, but only some 
of them is a bit suspect. I think we either want to allow even more or continue 
to be restrictive for now.

I think fundamentally, there are two questions that we need to answer:

1. Is this something that the user/tool author would likely want to do?
2. Is add the replacement order-dependent in any way?

I have no clue about #1, I'd have to see use cases. E.g. what use case are you 
trying to solve here?

But lets look at #2: I think I have come up with an easy definition of what 
makes something order-dependent. Lets assume we have two replacements A and B 
that both refer to the same original code (I am using A and B as single 
replacements or as sets of a single replacement for simplicity). The question 
is whether A.add(B) is order-dependent. I think we should define this as 
(assuming we have a function that shifts a replacement by another replacement 
like your getReplacementInChangedCode from https://reviews.llvm.org/D24383):

A.add(B) is order-dependent (and thus should conflict, if 
A.merge(getReplacementInChangedCode(B)) != 
B.merge(getReplacementInChangedCode(A)).

I think, this enables exactly the kinds of additions that we have so far 
enabled, which seems good. It also enables overlapping deletions, e.g. deleting 
range [0-2] and [1-3] will result in deleting [0-3], not matter in which order.


https://reviews.llvm.org/D24717



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

[PATCH] D24719: [include-fixer] Add customized editor settings documents.

[Haojian Wu via cfe-commits]

hokein created this revision.
hokein added a reviewer: bkramer.
hokein added a subscriber: cfe-commits.

https://reviews.llvm.org/D24719

Files:
  docs/include-fixer.rst

Index: docs/include-fixer.rst
===
--- docs/include-fixer.rst
+++ docs/include-fixer.rst
@@ -81,13 +81,40 @@
 You can customize the number of headers being shown by setting
 ``let g:clang_include_fixer_maximum_suggested_headers=5``
 
+Customized settings in `.vimrc`:
+
+- ``let g:clang_include_fixer_path = "clang-include-fixer"``
+
+  Set clang-include-fixer binary file path.
+
+- ``let g:clang_include_fixer_maximum_suggested_headers = 3``
+
+  Set the maximum number of ``#includes`` to show. Default is 3.
+
+- ``let g:clang_include_fixer_increment_num = 5``
+
+  Set the increment number of #includes to show every time when pressing ``m``.
+  Default is 5.
+
+- ``let g:clang_include_fixer_jump_to_include = 0``
+
+  Set to 1 if you want to jump to the new inserted ``#include`` line. Default 
is
+  0.
+
+- ``let g:clang_include_fixer_query_mode = 0``
+
+  Set to 1 if you want to insert ``#include`` for the symbol under the cursor.
+  Default is 0. Compared to normal mode, this mode won't parse the source file
+  and only search the sysmbol from database, which is faster than normal mode.
+
 See ``clang-include-fixer.py`` for more details.
 
 Integrate with Emacs
 
 To run `clang-include-fixer` on a potentially unsaved buffer in Emacs.
-Ensure that Emacs finds ``clang-include-fixer.el`` by adding the directory 
containing the file to the ``load-path``
-and requiring the `clang-include-fixer` in your ```.emacs``:
+Ensure that Emacs finds ``clang-include-fixer.el`` by adding the directory
+containing the file to the ``load-path`` and requiring the 
`clang-include-fixer`
+in your ``.emacs``:
 
 .. code-block:: console
 
@@ -100,6 +127,19 @@
 
 - Add the path to :program:`clang-include-fixer` to the PATH environment 
variable.
 
+Customized settings in `.emacs`:
+
+- ``(custom-set-variables '(clang-include-fixer-executable 
"/path/to/include-fixer"))``
+
+  Set clang-include-fixer binary file path.
+
+- ``(custom-set-variables '(clang-include-fixer-query-mode t))``
+
+  Set to `t` if you want to insert ``#include`` for the symbol under the 
cursor.
+  Default is `nil`. Compared to normal mode, this mode won't parse the source
+  file and only search the sysmbol from database, which is faster than normal
+  mode.
+
 See ``clang-include-fixer.el`` for more details.
 
 How it Works


Index: docs/include-fixer.rst
===
--- docs/include-fixer.rst
+++ docs/include-fixer.rst
@@ -81,13 +81,40 @@
 You can customize the number of headers being shown by setting
 ``let g:clang_include_fixer_maximum_suggested_headers=5``
 
+Customized settings in `.vimrc`:
+
+- ``let g:clang_include_fixer_path = "clang-include-fixer"``
+
+  Set clang-include-fixer binary file path.
+
+- ``let g:clang_include_fixer_maximum_suggested_headers = 3``
+
+  Set the maximum number of ``#includes`` to show. Default is 3.
+
+- ``let g:clang_include_fixer_increment_num = 5``
+
+  Set the increment number of #includes to show every time when pressing ``m``.
+  Default is 5.
+
+- ``let g:clang_include_fixer_jump_to_include = 0``
+
+  Set to 1 if you want to jump to the new inserted ``#include`` line. Default is
+  0.
+
+- ``let g:clang_include_fixer_query_mode = 0``
+
+  Set to 1 if you want to insert ``#include`` for the symbol under the cursor.
+  Default is 0. Compared to normal mode, this mode won't parse the source file
+  and only search the sysmbol from database, which is faster than normal mode.
+
 See ``clang-include-fixer.py`` for more details.
 
 Integrate with Emacs
 
 To run `clang-include-fixer` on a potentially unsaved buffer in Emacs.
-Ensure that Emacs finds ``clang-include-fixer.el`` by adding the directory containing the file to the ``load-path``
-and requiring the `clang-include-fixer` in your ```.emacs``:
+Ensure that Emacs finds ``clang-include-fixer.el`` by adding the directory
+containing the file to the ``load-path`` and requiring the `clang-include-fixer`
+in your ``.emacs``:
 
 .. code-block:: console
 
@@ -100,6 +127,19 @@
 
 - Add the path to :program:`clang-include-fixer` to the PATH environment variable.
 
+Customized settings in `.emacs`:
+
+- ``(custom-set-variables '(clang-include-fixer-executable "/path/to/include-fixer"))``
+
+  Set clang-include-fixer binary file path.
+
+- ``(custom-set-variables '(clang-include-fixer-query-mode t))``
+
+  Set to `t` if you want to insert ``#include`` for the symbol under the cursor.
+  Default is `nil`. Compared to normal mode, this mode won't parse the source
+  file and only search the sysmbol from database, which is faster than normal
+  mode.
+
 See ``clang-include-fixer.el`` for more details.
 
 How it Works

Re: [PATCH] D24192: [clang-refactor] introducing clang-refactor

[Eric Liu via cfe-commits]

ioeric added inline comments.


Comment at: clang-refactor/driver/ModuleManager.h:14-20
@@ +13,9 @@
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+#include 
+#include 
+
+#include "core/RefactoringModule.h"
+

curdeius wrote:
> I thought that idea behind sorting includes using clang-format is to avoid 
> handling groups and order manually.
> I don't think that there is any policy prohibiting separating includes into 
> groups, but AFAIK, there is one that says that STL includes should be the 
> last (you should include in order from the most specific to the most generic, 
> i.e. subproject, clang, llvm, STL).
Fortunately, clang-format is smart enough to categorize #include groups (e.g. 
LLVM includes, STL includes, main header etc). We actually encourage people to 
combine #includes groups into one block and let clang-format handle the 
categorization (fyi: clang-format only sort includes within a block). The point 
is to free you from worrying about the formatting.


https://reviews.llvm.org/D24192



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

Re: [PATCH] D24606: Recommit r281457 "Supports adding insertion around non-insertion replacements".

[Eric Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281891: Recommit r281457 "Supports adding insertion around 
non-insertion replacements". (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D24606?vs=71777&id=71779#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24606

Files:
  cfe/trunk/include/clang/Tooling/Core/Replacement.h
  cfe/trunk/lib/Tooling/Core/Replacement.cpp
  cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -134,6 +134,14 @@
 ReplacementText);
 }
 
+llvm::Error makeConflictReplacementsError(const Replacement &New,
+  const Replacement &Existing) {
+  return llvm::make_error(
+  "New replacement:\n" + New.toString() +
+  "\nconflicts with existing replacement:\n" + Existing.toString(),
+  llvm::inconvertibleErrorCode());
+}
+
 llvm::Error Replacements::add(const Replacement &R) {
   // Check the file path.
   if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
@@ -160,31 +168,45 @@
   // entries that start at the end can still be conflicting if R is an
   // insertion.
   auto I = Replaces.lower_bound(AtEnd);
-  // If it starts at the same offset as R (can only happen if R is an
-  // insertion), we have a conflict.  In that case, increase I to fall through
-  // to the conflict check.
-  if (I != Replaces.end() && R.getOffset() == I->getOffset())
-++I;
+  // If `I` starts at the same offset as `R`, `R` must be an insertion.
+  if (I != Replaces.end() && R.getOffset() == I->getOffset()) {
+assert(R.getLength() == 0);
+// `I` is also an insertion, `R` and `I` conflict.
+if (I->getLength() == 0)
+  return makeConflictReplacementsError(R, *I);
+// Insertion `R` is adjacent to a non-insertion replacement `I`, so they
+// are order-independent. It is safe to assume that `R` will not conflict
+// with any replacement before `I` since all replacements before `I` must
+// either end before `R` or end at `R` but has length > 0 (if the
+// replacement before `I` is an insertion at `R`, it would have been `I`
+// since it is a lower bound of `AtEnd` and ordered before the current `I`
+// in the set).
+Replaces.insert(R);
+return llvm::Error::success();
+  }
 
-  // I is the smallest iterator whose entry cannot overlap.
+  // `I` is the smallest iterator (after `R`) whose entry cannot overlap.
   // If that is begin(), there are no overlaps.
   if (I == Replaces.begin()) {
 Replaces.insert(R);
 return llvm::Error::success();
   }
   --I;
   // If the previous entry does not overlap, we know that entries before it
   // can also not overlap.
-  if (R.getOffset() != I->getOffset() &&
-  !Range(R.getOffset(), R.getLength())
+  if (!Range(R.getOffset(), R.getLength())
.overlapsWith(Range(I->getOffset(), I->getLength( {
+// If `R` and `I` do not have the same offset, it is safe to add `R` since
+// it must come after `I`. Otherwise:
+//   - If `R` is an insertion, `I` must not be an insertion since it would
+//   have come after `AtEnd`.
+//   - If `R` is not an insertion, `I` must be an insertion; otherwise, `R`
+//   and `I` would have overlapped.
+// In either case, we can safely insert `R`.
 Replaces.insert(R);
 return llvm::Error::success();
   }
-  return llvm::make_error(
-  "New replacement:\n" + R.toString() +
-  "\nconflicts with existing replacement:\n" + I->toString(),
-  llvm::inconvertibleErrorCode());
+  return makeConflictReplacementsError(R, *I);
 }
 
 namespace {
Index: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
===
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp
@@ -115,24 +115,26 @@
   llvm::consumeError(std::move(Err));
 }
 
-TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+TEST_F(ReplacementTest, AddAdjacentInsertionAndReplacement) {
   Replacements Replaces;
   // Test adding an insertion at the offset of an existing replacement.
   auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 
   Replaces.clear();
   // Test overlap with an existing insertion.
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   l