[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D35082#890150, @rjmccall wrote:

> Non-canonical information is not supposed to be mangled.
>
> It's not clear to me what the language rule you're really trying to implement 
> is.  Maybe you really do need a canonical __private address space, in which 
> case you are going to have to change a lot of code in Sema to add the address 
> space qualifier to every local and temporary allocation.  But that's not 
> obvious to me yet, because you haven't really explained why it needs to be 
> explicit in the representation.


In OpenCL all memory is in certain address space, therefore every l-value and 
pointee in a pointer should have an address space. Private address space has 
equal status as global or local address space. There are language rules about 
pointers to what address space can be assigned to pointers to what address 
space. Therefore all address space needs to be canonical. This patch already 
puts every local variable and function parameter in private address space, as 
is done in deduceOpenCLImplicitAddrSpace(). We need private address space to be 
explicit because we need to display explicit private address space and implicit 
address space differently, also because in certain semantic checks we need to 
know if a private address space is explicit or not.

> If you just want pointers to __private to be mangled with an address-space 
> qualifier — meaning I guess that the mangling e.g. Pi will be completely 
> unused — that should be easy enough to handle in the mangler.  But if you 
> need to distinguish between __private-qualified types and unqualified types, 
> and that distinction isn't purely to implement some syntactic restriction 
> about not writing e.g. __private __global, then that's not good enough and 
> you do need a canonical __private.

The mangler does not mangle an empty type qualifier, therefore if private 
address space is represented as the default address space (i.e., no address 
space), it will not be mangled at all. This is also related to the substitution 
and cannot be easily changed without breaking lots of stuff in the mangler.

> Telling me that you're in a hurry isn't helpful; preserving a reasonable 
> representation and not allowing corner cases to become maintenance problems 
> is far more important to the project than landing patches in trunk on some 
> particular schedule.

The introduction of explicit private address space and the implicit address 
space flag in AST is precisely for handling those corner cases in the OpenCL 
language rules so that they won't become maintenance problems.


https://reviews.llvm.org/D35082



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

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Non-canonical information is not supposed to be mangled.

It's not clear to me what the language rule you're really trying to implement 
is.  Maybe you really do need a canonical __private address space, in which 
case you are going to have to change a lot of code in Sema to add the address 
space qualifier to every local and temporary allocation.  But that's not 
obvious to me yet, because you haven't really explained why it needs to be 
explicit in the representation.

If you just want pointers to __private to be mangled with an address-space 
qualifier — meaning I guess that the mangling e.g. Pi will be completely unused 
— that should be easy enough to handle in the mangler.  But if you need to 
distinguish between __private-qualified types and unqualified types, and that 
distinction isn't purely to implement some syntactic restriction about not 
writing e.g. __private __global, then that's not good enough and you do need a 
canonical __private.

Telling me that you're in a hurry isn't helpful; preserving a reasonable 
representation and not allowing corner cases to become maintenance problems is 
far more important to the project than landing patches in trunk on some 
particular schedule.


https://reviews.llvm.org/D35082



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

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D35082#890143, @rjmccall wrote:

> You have an important backend change relying on being able to preserve type 
> sugar better in diagnostics?  The only apparent semantic change in this patch 
> is that you're changing the mangling, which frankly seems incorrect.


Can you elaborate on why the mangling is incorrect?


https://reviews.llvm.org/D35082



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

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You have an important backend change relying on being able to preserve type 
sugar better in diagnostics?  The only apparent semantic change in this patch 
is that you're changing the mangling, which frankly seems incorrect.


https://reviews.llvm.org/D35082



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

[PATCH] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't see any reason not to use linkonce_odr + hidden even outside of -Oz.  
Otherwise LGTM.


https://reviews.llvm.org/D38606



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

[PATCH] D37826: Refine generation of TBAA information in clang

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D37826



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

[PATCH] D38596: Implement attribute target multiversioning

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

This generally works for me modulo the things that Hal has mentioned. You'll 
probably want to add Richard to the review list for the Sema bits as well.

Thanks!




Comment at: lib/Sema/SemaDecl.cpp:3214
   if (!isFriend && NewMethod->getLexicalDeclContext()->isRecord() &&
-  !IsClassScopeExplicitSpecialization) {
+  !IsClassScopeExplicitSpecialization ) {
 //-- Member function declarations with the same name and the

Extra whitespace.


https://reviews.llvm.org/D38596



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

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

[Eric Fiselier via cfe-commits]

On Thu, Oct 5, 2017 at 10:20 AM, Chris Bieneman via Phabricator <
revi...@reviews.llvm.org> wrote:

> beanz added a comment.
>
> Building libcxx without LLVM's CMake modules is very important, not just
> to Apple.


*Why* is it important? The reason isn't obvious to me. The only additional
cost is downloading the LLVM sources,
and I don't see why that's show stopping. Sure it takes *slightly longer*
than before, but that can't be the issue here, is it?



> This is how several open source distributions work, and it would be a huge
> disservice to break this. Same is true for compiler-rt, libcxxabi,
> libunwind, etc.
>
> Historically we've been drawing the line that building and running tests
> for runtime projects can require LLVM, but building the runtime libraries
> themselves must work without LLVM. I believe that is still the correct line
> to draw.
>
>
> https://reviews.llvm.org/D31363
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Wei Mi via Phabricator via cfe-commits]

wmi updated this revision to Diff 117947.
wmi added a comment.

Address Hal's comment.


Repository:
  rL LLVM

https://reviews.llvm.org/D36562

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGRecordLayoutBuilder.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/finegrain-bitfield-access.cpp

Index: test/CodeGenCXX/finegrain-bitfield-access.cpp
===
--- test/CodeGenCXX/finegrain-bitfield-access.cpp
+++ test/CodeGenCXX/finegrain-bitfield-access.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \
+// RUN:   -emit-llvm -fsanitize=address -o - %s | FileCheck %s --check-prefix=SANITIZE
+// Check -fsplit-bitfields will be ignored since sanitizer is enabled.
+
+struct S1 {
+  unsigned f1:2;
+  unsigned f2:6;
+  unsigned f3:8;
+  unsigned f4:4;
+  unsigned f5:8;
+};
+
+S1 a1;
+unsigned read8_1() {
+  // CHECK-LABEL: @_Z7read8_1v
+  // CHECK: %bf.load = load i8, i8* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 1), align 1
+  // CHECK-NEXT: %bf.cast = zext i8 %bf.load to i32
+  // CHECK-NEXT: ret i32 %bf.cast
+  // SANITIZE-LABEL: @_Z7read8_1v
+  // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE: %bf.lshr = lshr i32 %bf.load, 8
+  // SANITIZE: %bf.clear = and i32 %bf.lshr, 255
+  // SANITIZE: ret i32 %bf.clear
+  return a1.f3;
+}
+void write8_1() {
+  // CHECK-LABEL: @_Z8write8_1v
+  // CHECK: store i8 3, i8* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 1), align 1
+  // CHECK-NEXT: ret void
+  // SANITIZE-LABEL: @_Z8write8_1v
+  // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE-NEXT: %bf.clear = and i32 %bf.load, -65281
+  // SANITIZE-NEXT: %bf.set = or i32 %bf.clear, 768
+  // SANITIZE-NEXT: store i32 %bf.set, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE-NEXT: ret void
+  a1.f3 = 3;
+}
+
+unsigned read8_2() {
+  // CHECK-LABEL: @_Z7read8_2v
+  // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+  // CHECK-NEXT: %bf.lshr = lshr i16 %bf.load, 4
+  // CHECK-NEXT: %bf.clear = and i16 %bf.lshr, 255
+  // CHECK-NEXT: %bf.cast = zext i16 %bf.clear to i32
+  // CHECK-NEXT: ret i32 %bf.cast
+  // SANITIZE-LABEL: @_Z7read8_2v
+  // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE-NEXT: %bf.lshr = lshr i32 %bf.load, 20
+  // SANITIZE-NEXT: %bf.clear = and i32 %bf.lshr, 255
+  // SANITIZE-NEXT: ret i32 %bf.clear
+  return a1.f5;
+}
+void write8_2() {
+  // CHECK-LABEL: @_Z8write8_2v
+  // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+  // CHECK-NEXT: %bf.clear = and i16 %bf.load, -4081
+  // CHECK-NEXT: %bf.set = or i16 %bf.clear, 48
+  // CHECK-NEXT: store i16 %bf.set, i16* getelementptr inbounds (%struct.S1, %struct.S1* @a1, i32 0, i32 2), align 2
+  // CHECK-NEXT: ret void
+  // SANITIZE-LABEL: @_Z8write8_2v
+  // SANITIZE: %bf.load = load i32, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE-NEXT: %bf.clear = and i32 %bf.load, -267386881
+  // SANITIZE-NEXT: %bf.set = or i32 %bf.clear, 3145728
+  // SANITIZE-NEXT: store i32 %bf.set, i32* getelementptr inbounds {{.*}}, align 4
+  // SANITIZE-NEXT: ret void
+  a1.f5 = 3;
+}
+
+struct S2 {
+  unsigned long f1:16;
+  unsigned long f2:16;
+  unsigned long f3:6;
+};
+
+S2 a2;
+unsigned read16_1() {
+  // CHECK-LABEL: @_Z8read16_1v
+  // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 0), align 8
+  // CHECK-NEXT: %bf.cast = zext i16 %bf.load to i64
+  // CHECK-NEXT: %conv = trunc i64 %bf.cast to i32
+  // CHECK-NEXT: ret i32 %conv
+  // SANITIZE-LABEL: @_Z8read16_1v
+  // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+  // SANITIZE-NEXT: %bf.clear = and i64 %bf.load, 65535
+  // SANITIZE-NEXT: %conv = trunc i64 %bf.clear to i32
+  // SANITIZE-NEXT: ret i32 %conv
+  return a2.f1;
+}
+unsigned read16_2() {
+  // CHECK-LABEL: @_Z8read16_2v
+  // CHECK: %bf.load = load i16, i16* getelementptr inbounds (%struct.S2, %struct.S2* @a2, i32 0, i32 1), align 2
+  // CHECK-NEXT: %bf.cast = zext i16 %bf.load to i64
+  // CHECK-NEXT: %conv = trunc i64 %bf.cast to i32
+  // CHECK-NEXT: ret i32 %conv
+  // SANITIZE-LABEL: @_Z8read16_2v
+  // SANITIZE: %bf.load = load i64, i64* bitcast {{.*}}, align 8
+  // SANITIZE-NEXT: %bf.lshr = lshr i64 %bf.load, 16
+  // SANITIZE-NEXT: %bf.clear = and i64 %bf.lshr, 65535
+  // SANITIZE-NEXT: %conv = trunc i64 %bf.clear to i32
+  // SANITIZE-NEXT: ret i32 %conv
+  return a2.f2;
+}
+
+void write16_1() {
+  // 

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Wei Mi via Phabricator via cfe-commits]

wmi marked an inline comment as done.
wmi added inline comments.



Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:444
 // Add bitfields to the run as long as they qualify.
-if (Field != FieldEnd && Field->getBitWidthValue(Context) != 0 &&
-Tail == getFieldBitOffset(*Field)) {
-  Tail += Field->getBitWidthValue(Context);
-  ++Field;
-  continue;
+if (Field != FieldEnd && !SingleFieldRun) {
+  SingleFieldRun = betterBeSingleFieldRun(Field);

hfinkel wrote:
> The logic here is not obvious. Can you please add a comment. SingleFieldRun 
> here is only not equal to `betterBeSingleFieldRun(Field)` if we've skipped 
> 0-length bitfields, right? Please explain what's going on and also please 
> make sure there's a test case.
I restructure the code a little bit and hope the logic is more clear. I already 
have a testcase added for it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36562



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

[PATCH] D38549: [clang-tidy] Link targets and Asm parsers

[Alexander Kornienko via Phabricator via cfe-commits]

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

The first attempt to fix this (https://reviews.llvm.org/D17981) was stuck on 
figuring out whether we care about the binary size. It seems to me that the 
binary size is secondary to maintaining proper functionality for platforms that 
use inline assembly in headers. Thus, LG


https://reviews.llvm.org/D38549



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

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Peter Szecsi via Phabricator via cfe-commits]

szepet closed this revision.
szepet added a comment.

This patch was commited in r302975. (https://reviews.llvm.org/rL302975)


https://reviews.llvm.org/D32981



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

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

Mostly looks good, other than the regression in enum parsing.




Comment at: clang/include/clang/Basic/LangOptions.def:140
 
+LANGOPT(DoubleSquareBracketsAttributes, 1, 0, "'[[]]' attributes extension for 
all language standard modes")
+

I would probably singularize this to `DoubleSquareBracketAttributes`. But I'm 
happy with either.



Comment at: clang/lib/Parse/ParseDecl.cpp:4223-4225
+// C++11 attributes are prohibited in this location.
+if (ParsedCXX11Attrs)
+  ProhibitAttributes(attrs);

This change still looks wrong to me. C++11 attributes are not prohibited in an 
//opaque-enum-declaration//.



Comment at: clang/lib/Parse/ParseDecl.cpp:5973-5974
+
+// If there are attributes following the identifier list, parse them and 
+// prohibit them.
+MaybeParseCXX11Attributes(FnAttrs);

Do you anticipate people trying this? Is the concern here that a function 
declarator can normally be followed by attributes, and so consistency might 
imply that we allow attributes on the function declarator after an 
identifier-list instead?



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3855-3857
+  AttributeList::Syntax Syntax = getLangOpts().CPlusPlus11
+ ? AttributeList::AS_CXX11
+ : AttributeList::AS_C2x;

This uses `AS_C2x` if `-fdouble-square-bracket-attributes` is enabled in C++98, 
which is probably not the desired behavior.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3913
 
-/// ParseCXX11AttributeSpecifier - Parse a C++11 attribute-specifier.
+/// ParseCXX11Attributespecifier - Parse a C++11 or C2x attribute-specifier.
 ///

Accidental lowercasing of an `S`?



Comment at: clang/lib/Parse/ParseTentative.cpp:592
   bool OuterMightBeMessageSend) {
-  if (Tok.is(tok::kw_alignas))
+  if (Tok.is(tok::kw_alignas) && getLangOpts().CPlusPlus11)
 return CAK_AttributeSpecifier;

This change is redundant. We wouldn't lex a `kw_alignas` token outside C++11.



Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.align/p6.cpp:31
 
-enum alignas(2) E : char; // expected-note {{declared with 'alignas' attribute 
here}}
-enum E : char {}; // expected-error {{'alignas' must be specified on 
definition if it is specified on any declaration}}
+enum alignas(2) E : char; // expected-error {{an attribute list cannot appear 
here}}
+enum E : char {};

This looks like the bug I pointed out above. This is a regression; this code is 
valid.



Comment at: include/clang/Driver/Options.td:607
 
+def fc_attributes : Flag<["-"], "fc-attributes">, Group,
+  Flags<[DriverOption, CC1Option]>, HelpText<"Enable '[[]]' attributes in C">;

aaron.ballman wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > rsmith wrote:
> > > > Hmm. On reflection, if we're going to have a flag to enable C++11 
> > > > attributes in other language modes, it should probably work in C++98 
> > > > mode too, so calling this `-fc-attributes` is probably not the best 
> > > > idea. `-fc++11-attributes` might make sense, though.
> > > I can't think of a reason why you'd want to control C and C++ attributes 
> > > separately, so I think it makes sense to add a more general name for this.
> > > 
> > > I'm definitely not keen on that flag name. I wouldn't be opposed to 
> > > `-fattributes`, but that may lead to confusion about other 
> > > vendor-specific attributes (which we could always decide to use flags 
> > > like `-fdeclspec-attributes` etc).
> > > 
> > > What should happen if a user compiles with `-std=c++11 
> > > -fno--attributes`? Do we want to support such a construct?
> > I wouldn't recommend anyone actually does that, but I'd expect clang to 
> > respect their wishes if they ask for it, just like we do for 
> > `-fms-extensions -fno-declspec`.
> Would you be okay with `-fattributes` as the flag name, or do you prefer 
> `-fdouble-square-bracket-attributes`?
I think `-fattributes` is problematically non-specific. I think it would be 
surprising that `-fno-attributes` does not turn off support for 
`__attribute__`, for example.


https://reviews.llvm.org/D37436



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

[PATCH] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.

We found that the IR IRGen emits when expanding __builtin_os_log_format is 
quite big and has a lot of redundancy.

For example, when clang compiles the following code:

  void foo1(void *buf) {
__builtin_os_log_format(buf, "%d %d", 11, 22);
  }

The IR looks like this:

  define void @foo1(i8* %buf) #0 {
  entry:
%buf.addr = alloca i8*, align 8
store i8* %buf, i8** %buf.addr, align 8
%0 = load i8*, i8** %buf.addr, align 8
%summary = getelementptr i8, i8* %0, i64 0
store i8 0, i8* %summary, align 1
%numArgs = getelementptr i8, i8* %0, i64 1
store i8 2, i8* %numArgs, align 1
%argDescriptor = getelementptr i8, i8* %0, i64 2
store i8 0, i8* %argDescriptor, align 1
%argSize = getelementptr i8, i8* %0, i64 3
store i8 4, i8* %argSize, align 1
%1 = getelementptr i8, i8* %0, i64 4
%2 = bitcast i8* %1 to i32*
store i32 11, i32* %2, align 1
%argDescriptor1 = getelementptr i8, i8* %0, i64 8
store i8 0, i8* %argDescriptor1, align 1
%argSize2 = getelementptr i8, i8* %0, i64 9
store i8 4, i8* %argSize2, align 1
%3 = getelementptr i8, i8* %0, i64 10
%4 = bitcast i8* %3 to i32*
store i32 22, i32* %4, align 1
ret void
  }

The IR generated when compiling a similar call like 
"__builtin_os_log_format(buf, "%d %d", 33, 44)" is almost the same except for 
the values of the integer constants stored, so there is an opportunity for code 
reductionton here.

To reduce code size, this patch modifies IRGen to emit a helper function that 
can be used by different call sites that call __builtin_os_log_format in a 
program. When compiling with -Oz, the generated helper function is marked as 
linkonce_odr, hidden, and noinline so that the linker can merge identical 
helper functions from different translation units. When compiling with other 
optimization levels, the function is marked as 'internal' and the generated IR 
should look mostly the same after inlining.

This patch also fixes a bug where the generated IR writes past the buffer when 
%m is the last directive. For example, the size of 'buf' in the following code 
is 4 but IRGen emits a store that writes a 4-byte value at buf+4.

  char buf[__builtin_os_log_format_buffer_size("%m")];
  __builtin_os_log_format(buf, "%m");

Original patch was written by Duncan.

rdar://problem/34065973
dar://problem/34196543


https://reviews.llvm.org/D38606

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/builtins.c
  test/CodeGenObjC/os_log.m

Index: test/CodeGenObjC/os_log.m
===
--- test/CodeGenObjC/os_log.m
+++ test/CodeGenObjC/os_log.m
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple x86_64-darwin-apple -fobjc-arc -O2 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple x86_64-darwin-apple -fobjc-arc -O0 | FileCheck %s -check-prefix=CHECK-O0
 
 // Make sure we emit clang.arc.use before calling objc_release as part of the
 // cleanup. This way we make sure the object will not be released until the
@@ -12,28 +13,67 @@
 // Behavior of __builtin_os_log differs between platforms, so only test on X86
 #ifdef __x86_64__
 // CHECK-LABEL: define i8* @test_builtin_os_log
+// CHECK-O0-LABEL: define i8* @test_builtin_os_log
+// CHECK: (i8* returned %[[BUF:.*]])
+// CHECK-O0: (i8* %[[BUF:.*]])
 void *test_builtin_os_log(void *buf) {
   return __builtin_os_log_format(buf, "capabilities: %@", GenString());
 
-  // CHECK: store i8 2, i8*
-  // CHECK: [[NUM_ARGS:%.*]] = getelementptr i8, i8* {{.*}}, i64 1
-  // CHECK: store i8 1, i8* [[NUM_ARGS]]
-  //
-  // CHECK: [[ARG1_DESC:%.*]] = getelementptr i8, i8* {{.*}}, i64 2
-  // CHECK: store i8 64, i8* [[ARG1_DESC]]
-  // CHECK: [[ARG1_SIZE:%.*]] = getelementptr i8, i8* {{.*}}, i64 3
-  // CHECK: store i8 8, i8* [[ARG1_SIZE]]
-  // CHECK: [[ARG1:%.*]] = getelementptr i8, i8* {{.*}}, i64 4
-  // CHECK: [[ARG1_CAST:%.*]] = bitcast i8* [[ARG1]] to
-
-  // CHECK: [[STRING:%.*]] = {{.*}} call {{.*}} @GenString()
-  // CHECK: [[STRING_CAST:%.*]] = bitcast {{.*}} [[STRING]] to
-  // CHECK: call {{.*}} @objc_retainAutoreleasedReturnValue(i8* [[STRING_CAST]])
-  // CHECK: store {{.*}} [[STRING]], {{.*}} [[ARG1_CAST]]
-
-  // CHECK: call void (...) @clang.arc.use({{.*}} [[STRING]])
-  // CHECK: call void @objc_release(i8* [[STRING_CAST]])
-  // CHECK: ret i8*
+  // CHECK: %[[CALL:.*]] = tail call %[[V0:.*]]* (...) @GenString()
+  // CHECK: %[[V0]] = bitcast %[[V0]]* %[[CALL]] to i8*
+  // CHECK: %[[V1:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %[[V0]])
+  // CHECK: %[[V2:.*]] = ptrtoint %[[V0]]* %[[CALL]] to i64
+  // CHECK: store i8 2, i8* %[[BUF]], align 1
+  // CHECK: %[[NUMARGS_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
+  // CHECK: store i8 1, i8* %[[NUMARGS_I]], align 1
+  // CHECK: %[[ARGDESCRIPTOR_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
+  // CHECK: store i8 64, i8* %[[ARGDESCRIPTOR_I]], 

r315030 - [CMake] Add install-distribution-toolchain to stage2 target list

[Chris Bieneman via cfe-commits]

Author: cbieneman
Date: Thu Oct  5 15:48:34 2017
New Revision: 315030

URL: http://llvm.org/viewvc/llvm-project?rev=315030=rev
Log:
[CMake] Add install-distribution-toolchain to stage2 target list

This allows generating toolchains with just the distribution components instead 
of toolchains with everything.

Modified:
cfe/trunk/cmake/caches/Apple-stage1.cmake

Modified: cfe/trunk/cmake/caches/Apple-stage1.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Apple-stage1.cmake?rev=315030=315029=315030=diff
==
--- cfe/trunk/cmake/caches/Apple-stage1.cmake (original)
+++ cfe/trunk/cmake/caches/Apple-stage1.cmake Thu Oct  5 15:48:34 2017
@@ -46,6 +46,7 @@ set(CLANG_BOOTSTRAP_TARGETS
   distribution
   install-distribution
   install-xcode-toolchain
+  install-distribution-toolchain
   clang CACHE STRING "")
 
 #bootstrap


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

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:411
+  // component so as it can be accessed directly with lower cost.
+  auto betterBeSingleFieldRun = [&](RecordDecl::field_iterator Field) {
+if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)

betterBeSingleFieldRun -> IsBetterAsSingleFieldRun



Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:444
 // Add bitfields to the run as long as they qualify.
-if (Field != FieldEnd && Field->getBitWidthValue(Context) != 0 &&
-Tail == getFieldBitOffset(*Field)) {
-  Tail += Field->getBitWidthValue(Context);
-  ++Field;
-  continue;
+if (Field != FieldEnd && !SingleFieldRun) {
+  SingleFieldRun = betterBeSingleFieldRun(Field);

The logic here is not obvious. Can you please add a comment. SingleFieldRun 
here is only not equal to `betterBeSingleFieldRun(Field)` if we've skipped 
0-length bitfields, right? Please explain what's going on and also please make 
sure there's a test case.


Repository:
  rL LLVM

https://reviews.llvm.org/D36562



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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:29
+namespace {
+TargetInfo::IntType GetDefaultWCharType(const llvm::Triple ) {
+  const llvm::Triple::ArchType Arch = T.getArch();

compnerd wrote:
> rnk wrote:
> > How is this better than what we had before? It's totally inconsistent with 
> > our existing strategy for figuring out type widths and sizes. Our current 
> > approach can be extended for new targets, this requires modifying shared 
> > TargetInfo code. This refactoring *should* be NFC anyway, so if we did want 
> > to do it, we'd want to split it out.
> The previous thing was split across and was fairly difficult to identify what 
> was going on.  I tend to think that this makes it easier to identify what is 
> going on.  However, if you have a strong opinion on this, Im willing to 
> switch it back (though, possibly retain some of the adjustments to make it 
> easier to follow).
I do, I want to see the minimal functional change. It'll make it easier to spot 
awkward places where we duplicate logic.


https://reviews.llvm.org/D37891



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

[clang-tools-extra] r315028 - [clangd] Attempt to fix compilation with MSVC.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Oct  5 15:15:15 2017
New Revision: 315028

URL: http://llvm.org/viewvc/llvm-project?rev=315028=rev
Log:
[clangd] Attempt to fix compilation with MSVC.

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=315028=315027=315028=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Oct  5 15:15:15 2017
@@ -50,6 +50,9 @@ typedef std::string VFSTag;
 /// FileSystemProvider when this value was computed.
 template  class Tagged {
 public:
+  // MSVC requires future<> arguments to be default-constructible.
+  Tagged() = default;
+
   template 
   Tagged(U &, VFSTag Tag)
   : Value(std::forward(Value)), Tag(std::move(Tag)) {}
@@ -61,8 +64,8 @@ public:
   Tagged(Tagged &)
   : Value(std::move(Other.Value)), Tag(std::move(Other.Tag)) {}
 
-  T Value;
-  VFSTag Tag;
+  T Value = T();
+  VFSTag Tag = VFSTag();
 };
 
 template 


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

[PATCH] D38601: Enable amdfam15,amdfam15h, and amdfam10h support

[Erich Keane via Phabricator via cfe-commits]

erichkeane abandoned this revision.
erichkeane added a comment.

Talking to Craig, this is just wrong for these features.  Attribute-target does 
some funky things in order to make this work, so I'll just bring those 'hacks' 
into my other patch.


https://reviews.llvm.org/D38601



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

RE: [clang-tools-extra] r314989 - [clangd] Added async API to run code completion.

[Yung, Douglas via cfe-commits]

Hi Ilya,

Your change has broken the build on the PS4 Windows bot.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/12525

Can you take a look and fix it?

Douglas Yung

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Ilya Biryukov via cfe-commits
> Sent: Thursday, October 05, 2017 10:04
> To: cfe-commits@lists.llvm.org
> Subject: [clang-tools-extra] r314989 - [clangd] Added async API to run code
> completion.
> 
> Author: ibiryukov
> Date: Thu Oct  5 10:04:13 2017
> New Revision: 314989
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=314989=rev
> Log:
> [clangd] Added async API to run code completion.
> 
> Summary:
> ClangdServer now provides async code completion API.
> It is still used synchronously by ClangdLSPServer, more work is needed to
> allow processing other requests in parallel while completion (or any other
> request) is running.
> 
> Reviewers: klimek, bkramer, krasimir
> 
> Reviewed By: klimek
> 
> Subscribers: cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D38583
> 
> Modified:
> clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.h
> clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
> 
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> extra/trunk/clangd/ClangdLSPServer.cpp?rev=314989=314988=314989=dif
> f
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct  5
> +++ 10:04:13 2017
> @@ -149,6 +149,9 @@ void ClangdLSPServer::onCompletion(TextD
> .codeComplete(Params.textDocument.uri.file,
>   Position{Params.position.line,
>Params.position.character})
> +   .get() // FIXME(ibiryukov): This could be made async if we
> +  // had an API that would allow to attach callbacks
> to
> +  // futures returned by ClangdServer.
> .Value;
> 
>std::string Completions;
> 
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> extra/trunk/clangd/ClangdServer.cpp?rev=314989=314988=314989=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct  5 10:04:13
> +++ 2017
> @@ -194,18 +194,19 @@ std::future ClangdServer::forceRep
>   std::move(TaggedFS));  }
> 
> -Tagged
> +std::future>
>  ClangdServer::codeComplete(PathRef File, Position Pos,
> llvm::Optional OverridenContents,
> IntrusiveRefCntPtr *UsedFS) {
> -  std::string DraftStorage;
> -  if (!OverridenContents) {
> +  std::string Contents;
> +  if (OverridenContents) {
> +Contents = *OverridenContents;
> +  } else {
>  auto FileContents = DraftMgr.getDraft(File);
>  assert(FileContents.Draft &&
> "codeComplete is called for non-added document");
> 
> -DraftStorage = std::move(*FileContents.Draft);
> -OverridenContents = DraftStorage;
> +Contents = std::move(*FileContents.Draft);
>}
> 
>auto TaggedFS = FSProvider.getTaggedFileSystem(File);
> @@ -215,12 +216,36 @@ ClangdServer::codeComplete(PathRef File,
>std::shared_ptr Resources = Units.getFile(File);
>assert(Resources && "Calling completion on non-added file");
> 
> -  auto Preamble = Resources->getPossiblyStalePreamble();
> -  std::vector Result = clangd::codeComplete(
> -  File, Resources->getCompileCommand(),
> -  Preamble ? >Preamble : nullptr, *OverridenContents, Pos,
> -  TaggedFS.Value, PCHs, SnippetCompletions, Logger);
> -  return make_tagged(std::move(Result), TaggedFS.Tag);
> +  using PackagedTask =
> +  std::packaged_task()>;
> +
> +  // Remember the current Preamble and use it when async task starts
> executing.
> +  // At the point when async task starts executing, we may have a
> + different  // Preamble in Resources. However, we assume the Preamble
> + that we obtain here  // is reusable in completion more often.
> +  std::shared_ptr Preamble =
> +  Resources->getPossiblyStalePreamble();
> +  // A task that will be run asynchronously.
> +  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble
> variable.
> +if (!Preamble) {
> +  // Maybe we built some preamble before processing this request.
> +  Preamble = Resources->getPossiblyStalePreamble();
> +}
> +// FIXME(ibiryukov): even if 

r315025 - For dllexport class templates, export specializations of member functions (PR34849)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Oct  5 14:45:27 2017
New Revision: 315025

URL: http://llvm.org/viewvc/llvm-project?rev=315025=rev
Log:
For dllexport class templates, export specializations of member functions 
(PR34849)

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=315025=315024=315025=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Oct  5 14:45:27 2017
@@ -6041,6 +6041,21 @@ static void checkDLLAttributeRedeclarati
diag::warn_dllimport_dropped_from_inline_function)
 << NewDecl << OldImportAttr;
   }
+
+  // A specialization of a class template member function is processed here
+  // since it's a redeclaration. If the parent class is dllexport, the
+  // specialization inherits that attribute. This doesn't happen automatically
+  // since the parent class isn't instantiated until later.
+  if (IsSpecialization && isa(NewDecl) && !NewImportAttr &&
+  !NewExportAttr) {
+if (const DLLExportAttr *ParentExportAttr = cast(NewDecl)
+ ->getParent()
+ ->getAttr()) {
+  DLLExportAttr *NewAttr = ParentExportAttr->clone(S.Context);
+  NewAttr->setInherited(true);
+  NewDecl->addAttr(NewAttr);
+}
+  }
 }
 
 /// Given that we are within the definition of the given function,

Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=315025=315024=315025=diff
==
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Thu Oct  5 14:45:27 2017
@@ -831,6 +831,14 @@ template  struct ExplicitIns
 template struct __declspec(dllexport) __declspec(dllimport) 
ExplicitInstantiationTwoAttributes;
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void 
@"\01?f@?$ExplicitInstantiationTwoAttributes@H@@QAEXXZ"
 
+// Specializations of exported class template functions get exported.
+namespace pr34849 {
+template  struct __declspec(dllexport) ExportedClass { void foo(); 
};
+template<> void ExportedClass::foo() {}
+template struct ExportedClass;
+// M32-DAG: define dllexport x86_thiscallcc void 
@"\01?foo@?$ExportedClass@H@pr34849@@QAEXXZ"
+}
+
 
 
//===--===//
 // Classes with template base classes


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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Does dlopen cause issues even with `RTLD_GLOBAL`?


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38601: Enable amdfam15,amdfam15h, and amdfam10h support

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.

buildin_cpu_is already supports amdfam10h and amdfam15h, the 'sans-h'
version to the check and enable it.

Addtionally, add amdfam15 as an alias for bdver1, like amdfam10 is for
barcelona.


https://reviews.llvm.org/D38601

Files:
  lib/Basic/Targets/X86.cpp
  test/CodeGen/target-builtin-noerror.c
  test/Preprocessor/predefined-arch-macros.c


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1291,8 +1291,8 @@
 bool X86TargetInfo::validateCpuIs(StringRef FeatureStr) const {
   return llvm::StringSwitch(FeatureStr)
   .Case("amd", true)
-  .Case("amdfam10h", true)
-  .Case("amdfam15h", true)
+  .Cases("amdfam10h", "amdfam10", true)
+  .Cases("amdfam15h", "amdfam15", true)
   .Case("atom", true)
   .Case("barcelona", true)
   .Case("bdver1", true)
@@ -1563,7 +1563,7 @@
   .Cases("amdfam10", "barcelona", CK_AMDFAM10)
   .Case("btver1", CK_BTVER1)
   .Case("btver2", CK_BTVER2)
-  .Case("bdver1", CK_BDVER1)
+  .Cases("amdfam15", "bdver1", CK_BDVER1)
   .Case("bdver2", CK_BDVER2)
   .Case("bdver3", CK_BDVER3)
   .Case("bdver4", CK_BDVER4)
Index: test/CodeGen/target-builtin-noerror.c
===
--- test/CodeGen/target-builtin-noerror.c
+++ test/CodeGen/target-builtin-noerror.c
@@ -80,7 +80,9 @@
 void verifycpustrings() {
   (void)__builtin_cpu_is("amd");
   (void)__builtin_cpu_is("amdfam10h");
+  (void)__builtin_cpu_is("amdfam10");
   (void)__builtin_cpu_is("amdfam15h");
+  (void)__builtin_cpu_is("amdfam15");
   (void)__builtin_cpu_is("atom");
   (void)__builtin_cpu_is("barcelona");
   (void)__builtin_cpu_is("bdver1");
Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1673,6 +1673,9 @@
 // RUN: %clang -march=bdver1 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BDVER1_M32
+// RUN: %clang -march=amdfam15 -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BDVER1_M32
 // CHECK_BDVER1_M32-NOT: #define __3dNOW_A__ 1
 // CHECK_BDVER1_M32-NOT: #define __3dNOW__ 1
 // CHECK_BDVER1_M32: #define __AES__ 1
@@ -1703,6 +1706,9 @@
 // RUN: %clang -march=bdver1 -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BDVER1_M64
+// RUN: %clang -march=amdfam15 -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BDVER1_M64
 // CHECK_BDVER1_M64-NOT: #define __3dNOW_A__ 1
 // CHECK_BDVER1_M64-NOT: #define __3dNOW__ 1
 // CHECK_BDVER1_M64: #define __AES__ 1


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1291,8 +1291,8 @@
 bool X86TargetInfo::validateCpuIs(StringRef FeatureStr) const {
   return llvm::StringSwitch(FeatureStr)
   .Case("amd", true)
-  .Case("amdfam10h", true)
-  .Case("amdfam15h", true)
+  .Cases("amdfam10h", "amdfam10", true)
+  .Cases("amdfam15h", "amdfam15", true)
   .Case("atom", true)
   .Case("barcelona", true)
   .Case("bdver1", true)
@@ -1563,7 +1563,7 @@
   .Cases("amdfam10", "barcelona", CK_AMDFAM10)
   .Case("btver1", CK_BTVER1)
   .Case("btver2", CK_BTVER2)
-  .Case("bdver1", CK_BDVER1)
+  .Cases("amdfam15", "bdver1", CK_BDVER1)
   .Case("bdver2", CK_BDVER2)
   .Case("bdver3", CK_BDVER3)
   .Case("bdver4", CK_BDVER4)
Index: test/CodeGen/target-builtin-noerror.c
===
--- test/CodeGen/target-builtin-noerror.c
+++ test/CodeGen/target-builtin-noerror.c
@@ -80,7 +80,9 @@
 void verifycpustrings() {
   (void)__builtin_cpu_is("amd");
   (void)__builtin_cpu_is("amdfam10h");
+  (void)__builtin_cpu_is("amdfam10");
   (void)__builtin_cpu_is("amdfam15h");
+  (void)__builtin_cpu_is("amdfam15");
   (void)__builtin_cpu_is("atom");
   (void)__builtin_cpu_is("barcelona");
   (void)__builtin_cpu_is("bdver1");
Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1673,6 +1673,9 @@
 // RUN: %clang -march=bdver1 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BDVER1_M32
+// RUN: %clang -march=amdfam15 -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck 

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Dan Albert via Phabricator via cfe-commits]

danalbert created this revision.

This doesn't only happen for incorrectly built apps, it also happens
for libraries loaded with dlopen.


Repository:
  rL LLVM

https://reviews.llvm.org/D38599

Files:
  src/private_typeinfo.cpp


Index: src/private_typeinfo.cpp
===
--- src/private_typeinfo.cpp
+++ src/private_typeinfo.cpp
@@ -10,39 +10,19 @@
 #include "private_typeinfo.h"
 
 // The flag _LIBCXX_DYNAMIC_FALLBACK is used to make dynamic_cast more
-// forgiving when type_info's mistakenly have hidden visibility and thus
-// multiple type_infos can exist for a single type.
-// 
+// forgiving when multiple type_infos exist for a single type. This happens if
+// the libraries are mistakenly built with the type_infos having hidden
+// visibility, but also occurs when the libraries are loaded with dlopen.
+//
 // When _LIBCXX_DYNAMIC_FALLBACK is defined, and only in the case where
 // there is a detected inconsistency in the type_info hierarchy during a
 // dynamic_cast, then the equality operation will fall back to using strcmp
 // on type_info names to determine type_info equality.
-// 
-// This change happens *only* under dynamic_cast, and only when
-// dynamic_cast is faced with the choice:  abort, or possibly give back the
-// wrong answer.  If when the dynamic_cast is done with this fallback
-// algorithm and an inconsistency is still detected, dynamic_cast will call
-// abort with an appropriate message.
-// 
-// The current implementation of _LIBCXX_DYNAMIC_FALLBACK requires a
-// printf-like function called syslog:
-// 
-// void syslog(int facility_priority, const char* format, ...);
-// 
-// If you want this functionality but your platform doesn't have syslog,
-// just implement it in terms of fprintf(stderr, ...).
-// 
+//
 // _LIBCXX_DYNAMIC_FALLBACK is currently off by default.
 
-
 #include 
 
-
-#ifdef _LIBCXX_DYNAMIC_FALLBACK
-#include "abort_message.h"
-#include 
-#endif
-
 // On Windows, typeids are different between DLLs and EXEs, so comparing
 // type_info* will work for typeids from the same compiled file but fail
 // for typeids from a DLL and an executable. Among other things, exceptions
@@ -647,11 +627,11 @@
 //   find (static_ptr, static_type), either on a public or private path
 if (info.path_dst_ptr_to_static_ptr == unknown)
 {
-// We get here only if there is some kind of visibility problem
-//   in client code.
-syslog(LOG_ERR, "dynamic_cast error 1: Both of the following 
type_info's "
-"should have public visibility.  At least one of them is 
hidden. %s" 
-", %s.\n", static_type->name(), dynamic_type->name());
+// We get here only if there is some kind of visibility problem in
+// client code. Possibly because the binaries were built
+// incorrectly, but possibly because the library was loaded with
+// dlopen.
+//
 // Redo the search comparing type_info's using strcmp
 info = {dst_type, static_ptr, static_type, src2dst_offset, 0};
 info.number_of_dst_type = 1;
@@ -672,10 +652,6 @@
 if (info.path_dst_ptr_to_static_ptr == unknown &&
 info.path_dynamic_ptr_to_static_ptr == unknown)
 {
-syslog(LOG_ERR, "dynamic_cast error 2: One or more of the 
following type_info's "
-" has hidden visibility.  They should all have 
public visibility.  "
-" %s, %s, %s.\n", static_type->name(), 
dynamic_type->name(),
-dst_type->name());
 // Redo the search comparing type_info's using strcmp
 info = {dst_type, static_ptr, static_type, src2dst_offset, 0};
 dynamic_type->search_below_dst(, dynamic_ptr, public_path, 
true);


Index: src/private_typeinfo.cpp
===
--- src/private_typeinfo.cpp
+++ src/private_typeinfo.cpp
@@ -10,39 +10,19 @@
 #include "private_typeinfo.h"
 
 // The flag _LIBCXX_DYNAMIC_FALLBACK is used to make dynamic_cast more
-// forgiving when type_info's mistakenly have hidden visibility and thus
-// multiple type_infos can exist for a single type.
-// 
+// forgiving when multiple type_infos exist for a single type. This happens if
+// the libraries are mistakenly built with the type_infos having hidden
+// visibility, but also occurs when the libraries are loaded with dlopen.
+//
 // When _LIBCXX_DYNAMIC_FALLBACK is defined, and only in the case where
 // there is a detected inconsistency in the type_info hierarchy during a
 // dynamic_cast, then the equality operation will fall back to using strcmp
 // on type_info names to determine type_info equality.
-// 
-// This change happens *only* under dynamic_cast, and only when
-// dynamic_cast is faced with the choice:  abort, or possibly give back the
-// wrong answer.  

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Francisco Lopes da Silva via Phabricator via cfe-commits]

francisco.lopes added a comment.

In https://reviews.llvm.org/D38048#889568, @rwols wrote:

> Since we're throwing around gifs for fun here's how signatureHelp looks in 
> practice for Sublime Text


Very cool!


https://reviews.llvm.org/D38048



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

[PATCH] D38596: Implement attribute target multiversioning

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D38596#889697, @erichkeane wrote:

> In https://reviews.llvm.org/D38596#889682, @hfinkel wrote:
>
> > Can you explain briefly what is required of the system in order to support 
> > this (is it just ifunc support in the dynamic linker / loader)?
>
>
> Yep, this feature depends entirely on ifuncs.
>
> > In general, there are a number of places in this patch, at the Sema layer 
> > (including in the error messages), that talk about how this is an x86-only 
> > feature. As soon as someone adds support for a second architecture, this 
> > will cause unnecessary churn (and perhaps miss places resulting in stale 
> > comments). Can you create some callbacks, in TargetInfo or similar, so that 
> > we can just naturally make this work on other architectures?
>
> Yep, I have that enforced, with the hope that it would make it easier to 
> identify all the places that this change needs to be made.  Currently, there 
> are a few 'TargetInfo' features that need to be supported 
> (isValidCPUName/isValidCPUFeature/compareCpusAndFeatures), the SEMA check 
> relaxed (though I could perhaps add a 
> TargetInfo::IsFunctionMultiVersioningSupported function if thats what you're 
> saying?), and 
> CodeGenFunction::FormResolverCondition/CodeGenFunction::EmitMultiVersionResolver
>  ( which could easily have FormResolverCondition moved to TargetInfo, 
> assuming we OK with a CodeGen file calling into a TargetInfo?).  Are these 
> the changes you mean?


I added some comments inline.

> 
> 
>> 
>> 
>>> Function templates are NOT supported (not supported in GCC either).
>> 
>> Can you please elaborate on this? I'd like to see this feature, but I don't 
>> believe that we should introduce features that artificially force users to 
>> choose between good code design and technical capabilities. The fact that 
>> GCC does not support this feature on templates seems unfortunate, but not in 
>> itself a justification to replicate that restriction. I'm obviously fine 
>> with adding template functions in a follow-up commit, but I want to 
>> understand whether there's something in the design (of the feature or of 
>> your implementation approach) that makes this hard. I wouldn't want this to 
>> ship without template support.
> 
> I don't think there are limitations beyond my understanding of the code 
> around redefinitions of templates.  I investigated for a while and was unable 
> to figure it out over a few days, but that doesn't mean terribly much.  I 
> gave up temporarily in exchange for getting the rest of the feature in (and 
> the fact that GCC doesn't support it made it an explainable stopping point; 
> additionally, none of our users of 'attribute-target' actually have C++ 
> codebases).

I anticipate this will change.

>   I believe I'd be able to add function-template support with some additional 
> work, and am definitely willing to do so.

Great, thanks!

> Thanks Hal, I appreciate your advice/feedback!  I realize this is a huge 
> patch, so anything I can get to make this more acceptable is greatly 
> appreciated.






Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9305
+: Error<"function multiversioning with 'target' is only supported on "
+"X86/x86_64 architectures">;
+def err_bad_multiversion_option

I'd not mention x86 here. As soon as we add support for another architecture, 
we'll need to reword this anyway.

  : Error<"function multiversioning with 'target' is not supported on this 
architecture and platform">;




Comment at: lib/Basic/Targets/X86.cpp:1295
   .Case("amdfam10h", true)
+  .Case("amdfam10", true)
   .Case("amdfam15h", true)

Can you please separate out these changes adding the amdfam10 target strings 
into a separate patch?



Comment at: lib/Basic/Targets/X86.cpp:1330
+  // ordering of the GCC Target attribute.
+  const auto TargetArray = {"avx512vpopcntdq",
+"avx5124fmaps",

erichkeane wrote:
> hfinkel wrote:
> > How are we expected to maintain this array?
> Ugg... I know.  I have no idea, it is ANOTHER implementation of this foolish 
> list, just in a slightly different order.  We NEED to know the order, so we 
> know how to order the comparisons.  They are generally arbitrary orderings, 
> so I have no idea besides "with hard work".  If you have a suggestion, or 
> perhaps a way to reorder one of the OTHER lists, I'd love to hear it.
At the risk of a comment that will become stale at some point, is it possible 
to say something here like:

  // This list has to match the list in GCC. In GCC 6.whatever, the list is in 
path/to/file/in/gcc.c

At least that would give someone a clue of how to update this list if necessary.




Comment at: lib/Sema/SemaDecl.cpp:9249
+
+  // Need to check isValidCPUName since it checks X86 vs X86-64 CPUs.  From
+  // there, the subset of 

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Looks good to me.


https://reviews.llvm.org/D38473



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

[PATCH] D38596: Implement attribute target multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D38596#889682, @hfinkel wrote:

> Can you explain briefly what is required of the system in order to support 
> this (is it just ifunc support in the dynamic linker / loader)?


Yep, this feature depends entirely on ifuncs.

> In general, there are a number of places in this patch, at the Sema layer 
> (including in the error messages), that talk about how this is an x86-only 
> feature. As soon as someone adds support for a second architecture, this will 
> cause unnecessary churn (and perhaps miss places resulting in stale 
> comments). Can you create some callbacks, in TargetInfo or similar, so that 
> we can just naturally make this work on other architectures?

Yep, I have that enforced, with the hope that it would make it easier to 
identify all the places that this change needs to be made.  Currently, there 
are a few 'TargetInfo' features that need to be supported 
(isValidCPUName/isValidCPUFeature/compareCpusAndFeatures), the SEMA check 
relaxed (though I could perhaps add a 
TargetInfo::IsFunctionMultiVersioningSupported function if thats what you're 
saying?), and 
CodeGenFunction::FormResolverCondition/CodeGenFunction::EmitMultiVersionResolver
 ( which could easily have FormResolverCondition moved to TargetInfo, assuming 
we OK with a CodeGen file calling into a TargetInfo?).  Are these the changes 
you mean?

> 
> 
>> Function templates are NOT supported (not supported in GCC either).
> 
> Can you please elaborate on this? I'd like to see this feature, but I don't 
> believe that we should introduce features that artificially force users to 
> choose between good code design and technical capabilities. The fact that GCC 
> does not support this feature on templates seems unfortunate, but not in 
> itself a justification to replicate that restriction. I'm obviously fine with 
> adding template functions in a follow-up commit, but I want to understand 
> whether there's something in the design (of the feature or of your 
> implementation approach) that makes this hard. I wouldn't want this to ship 
> without template support.

I don't think there are limitations beyond my understanding of the code around 
redefinitions of templates.  I investigated for a while and was unable to 
figure it out over a few days, but that doesn't mean terribly much.  I gave up 
temporarily in exchange for getting the rest of the feature in (and the fact 
that GCC doesn't support it made it an explainable stopping point; 
additionally, none of our users of 'attribute-target' actually have C++ 
codebases).  I believe I'd be able to add function-template support with some 
additional work, and am definitely willing to do so.

Thanks Hal, I appreciate your advice/feedback!  I realize this is a huge patch, 
so anything I can get to make this more acceptable is greatly appreciated.




Comment at: lib/Basic/Targets/X86.cpp:1330
+  // ordering of the GCC Target attribute.
+  const auto TargetArray = {"avx512vpopcntdq",
+"avx5124fmaps",

hfinkel wrote:
> How are we expected to maintain this array?
Ugg... I know.  I have no idea, it is ANOTHER implementation of this foolish 
list, just in a slightly different order.  We NEED to know the order, so we 
know how to order the comparisons.  They are generally arbitrary orderings, so 
I have no idea besides "with hard work".  If you have a suggestion, or perhaps 
a way to reorder one of the OTHER lists, I'd love to hear it.


https://reviews.llvm.org/D38596



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

[PATCH] D38596: Implement attribute target multiversioning

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Can you explain briefly what is required of the system in order to support this 
(is it just ifunc support in the dynamic linker / loader)?

In general, there are a number of places in this patch, at the Sema layer 
(including in the error messages), that talk about how this is an x86-only 
feature. As soon as someone adds support for a second architecture, this will 
cause unnecessary churn (and perhaps miss places resulting in stale comments). 
Can you create some callbacks, in TargetInfo or similar, so that we can just 
naturally make this work on other architectures?

> Function templates are NOT supported (not supported in GCC either).

Can you please elaborate on this? I'd like to see this feature, but I don't 
believe that we should introduce features that artificially force users to 
choose between good code design and technical capabilities. The fact that GCC 
does not support this feature on templates seems unfortunate, but not in itself 
a justification to replicate that restriction. I'm obviously fine with adding 
template functions in a follow-up commit, but I want to understand whether 
there's something in the design (of the feature or of your implementation 
approach) that makes this hard. I wouldn't want this to ship without template 
support.




Comment at: lib/Basic/Targets/X86.cpp:1330
+  // ordering of the GCC Target attribute.
+  const auto TargetArray = {"avx512vpopcntdq",
+"avx5124fmaps",

How are we expected to maintain this array?


https://reviews.llvm.org/D38596



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

[libclc] r315018 - configure: Fix handling of directories with compats only source lists

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Thu Oct  5 13:16:28 2017
New Revision: 315018

URL: http://llvm.org/viewvc/llvm-project?rev=315018=rev
Log:
configure: Fix handling of directories with compats only source lists

Reviewer: Jeroen Ketema
Signed-off-by: Jan Vesely 

Modified:
libclc/trunk/configure.py

Modified: libclc/trunk/configure.py
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/configure.py?rev=315018=315017=315018=diff
==
--- libclc/trunk/configure.py (original)
+++ libclc/trunk/configure.py Thu Oct  5 13:16:28 2017
@@ -185,7 +185,8 @@ for target in targets:
 
   incdirs = filter(os.path.isdir,
[os.path.join(srcdir, subdir, 'include') for subdir in subdirs])
-  libdirs = filter(lambda d: os.path.isfile(os.path.join(d, 'SOURCES')),
+  libdirs = filter(lambda d: os.path.isfile(os.path.join(d, 'SOURCES')) or
+ os.path.isfile(os.path.join(d, 'SOURCES_' + 
llvm_string_version)),
[os.path.join(srcdir, subdir, 'lib') for subdir in subdirs])
 
   # The above are iterables in python3 but we might use them multiple times
@@ -218,7 +219,8 @@ for target in targets:
 
 for libdir in libdirs:
   subdir_list_file = os.path.join(libdir, 'SOURCES')
-  manifest_deps.add(subdir_list_file)
+  if os.path.exists(subdir_list_file):
+manifest_deps.add(subdir_list_file)
   override_list_file = os.path.join(libdir, 'OVERRIDES')
   compat_list_file = os.path.join(libdir,
 'SOURCES_' + llvm_string_version)
@@ -227,6 +229,7 @@ for target in targets:
 
   # Build compat list
   if os.path.exists(compat_list_file):
+manifest_deps.add(compat_list_file)
 for compat in open(compat_list_file).readlines():
   compat = compat.rstrip()
   compats.append(compat)
@@ -243,7 +246,8 @@ for target in targets:
   override = override.rstrip()
   sources_seen.add(override)
 
-  for src in open(subdir_list_file).readlines() + compats:
+  files = open(subdir_list_file).readlines() if 
os.path.exists(subdir_list_file) else []
+  for src in files + compats:
 src = src.rstrip()
 if src not in sources_seen:
   sources_seen.add(src)


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

[PATCH] D38525: Cleanup and generalize -shared-libasan.

[Evgenii Stepanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315015: Cleanup and generalize -shared-libasan. (authored by 
eugenis).

Changed prior to commit:
  https://reviews.llvm.org/D38525?vs=117875=117878#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38525

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/SanitizerArgs.h
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
  cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
  cfe/trunk/test/Driver/sanitizer-ld.c

Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -610,10 +610,11 @@
   CoverageFeatures |= CoverageFunc;
   }
 
+  SharedRuntime =
+  Args.hasFlag(options::OPT_shared_libsan, options::OPT_static_libsan,
+   TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia());
+
   if (AllAddedKinds & Address) {
-AsanSharedRuntime =
-Args.hasArg(options::OPT_shared_libasan) ||
-TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 NeedPIE |= TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 if (Arg *A =
 Args.getLastArg(options::OPT_fsanitize_address_field_padding)) {
Index: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
@@ -81,7 +81,7 @@
   if (!Args.hasArg(options::OPT_shared)) {
 std::string Dyld = D.DyldPrefix;
 if (ToolChain.getSanitizerArgs().needsAsanRt() &&
-ToolChain.getSanitizerArgs().needsSharedAsanRt())
+ToolChain.getSanitizerArgs().needsSharedRt())
   Dyld += "asan/";
 Dyld += "ld.so.1";
 CmdArgs.push_back("-dynamic-linker");
Index: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
@@ -375,7 +375,7 @@
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
-if (TC.getSanitizerArgs().needsSharedAsanRt() ||
+if (TC.getSanitizerArgs().needsSharedRt() ||
 Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
   for (const auto  : {"asan_dynamic", "asan_dynamic_runtime_thunk"})
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -560,27 +560,35 @@
  SmallVectorImpl ) {
   const SanitizerArgs  = TC.getSanitizerArgs();
   // Collect shared runtimes.
-  if (SanArgs.needsAsanRt() && SanArgs.needsSharedAsanRt()) {
-SharedRuntimes.push_back("asan");
+  if (SanArgs.needsSharedRt()) {
+if (SanArgs.needsAsanRt()) {
+  SharedRuntimes.push_back("asan");
+  if (!Args.hasArg(options::OPT_shared) && !TC.getTriple().isAndroid())
+HelperStaticRuntimes.push_back("asan-preinit");
+}
+
+if (SanArgs.needsUbsanRt()) {
+  if (SanArgs.requiresMinimalRuntime()) {
+SharedRuntimes.push_back("ubsan_minimal");
+  } else {
+SharedRuntimes.push_back("ubsan_standalone");
+  }
+}
   }
 
   // The stats_client library is also statically linked into DSOs.
   if (SanArgs.needsStatsRt())
 StaticRuntimes.push_back("stats_client");
 
   // Collect static runtimes.
-  if (Args.hasArg(options::OPT_shared) || TC.getTriple().isAndroid()) {
-// Don't link static runtimes into DSOs or if compiling for Android.
+  if (Args.hasArg(options::OPT_shared) || SanArgs.needsSharedRt()) {
+// Don't link static runtimes into DSOs or if -shared-libasan.
 return;
   }
   if (SanArgs.needsAsanRt()) {
-if (SanArgs.needsSharedAsanRt()) {
-  HelperStaticRuntimes.push_back("asan-preinit");
-} else {
-  StaticRuntimes.push_back("asan");
-  if (SanArgs.linkCXXRuntimes())
-StaticRuntimes.push_back("asan_cxx");
-}
+StaticRuntimes.push_back("asan");
+if (SanArgs.linkCXXRuntimes())
+  StaticRuntimes.push_back("asan_cxx");
   }
   if (SanArgs.needsDfsanRt())
 StaticRuntimes.push_back("dfsan");
Index: cfe/trunk/include/clang/Driver/SanitizerArgs.h
===
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h
@@ -33,7 +33,7 @@
   bool MsanUseAfterDtor = false;
   bool CfiCrossDso = false;
   int AsanFieldPadding = 0;
-  bool AsanSharedRuntime = false;
+  bool 

r315015 - Cleanup and generalize -shared-libasan.

[Evgeniy Stepanov via cfe-commits]

Author: eugenis
Date: Thu Oct  5 13:14:00 2017
New Revision: 315015

URL: http://llvm.org/viewvc/llvm-project?rev=315015=rev
Log:
Cleanup and generalize -shared-libasan.

Summary:
* Rename -shared-libasan to -shared-libsan, keeping the old name as alias.
* Add -static-libsan for targets that default to shared.
* Remove an Android special case. It is now possible (but untested) to use 
static compiler-rt libraries there.
* Support libclang_rt.ubsan_standalone as a shared library.

Unlike GCC, this change applies -shared-libsan / -static-libsan to all 
sanitizers.
I don't see a point in multiple flags like -shared-libubsan, considering that 
most sanitizers
are not compatible with each other, and each link has basically a single 
shared/static choice.

Reviewers: vitalybuka, kcc, rsmith

Subscribers: srhines, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=315015=315014=315015=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Oct  5 13:14:00 2017
@@ -592,7 +592,9 @@ def fapple_kext : Flag<["-"], "fapple-ke
   HelpText<"Use Apple's kernel extensions ABI">;
 def fapple_pragma_pack : Flag<["-"], "fapple-pragma-pack">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable Apple gcc-compatible #pragma pack handling">;
-def shared_libasan : Flag<["-"], "shared-libasan">;
+def shared_libsan : Flag<["-"], "shared-libsan">;
+def static_libsan : Flag<["-"], "static-libsan">;
+def : Flag<["-"], "shared-libasan">, Alias;
 def fasm : Flag<["-"], "fasm">, Group;
 
 def fasm_blocks : Flag<["-"], "fasm-blocks">, Group, 
Flags<[CC1Option]>;

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=315015=315014=315015=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Thu Oct  5 13:14:00 2017
@@ -33,7 +33,7 @@ class SanitizerArgs {
   bool MsanUseAfterDtor = false;
   bool CfiCrossDso = false;
   int AsanFieldPadding = 0;
-  bool AsanSharedRuntime = false;
+  bool SharedRuntime = false;
   bool AsanUseAfterScope = true;
   bool AsanGlobalsDeadStripping = false;
   bool LinkCXXRuntimes = false;
@@ -49,8 +49,9 @@ class SanitizerArgs {
   /// Parses the sanitizer arguments from an argument list.
   SanitizerArgs(const ToolChain , const llvm::opt::ArgList );
 
+  bool needsSharedRt() const { return SharedRuntime; }
+
   bool needsAsanRt() const { return Sanitizers.has(SanitizerKind::Address); }
-  bool needsSharedAsanRt() const { return AsanSharedRuntime; }
   bool needsTsanRt() const { return Sanitizers.has(SanitizerKind::Thread); }
   bool needsMsanRt() const { return Sanitizers.has(SanitizerKind::Memory); }
   bool needsFuzzer() const { return Sanitizers.has(SanitizerKind::Fuzzer); }

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=315015=315014=315015=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Thu Oct  5 13:14:00 2017
@@ -610,10 +610,11 @@ SanitizerArgs::SanitizerArgs(const ToolC
   CoverageFeatures |= CoverageFunc;
   }
 
+  SharedRuntime =
+  Args.hasFlag(options::OPT_shared_libsan, options::OPT_static_libsan,
+   TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia());
+
   if (AllAddedKinds & Address) {
-AsanSharedRuntime =
-Args.hasArg(options::OPT_shared_libasan) ||
-TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 NeedPIE |= TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 if (Arg *A =
 Args.getLastArg(options::OPT_fsanitize_address_field_padding)) {

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=315015=315014=315015=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Thu Oct  5 13:14:00 2017
@@ -560,8 +560,20 @@ collectSanitizerRuntimes(const ToolChain
  SmallVectorImpl ) {
   const 

[PATCH] D38525: Cleanup and generalize -shared-libasan.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:614
+  if (Arg *A = Args.getLastArg(options::OPT_shared_libsan,
+   options::OPT_static_libsan))
+SharedRuntime = A->getOption().matches(options::OPT_shared_libsan);

vitalybuka wrote:
> ```
> SharedRuntime = Args.hasFlag(options::OPT_shared_libsan,,
>  options::OPT_static_libsan,
>  TC.getTriple().isAndroid() || 
> TC.getTriple().isOSFuchsia());
> ```
good!


https://reviews.llvm.org/D38525



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

[PATCH] D38525: Cleanup and generalize -shared-libasan.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis updated this revision to Diff 117875.
eugenis added a comment.

address comment


https://reviews.llvm.org/D38525

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -16,11 +16,24 @@
 // CHECK-ASAN-LINUX: "-lrt"
 // CHECK-ASAN-LINUX: "-ldl"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -shared-libsan \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SHARED-ASAN-LINUX %s
+
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -shared-libasan \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-SHARED-ASAN-LINUX %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address \
+// RUN: -shared-libsan -static-libsan -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SHARED-ASAN-LINUX %s
 //
 // CHECK-SHARED-ASAN-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SHARED-ASAN-LINUX-NOT: "-lc"
@@ -34,7 +47,7 @@
 // CHECK-SHARED-ASAN-LINUX-NOT: "--dynamic-list"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.so -shared 2>&1 \
-// RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -shared-libasan \
+// RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -shared-libsan \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-DSO-SHARED-ASAN-LINUX %s
@@ -131,6 +144,39 @@
 // CHECK-ASAN-ANDROID-NOT: "-lpthread"
 // CHECK-ASAN-ANDROID: libclang_rt.asan-arm-android.so"
 // CHECK-ASAN-ANDROID-NOT: "-lpthread"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=address \
+// RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
+// RUN: -static-libsan \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-ANDROID-STATICLIBASAN %s
+//
+// CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
+// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
+// RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
+// RUN:   | FileCheck --check-prefix=CHECK-UBSAN-ANDROID %s
+//
+// CHECK-UBSAN-ANDROID: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-UBSAN-ANDROID-NOT: "-lc"
+// CHECK-UBSAN-ANDROID-NOT: "-pie"
+// CHECK-UBSAN-ANDROID-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID: libclang_rt.ubsan_standalone-arm-android.so"
+// CHECK-UBSAN-ANDROID-NOT: "-lpthread"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
+// RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
+// RUN: -static-libsan \
+// RUN:   | FileCheck --check-prefix=CHECK-UBSAN-ANDROID-STATICLIBASAN %s
+//
+// CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN: libclang_rt.ubsan_standalone-arm-android.a"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target i686-linux-android -fuse-ld=ld -fsanitize=address \
@@ -147,10 +193,10 @@
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fsanitize=address \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
-// RUN: -shared-libasan \
+// RUN: -shared-libsan \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-ANDROID-SHARED-LIBASAN %s
 //
-// CHECK-ASAN-ANDROID-SHARED-LIBASAN-NOT: argument unused during compilation: '-shared-libasan'
+// CHECK-ASAN-ANDROID-SHARED-LIBASAN-NOT: argument unused during compilation: '-shared-libsan'
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=address \
@@ -214,6 +260,13 @@
 // RUN: -target i386-unknown-linux -fuse-ld=ld \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 

[PATCH] D38525: Cleanup and generalize -shared-libasan.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:563
+if (SanArgs.needsUbsanRt()) {
+  if (SanArgs.requiresMinimalRuntime()) {
+SharedRuntimes.push_back("ubsan_minimal");

vitalybuka wrote:
> Shouldn't ubsan changes be in the separate patch?
They are quite hard to separate, and without ubsan there is not much point to 
this change.



https://reviews.llvm.org/D38525



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

[PATCH] D38596: Implement attribute target multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.

GCC's attribute 'target', in addition to being an optimization hint,
also allows function multiversioning.  We currently have the former
implemented, this is the latter's implementation.

Note that it ends up having to permit redefinition of functions so
that they can all be emitted.  Additionally, all versions of the function
must be emitted, so this also manages that.

Function templates are NOT supported (not supported in GCC either).

Options on how to split this patch up would also be particularly solicited,
since this IS a large patch.


https://reviews.llvm.org/D38596

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Sema/Sema.h
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Sema/SemaDecl.cpp
  test/CodeGen/attr-target-multiversion.c
  test/CodeGenCXX/attr-target-multiversion.cpp
  test/Sema/attr-target-multiversion.c
  test/SemaCXX/attr-target-multiversion.cpp

Index: test/SemaCXX/attr-target-multiversion.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-target-multiversion.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
+
+struct S {
+  __attribute__((target("arch=sandybridge")))
+  void mv(){}
+  __attribute__((target("arch=ivybridge")))
+  void mv(){}
+  __attribute__((target("default")))
+  void mv(){}
+};
+
+// Note: Template attribute 'target' isn't implemented in GCC either, and would
+// end up causing some nasty issues attempting it, so ensure that it still gives
+// the same errors as without the attribute.
+template
+__attribute__((target("arch=sandybridge")))
+void mv_temp(){}
+
+template
+__attribute__((target("arch=ivybridge")))
+//expected-error@+2 {{redefinition of}}
+//expected-note@-5{{previous definition is here}}
+void mv_temp(){}
+
+template
+__attribute__((target("default")))
+void mv_temp(){}
+
+void foo() {
+  //expected-error@+2{{no matching function for call to}}
+  //expected-note@-8{{candidate template ignored}}
+  mv_temp();
+}
Index: test/Sema/attr-target-multiversion.c
===
--- /dev/null
+++ test/Sema/attr-target-multiversion.c
@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only -DCHECK_DEFAULT %s
+
+#if defined(CHECK_DEFAULT)
+__attribute__((target("arch=sandybridge")))
+//expected-error@+1 {{function multiversioning with 'target' requires a 'default' implementation}}
+void no_default(){}
+__attribute__((target("arch=ivybridge")))
+void no_default(){}
+#else
+// Only multiversioning causes issues with redeclaration changing 'target'.
+__attribute__((target("arch=sandybridge")))
+void fine_since_no_mv();
+void fine_since_no_mv();
+
+void also_fine_since_no_mv();
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv();
+
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv2();
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv2();
+void also_fine_since_no_mv2();
+
+__attribute__((target("arch=sandybridge")))
+void mv(){}
+__attribute__((target("arch=ivybridge")))
+void mv(){}
+__attribute__((target("default")))
+void mv(){}
+
+void redecl_causes_mv();
+__attribute__((target("arch=sandybridge")))
+void redecl_causes_mv();
+// expected-error@+3 {{function redeclaration causes a multiversioned function, but a previous declaration lacks a 'target' attribute}}
+// expected-note@-4 {{previous declaration is here}}
+__attribute__((target("arch=ivybridge")))
+void redecl_causes_mv();
+
+__attribute__((target("arch=sandybridge")))
+void redecl_causes_mv2();
+void redecl_causes_mv2();
+// expected-error@+3 {{function redeclaration causes a multiversioned function, but a previous declaration lacks a 'target' attribute}}
+// expected-note@-2 {{previous declaration is here}}
+__attribute__((target("arch=ivybridge")))
+void redecl_causes_mv2();
+
+__attribute__((target("arch=sandybridge")))
+void redecl_without_attr();
+__attribute__((target("arch=ivybridge")))
+void redecl_without_attr();
+// expected-error@+2 {{function redeclaration is missing 'target' attribute in a multiversioned function}}
+// expected-note@-4 {{previous declaration is here}}
+void redecl_without_attr();
+
+__attribute__((target("arch=sandybridge")))
+void multiversion_with_predecl();
+__attribute__((target("default")))
+void multiversion_with_predecl();
+__attribute__((target("arch=sandybridge")))
+void multiversion_with_predecl(){}
+__attribute__((target("default")))
+void multiversion_with_predecl(){}
+//expected-error@+3 {{redefinition of 

[PATCH] D38525: Cleanup and generalize -shared-libasan.

[Vitaly Buka via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/SanitizerArgs.cpp:614
+  if (Arg *A = Args.getLastArg(options::OPT_shared_libsan,
+   options::OPT_static_libsan))
+SharedRuntime = A->getOption().matches(options::OPT_shared_libsan);

```
SharedRuntime = Args.hasFlag(options::OPT_shared_libsan,,
 options::OPT_static_libsan,
 TC.getTriple().isAndroid() || 
TC.getTriple().isOSFuchsia());
```



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:563
+if (SanArgs.needsUbsanRt()) {
+  if (SanArgs.requiresMinimalRuntime()) {
+SharedRuntimes.push_back("ubsan_minimal");

Shouldn't ubsan changes be in the separate patch?


https://reviews.llvm.org/D38525



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

r315005 - Fix two-phase name lookup for non-dependent overloaded operators.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Oct  5 12:35:51 2017
New Revision: 315005

URL: http://llvm.org/viewvc/llvm-project?rev=315005=rev
Log:
Fix two-phase name lookup for non-dependent overloaded operators.

If we resolve an overloaded operator call to a specific function during
template definition, don't perform ADL during template instantiation.
Doing so finds overloads that we're not supposed to find.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/SemaCXX/overloaded-operator.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=315005=315004=315005=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Oct  5 12:35:51 2017
@@ -2914,12 +2914,13 @@ public:
   ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
  UnaryOperatorKind Opc,
  const UnresolvedSetImpl ,
- Expr *input);
+ Expr *input, bool RequiresADL = true);
 
   ExprResult CreateOverloadedBinOp(SourceLocation OpLoc,
BinaryOperatorKind Opc,
const UnresolvedSetImpl ,
-   Expr *LHS, Expr *RHS);
+   Expr *LHS, Expr *RHS,
+   bool RequiresADL = true);
 
   ExprResult CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
 SourceLocation RLoc,

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=315005=315004=315005=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Oct  5 12:35:51 2017
@@ -11927,7 +11927,7 @@ static bool IsOverloaded(const Unresolve
 ExprResult
 Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
   const UnresolvedSetImpl ,
-  Expr *Input) {
+  Expr *Input, bool PerformADL) {
   OverloadedOperatorKind Op = UnaryOperator::getOverloadedOperator(Opc);
   assert(Op != OO_None && "Invalid opcode for overloaded unary operator");
   DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(Op);
@@ -11978,9 +11978,11 @@ Sema::CreateOverloadedUnaryOp(SourceLoca
   AddMemberOperatorCandidates(Op, OpLoc, ArgsArray, CandidateSet);
 
   // Add candidates from ADL.
-  AddArgumentDependentLookupCandidates(OpName, OpLoc, ArgsArray,
-   /*ExplicitTemplateArgs*/nullptr,
-   CandidateSet);
+  if (PerformADL) {
+AddArgumentDependentLookupCandidates(OpName, OpLoc, ArgsArray,
+ /*ExplicitTemplateArgs*/nullptr,
+ CandidateSet);
+  }
 
   // Add builtin operator candidates.
   AddBuiltinOperatorCandidates(Op, OpLoc, ArgsArray, CandidateSet);
@@ -12118,7 +12120,7 @@ ExprResult
 Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
 BinaryOperatorKind Opc,
 const UnresolvedSetImpl ,
-Expr *LHS, Expr *RHS) {
+Expr *LHS, Expr *RHS, bool PerformADL) {
   Expr *Args[2] = { LHS, RHS };
   LHS=RHS=nullptr; // Please use only Args instead of LHS/RHS couple
 
@@ -12149,7 +12151,7 @@ Sema::CreateOverloadedBinOp(SourceLocati
 UnresolvedLookupExpr *Fn
   = UnresolvedLookupExpr::Create(Context, NamingClass,
  NestedNameSpecifierLoc(), OpNameInfo,
- /*ADL*/ true, IsOverloaded(Fns),
+ /*ADL*/PerformADL, IsOverloaded(Fns),
  Fns.begin(), Fns.end());
 return new (Context)
 CXXOperatorCallExpr(Context, Op, Fn, Args, Context.DependentTy,
@@ -12192,7 +12194,7 @@ Sema::CreateOverloadedBinOp(SourceLocati
   // Add candidates from ADL. Per [over.match.oper]p2, this lookup is not
   // performed for an assignment operator (nor for operator[] nor operator->,
   // which don't get here).
-  if (Opc != BO_Assign)
+  if (Opc != BO_Assign && PerformADL)
 AddArgumentDependentLookupCandidates(OpName, OpLoc, Args,
  /*ExplicitTemplateArgs*/ nullptr,
  CandidateSet);

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=315005=315004=315005=diff

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Raoul Wols via Phabricator via cfe-commits]

rwols added a comment.

In https://reviews.llvm.org/D38048#887960, @ilya-biryukov wrote:

> LGTM. Do you need my help in landing this?


Yes, I don't have commit access, go ahead and merge. If you find any merge 
conflicts let me know I can probably fix that.
Since we're throwing around gifs for fun here's how signatureHelp looks in 
practice for Sublime Text:

F5410662: sighelpexample2.mov 


https://reviews.llvm.org/D38048



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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:29
+namespace {
+TargetInfo::IntType GetDefaultWCharType(const llvm::Triple ) {
+  const llvm::Triple::ArchType Arch = T.getArch();

rnk wrote:
> How is this better than what we had before? It's totally inconsistent with 
> our existing strategy for figuring out type widths and sizes. Our current 
> approach can be extended for new targets, this requires modifying shared 
> TargetInfo code. This refactoring *should* be NFC anyway, so if we did want 
> to do it, we'd want to split it out.
The previous thing was split across and was fairly difficult to identify what 
was going on.  I tend to think that this makes it easier to identify what is 
going on.  However, if you have a strong opinion on this, Im willing to switch 
it back (though, possibly retain some of the adjustments to make it easier to 
follow).


https://reviews.llvm.org/D37891



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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:29
+namespace {
+TargetInfo::IntType GetDefaultWCharType(const llvm::Triple ) {
+  const llvm::Triple::ArchType Arch = T.getArch();

How is this better than what we had before? It's totally inconsistent with our 
existing strategy for figuring out type widths and sizes. Our current approach 
can be extended for new targets, this requires modifying shared TargetInfo 
code. This refactoring *should* be NFC anyway, so if we did want to do it, we'd 
want to split it out.


https://reviews.llvm.org/D37891



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

[libclc] r314998 - Add vload_half helpers for ptx

[Jeroen Ketema via cfe-commits]

Author: jketema
Date: Thu Oct  5 11:17:40 2017
New Revision: 314998

URL: http://llvm.org/viewvc/llvm-project?rev=314998=rev
Log:
Add vload_half helpers for ptx

The removes the vload_half unresolved calls from the nvptx libraries.

Reviewed-by: Jan Vesely 

Added:
libclc/trunk/ptx/lib/shared/vload_half_helpers.ll
Modified:
libclc/trunk/ptx/lib/SOURCES_3.9
libclc/trunk/ptx/lib/SOURCES_4.0
libclc/trunk/ptx/lib/SOURCES_5.0

Modified: libclc/trunk/ptx/lib/SOURCES_3.9
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/ptx/lib/SOURCES_3.9?rev=314998=314997=314998=diff
==
--- libclc/trunk/ptx/lib/SOURCES_3.9 (original)
+++ libclc/trunk/ptx/lib/SOURCES_3.9 Thu Oct  5 11:17:40 2017
@@ -1 +1,2 @@
+shared/vload_half_helpers.ll
 shared/vstore_half_helpers.ll

Modified: libclc/trunk/ptx/lib/SOURCES_4.0
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/ptx/lib/SOURCES_4.0?rev=314998=314997=314998=diff
==
--- libclc/trunk/ptx/lib/SOURCES_4.0 (original)
+++ libclc/trunk/ptx/lib/SOURCES_4.0 Thu Oct  5 11:17:40 2017
@@ -1 +1,2 @@
+shared/vload_half_helpers.ll
 shared/vstore_half_helpers.ll

Modified: libclc/trunk/ptx/lib/SOURCES_5.0
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/ptx/lib/SOURCES_5.0?rev=314998=314997=314998=diff
==
--- libclc/trunk/ptx/lib/SOURCES_5.0 (original)
+++ libclc/trunk/ptx/lib/SOURCES_5.0 Thu Oct  5 11:17:40 2017
@@ -1 +1,2 @@
+shared/vload_half_helpers.ll
 shared/vstore_half_helpers.ll

Added: libclc/trunk/ptx/lib/shared/vload_half_helpers.ll
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/ptx/lib/shared/vload_half_helpers.ll?rev=314998=auto
==
--- libclc/trunk/ptx/lib/shared/vload_half_helpers.ll (added)
+++ libclc/trunk/ptx/lib/shared/vload_half_helpers.ll Thu Oct  5 11:17:40 2017
@@ -0,0 +1,23 @@
+define float @__clc_vload_half_float_helper__private(half addrspace(0)* 
nocapture %ptr) nounwind alwaysinline {
+  %data = load half, half addrspace(0)* %ptr
+  %res = fpext half %data to float
+  ret float %res
+}
+
+define float @__clc_vload_half_float_helper__global(half addrspace(1)* 
nocapture %ptr) nounwind alwaysinline {
+  %data = load half, half addrspace(1)* %ptr
+  %res = fpext half %data to float
+  ret float %res
+}
+
+define float @__clc_vload_half_float_helper__local(half addrspace(3)* 
nocapture %ptr) nounwind alwaysinline {
+  %data = load half, half addrspace(3)* %ptr
+  %res = fpext half %data to float
+  ret float %res
+}
+
+define float @__clc_vload_half_float_helper__constant(half addrspace(4)* 
nocapture %ptr) nounwind alwaysinline {
+  %data = load half, half addrspace(4)* %ptr
+  %res = fpext half %data to float
+  ret float %res
+}


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

[PATCH] D38113: OpenCL: Assume functions are convergent

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 117855.
arsenm added a comment.

Check noduplicate


https://reviews.llvm.org/D38113

Files:
  include/clang/Basic/LangOptions.h
  lib/CodeGen/CGCall.cpp
  test/CodeGenOpenCL/amdgpu-attrs.cl
  test/CodeGenOpenCL/convergent.cl

Index: test/CodeGenOpenCL/convergent.cl
===
--- test/CodeGenOpenCL/convergent.cl
+++ test/CodeGenOpenCL/convergent.cl
@@ -1,9 +1,19 @@
-// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - | opt -instnamer -S | FileCheck %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - | opt -instnamer -S | FileCheck -enable-var-scope %s
+
+// This is initially assumed convergent, but can be deduced to not require it.
+
+// CHECK-LABEL: define spir_func void @non_convfun() local_unnamed_addr #0
+// CHECK: ret void
+__attribute__((noinline))
+void non_convfun(void) {
+  volatile int* p;
+  *p = 0;
+}
 
 void convfun(void) __attribute__((convergent));
-void non_convfun(void);
 void nodupfun(void) __attribute__((noduplicate));
 
+// External functions should be assumed convergent.
 void f(void);
 void g(void);
 
@@ -17,19 +27,23 @@
 //  non_convfun();
 //}
 //
-// CHECK: define spir_func void @test_merge_if(i32 %[[a:.+]])
-// CHECK: %[[tobool:.+]] = icmp eq i32 %[[a]], 0
+// CHECK-LABEL: define spir_func void @test_merge_if(i32 %a) local_unnamed_addr #1 {
+// CHECK: %[[tobool:.+]] = icmp eq i32 %a, 0
 // CHECK: br i1 %[[tobool]], label %[[if_end3_critedge:.+]], label %[[if_then:.+]]
+
 // CHECK: [[if_then]]:
 // CHECK: tail call spir_func void @f()
 // CHECK: tail call spir_func void @non_convfun()
 // CHECK: tail call spir_func void @g()
+
 // CHECK: br label %[[if_end3:.+]]
+
 // CHECK: [[if_end3_critedge]]:
 // CHECK: tail call spir_func void @non_convfun()
 // CHECK: br label %[[if_end3]]
+
 // CHECK: [[if_end3]]:
-// CHECK-LABEL: ret void
+// CHECK: ret void
 
 void test_merge_if(int a) {
   if (a) {
@@ -41,22 +55,22 @@
   }
 }
 
-// CHECK-DAG: declare spir_func void @f()
-// CHECK-DAG: declare spir_func void @non_convfun()
-// CHECK-DAG: declare spir_func void @g()
+// CHECK-DAG: declare spir_func void @f() local_unnamed_addr #2
+// CHECK-DAG: declare spir_func void @g() local_unnamed_addr #2
+
 
 // Test two if's are not merged.
-// CHECK: define spir_func void @test_no_merge_if(i32 %[[a:.+]])
-// CHECK:  %[[tobool:.+]] = icmp eq i32 %[[a]], 0
+// CHECK-LABEL: define spir_func void @test_no_merge_if(i32 %a) local_unnamed_addr #1
+// CHECK:  %[[tobool:.+]] = icmp eq i32 %a, 0
 // CHECK: br i1 %[[tobool]], label %[[if_end:.+]], label %[[if_then:.+]]
 // CHECK: [[if_then]]:
 // CHECK: tail call spir_func void @f()
 // CHECK-NOT: call spir_func void @convfun()
 // CHECK-NOT: call spir_func void @g()
 // CHECK: br label %[[if_end]]
 // CHECK: [[if_end]]:
 // CHECK:  %[[tobool_pr:.+]] = phi i1 [ true, %[[if_then]] ], [ false, %{{.+}} ]
-// CHECK:  tail call spir_func void @convfun() #[[attr5:.+]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4:.+]]
 // CHECK:  br i1 %[[tobool_pr]], label %[[if_then2:.+]], label %[[if_end3:.+]]
 // CHECK: [[if_then2]]:
 // CHECK: tail call spir_func void @g()
@@ -74,20 +88,20 @@
   }
 }
 
-// CHECK: declare spir_func void @convfun(){{[^#]*}} #[[attr2:[0-9]+]]
+// CHECK: declare spir_func void @convfun(){{[^#]*}} #2
 
 // Test loop is unrolled for convergent function.
-// CHECK-LABEL: define spir_func void @test_unroll()
-// CHECK:  tail call spir_func void @convfun() #[[attr5:[0-9]+]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
-// CHECK:  tail call spir_func void @convfun() #[[attr5]]
+// CHECK-LABEL: define spir_func void @test_unroll() local_unnamed_addr #1
+// CHECK:  tail call spir_func void @convfun() #[[attr4:[0-9]+]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
+// CHECK:  tail call spir_func void @convfun() #[[attr4]]
 // CHECK-LABEL:  ret void
 
 void test_unroll() {
@@ -101,7 +115,7 @@
 // CHECK: [[for_cond_cleanup:.+]]:
 // CHECK:  ret void
 // CHECK: [[for_body]]:
-// CHECK:  tail call spir_func void @nodupfun() #[[attr6:[0-9]+]]
+// CHECK:  tail call spir_func void 

r314995 - [OPENMP] Fix mapping|privatization of implicitly captured variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Oct  5 10:51:39 2017
New Revision: 314995

URL: http://llvm.org/viewvc/llvm-project?rev=314995=rev
Log:
[OPENMP] Fix mapping|privatization of implicitly captured variables.

If the `defaultmap(tofrom:scalar)` clause is specified, the scalars must
be mapped with 'tofrom' modifiers, otherwise they must be captured as
firstprivates.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=314995=314994=314995=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Oct  5 10:51:39 2017
@@ -45,7 +45,13 @@ namespace {
 enum DefaultDataSharingAttributes {
   DSA_unspecified = 0, /// \brief Data sharing attribute not specified.
   DSA_none = 1 << 0,   /// \brief Default data sharing attribute 'none'.
-  DSA_shared = 1 << 1  /// \brief Default data sharing attribute 'shared'.
+  DSA_shared = 1 << 1, /// \brief Default data sharing attribute 'shared'.
+};
+
+/// Attributes of the defaultmap clause.
+enum DefaultMapAttributes {
+  DMA_unspecified,   /// Default mapping is not specified.
+  DMA_tofrom_scalar, /// Default mapping is 'tofrom:scalar'.
 };
 
 /// \brief Stack for tracking declarations used in OpenMP directives and
@@ -115,6 +121,8 @@ private:
 LoopControlVariablesMapTy LCVMap;
 DefaultDataSharingAttributes DefaultAttr = DSA_unspecified;
 SourceLocation DefaultAttrLoc;
+DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
+SourceLocation DefaultMapAttrLoc;
 OpenMPDirectiveKind Directive = OMPD_unknown;
 DeclarationNameInfo DirectiveName;
 Scope *CurScope = nullptr;
@@ -341,6 +349,12 @@ public:
 Stack.back().first.back().DefaultAttr = DSA_shared;
 Stack.back().first.back().DefaultAttrLoc = Loc;
   }
+  /// Set default data mapping attribute to 'tofrom:scalar'.
+  void setDefaultDMAToFromScalar(SourceLocation Loc) {
+assert(!isStackEmpty());
+Stack.back().first.back().DefaultMapAttr = DMA_tofrom_scalar;
+Stack.back().first.back().DefaultMapAttrLoc = Loc;
+  }
 
   DefaultDataSharingAttributes getDefaultDSA() const {
 return isStackEmpty() ? DSA_unspecified
@@ -350,6 +364,17 @@ public:
 return isStackEmpty() ? SourceLocation()
   : Stack.back().first.back().DefaultAttrLoc;
   }
+  DefaultMapAttributes getDefaultDMA() const {
+return isStackEmpty() ? DMA_unspecified
+  : Stack.back().first.back().DefaultMapAttr;
+  }
+  DefaultMapAttributes getDefaultDMAAtLevel(unsigned Level) const {
+return Stack.back().first[Level].DefaultMapAttr;
+  }
+  SourceLocation getDefaultDMALocation() const {
+return isStackEmpty() ? SourceLocation()
+  : Stack.back().first.back().DefaultMapAttrLoc;
+  }
 
   /// \brief Checks if the specified variable is a threadprivate.
   bool isThreadPrivate(VarDecl *D) {
@@ -1242,7 +1267,8 @@ bool Sema::IsOpenMPCapturedByRef(ValueDe
   IsByRef = !(Ty->isPointerType() && IsVariableAssociatedWithSection);
 } else {
   // By default, all the data that has a scalar type is mapped by copy.
-  IsByRef = !Ty->isScalarType();
+  IsByRef = !Ty->isScalarType() ||
+DSAStack->getDefaultDMAAtLevel(Level) == DMA_tofrom_scalar;
 }
   }
 
@@ -1804,7 +1830,7 @@ public:
 if (auto *VD = dyn_cast(E->getDecl())) {
   VD = VD->getCanonicalDecl();
   // Skip internally declared variables.
-  if (VD->isLocalVarDecl() && !CS->capturesVariable(VD))
+  if (VD->hasLocalStorage() && !CS->capturesVariable(VD))
 return;
 
   auto DVar = Stack->getTopDSA(VD, false);
@@ -1848,20 +1874,16 @@ public:
MC.getAssociatedExpression()));
  });
 })) {
-  bool CapturedByCopy = false;
+  bool IsFirstprivate = false;
   // By default lambdas are captured as firstprivates.
   if (const auto *RD =
   VD->getType().getNonReferenceType()->getAsCXXRecordDecl())
-if (RD->isLambda())
-  CapturedByCopy = true;
-  CapturedByCopy =
-  CapturedByCopy ||
-  llvm::any_of(
-  CS->captures(), [VD](const CapturedStmt::Capture ) {
-return I.capturesVariableByCopy() &&
-   I.getCapturedVar()->getCanonicalDecl() == VD;
-  });
-  if (CapturedByCopy)
+IsFirstprivate = RD->isLambda();
+  IsFirstprivate =
+  IsFirstprivate ||
+  (VD->getType().getNonReferenceType()->isScalarType() &&
+   Stack->getDefaultDMA() != DMA_tofrom_scalar);
+  if 

[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314992: [Analyzer Tests] Run static analyzer integration 
tests until the end, (authored by george.karpenkov).

Changed prior to commit:
  https://reviews.llvm.org/D38589?vs=117847=117848#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38589

Files:
  cfe/trunk/utils/analyzer/SATestBuild.py


Index: cfe/trunk/utils/analyzer/SATestBuild.py
===
--- cfe/trunk/utils/analyzer/SATestBuild.py
+++ cfe/trunk/utils/analyzer/SATestBuild.py
@@ -570,7 +570,11 @@
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +590,6 @@
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +626,15 @@
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-return (NumDiffs > 0)
+return TestsPassed
 
 
 def cleanupReferenceResults(SBOutputDir):
@@ -686,6 +688,11 @@
 
 
 def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+"""
+Test a given project.
+:return TestsPassed: Whether tests have passed according
+to the :param Strictness: criteria.
+"""
 print " \n\n--- Building project %s" % (ID,)
 
 TBegin = time.time()
@@ -704,11 +711,13 @@
 
 if IsReferenceBuild:
 cleanupReferenceResults(SBOutputDir)
+TestsPassed = True
 else:
-runCmpResults(Dir, Strictness)
+TestsPassed = runCmpResults(Dir, Strictness)
 
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time() - TBegin))
+return TestsPassed
 
 
 def isCommentCSVLine(Entries):
@@ -749,6 +758,7 @@
 
 
 def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0):
+TestsPassed = True
 with projectFileHandler() as PMapFile:
 validateProjectFile(PMapFile)
 
@@ -760,7 +770,7 @@
 
 # Test the projects.
 for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile):
-testProject(
+TestsPassed &= testProject(
 ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness)
 
 # Re-add reference results to SVN.
@@ -793,4 +803,6 @@
 IsReference = True
 UpdateSVN = True
 
-testAll(IsReference, UpdateSVN, Strictness)
+TestsPassed = testAll(IsReference, UpdateSVN, Strictness)
+if not TestsPassed:
+sys.exit(-1)


Index: cfe/trunk/utils/analyzer/SATestBuild.py
===
--- cfe/trunk/utils/analyzer/SATestBuild.py
+++ cfe/trunk/utils/analyzer/SATestBuild.py
@@ -570,7 +570,11 @@
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +590,6 @@
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +626,15 @@
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-

r314992 - [Analyzer Tests] Run static analyzer integration tests until the end,

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Thu Oct  5 10:32:06 2017
New Revision: 314992

URL: http://llvm.org/viewvc/llvm-project?rev=314992=rev
Log:
[Analyzer Tests] Run static analyzer integration tests until the end,

Do not stop at the first failure.

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

Modified:
cfe/trunk/utils/analyzer/SATestBuild.py

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=314992=314991=314992=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Thu Oct  5 10:32:06 2017
@@ -570,7 +570,11 @@ def runCmpResults(Dir, Strictness=0):
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +590,6 @@ def runCmpResults(Dir, Strictness=0):
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +626,15 @@ def runCmpResults(Dir, Strictness=0):
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-return (NumDiffs > 0)
+return TestsPassed
 
 
 def cleanupReferenceResults(SBOutputDir):
@@ -686,6 +688,11 @@ def updateSVN(Mode, PMapFile):
 
 
 def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+"""
+Test a given project.
+:return TestsPassed: Whether tests have passed according
+to the :param Strictness: criteria.
+"""
 print " \n\n--- Building project %s" % (ID,)
 
 TBegin = time.time()
@@ -704,11 +711,13 @@ def testProject(ID, ProjectBuildMode, Is
 
 if IsReferenceBuild:
 cleanupReferenceResults(SBOutputDir)
+TestsPassed = True
 else:
-runCmpResults(Dir, Strictness)
+TestsPassed = runCmpResults(Dir, Strictness)
 
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time() - TBegin))
+return TestsPassed
 
 
 def isCommentCSVLine(Entries):
@@ -749,6 +758,7 @@ def validateProjectFile(PMapFile):
 
 
 def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0):
+TestsPassed = True
 with projectFileHandler() as PMapFile:
 validateProjectFile(PMapFile)
 
@@ -760,7 +770,7 @@ def testAll(IsReferenceBuild=False, Upda
 
 # Test the projects.
 for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile):
-testProject(
+TestsPassed &= testProject(
 ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness)
 
 # Re-add reference results to SVN.
@@ -793,4 +803,6 @@ if __name__ == '__main__':
 IsReference = True
 UpdateSVN = True
 
-testAll(IsReference, UpdateSVN, Strictness)
+TestsPassed = testAll(IsReference, UpdateSVN, Strictness)
+if not TestsPassed:
+sys.exit(-1)


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

[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure

[Devin Coughlin via Phabricator via cfe-commits]

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

Oops, wrong button! LGTM!


https://reviews.llvm.org/D38589



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

[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin requested changes to this revision.
dcoughlin added a comment.
This revision now requires changes to proceed.

This looks good to me.


https://reviews.llvm.org/D38589



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

[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun.

This is especially important for updating reference results.


https://reviews.llvm.org/D38589

Files:
  utils/analyzer/SATestBuild.py

Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -46,6 +46,7 @@
 
 import os
 import csv
+from collections import namedtuple
 import sys
 import glob
 import math
@@ -570,7 +571,11 @@
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +591,6 @@
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +627,15 @@
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-return (NumDiffs > 0)
+return TestsPassed
 
 
 def cleanupReferenceResults(SBOutputDir):
@@ -686,6 +689,11 @@
 
 
 def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+"""
+Test a given project.
+:return TestsPassed: Whether tests have passed according
+to the :param Strictness: criteria.
+"""
 print " \n\n--- Building project %s" % (ID,)
 
 TBegin = time.time()
@@ -704,11 +712,13 @@
 
 if IsReferenceBuild:
 cleanupReferenceResults(SBOutputDir)
+TestsPassed = True
 else:
-runCmpResults(Dir, Strictness)
+TestsPassed = runCmpResults(Dir, Strictness)
 
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time() - TBegin))
+return TestsPassed
 
 
 def isCommentCSVLine(Entries):
@@ -749,6 +759,7 @@
 
 
 def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0):
+TestsPassed = True
 with projectFileHandler() as PMapFile:
 validateProjectFile(PMapFile)
 
@@ -760,7 +771,7 @@
 
 # Test the projects.
 for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile):
-testProject(
+TestsPassed &= testProject(
 ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness)
 
 # Re-add reference results to SVN.
@@ -793,4 +804,6 @@
 IsReference = True
 UpdateSVN = True
 
-testAll(IsReference, UpdateSVN, Strictness)
+TestsPassed = testAll(IsReference, UpdateSVN, Strictness)
+if not TestsPassed:
+sys.exit(-1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38589: [Analyzer] Run tests until the end, do not stop at the first failure

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 117847.

https://reviews.llvm.org/D38589

Files:
  utils/analyzer/SATestBuild.py


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -570,7 +570,11 @@
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +590,6 @@
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +626,15 @@
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-return (NumDiffs > 0)
+return TestsPassed
 
 
 def cleanupReferenceResults(SBOutputDir):
@@ -686,6 +688,11 @@
 
 
 def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+"""
+Test a given project.
+:return TestsPassed: Whether tests have passed according
+to the :param Strictness: criteria.
+"""
 print " \n\n--- Building project %s" % (ID,)
 
 TBegin = time.time()
@@ -704,11 +711,13 @@
 
 if IsReferenceBuild:
 cleanupReferenceResults(SBOutputDir)
+TestsPassed = True
 else:
-runCmpResults(Dir, Strictness)
+TestsPassed = runCmpResults(Dir, Strictness)
 
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time() - TBegin))
+return TestsPassed
 
 
 def isCommentCSVLine(Entries):
@@ -749,6 +758,7 @@
 
 
 def testAll(IsReferenceBuild=False, UpdateSVN=False, Strictness=0):
+TestsPassed = True
 with projectFileHandler() as PMapFile:
 validateProjectFile(PMapFile)
 
@@ -760,7 +770,7 @@
 
 # Test the projects.
 for (ProjName, ProjBuildMode) in iterateOverProjects(PMapFile):
-testProject(
+TestsPassed &= testProject(
 ProjName, int(ProjBuildMode), IsReferenceBuild, Strictness)
 
 # Re-add reference results to SVN.
@@ -793,4 +803,6 @@
 IsReference = True
 UpdateSVN = True
 
-testAll(IsReference, UpdateSVN, Strictness)
+TestsPassed = testAll(IsReference, UpdateSVN, Strictness)
+if not TestsPassed:
+sys.exit(-1)


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -570,7 +570,11 @@
   0 - success if there are no crashes or analyzer failure.
   1 - success if there are no difference in the number of reported bugs.
   2 - success if all the bug reports are identical.
+
+:return success: Whether tests pass according to the Strictness
+criteria.
 """
+TestsPassed = True
 TBegin = time.time()
 
 RefDir = os.path.join(Dir, SBOutputDirReferencePrefix + SBOutputDirName)
@@ -586,8 +590,6 @@
 RefList.remove(RefLogDir)
 NewList.remove(os.path.join(NewDir, LogFolderName))
 
-if len(RefList) == 0 or len(NewList) == 0:
-return False
 assert(len(RefList) == len(NewList))
 
 # There might be more then one folder underneath - one per each scan-build
@@ -624,15 +626,15 @@
   (NumDiffs, DiffsPath,)
 if Strictness >= 2 and NumDiffs > 0:
 print "Error: Diffs found in strict mode (2)."
-sys.exit(-1)
+TestsPassed = False
 elif Strictness >= 1 and ReportsInRef != ReportsInNew:
 print "Error: The number of results are different in "\
   "strict mode (1)."
-sys.exit(-1)
+TestsPassed = False
 
 print "Diagnostic comparison complete (time: %.2f)." % (
   time.time() - TBegin)
-return (NumDiffs > 0)
+return TestsPassed
 
 
 def cleanupReferenceResults(SBOutputDir):
@@ -686,6 +688,11 @@
 
 
 def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+"""
+Test a given project.
+:return TestsPassed: Whether tests have passed according
+to the :param Strictness: 

[PATCH] D38583: [clangd] Added async API to run code completion.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314989: [clangd] Added async API to run code completion. 
(authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D38583

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed asynchronously. You can use the returned future to
+  /// wait for the results of the async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: 

[clang-tools-extra] r314989 - [clangd] Added async API to run code completion.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Oct  5 10:04:13 2017
New Revision: 314989

URL: http://llvm.org/viewvc/llvm-project?rev=314989=rev
Log:
[clangd] Added async API to run code completion.

Summary:
ClangdServer now provides async code completion API.
It is still used synchronously by ClangdLSPServer, more work is needed
to allow processing other requests in parallel while completion (or
any other request) is running.

Reviewers: klimek, bkramer, krasimir

Reviewed By: klimek

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=314989=314988=314989=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct  5 10:04:13 2017
@@ -149,6 +149,9 @@ void ClangdLSPServer::onCompletion(TextD
.codeComplete(Params.textDocument.uri.file,
  Position{Params.position.line,
   Params.position.character})
+   .get() // FIXME(ibiryukov): This could be made async if we
+  // had an API that would allow to attach callbacks to
+  // futures returned by ClangdServer.
.Value;
 
   std::string Completions;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=314989=314988=314989=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct  5 10:04:13 2017
@@ -194,18 +194,19 @@ std::future ClangdServer::forceRep
  std::move(TaggedFS));
 }
 
-Tagged
+std::future>
 ClangdServer::codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents,
IntrusiveRefCntPtr *UsedFS) {
-  std::string DraftStorage;
-  if (!OverridenContents) {
+  std::string Contents;
+  if (OverridenContents) {
+Contents = *OverridenContents;
+  } else {
 auto FileContents = DraftMgr.getDraft(File);
 assert(FileContents.Draft &&
"codeComplete is called for non-added document");
 
-DraftStorage = std::move(*FileContents.Draft);
-OverridenContents = DraftStorage;
+Contents = std::move(*FileContents.Draft);
   }
 
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
@@ -215,12 +216,36 @@ ClangdServer::codeComplete(PathRef File,
   std::shared_ptr Resources = Units.getFile(File);
   assert(Resources && "Calling completion on non-added file");
 
-  auto Preamble = Resources->getPossiblyStalePreamble();
-  std::vector Result = clangd::codeComplete(
-  File, Resources->getCompileCommand(),
-  Preamble ? >Preamble : nullptr, *OverridenContents, Pos,
-  TaggedFS.Value, PCHs, SnippetCompletions, Logger);
-  return make_tagged(std::move(Result), TaggedFS.Tag);
+  using PackagedTask =
+  std::packaged_task()>;
+
+  // Remember the current Preamble and use it when async task starts executing.
+  // At the point when async task starts executing, we may have a different
+  // Preamble in Resources. However, we assume the Preamble that we obtain here
+  // is reusable in completion more often.
+  std::shared_ptr Preamble =
+  Resources->getPossiblyStalePreamble();
+  // A task that will be run asynchronously.
+  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
+if (!Preamble) {
+  // Maybe we built some preamble before processing this request.
+  Preamble = Resources->getPossiblyStalePreamble();
+}
+// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
+// both the old and the new version in case only one of them matches.
+
+std::vector Result = clangd::codeComplete(
+File, Resources->getCompileCommand(),
+Preamble ? >Preamble : nullptr, Contents, Pos, 
TaggedFS.Value,
+PCHs, SnippetCompletions, Logger);
+return make_tagged(std::move(Result), std::move(TaggedFS.Tag));
+  });
+
+  auto Future = Task.get_future();
+  // FIXME(ibiryukov): to reduce overhead for wrapping the same callable
+  // multiple times, ClangdScheduler should return future<> itself.
+  WorkScheduler.addToFront([](PackagedTask Task) { Task(); }, std::move(Task));
+  return Future;
 }
 
 std::vector ClangdServer::formatRange(PathRef 

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Added another missing 'the'.


https://reviews.llvm.org/D38583

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed asynchronously. You can use the returned future to
+  /// wait for the results of the async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -194,18 +194,19 @@
  std::move(TaggedFS));
 }
 
-Tagged
+std::future>
 ClangdServer::codeComplete(PathRef File, Position 

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb added a comment.

FYI, I was able to do simple testing (C++ programs throwing exceptions) using 
GCC 6.3 to compile a FreeBSD userland for both O32 and N64.  Still working on 
cross-compiling LLVM so I can run the tests under qemu.


https://reviews.llvm.org/D38110



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

[PATCH] D31363: [libc++] Remove cmake glob for source files

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Building libcxx without LLVM's CMake modules is very important, not just to 
Apple. This is how several open source distributions work, and it would be a 
huge disservice to break this. Same is true for compiler-rt, libcxxabi, 
libunwind, etc.

Historically we've been drawing the line that building and running tests for 
runtime projects can require LLVM, but building the runtime libraries 
themselves must work without LLVM. I believe that is still the correct line to 
draw.


https://reviews.llvm.org/D31363



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

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! All addressed (except removing ShutdownParams, which I'm not sure is 
worthwhile).




Comment at: clangd/ClangdLSPServer.h:47
   // Implement ProtocolCallbacks.
-  void onInitialize(StringRef ID, InitializeParams IP,
-JSONOutput ) override;
-  void onShutdown(JSONOutput ) override;
-  void onDocumentDidOpen(DidOpenTextDocumentParams Params,
- JSONOutput ) override;
-  void onDocumentDidChange(DidChangeTextDocumentParams Params,
-   JSONOutput ) override;
-  void onDocumentDidClose(DidCloseTextDocumentParams Params,
-  JSONOutput ) override;
-  void onDocumentOnTypeFormatting(DocumentOnTypeFormattingParams Params,
-  StringRef ID, JSONOutput ) override;
-  void onDocumentRangeFormatting(DocumentRangeFormattingParams Params,
- StringRef ID, JSONOutput ) override;
-  void onDocumentFormatting(DocumentFormattingParams Params, StringRef ID,
-JSONOutput ) override;
-  void onCodeAction(CodeActionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onCompletion(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID,
-JSONOutput ) override;
+  void onInitialize(Ctx , InitializeParams ) override;
+  void onShutdown(Ctx , NoParams ) override;

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Maybe there's a way to have a typed return value instead of `Ctx`?
> > This would give an interface that's harder to misuse: one can't forget to 
> > call `reply` or call it twice.
> > 
> > Here's on design that comes to mind.
> > Notification handlers could have `void` return, normal requests can return 
> > `Optional` or `Optional` (with result json).
> > `Optional` is be used to indicate whether error occurred while processing 
> > the request.
> > 
> After putting more thought into it, `Ctx`-based API seems to have an 
> advantage: it will allow us to easily implement async responses.
> E.g., we can run code completion on a background thread and continue 
> processing other requests. When completion is ready, we will simply call 
> `Ctx.reply` to return results to the language client.
> 
> To make that possible, can we allow moving `RequestContext` and pass it 
> by-value instead of by-ref?
Yeah I thought about returning a value... it certainly reads more nicely, but I 
don't think we're ready to do a good job in this patch:
 - return value should be an object ready to be serialized (rather than a JSON 
string) - I don't want to bring that in scope here, but it might affect the 
details of the API
 - there's several cases we know about (return object, no reply, error reply) 
and some we're not sure about (async as you mention - any multiple responses)? 
I think this needs some design, and I don't yet know the project well enough to 
drive it.

I've switched to passing Ctx by value as you suggest (though it's certainly 
cheap enough to copy, too).



Comment at: clangd/ClangdLSPServer.h:48
+  void onInitialize(Ctx , InitializeParams ) override;
+  void onShutdown(Ctx , NoParams ) override;
+  void onDocumentDidOpen(Ctx , DidOpenTextDocumentParams ) override;

ilya-biryukov wrote:
> Maybe there's a way to get rid of `NoParams`?
> E.g. by adding a overload to `HandlerRegisterer`?
Even if there was, I think I prefer the regularity (changed this to 
ShutdownParams - oops!).

Otherwise the signature's dependent on some combination of {current LSP, 
whether we actually implement the options, whether we've defined any 
extensions}. It's harder to remember, means changing lots of places when these 
factors change, and complicates the generic code.

Maybe I've just spent too long around Stubby, though - WDYT?


https://reviews.llvm.org/D38464



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

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 117835.
sammccall marked 5 inline comments as done.
sammccall added a comment.

- ClangLSPServer.h should refer to ShutdownParams, not NoParams
- Address review comments


https://reviews.llvm.org/D38464

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/fixits.test

Index: test/clangd/fixits.test
===
--- test/clangd/fixits.test
+++ test/clangd/fixits.test
@@ -15,13 +15,13 @@
 
  {"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
 #
-# CHECK: {"jsonrpc":"2.0","id":2, "result": [{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
+# CHECK: {"jsonrpc":"2.0","id":2,"result":[{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
 #
 Content-Length: 771
 
 {"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
 # Make sure unused "code" and "source" fields ignored gracefully
-# CHECK: {"jsonrpc":"2.0","id":2, "result": [{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
+# CHECK: {"jsonrpc":"2.0","id":2,"result":[{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
 #
 Content-Length: 44
 
Index: 

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 117832.
compnerd added a comment.

Moves the logic back into Basic.  The flags are now optional, but controlled by 
the driver.  The test adjustments are to map the old `-fshort-wchar` to 
`-fwchar-type=short -fno-signed-wchar` for the most part, there is one case 
where we were checking that we were passing `-fshort-wchar` through to the 
frontend, which we no longer do.


https://reviews.llvm.org/D37891

Files:
  include/clang/Basic/DiagnosticFrontendKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/ARM.cpp
  lib/Basic/Targets/AVR.h
  lib/Basic/Targets/OSTargets.h
  lib/Basic/Targets/X86.h
  lib/Basic/Targets/XCore.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CXX/conv/conv.prom/p2.cpp
  test/CodeGen/arm-metadata.c
  test/CodeGen/pascal-wchar-string.c
  test/CodeGen/string-literal-short-wstring.c
  test/CodeGen/string-literal-unicode-conversion.c
  test/CodeGen/wchar-size.c
  test/Driver/clang_f_opts.c
  test/Headers/wchar_limits.cpp
  test/Index/index-pch.cpp
  test/Lexer/wchar.c
  test/Preprocessor/init.c
  test/Preprocessor/pr19649-unsigned-wchar_t.c
  test/Preprocessor/wchar_t.c
  test/Sema/wchar.c
  test/SemaCXX/short-wchar-sign.cpp

Index: test/SemaCXX/short-wchar-sign.cpp
===
--- test/SemaCXX/short-wchar-sign.cpp
+++ test/SemaCXX/short-wchar-sign.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-mingw32 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fshort-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -pedantic -verify %s
 // expected-no-diagnostics

Index: test/Sema/wchar.c
===
--- test/Sema/wchar.c
+++ test/Sema/wchar.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// RUN: %clang_cc1 %s -fsyntax-only -fshort-wchar -verify -DSHORT_WCHAR
+// RUN: %clang_cc1 %s -fsyntax-only -fwchar-type=short -fno-signed-wchar -verify -DSHORT_WCHAR
 
 typedef __WCHAR_TYPE__ wchar_t;
 
Index: test/Preprocessor/wchar_t.c
===
--- /dev/null
+++ test/Preprocessor/wchar_t.c
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -triple i386-pc-solaris -dM -E %s -o - | FileCheck %s -check-prefix CHECK-SOLARIS
+// CHECK-SOLARIS-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple avr-unknown-unknown -fwchar-type=int -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-AVR
+// CHECK-AVR-DAG: #define __WCHAR_MAX__ 32767
+// CHECK-AVR-DAG: #define __WCHAR_TYPE__ int
+// CHECK-AVR-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple arm-unknown-none-gnu -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM-APCS
+// CHECK-ARM-APCS-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM-APCS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM-APCS-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple arm-unknown-netbsd-gnu -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM-NETBSD-AAPCS
+// CHECK-ARM-NETBSD-AAPCS-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM-NETBSD-AAPCS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM-NETBSD-AAPCS-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple arm-unknown-openbsd -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM-OPENBSD
+// CHECK-ARM-OPENBSD-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM-OPENBSD-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM-OPENBSD-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM64-DARWIN
+// CHECK-ARM64-DARWIN-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM64-DARWIN-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM64-DARWIN-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple aarch64-unknown-netbsd -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM64-NETBSD
+// CHECK-ARM64-NETBSD-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM64-NETBSD-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM64-NETBSD-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple aarch64-unknown-openbsd -fsigned-wchar -dM -E %s -o - | FileCheck %s -check-prefix CHECK-ARM64-OPENBSD
+// CHECK-ARM64-OPENBSD-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-ARM64-OPENBSD-DAG: #define __WCHAR_TYPE__ int
+// CHECK-ARM64-OPENBSD-NOT: #define __WCHAR_UNSIGNED__ 0
+
+// RUN: %clang_cc1 -triple aarch64-unknown-none -fwchar-type=int -fno-signed-wchar -dM -E %s -o - | 

[PATCH] D37826: Refine generation of TBAA information in clang

[Ivan A. Kosarev via Phabricator via cfe-commits]

kosarev added inline comments.



Comment at: lib/CodeGen/CodeGenTBAA.h:129
 
-  /// getBaseTypeInfo - Get metadata node for a given base access type.
-  llvm::MDNode *getBaseTypeInfo(QualType QType);
+  /// getTBAABaseTypeMetadata - Get metadata that describes the given base
+  /// access type. Return null if the type is not suitable for use in TBAA

Wrong function name. Will fix on next review cycle or commit.


Repository:
  rL LLVM

https://reviews.llvm.org/D37826



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

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: dberlin.
aaron.ballman added a subscriber: dberlin.
aaron.ballman added a comment.

Adding @dberlin for licensing discussion questions.




Comment at: LICENSE.TXT:64
 clang-tidy  clang-tidy/hicpp
+clang-tidy  
clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt

This should point to a directory instead of an individual file.



Comment at: 
clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt:2
+--
+clang-tidy Cognitive Complexity Check
+--

I think that this file should be a general LICENSE.TXT instead of a tied to a 
specific file name. The license is for the algorithm, not for the file name.



Comment at: 
clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt:13
+
+From: Olivier Gaudin 
+Date: Tue, 01 Aug 2017 07:06:11 +

This isn't actually a license, however. I think we need to ask sonarsource 
either for a license statement they're comfortable with, or approach them with 
one in hand that they can approve. However, @dberlin might have more input here.


Repository:
  rL LLVM

https://reviews.llvm.org/D36836



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

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Francisco Lopes da Silva via Phabricator via cfe-commits]

francisco.lopes added a comment.

In https://reviews.llvm.org/D38048#889176, @ilya-biryukov wrote:

> @francisco.lopes, thanks for sharing your experience, I definitely like the 
> final UX you achieved, we should aim for something similar in clangd.
>  It also looks like @rwols's  idea of how things should look like is very 
> close to your implementation.


Nice!

> I do think that showing signatures for non-overloaded functions in completion 
> may still be useful. Overloaded functions can be grouped by name and include 
> some indicators that there are more overloads (something like `foo(int)   
> void  (+5 overloads)`.
>  How do you choose return type for the completion item if function is 
> overloaded? Show it only when it's the same for all overloads? Go with one of 
> the return types?

In fact it didn't happen to me to think about the return information in the 
popup menu at the time, so it was not changed!, and it should just show one 
return type from one of the overloads randomly (I must never look at return 
types...).
I do think the (+ 5 overloads) approach is a good one too, though I prefer to 
not oversize the menu width with parameters.

It will be cool if clangd and its Language Server Protocol implementation 
manages to have feature parity with this, personally I'd be looking in 
migrating from YouCompleteMe to a LSP solution that's more modular and 
lightweight, possibly bringing to it
what gets implemented here myself.


https://reviews.llvm.org/D38048



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

[PATCH] D37826: Refine generation of TBAA information in clang

[Ivan A. Kosarev via Phabricator via cfe-commits]

kosarev updated this revision to Diff 117823.
kosarev added a comment.

- Rebased on top of https://reviews.llvm.org/D38503.
- The tbaa-for-vptr.cpp test is changed to be more specific about the 
instructions to test. Otherwise, we fail on some triples, such as 
s390x-ibm-linux and x86_64-pc-mingw32. This is due to the changes for 
EmitLoadOfScalar() and EmitStoreOfScalar() where we previously were defaulting 
to empty TBAA info and now generate a TBAA descriptor based on the specified 
access type. This results in the instructions that load the pointer-to-function 
value having their TBAA tags (the may-alias one, namely) so that these 
instructions match the pattern in the test file and NUM gets a wrong value.


Repository:
  rL LLVM

https://reviews.llvm.org/D37826

Files:
  lib/CodeGen/CGAtomic.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGValue.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTBAA.cpp
  lib/CodeGen/CodeGenTBAA.h
  test/CodeGen/tbaa-for-vptr.cpp

Index: test/CodeGen/tbaa-for-vptr.cpp
===
--- test/CodeGen/tbaa-for-vptr.cpp
+++ test/CodeGen/tbaa-for-vptr.cpp
@@ -23,12 +23,12 @@
 }
 
 // CHECK-LABEL: @_Z7CallFoo
-// CHECK: %{{.*}} = load {{.*}} !tbaa ![[NUM:[0-9]+]]
+// CHECK: %{{.*}} = load i32 (%struct.A*)**, {{.*}} !tbaa ![[NUM:[0-9]+]]
 // CHECK: br i1
-// CHECK: load {{.*}}, !tbaa ![[NUM]]
+// CHECK: load i8*, {{.*}}, !tbaa ![[NUM]]
 //
 // CHECK-LABEL: @_ZN1AC2Ev
-// CHECK: store {{.*}} !tbaa ![[NUM]]
+// CHECK: store i32 (...)** {{.*}}, !tbaa ![[NUM]]
 //
 // CHECK: [[NUM]] = !{[[TYPE:!.*]], [[TYPE]], i64 0}
 // CHECK: [[TYPE]] = !{!"vtable pointer", !{{.*}}
Index: lib/CodeGen/CodeGenTBAA.h
===
--- lib/CodeGen/CodeGenTBAA.h
+++ lib/CodeGen/CodeGenTBAA.h
@@ -32,22 +32,15 @@
 namespace CodeGen {
 class CGRecordLayout;
 
-struct TBAAPathTag {
-  TBAAPathTag(const Type *B, const llvm::MDNode *A, uint64_t O)
-: BaseT(B), AccessN(A), Offset(O) {}
-  const Type *BaseT;
-  const llvm::MDNode *AccessN;
-  uint64_t Offset;
-};
-
 // TBAAAccessInfo - Describes a memory access in terms of TBAA.
 struct TBAAAccessInfo {
-  TBAAAccessInfo(QualType BaseType, llvm::MDNode *AccessType, uint64_t Offset)
+  TBAAAccessInfo(llvm::MDNode *BaseType, llvm::MDNode *AccessType,
+ uint64_t Offset)
 : BaseType(BaseType), AccessType(AccessType), Offset(Offset)
   {}
 
   explicit TBAAAccessInfo(llvm::MDNode *AccessType)
-: TBAAAccessInfo(/* BaseType= */ QualType(), AccessType, /* Offset= */ 0)
+: TBAAAccessInfo(/* BaseType= */ nullptr, AccessType, /* Offset= */ 0)
   {}
 
   TBAAAccessInfo()
@@ -57,7 +50,7 @@
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
-  QualType BaseType;
+  llvm::MDNode *BaseType;
 
   /// AccessType - The final access type. May be null if there is no TBAA
   /// information available about this access.
@@ -82,10 +75,10 @@
   /// MetadataCache - This maps clang::Types to scalar llvm::MDNodes describing
   /// them.
   llvm::DenseMap MetadataCache;
-  /// This maps clang::Types to a struct node in the type DAG.
-  llvm::DenseMap StructTypeMetadataCache;
-  /// This maps TBAAPathTags to a tag node.
-  llvm::DenseMap StructTagMetadataCache;
+  /// This maps clang::Types to a base access type in the type DAG.
+  llvm::DenseMap BaseTypeMetadataCache;
+  /// This maps TBAA access descriptors to tag nodes.
+  llvm::DenseMap AccessTagMetadataCache;
 
   /// StructMetadataCache - This maps clang::Types to llvm::MDNodes describing
   /// them for struct assignments.
@@ -133,8 +126,10 @@
   /// the given type.
   llvm::MDNode *getTBAAStructInfo(QualType QTy);
 
-  /// getBaseTypeInfo - Get metadata node for a given base access type.
-  llvm::MDNode *getBaseTypeInfo(QualType QType);
+  /// getTBAABaseTypeMetadata - Get metadata that describes the given base
+  /// access type. Return null if the type is not suitable for use in TBAA
+  /// access tags.
+  llvm::MDNode *getBaseTypeInfo(QualType QTy);
 
   /// getAccessTagInfo - Get TBAA tag for a given memory access.
   llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
@@ -149,31 +144,31 @@
 
 namespace llvm {
 
-template<> struct DenseMapInfo {
-  static clang::CodeGen::TBAAPathTag getEmptyKey() {
-return clang::CodeGen::TBAAPathTag(
-  DenseMapInfo::getEmptyKey(),
-  DenseMapInfo::getEmptyKey(),
+template<> struct DenseMapInfo {
+  static clang::CodeGen::TBAAAccessInfo getEmptyKey() {
+return clang::CodeGen::TBAAAccessInfo(
+  DenseMapInfo::getEmptyKey(),
+  DenseMapInfo::getEmptyKey(),
   DenseMapInfo::getEmptyKey());
   }
 
-  static 

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D35082#889053, @rjmccall wrote:

> Are you sure it's a good idea to not print the address space when it's 
> implicit?  Won't that often lead to really confusing diagnostics?
>
> Also, we do already have a way of expressing that an extended qualifier was 
> explicit: AttributedType.  We have very similar sorts of superficial 
> well-formedness checks to what I think you're trying to do in ObjC ARC.


Based on my observation, in most cases it is OK not to print the implicit 
address space, and printing implicit address space could cause quite annoying 
cluttering. In some cases printing implicit address space may be desired. I can 
improve the printing in some future patch, e.g. only hide the implicit address 
space in situations which causes cluttering but not providing much useful 
information.

I think AttributedType sounds an interesting idea and worth exploring.

I just felt this review dragged too long (~ 3 months) already. We have an 
important backend change depending on this feature. Since the current solution 
achieves its goals already, can we leave re-implementing it by AttributedType 
for the future? Thanks.


https://reviews.llvm.org/D35082



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

[PATCH] D38583: [clangd] Added async API to run code completion.

[Manuel Klimek via Phabricator via cfe-commits]

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

LG




Comment at: clangd/ClangdServer.h:236-237
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///

ilya-biryukov wrote:
> klimek wrote:
> > Extra space before asynchronously.
> > Replace first ',' with ';'
> > ... wait for the results of the async request...
> Split into two sentences on `,` instead of replacing it with `;`. Feels even 
> better.
..."the" returned future...



https://reviews.llvm.org/D38583



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

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 117824.
lebedev.ri added a comment.

Ping.

- Rebased
- Added the legal status info into `LICENSE.txt`, referencing 
`clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt`, as 
suggested by @aaron.ballman

Question:
Is it expected that clang-tidy somehow parses the `DiagnosticIDs::Note`'s as 
`FixIt`'s, and thus fails with the following assert:

  Running ['clang-tidy', 
'/build/llvm-build-Clang-release/tools/clang/tools/extra/test/clang-tidy/Output/readability-function-cognitive-complexity.cpp.tmp.cpp',
 '-fix', '--checks=-*,readability-function-cognitive-complexity', 
'-config={CheckOptions: [{key: 
readability-function-cognitive-complexity.Threshold, value: 0}]}', '--', 
'-std=c++11', '-nostdinc++']...
  clang-tidy failed:
  Fix conflicts with existing fix! The new replacement overlaps with an 
existing replacement.
  New replacement: 
/build/llvm-build-Clang-release/tools/clang/tools/extra/test/clang-tidy/Output/readability-function-cognitive-complexity.cpp.tmp.cpp:
 3484:+5:""
  Existing replacement: 
/build/llvm-build-Clang-release/tools/clang/tools/extra/test/clang-tidy/Output/readability-function-cognitive-complexity.cpp.tmp.cpp:
 3485:+2:"&"
  clang-tidy: 
/build/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:86: virtual 
void (anonymous 
namespace)::ClangTidyDiagnosticRenderer::emitCodeContext(clang::FullSourceLoc, 
DiagnosticsEngine::Level, SmallVectorImpl &, 
ArrayRef): Assertion `false && "Fix conflicts with existing 
fix!"' failed.


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  LICENSE.TXT
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  test/clang-tidy/readability-function-cognitive-complexity.cpp

Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,952 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [readability-function-cognitive-complexity]
+  CC100;
+
+  if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// 

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.h:236-237
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///

klimek wrote:
> Extra space before asynchronously.
> Replace first ',' with ';'
> ... wait for the results of the async request...
Split into two sentences on `,` instead of replacing it with `;`. Feels even 
better.


https://reviews.llvm.org/D38583



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

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 117822.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Fixed grammar in a comment.


https://reviews.llvm.org/D38583

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed asynchronously. You can use returned future to wait
+  /// for the results of the async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -194,18 +194,19 @@
  std::move(TaggedFS));
 }
 
-Tagged
+std::future>
 

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Added a comment about Preamble assignment.


https://reviews.llvm.org/D38583

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -194,18 +194,19 @@
  std::move(TaggedFS));
 }
 
-Tagged
+std::future>
 ClangdServer::codeComplete(PathRef File, 

[PATCH] D38583: [clangd] Added async API to run code completion.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clangd/ClangdServer.cpp:225
+  // A task that will be run asynchronously.
+  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
+if (!Preamble) {

ilya-biryukov wrote:
> klimek wrote:
> > It doesn't seem like we use Preamble anywhere else but in the lambda - so 
> > why not get it in the lambda?
> TLDR; To use in async requests exactly the same preamble that was previously 
> used for sync requests.
> 
> Those are two different time points. Our callers might change the file when 
> we'll be executing this lambda. I assume that most of time we'll want the 
> preamble that was built at the point where `codeComplete` is called, not 
> after that.
> 
> On the other hand, after file was changed, code completion results will 
> probably be irrelevant and will be discarded by clients anyway, so that might 
> not matter. I've opted for not changing the behavior and using the same 
> preamble that was previously used by the synchronous version. (Unless 
> `Preamble` was null in the sync case, where we would only improve 
> performance).
That makes sense, but needs a comment in the code :)



Comment at: clangd/ClangdServer.h:236-237
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///

Extra space before asynchronously.
Replace first ',' with ';'
... wait for the results of the async request...


https://reviews.llvm.org/D38583



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

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.cpp:222
+
+  std::shared_ptr Preamble =
+  Resources->getPossiblyStalePreamble();

klimek wrote:
> I think we use "const type" everywhere.
Sorry, my previously preferred style keeps sneaking in.



Comment at: clangd/ClangdServer.cpp:225
+  // A task that will be run asynchronously.
+  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
+if (!Preamble) {

klimek wrote:
> It doesn't seem like we use Preamble anywhere else but in the lambda - so why 
> not get it in the lambda?
TLDR; To use in async requests exactly the same preamble that was previously 
used for sync requests.

Those are two different time points. Our callers might change the file when 
we'll be executing this lambda. I assume that most of time we'll want the 
preamble that was built at the point where `codeComplete` is called, not after 
that.

On the other hand, after file was changed, code completion results will 
probably be irrelevant and will be discarded by clients anyway, so that might 
not matter. I've opted for not changing the behavior and using the same 
preamble that was previously used by the synchronous version. (Unless 
`Preamble` was null in the sync case, where we would only improve performance).


https://reviews.llvm.org/D38583



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

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Moved 'const' to the left.


https://reviews.llvm.org/D38583

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -194,18 +194,19 @@
  std::move(TaggedFS));
 }
 
-Tagged
+std::future>
 ClangdServer::codeComplete(PathRef File, Position Pos,
  

[PATCH] D38583: [clangd] Added async API to run code completion.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clangd/ClangdServer.cpp:222
+
+  std::shared_ptr Preamble =
+  Resources->getPossiblyStalePreamble();

I think we use "const type" everywhere.



Comment at: clangd/ClangdServer.cpp:225
+  // A task that will be run asynchronously.
+  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
+if (!Preamble) {

It doesn't seem like we use Preamble anywhere else but in the lambda - so why 
not get it in the lambda?


https://reviews.llvm.org/D38583



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

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

[Michael Ferguson via Phabricator via cfe-commits]

mppf added a comment.

@rjmccall , @rsmith - I added a comment about complete object type. I think 
I've addressed all the concerns at this point, but speak up if I'm 
misunderstanding.

I don't have commit access so I'll need help committing it. Thanks for your 
help!


https://reviews.llvm.org/D38473



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

[PATCH] D38583: [clangd] Added async API to run code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

ClangdServer now provides async code completion API.
It is still used synchronously by ClangdLSPServer, more work is needed
to allow processing other requests in parallel while completion (or
any other request) is running.


https://reviews.llvm.org/D38583

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -473,13 +473,13 @@
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
   Server.addDocument(FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
+  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag);
 }
 
 // Only enable this test on Unix
@@ -631,7 +631,7 @@
 
   {
 auto CodeCompletionResults1 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc"));
   }
@@ -641,14 +641,15 @@
 Server
 .codeComplete(FooCpp, CompletePos,
   StringRef(OverridenSourceContents))
+.get()
 .Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
   }
 
   {
 auto CodeCompletionResults2 =
-Server.codeComplete(FooCpp, CompletePos, None).Value;
+Server.codeComplete(FooCpp, CompletePos, None).get().Value;
 EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba"));
 EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc"));
   }
@@ -840,7 +841,13 @@
 AddDocument(FileIndex);
 
   Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-  Server.codeComplete(FilePaths[FileIndex], Pos);
+  // FIXME(ibiryukov): Also test async completion requests.
+  // Simply putting CodeCompletion into async requests now would make
+  // tests slow, since there's no way to cancel previous completion
+  // requests as opposed to AddDocument/RemoveDocument, which are implicitly
+  // cancelled by any subsequent AddDocument/RemoveDocument request to the
+  // same file.
+  Server.codeComplete(FilePaths[FileIndex], Pos).wait();
 };
 
 auto FindDefinitionsRequest = [&]() {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -231,19 +231,25 @@
   /// and AST and rebuild them from scratch.
   std::future forceReparse(PathRef File);
 
-  /// Run code completion for \p File at \p Pos. If \p OverridenContents is not
-  /// None, they will used only for code completion, i.e. no diagnostics update
-  /// will be scheduled and a draft for \p File will not be updated.
-  /// If \p OverridenContents is None, contents of the current draft for \p File
-  /// will be used.
-  /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
-  /// for completion.
-  /// This method should only be called for currently tracked
-  /// files.
-  Tagged
+  /// Run code completion for \p File at \p Pos.
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///
+  /// If \p OverridenContents is not None, they will used only for code
+  /// completion, i.e. no diagnostics update will be scheduled and a draft for
+  /// \p File will not be updated. If \p OverridenContents is None, contents of
+  /// the current draft for \p File will be used. If \p UsedFS is non-null, it
+  /// will be overwritten by vfs::FileSystem used for completion.
+  ///
+  /// This method should only be called for currently tracked files. However, it
+  /// is safe to call removeDocument for \p File after this method returns, even
+  /// while returned future is not yet ready.
+  std::future>
   codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   Tagged findDefinitions(PathRef File, Position Pos);
 
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -194,18 +194,19 @@
  

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

[Michael Ferguson via Phabricator via cfe-commits]

mppf updated this revision to Diff 117806.
mppf added a comment.

- Update comment for "complete object type"


https://reviews.llvm.org/D38473

Files:
  include/clang/CodeGen/CodeGenABITypes.h
  lib/CodeGen/CodeGenABITypes.cpp
  unittests/CodeGen/CMakeLists.txt
  unittests/CodeGen/CodeGenExternalTest.cpp

Index: unittests/CodeGen/CodeGenExternalTest.cpp
===
--- /dev/null
+++ unittests/CodeGen/CodeGenExternalTest.cpp
@@ -0,0 +1,302 @@
+//===- unittests/CodeGen/CodeGenExternalTest.cpp - test external CodeGen -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/GlobalDecl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+// Mocks up a language using Clang code generation as a library and
+// tests some basic functionality there.
+//   - CodeGen->GetAddrOfGlobal
+//   - CodeGen::convertTypeForMemory
+//   - CodeGen::getLLVMFieldNumber
+
+static const bool DebugThisTest = false;
+
+// forward declarations
+struct MyASTConsumer;
+static void test_codegen_fns(MyASTConsumer *my);
+static bool test_codegen_fns_ran;
+
+// This forwards the calls to the Clang CodeGenerator
+// so that we can test CodeGen functions while it is open.
+// It accumulates toplevel decls in HandleTopLevelDecl and
+// calls test_codegen_fns() in HandleTranslationUnit
+// before forwarding that function to the CodeGenerator.
+
+struct MyASTConsumer : public ASTConsumer {
+  std::unique_ptr Builder;
+  std::vector toplevel_decls;
+
+  MyASTConsumer(std::unique_ptr Builder_in)
+: ASTConsumer(), Builder(std::move(Builder_in))
+  {
+  }
+
+  ~MyASTConsumer() { }
+
+  void Initialize(ASTContext ) override;
+  void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
+  bool HandleTopLevelDecl(DeclGroupRef D) override;
+  void HandleInlineFunctionDefinition(FunctionDecl *D) override;
+  void HandleInterestingDecl(DeclGroupRef D) override;
+  void HandleTranslationUnit(ASTContext ) override;
+  void HandleTagDeclDefinition(TagDecl *D) override;
+  void HandleTagDeclRequiredDefinition(const TagDecl *D) override;
+  void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) override;
+  void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
+  void HandleImplicitImportDecl(ImportDecl *D) override;
+  void CompleteTentativeDefinition(VarDecl *D) override;
+  void AssignInheritanceModel(CXXRecordDecl *RD) override;
+  void HandleVTable(CXXRecordDecl *RD) override;
+  ASTMutationListener *GetASTMutationListener() override;
+  ASTDeserializationListener *GetASTDeserializationListener() override;
+  void PrintStats() override;
+  bool shouldSkipFunctionBody(Decl *D) override;
+};
+
+void MyASTConsumer::Initialize(ASTContext ) {
+  Builder->Initialize(Context);
+}
+
+bool MyASTConsumer::HandleTopLevelDecl(DeclGroupRef DG) {
+
+  for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I) {
+toplevel_decls.push_back(*I);
+  }
+
+  return Builder->HandleTopLevelDecl(DG);
+}
+
+void MyASTConsumer::HandleInlineFunctionDefinition(FunctionDecl *D) {
+  Builder->HandleInlineFunctionDefinition(D);
+}
+
+void MyASTConsumer::HandleInterestingDecl(DeclGroupRef D) {
+  Builder->HandleInterestingDecl(D);
+}
+
+void MyASTConsumer::HandleTranslationUnit(ASTContext ) {
+  test_codegen_fns(this);
+  // HandleTranslationUnit can close the module
+  Builder->HandleTranslationUnit(Context);
+}
+
+void MyASTConsumer::HandleTagDeclDefinition(TagDecl *D) {
+  Builder->HandleTagDeclDefinition(D);
+}
+
+void MyASTConsumer::HandleTagDeclRequiredDefinition(const TagDecl *D) {
+  Builder->HandleTagDeclRequiredDefinition(D);
+}
+
+void MyASTConsumer::HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {
+  Builder->HandleCXXImplicitFunctionInstantiation(D);
+}
+
+void MyASTConsumer::HandleTopLevelDeclInObjCContainer(DeclGroupRef D) {
+  Builder->HandleTopLevelDeclInObjCContainer(D);
+}
+
+void MyASTConsumer::HandleImplicitImportDecl(ImportDecl *D) {
+  Builder->HandleImplicitImportDecl(D);
+}
+
+void MyASTConsumer::CompleteTentativeDefinition(VarDecl *D) {
+  Builder->CompleteTentativeDefinition(D);
+}
+

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8719
+  // Type limit values are handled later by CheckTautologicalComparison().
+  if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
 return;

rsmith wrote:
> Is this necessary? (Aren't the type limit values always within the range of 
> the type in question?)
> 
> Can we avoid evaluating `Constant` a extra time here? (We already have its 
> value in `Value`.)
Uhm, good question :)
If i remove this, `check-clang-sema` and `check-clang-semacxx` still pass.
I agree that it does not make much sense. Initially it was only checking for 
`Value == 0`.
Git suggests that initially this branch was added by @rtrieu, maybe can help.


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



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

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 117802.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a subscriber: rtrieu.
lebedev.ri added a comment.

- Address review notes
- ~~Avoid re-evaluating ICE when called from `DiagnoseOutOfRangeComparison()` 
which already knows the evaluated value~~

  Don't even check if this is a Type Limit in `DiagnoseOutOfRangeComparison()`. 
Perhaps @rtrieu can shed some light on this.
- Enhance test coverage a little (add a test with macro, with template)


Repository:
  rL LLVM

https://reviews.llvm.org/D38101

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/outof-range-constant-compare.c
  test/Sema/tautological-constant-compare.c
  test/Sema/tautological-unsigned-zero-compare.c

Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,47 +1,370 @@
 // RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
 
-unsigned value(void);
+unsigned uvalue(void);
+signed int svalue(void);
 
-int main() {
-  unsigned un = value();
+#define macro(val) val
+
+#ifdef __cplusplus
+template
+void TFunc() {
+  // Make sure that we do warn for normal variables in template functions !
+  unsigned char c = svalue();
+#ifdef TEST
+  if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+  return;
+#else
+  if (c < 0)
+  return;
+#endif
+
+  if (c < macro(0))
+  return;
+
+  T v = svalue();
+  if (v < 0)
+  return;
+}
+#endif
+
+int main()
+{
+#ifdef __cplusplus
+  TFunc();
+  TFunc();
+#endif
+
+  unsigned un = uvalue();
 
 #ifdef TEST
+  if (un == 0)
+  return 0;
+  if (un != 0)
+  return 0;
   if (un < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
-return 0;
+  return 0;
+  if (un <= 0)
+  return 0;
+  if (un > 0)
+  return 0;
   if (un >= 0) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
-return 0;
+  return 0;
+
+  if (0 == un)
+  return 0;
+  if (0 != un)
+  return 0;
+  if (0 < un)
+  return 0;
   if (0 <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
-return 0;
+  return 0;
   if (0 > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
-return 0;
-  if (un < 0U) // expected-warning {{comparison of unsigned expression < 0 is always false}}
-return 0;
-  if (un >= 0U) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
-return 0;
-  if (0U <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
-return 0;
-  if (0U > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
-return 0;
+  return 0;
+  if (0 >= un)
+  return 0;
+
+  if (un == 0UL)
+  return 0;
+  if (un != 0UL)
+  return 0;
+  if (un < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+  return 0;
+  if (un <= 0UL)
+  return 0;
+  if (un > 0UL)
+  return 0;
+  if (un >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+  return 0;
+
+  if (0UL == un)
+  return 0;
+  if (0UL != un)
+  return 0;
+  if (0UL < un)
+  return 0;
+  if (0UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+  return 0;
+  if (0UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+  return 0;
+  if (0UL >= un)
+  return 0;
 #else
 // expected-no-diagnostics
+  if (un == 0)
+  return 0;
+  if (un != 0)
+  return 0;
   if (un < 0)
-return 0;
+  return 0;
+  if (un <= 0)
+  return 0;
+  if (un > 0)
+  return 0;
   if (un >= 0)
-return 0;
+  return 0;
+
+  if (0 == un)
+  return 0;
+  if (0 != un)
+  return 0;
+  if (0 < un)
+  return 0;
   if (0 <= un)
-return 0;
+  return 0;
   if (0 > un)
-return 0;
-  if (un < 0U)
-return 0;
-  if (un >= 0U)
-return 0;
-  if (0U <= un)
-return 0;
-  if (0U > un)
-return 0;
+  return 0;
+  if (0 >= un)
+  return 0;
+
+  if (un == 0UL)
+  return 0;
+  if (un != 0UL)
+  return 0;
+  if (un < 0UL)
+  return 0;
+  if (un <= 0UL)
+  return 0;
+  if (un > 0UL)
+  return 0;
+  if (un >= 0UL)
+  return 0;
+
+  if (0UL == un)
+  return 0;
+  if (0UL != un)
+  return 0;
+  if (0UL < un)
+  return 0;
+  if (0UL <= un)
+  return 0;
+  if (0UL > un)
+  return 0;
+  if (0UL 

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D37436#881363, @aaron.ballman wrote:

> In https://reviews.llvm.org/D37436#871117, @aaron.ballman wrote:
>
> > Updated based on review feedback.
> >
> > - Rename CAttributes to DoubleSquareBracketAttributes
> > - Added Objective-C test cases to demonstrate no regressed behavior (based 
> > on a use-case pointed out during review)
> > - Fixed regression with GNU-style attributes in enumerations
> >
> >   I still need to rename the cc1 flag, pending agreement on the name.
>
>
> Ping -- the last outstanding issue that I'm aware of is the cc1 flag name, do 
> you have a preference there, Richard? If not, I'll be going with the verbose 
> `-fdouble-square-bracket-attributes`.


Post-cppcon-ping.


https://reviews.llvm.org/D37436



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

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.h:47
   // Implement ProtocolCallbacks.
-  void onInitialize(StringRef ID, InitializeParams IP,
-JSONOutput ) override;
-  void onShutdown(JSONOutput ) override;
-  void onDocumentDidOpen(DidOpenTextDocumentParams Params,
- JSONOutput ) override;
-  void onDocumentDidChange(DidChangeTextDocumentParams Params,
-   JSONOutput ) override;
-  void onDocumentDidClose(DidCloseTextDocumentParams Params,
-  JSONOutput ) override;
-  void onDocumentOnTypeFormatting(DocumentOnTypeFormattingParams Params,
-  StringRef ID, JSONOutput ) override;
-  void onDocumentRangeFormatting(DocumentRangeFormattingParams Params,
- StringRef ID, JSONOutput ) override;
-  void onDocumentFormatting(DocumentFormattingParams Params, StringRef ID,
-JSONOutput ) override;
-  void onCodeAction(CodeActionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onCompletion(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput ) override;
-  void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID,
-JSONOutput ) override;
+  void onInitialize(Ctx , InitializeParams ) override;
+  void onShutdown(Ctx , NoParams ) override;

ilya-biryukov wrote:
> Maybe there's a way to have a typed return value instead of `Ctx`?
> This would give an interface that's harder to misuse: one can't forget to 
> call `reply` or call it twice.
> 
> Here's on design that comes to mind.
> Notification handlers could have `void` return, normal requests can return 
> `Optional` or `Optional` (with result json).
> `Optional` is be used to indicate whether error occurred while processing the 
> request.
> 
After putting more thought into it, `Ctx`-based API seems to have an advantage: 
it will allow us to easily implement async responses.
E.g., we can run code completion on a background thread and continue processing 
other requests. When completion is ready, we will simply call `Ctx.reply` to 
return results to the language client.

To make that possible, can we allow moving `RequestContext` and pass it 
by-value instead of by-ref?



Comment at: clangd/JSONRPCDispatcher.h:55
+  /// Logs a message locally only. A newline will be added.
+  void log(const Twine ) { Out.log(Message + "\n"); }
+

It seems there are no any of `log`. Maybe remove it from this class?


https://reviews.llvm.org/D38464



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

r314979 - [CodeGen] Unify generation of scalar and struct-path TBAA tags

[Ivan A. Kosarev via cfe-commits]

Author: kosarev
Date: Thu Oct  5 04:08:17 2017
New Revision: 314979

URL: http://llvm.org/viewvc/llvm-project?rev=314979=rev
Log:
[CodeGen] Unify generation of scalar and struct-path TBAA tags

This patch makes it possible to produce access tags in a uniform
manner regardless whether the resulting tag will be a scalar or a
struct-path one. getAccessTagInfo() now takes care of the actual
translation of access descriptors to tags and can handle all
kinds of accesses. Facilities that specific to scalar accesses
are eliminated.

Some more details:
* DecorateInstructionWithTBAA() is not responsible for conversion
  of types to access tags anymore. Instead, it takes an access
  descriptor (TBAAAccessInfo) and generates corresponding access
  tag from it.
* getTBAAInfoForVTablePtr() reworked to
  getTBAAVTablePtrAccessInfo() that now returns the
  virtual-pointer access descriptor and not the virtual-point
  type metadata.
* Added function getTBAAMayAliasAccessInfo() that returns the
  descriptor for may-alias accesses.
* getTBAAStructTagInfo() renamed to getTBAAAccessTagInfo() as now
  it is the only way to generate access tag by a given access
  descriptor. It is capable of producing both scalar and
  struct-path tags, depending on options and availability of the
  base access type. getTBAAScalarTagInfo() and its cache
  ScalarTagMetadataCache are eliminated.
* Now that we do not need to care about whether the resulting
  access tag should be a scalar or struct-path one,
  getTBAAStructTypeInfo() is renamed to getBaseTypeInfo().
* Added function getTBAAAccessInfo() that constructs access
  descriptor by a given QualType access type.

This is part of D37826 reworked to be a separate patch to
simplify review.

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

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=314979=314978=314979=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Thu Oct  5 04:08:17 2017
@@ -1425,8 +1425,8 @@ llvm::Value *AtomicInfo::EmitAtomicLoadO
   // Other decoration.
   if (IsVolatile)
 Load->setVolatile(true);
-  if (LVal.getTBAAAccessType())
-CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAAccessType());
+  TBAAAccessInfo TBAAInfo(LVal.getTBAAAccessType());
+  CGF.CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
   return Load;
 }
 
@@ -1942,8 +1942,8 @@ void CodeGenFunction::EmitAtomicStore(RV
 // Other decoration.
 if (IsVolatile)
   store->setVolatile(true);
-if (dest.getTBAAAccessType())
-  CGM.DecorateInstructionWithTBAA(store, dest.getTBAAAccessType());
+TBAAAccessInfo TBAAInfo(dest.getTBAAAccessType());
+CGM.DecorateInstructionWithTBAA(store, TBAAInfo);
 return;
   }
 

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=314979=314978=314979=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Oct  5 04:08:17 2017
@@ -2383,7 +2383,7 @@ void CodeGenFunction::InitializeVTablePo
   VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, 
VTableField);
-  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAInfoForVTablePtr());
+  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAVTablePtrAccessInfo());
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   CGM.getCodeGenOpts().StrictVTablePointers)
 CGM.DecorateInstructionWithInvariantGroup(Store, Vptr.VTableClass);
@@ -2477,7 +2477,7 @@ llvm::Value *CodeGenFunction::GetVTableP
const CXXRecordDecl *RD) {
   Address VTablePtrSrc = Builder.CreateElementBitCast(This, VTableTy);
   llvm::Instruction *VTable = Builder.CreateLoad(VTablePtrSrc, "vtable");
-  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAInfoForVTablePtr());
+  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAVTablePtrAccessInfo());
 
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   CGM.getCodeGenOpts().StrictVTablePointers)

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=314979=314978=314979=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu Oct  5 04:08:17 2017
@@ -1526,12 +1526,9 @@ llvm::Value 

r314978 - Revert r314977 "[CodeGen] Unify generation of scalar and struct-path TBAA tags"

[Ivan A. Kosarev via cfe-commits]

Author: kosarev
Date: Thu Oct  5 04:05:43 2017
New Revision: 314978

URL: http://llvm.org/viewvc/llvm-project?rev=314978=rev
Log:
Revert r314977 "[CodeGen] Unify generation of scalar and struct-path TBAA tags"

D37826 has been mistakenly committed where it should be the patch from D38503.

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

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=314978=314977=314978=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Thu Oct  5 04:05:43 2017
@@ -205,7 +205,7 @@ namespace {
 addr = CGF.Builder.CreateStructGEP(addr, 0, CharUnits());
 
   return LValue::MakeAddr(addr, getValueType(), CGF.getContext(),
-  LVal.getBaseInfo(), LVal.getTBAAInfo());
+  LVal.getBaseInfo(), LVal.getTBAAAccessType());
 }
 
 /// \brief Emits atomic load.
@@ -1425,7 +1425,8 @@ llvm::Value *AtomicInfo::EmitAtomicLoadO
   // Other decoration.
   if (IsVolatile)
 Load->setVolatile(true);
-  CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAInfo());
+  if (LVal.getTBAAAccessType())
+CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAAccessType());
   return Load;
 }
 
@@ -1941,7 +1942,8 @@ void CodeGenFunction::EmitAtomicStore(RV
 // Other decoration.
 if (IsVolatile)
   store->setVolatile(true);
-CGM.DecorateInstructionWithTBAA(store, dest.getTBAAInfo());
+if (dest.getTBAAAccessType())
+  CGM.DecorateInstructionWithTBAA(store, dest.getTBAAAccessType());
 return;
   }
 

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=314978=314977=314978=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Oct  5 04:05:43 2017
@@ -2383,7 +2383,7 @@ void CodeGenFunction::InitializeVTablePo
   VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, 
VTableField);
-  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAVTablePtrAccessInfo());
+  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAInfoForVTablePtr());
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   CGM.getCodeGenOpts().StrictVTablePointers)
 CGM.DecorateInstructionWithInvariantGroup(Store, Vptr.VTableClass);
@@ -2477,7 +2477,7 @@ llvm::Value *CodeGenFunction::GetVTableP
const CXXRecordDecl *RD) {
   Address VTablePtrSrc = Builder.CreateElementBitCast(This, VTableTy);
   llvm::Instruction *VTable = Builder.CreateLoad(VTablePtrSrc, "vtable");
-  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAVTablePtrAccessInfo());
+  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAInfoForVTablePtr());
 
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   CGM.getCodeGenOpts().StrictVTablePointers)

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=314978=314977=314978=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu Oct  5 04:05:43 2017
@@ -1174,7 +1174,8 @@ LValue CodeGenFunction::EmitLValue(const
   llvm::Value *V = LV.getPointer();
   Scope.ForceCleanup({});
   return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
-  getContext(), LV.getBaseInfo(), 
LV.getTBAAInfo());
+  getContext(), LV.getBaseInfo(),
+  LV.getTBAAAccessType());
 }
 // FIXME: Is it possible to create an ExprWithCleanups that produces a
 // bitfield lvalue or some other non-simple lvalue?
@@ -1513,7 +1514,7 @@ llvm::Value *CodeGenFunction::EmitLoadOf
 
   // Atomic operations have to be done on integral types.
   LValue AtomicLValue =
-  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
+  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
   if (Ty->isAtomicType() || LValueIsSuitableForInlineAtomic(AtomicLValue)) {
 return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
@@ -1524,10 +1525,14 @@ llvm::Value *CodeGenFunction::EmitLoadOf
 

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv created this revision.

When a preamble ends in a conditional preprocessor block that is being
skipped, the preprocessor needs to continue skipping that block when
the preamble is used.

This fixes PR34570.


https://reviews.llvm.org/D38578

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/Index/preamble-conditionals-inverted.cpp
  test/Index/preamble-conditionals-skipping.cpp

Index: test/Index/preamble-conditionals-skipping.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-skipping.cpp
@@ -0,0 +1,16 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \
+// RUN:   local -std=c++14 %s 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "error:"
+
+#ifdef MYCPLUSPLUS
+extern "C" {
+#endif
+
+#ifdef MYCPLUSPLUS
+}
+#endif
+
+int main()
+{
+return 0;
+}
Index: test/Index/preamble-conditionals-inverted.cpp
===
--- test/Index/preamble-conditionals-inverted.cpp
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -3,6 +3,8 @@
 // RUN: | FileCheck %s --implicit-check-not "error:"
 #ifdef FOO_H
 
-void foo();
+void foo() {}
 
 #endif
+
+int foo() { return 0; }
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -2407,6 +2407,13 @@
 
   if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
 assert(!IsModule);
+const Preprocessor::PreambleSkipInfo  = PP.getPreambleSkipInfo();
+Record.push_back(SkipInfo.ReachedEOFWhileSkipping);
+AddSourceLocation(SkipInfo.HashToken, Record);
+AddSourceLocation(SkipInfo.IfTokenLoc, Record);
+Record.push_back(SkipInfo.FoundNonSkipPortion);
+Record.push_back(SkipInfo.FoundElse);
+AddSourceLocation(SkipInfo.ElseLoc, Record);
 for (const auto  : PP.getPreambleConditionalStack()) {
   AddSourceLocation(Cond.IfLoc, Record);
   Record.push_back(Cond.WasSkipping);
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2995,16 +2995,24 @@
 
 case PP_CONDITIONAL_STACK:
   if (!Record.empty()) {
+unsigned Idx = 0, End = Record.size() - 1;
+Preprocessor::PreambleSkipInfo SkipInfo;
+SkipInfo.ReachedEOFWhileSkipping = Record[Idx++];
+SkipInfo.HashToken = ReadSourceLocation(F, Record, Idx);
+SkipInfo.IfTokenLoc = ReadSourceLocation(F, Record, Idx);
+SkipInfo.FoundNonSkipPortion = Record[Idx++];
+SkipInfo.FoundElse = Record[Idx++];
+SkipInfo.ElseLoc = ReadSourceLocation(F, Record, Idx);
 SmallVector ConditionalStack;
-for (unsigned Idx = 0, N = Record.size() - 1; Idx < N; /* in loop */) {
+while (Idx < End) {
   auto Loc = ReadSourceLocation(F, Record, Idx);
   bool WasSkipping = Record[Idx++];
   bool FoundNonSkip = Record[Idx++];
   bool FoundElse = Record[Idx++];
   ConditionalStack.push_back(
   {Loc, WasSkipping, FoundNonSkip, FoundElse});
 }
-PP.setReplayablePreambleConditionalStack(ConditionalStack);
+PP.setReplayablePreambleConditionalStack(ConditionalStack, SkipInfo);
   }
   break;
 
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -544,6 +544,13 @@
"CurPPLexer is null when calling replayPreambleConditionalStack.");
 CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack());
 PreambleConditionalStack.doneReplaying();
+if (PreambleConditionalStack.reachedEOFWhileSkipping())
+  SkipExcludedConditionalBlock(
+  PreambleConditionalStack.SkipInfo.HashToken,
+  PreambleConditionalStack.SkipInfo.IfTokenLoc,
+  PreambleConditionalStack.SkipInfo.FoundNonSkipPortion,
+  PreambleConditionalStack.SkipInfo.FoundElse,
+  PreambleConditionalStack.SkipInfo.ElseLoc);
   }
 }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -350,16 +350,19 @@
 /// If ElseOk is true, then \#else directives are ok, if not, then we have
 /// already seen one so a \#else directive is a duplicate.  When this returns,
 /// the caller can lex the first valid token.
-void Preprocessor::SkipExcludedConditionalBlock(const Token ,
+void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashToken,
 SourceLocation IfTokenLoc,
  

r314977 - [CodeGen] Unify generation of scalar and struct-path TBAA tags

[Ivan A. Kosarev via cfe-commits]

Author: kosarev
Date: Thu Oct  5 03:47:51 2017
New Revision: 314977

URL: http://llvm.org/viewvc/llvm-project?rev=314977=rev
Log:
[CodeGen] Unify generation of scalar and struct-path TBAA tags

This patch makes it possible to produce access tags in a uniform
manner regardless whether the resulting tag will be a scalar or a
struct-path one. getAccessTagInfo() now takes care of the actual
translation of access descriptors to tags and can handle all
kinds of accesses. Facilities that specific to scalar accesses
are eliminated.

Some more details:
* DecorateInstructionWithTBAA() is not responsible for conversion
  of types to access tags anymore. Instead, it takes an access
  descriptor (TBAAAccessInfo) and generates corresponding access
  tag from it.
* getTBAAInfoForVTablePtr() reworked to
  getTBAAVTablePtrAccessInfo() that now returns the
  virtual-pointer access descriptor and not the virtual-point
  type metadata.
* Added function getTBAAMayAliasAccessInfo() that returns the
  descriptor for may-alias accesses.
* getTBAAStructTagInfo() renamed to getTBAAAccessTagInfo() as now
  it is the only way to generate access tag by a given access
  descriptor. It is capable of producing both scalar and
  struct-path tags, depending on options and availability of the
  base access type. getTBAAScalarTagInfo() and its cache
  ScalarTagMetadataCache are eliminated.
* Now that we do not need to care about whether the resulting
  access tag should be a scalar or struct-path one,
  getTBAAStructTypeInfo() is renamed to getBaseTypeInfo().
* Added function getTBAAAccessInfo() that constructs access
  descriptor by a given QualType access type.

This is part of D37826 reworked to be a separate patch to
simplify review.

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

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=314977=314976=314977=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Thu Oct  5 03:47:51 2017
@@ -205,7 +205,7 @@ namespace {
 addr = CGF.Builder.CreateStructGEP(addr, 0, CharUnits());
 
   return LValue::MakeAddr(addr, getValueType(), CGF.getContext(),
-  LVal.getBaseInfo(), LVal.getTBAAAccessType());
+  LVal.getBaseInfo(), LVal.getTBAAInfo());
 }
 
 /// \brief Emits atomic load.
@@ -1425,8 +1425,7 @@ llvm::Value *AtomicInfo::EmitAtomicLoadO
   // Other decoration.
   if (IsVolatile)
 Load->setVolatile(true);
-  if (LVal.getTBAAAccessType())
-CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAAccessType());
+  CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAInfo());
   return Load;
 }
 
@@ -1942,8 +1941,7 @@ void CodeGenFunction::EmitAtomicStore(RV
 // Other decoration.
 if (IsVolatile)
   store->setVolatile(true);
-if (dest.getTBAAAccessType())
-  CGM.DecorateInstructionWithTBAA(store, dest.getTBAAAccessType());
+CGM.DecorateInstructionWithTBAA(store, dest.getTBAAInfo());
 return;
   }
 

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=314977=314976=314977=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Oct  5 03:47:51 2017
@@ -2383,7 +2383,7 @@ void CodeGenFunction::InitializeVTablePo
   VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, 
VTableField);
-  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAInfoForVTablePtr());
+  CGM.DecorateInstructionWithTBAA(Store, CGM.getTBAAVTablePtrAccessInfo());
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   CGM.getCodeGenOpts().StrictVTablePointers)
 CGM.DecorateInstructionWithInvariantGroup(Store, Vptr.VTableClass);
@@ -2477,7 +2477,7 @@ llvm::Value *CodeGenFunction::GetVTableP
const CXXRecordDecl *RD) {
   Address VTablePtrSrc = Builder.CreateElementBitCast(This, VTableTy);
   llvm::Instruction *VTable = Builder.CreateLoad(VTablePtrSrc, "vtable");
-  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAInfoForVTablePtr());
+  CGM.DecorateInstructionWithTBAA(VTable, CGM.getTBAAVTablePtrAccessInfo());
 
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   

[PATCH] D38503: [CodeGen] Unify generation of scalar and struct-path TBAA tags

[Ivan A. Kosarev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314977: [CodeGen] Unify generation of scalar and struct-path 
TBAA tags (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38503?vs=117519=117791#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38503

Files:
  cfe/trunk/lib/CodeGen/CGAtomic.cpp
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGValue.h
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
  cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -578,42 +578,44 @@
   return TBAA->getTypeInfo(QTy);
 }
 
-llvm::MDNode *CodeGenModule::getTBAAInfoForVTablePtr() {
+TBAAAccessInfo CodeGenModule::getTBAAAccessInfo(QualType AccessType) {
+  return TBAAAccessInfo(getTBAATypeInfo(AccessType));
+}
+
+TBAAAccessInfo CodeGenModule::getTBAAVTablePtrAccessInfo() {
   if (!TBAA)
-return nullptr;
-  return TBAA->getTBAAInfoForVTablePtr();
+return TBAAAccessInfo();
+  return TBAA->getVTablePtrAccessInfo();
 }
 
 llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType QTy) {
   if (!TBAA)
 return nullptr;
   return TBAA->getTBAAStructInfo(QTy);
 }
 
-llvm::MDNode *CodeGenModule::getTBAAStructTagInfo(TBAAAccessInfo Info) {
+llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
   if (!TBAA)
 return nullptr;
-  return TBAA->getTBAAStructTagInfo(Info);
+  return TBAA->getBaseTypeInfo(QTy);
 }
 
-llvm::MDNode *CodeGenModule::getTBAAMayAliasTypeInfo() {
+llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
   if (!TBAA)
 return nullptr;
-  return TBAA->getMayAliasTypeInfo();
+  return TBAA->getAccessTagInfo(Info);
+}
+
+TBAAAccessInfo CodeGenModule::getTBAAMayAliasAccessInfo() {
+  if (!TBAA)
+return TBAAAccessInfo();
+  return TBAA->getMayAliasAccessInfo();
 }
 
-/// Decorate the instruction with a TBAA tag. For both scalar TBAA
-/// and struct-path aware TBAA, the tag has the same format:
-/// base type, access type and offset.
-/// When ConvertTypeToTag is true, we create a tag based on the scalar type.
 void CodeGenModule::DecorateInstructionWithTBAA(llvm::Instruction *Inst,
-llvm::MDNode *TBAAInfo,
-bool ConvertTypeToTag) {
-  if (ConvertTypeToTag && TBAA)
-Inst->setMetadata(llvm::LLVMContext::MD_tbaa,
-  TBAA->getTBAAScalarTagInfo(TBAAInfo));
-  else
-Inst->setMetadata(llvm::LLVMContext::MD_tbaa, TBAAInfo);
+TBAAAccessInfo TBAAInfo) {
+  if (llvm::MDNode *Tag = getTBAAAccessTagInfo(TBAAInfo))
+Inst->setMetadata(llvm::LLVMContext::MD_tbaa, Tag);
 }
 
 void CodeGenModule::DecorateInstructionWithInvariantGroup(
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -1174,8 +1174,7 @@
   llvm::Value *V = LV.getPointer();
   Scope.ForceCleanup({});
   return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
-  getContext(), LV.getBaseInfo(),
-  LV.getTBAAAccessType());
+  getContext(), LV.getBaseInfo(), LV.getTBAAInfo());
 }
 // FIXME: Is it possible to create an ExprWithCleanups that produces a
 // bitfield lvalue or some other non-simple lvalue?
@@ -1514,7 +1513,7 @@
 
   // Atomic operations have to be done on integral types.
   LValue AtomicLValue =
-  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
+  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
   if (Ty->isAtomicType() || LValueIsSuitableForInlineAtomic(AtomicLValue)) {
 return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
@@ -1525,14 +1524,10 @@
 Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
 Load->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
-  if (TBAAInfo.AccessType) {
-bool MayAlias = BaseInfo.getMayAlias();
-llvm::MDNode *TBAA = MayAlias
-? CGM.getTBAAMayAliasTypeInfo()
-: CGM.getTBAAStructTagInfo(TBAAInfo);
-if (TBAA)
-  CGM.DecorateInstructionWithTBAA(Load, TBAA, MayAlias);
-  }
+
+  if (BaseInfo.getMayAlias())
+TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
 
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
@@ -1599,7 +1594,7 @@
   Value = 

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@francisco.lopes, thanks for sharing your experience, I definitely like the 
final UX you achieved, we should aim for something similar in clangd.
It also looks like @rwols's  idea of how things should look like is very close 
to your implementation.

I do think that showing signatures for non-overloaded functions in completion 
may still be useful. Overloaded functions can be grouped by name and include 
some indicators that there are more overloads (something like `foo(int)   void  
(+5 overloads)`.
How do you choose return type for the completion item if function is 
overloaded? Show it only when it's the same for all overloads? Go with one of 
the return types?


https://reviews.llvm.org/D38048



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

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314975: [analyzer] Fix leak false positives on stuff put in 
C++/ObjC initializer lists. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D35216?vs=117699=117785#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35216

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/test/Analysis/initializer.cpp
  cfe/trunk/test/Analysis/objc-boxing.m
  cfe/trunk/test/Analysis/objc-for.m

Index: cfe/trunk/test/Analysis/initializer.cpp
===
--- cfe/trunk/test/Analysis/initializer.cpp
+++ cfe/trunk/test/Analysis/initializer.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
 
 void clang_analyzer_eval(bool);
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 class A {
   int x;
 public:
@@ -204,3 +206,17 @@
const char()[2];
 };
 }
+
+namespace CXX_initializer_lists {
+struct C {
+  C(std::initializer_list list);
+};
+void foo() {
+  C empty{}; // no-crash
+
+  // Do not warn that 'x' leaks. It might have been deleted by
+  // the destructor of 'c'.
+  int *x = new int;
+  C c{x}; // no-warning
+}
+}
Index: cfe/trunk/test/Analysis/objc-for.m
===
--- cfe/trunk/test/Analysis/objc-for.m
+++ cfe/trunk/test/Analysis/objc-for.m
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.cocoa.Loops,debug.ExprInspection -verify %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
 
 #define nil ((id)0)
 
@@ -20,11 +21,13 @@
 @interface NSArray : NSObject 
 - (NSUInteger)count;
 - (NSEnumerator *)objectEnumerator;
++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
 @end
 
 @interface NSDictionary : NSObject 
 - (NSUInteger)count;
 - (id)objectForKey:(id)key;
++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id /*  */ [])keys count:(NSUInteger)count;
 @end
 
 @interface NSDictionary (SomeCategory)
@@ -324,3 +327,19 @@
   for (id key in array)
 clang_analyzer_eval(0); // expected-warning{{FALSE}}
 }
+
+NSArray *globalArray;
+NSDictionary *globalDictionary;
+void boxedArrayEscape(NSMutableArray *array) {
+  if ([array count])
+return;
+  globalArray = @[array];
+  for (id key in array)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  if ([array count])
+return;
+  globalDictionary = @{ @"array" : array };
+  for (id key in array)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: cfe/trunk/test/Analysis/objc-boxing.m
===
--- cfe/trunk/test/Analysis/objc-boxing.m
+++ cfe/trunk/test/Analysis/objc-boxing.m
@@ -5,6 +5,16 @@
 typedef signed char BOOL;
 typedef long NSInteger;
 typedef unsigned long NSUInteger;
+
+@protocol NSObject
+@end
+@interface NSObject  {}
+@end
+@protocol NSCopying
+@end
+@protocol NSCoding
+@end
+
 @interface NSString @end
 @interface NSString (NSStringExtensionMethods)
 + (id)stringWithUTF8String:(const char *)nullTerminatedCString;
@@ -28,7 +38,15 @@
 + (NSNumber *)numberWithUnsignedInteger:(NSUInteger)value ;
 @end
 
+@interface NSValue : NSObject 
+- (void)getValue:(void *)value;
++ (NSValue *)valueWithBytes:(const void *)value
+   objCType:(const char *)type;
+@end
 
+typedef typeof(sizeof(int)) size_t;
+extern void *malloc(size_t);
+extern void free(void *);
 extern char *strdup(const char *str);
 
 id constant_string() {
@@ -39,6 +57,23 @@
 return @(strdup("boxed dynamic string")); // expected-warning{{Potential memory leak}}
 }
 
+typedef struct __attribute__((objc_boxable)) {
+  const char *str;
+} BoxableStruct;
+
+id leak_within_boxed_struct() {
+  BoxableStruct bs;
+  bs.str = strdup("dynamic string"); // The duped string shall be owned by val.
+  NSValue *val = @(bs); // no-warning
+  return val;
+}
+
+id leak_of_boxed_struct() {
+  BoxableStruct *bs = malloc(sizeof(BoxableStruct)); // The pointer stored in bs isn't owned by val.
+  NSValue *val = @(*bs); // expected-warning{{Potential leak of memory pointed to by 'bs'}}
+  return val;
+}
+
 id const_char_pointer(int *x) {
   if (x)
 return @(3);
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -827,6 +827,21 @@
   }
 }
 
+namespace {
+class CollectReachableSymbolsCallback final : public SymbolVisitor {
+  InvalidatedSymbols Symbols;
+
+public:
+  explicit 

r314975 - [analyzer] Fix leak false positives on stuff put in C++/ObjC initializer lists.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Oct  5 01:43:32 2017
New Revision: 314975

URL: http://llvm.org/viewvc/llvm-project?rev=314975=rev
Log:
[analyzer] Fix leak false positives on stuff put in C++/ObjC initializer lists.

The analyzer now realizes that C++ std::initializer_list objects and
Objective-C boxed structure/array/dictionary expressions can potentially
maintain a reference to the objects that were put into them. This avoids
false memory leak posivites and a few other issues.

This is a conservative behavior; for now, we do not model what actually happens
to the objects after being passed into such initializer lists.

rdar://problem/32918288
Differential Revision: https://reviews.llvm.org/D35216

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/initializer.cpp
cfe/trunk/test/Analysis/objc-boxing.m
cfe/trunk/test/Analysis/objc-for.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=314975=314974=314975=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Oct  5 01:43:32 2017
@@ -827,6 +827,21 @@ void ExprEngine::VisitCXXBindTemporaryEx
   }
 }
 
+namespace {
+class CollectReachableSymbolsCallback final : public SymbolVisitor {
+  InvalidatedSymbols Symbols;
+
+public:
+  explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
+  const InvalidatedSymbols () const { return Symbols; }
+
+  bool VisitSymbol(SymbolRef Sym) override {
+Symbols.insert(Sym);
+return true;
+  }
+};
+} // end anonymous namespace
+
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet ) {
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
@@ -1103,8 +1118,29 @@ void ExprEngine::Visit(const Stmt *S, Ex
 SVal result = svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
resultType,
currBldrCtx->blockCount());
-ProgramStateRef state = N->getState()->BindExpr(Ex, LCtx, result);
-Bldr2.generateNode(S, N, state);
+ProgramStateRef State = N->getState()->BindExpr(Ex, LCtx, result);
+
+// Escape pointers passed into the list, unless it's an ObjC boxed
+// expression which is not a boxable C structure.
+if (!(isa(Ex) &&
+  !cast(Ex)->getSubExpr()
+  ->getType()->isRecordType()))
+  for (auto Child : Ex->children()) {
+assert(Child);
+
+SVal Val = State->getSVal(Child, LCtx);
+
+CollectReachableSymbolsCallback Scanner =
+State->scanReachableSymbols(
+Val);
+const InvalidatedSymbols  = Scanner.getSymbols();
+
+State = getCheckerManager().runCheckersForPointerEscape(
+State, EscapedSymbols,
+/*CallEvent*/ nullptr, PSK_EscapeOther, nullptr);
+  }
+
+Bldr2.generateNode(S, N, State);
   }
 
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
@@ -2237,21 +2273,6 @@ void ExprEngine::VisitAtomicExpr(const A
   getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, 
*this);
 }
 
-namespace {
-class CollectReachableSymbolsCallback final : public SymbolVisitor {
-  InvalidatedSymbols Symbols;
-
-public:
-  CollectReachableSymbolsCallback(ProgramStateRef State) {}
-  const InvalidatedSymbols () const { return Symbols; }
-
-  bool VisitSymbol(SymbolRef Sym) override {
-Symbols.insert(Sym);
-return true;
-  }
-};
-} // end anonymous namespace
-
 // A value escapes in three possible cases:
 // (1) We are binding to something that is not a memory region.
 // (2) We are binding to a MemrRegion that does not have stack storage.

Modified: cfe/trunk/test/Analysis/initializer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/initializer.cpp?rev=314975=314974=314975=diff
==
--- cfe/trunk/test/Analysis/initializer.cpp (original)
+++ cfe/trunk/test/Analysis/initializer.cpp Thu Oct  5 01:43:32 2017
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
c++-inlining=constructors -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection
 -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
 
 void clang_analyzer_eval(bool);
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 class A {
   int x;
 public:
@@ -204,3 +206,17 @@ struct C {
const char()[2];
 };
 }
+
+namespace CXX_initializer_lists {
+struct C {
+  C(std::initializer_list list);
+};
+void 

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

[Daniel Kolozsvari via Phabricator via cfe-commits]

koldaniel updated this revision to Diff 117782.
Herald added a subscriber: szepet.

https://reviews.llvm.org/D35068

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/Driver/ToolChains/Clang.cpp
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m

Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -13,6 +13,9 @@
 # define BUILTIN(f) f
 #endif /* USE_BUILTINS */
 
+#include "Inputs/system-header-simulator-for-valist.h"
+#include "Inputs/system-header-simulator-for-simple-stream.h"
+
 typedef typeof(sizeof(int)) size_t;
 
 
@@ -202,3 +205,83 @@
   mkdtemp("XX");
 }
 
+
+//===--===
+// deprecated or unsafe buffer handling
+//===--===
+typedef int wchar_t;
+
+int sprintf(char *str, const char *format, ...);
+//int vsprintf (char *s, const char *format, va_list arg);
+int scanf(const char *format, ...);
+int wscanf(const wchar_t *format, ...);
+int fscanf(FILE *stream, const char *format, ...);
+int fwscanf(FILE *stream, const wchar_t *format, ...);
+int vscanf(const char *format, va_list arg);
+int vwscanf(const wchar_t *format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list arg);
+int vfwscanf(FILE *stream, const wchar_t *format, va_list arg);
+int sscanf(const char *s, const char *format, ...);
+int swscanf(const wchar_t *ws, const wchar_t *format, ...);
+int vsscanf(const char *s, const char *format, va_list arg);
+int vswscanf(const wchar_t *ws, const wchar_t *format, va_list arg);
+int swprintf(wchar_t *ws, size_t len, const wchar_t *format, ...);
+int snprintf(char *s, size_t n, const char *format, ...);
+int vswprintf(wchar_t *ws, size_t len, const wchar_t *format, va_list arg);
+int vsnprintf(char *s, size_t n, const char *format, va_list arg);
+void *memcpy(void *destination, const void *source, size_t num);
+void *memmove(void *destination, const void *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+char *strncat(char *destination, const char *source, size_t num);
+void *memset(void *ptr, int value, size_t num);
+
+void test_deprecated_or_unsafe_buffer_handling_1() {
+  char buf [5];
+  wchar_t wbuf [5];
+  int a;
+  FILE *file;
+  sprintf(buf, "a"); // expected-warning{{Using 'sprintf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'sprintf_s'}}
+  scanf("%d", ); // expected-warning{{Using 'scanf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'scanf_s'}}
+  scanf("%s", buf); // expected-warning{{Call to function 'scanf' is insecure as it does not provide bounding of the memory buffer. Replace with analogous functions that support length arguments or provides boundary checks such as 'scanf_s' in case of C11}} expected-warning{{Using 'scanf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'scanf_s'}}
+  scanf("%4s", buf); // expected-warning{{Using 'scanf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'scanf_s'}}
+  wscanf((const wchar_t*) L"%s", buf); // expected-warning{{Call to function 'wscanf' is insecure as it does not provide bounding of the memory buffer. Replace with analogous functions that support length arguments or provides boundary checks such as 'wscanf_s' in case of C11}} expected-warning{{Using 'wscanf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'wscanf_s'}}
+  fscanf(file, "%d", ); // expected-warning{{Using 'fscanf' is deprecated as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions introduced in C11 standard that supports length arguments or provides boundary checks such as 'fscanf_s'}}
+  fscanf(file, "%s", buf); // expected-warning{{Call to function 'fscanf' is insecure as it does not provide bounding of the 

[PATCH] D38576: |libunwind] [docs] Mention that SjLj works on any OS on the archs where supported by the compiler

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.

https://reviews.llvm.org/D38576

Files:
  docs/index.rst


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -50,6 +50,7 @@
 LinuxARM  Clang, GCC   EHABI
 Bare Metal   ARM  Clang, GCC   EHABI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -50,6 +50,7 @@
 LinuxARM  Clang, GCC   EHABI
 Bare Metal   ARM  Clang, GCC   EHABI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits