[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

2018-12-18 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

Just to be clear, this is just a syntactic change from what I originally 
posted. The old `#pragma clang attribute push NoReturn (...)` is semantically 
equivalent to the new `#pragma clang attribute NoReturn.push (...)`


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

https://reviews.llvm.org/D55628



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


[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

2018-12-18 Thread Simon Atanasyan via Phabricator via cfe-commits
atanasyan added inline comments.



Comment at: lib/Driver/ToolChains/FreeBSD.cpp:160
+if (ToolChain.getTriple().getOSMajorVersion() >= 9)
+  CmdArgs.push_back("--hash-style=gnu");
 CmdArgs.push_back("--enable-new-dtags");

MaskRay wrote:
> I can't find rationale behind the MIPS discrepancy in the original commit. I 
> can add the if branch back if you tell me why...
> I can't find rationale behind the MIPS discrepancy in the original commit. I 
> can add the if branch back if you tell me why...

Did you check that linker on MIPS FreeBSD really support --hash-style=gnu?

As far as I know ".gnu.hash and the MIPS ABI require .dynsym to be sorted in 
different ways.  .gnu.hash needs symbols to be grouped by hash code whereas the 
MIPS ABI requires a mapping between the GOT and the symbol table".


Repository:
  rC Clang

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

https://reviews.llvm.org/D55878



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


[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

2018-12-18 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 178837.
erik.pilkington added a comment.

After looking through some users of `#pragma clang attribute`, I don't think 
that the begin/end solution is what we want here. Some users of this attribute 
write macros that can expand to different attributes depending on their 
arguments, for instance:

  AVAILABILITY_BEGIN(macos(10.12)) // expands to an availability attribute
  AVAILABILITY_BEGIN(ios(10))
  // some code...
  AVAILABILITY_END
  AVAILABILITY_END

This code makes sense and is safe, but in this case rewriting 
AVAILABILITY_BEGIN to use a hypothetical `pragma clang attribute begin`/`end` 
would be a breaking change, which isn't acceptable. So I think that we want 
stack semantics, but scoped within the `AVAILABILITY_BEGIN` macro's namespace. 
That way, we can support multiple `push`es in the same macro, without having 
having different macros accidentally pop each other.

As for a syntax for this, I chose the following (basically, @dexonsmith's idea 
with a '.'):

  #pragma clang attribute NoReturn.push (__attribute__((noreturn)), 
apply_to=function)
  int foo();
  #pragma clang attribute NoReturn.pop

I think the '.' makes the nested relationship (i.e. the push is contained 
within the namespace) more clear to C programmers, and hopefully clears up the 
confusion. @AaronBallman: WDYT?


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

https://reviews.llvm.org/D55628

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Parser/pragma-attribute.cpp
  clang/test/Sema/pragma-attribute-namespace.c

Index: clang/test/Sema/pragma-attribute-namespace.c
===
--- /dev/null
+++ clang/test/Sema/pragma-attribute-namespace.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma clang attribute MyNamespace.push (__attribute__((annotate)), apply_to=function) // expected-error 2 {{'annotate' attribute}}
+
+int some_func(); // expected-note{{when applied to this declaration}}
+
+#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}}
+#pragma clang attribute NotMyNamespace.pop // expected-error{{'#pragma clang attribute NotMyNamespace.pop' with no matching '#pragma clang attribute NotMyNamespace.push'}}
+
+#pragma clang attribute MyOtherNamespace.push (__attribute__((annotate)), apply_to=function) // expected-error 2 {{'annotate' attribute}}
+
+int some_other_func(); // expected-note 2 {{when applied to this declaration}}
+
+// Out of order!
+#pragma clang attribute MyNamespace.pop
+
+int some_other_other_func(); // expected-note 1 {{when applied to this declaration}}
+
+#pragma clang attribute MyOtherNamespace.pop
+
+#pragma clang attribute Misc. () // expected-error{{namespace can only apply to 'push' or 'pop' directives}} expected-note {{omit the namespace to add attributes to the most-recently pushed attribute group}}
+
+#pragma clang attribute Misc push // expected-error{{expected '.' after pragma attribute namespace 'Misc'}}
Index: clang/test/Parser/pragma-attribute.cpp
===
--- clang/test/Parser/pragma-attribute.cpp
+++ clang/test/Parser/pragma-attribute.cpp
@@ -102,7 +102,7 @@
 
 #pragma clang attribute // expected-error {{expected 'push', 'pop', or '(' after '#pragma clang attribute'}}
 #pragma clang attribute 42 // expected-error {{expected 'push', 'pop', or '(' after '#pragma clang attribute'}}
-#pragma clang attribute pushpop // expected-error {{unexpected argument 'pushpop' to '#pragma clang attribute'; expected 'push' or 'pop'}}
+#pragma clang attribute pushpop // expected-error {{expected '.' after pragma attribute namespace 'pushpop'}}
 
 #pragma clang attribute push
 #pragma clang attribute pop
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -631,28 +631,46 @@
   {PragmaLoc, , std::move(SubjectMatchRules), /*IsUsed=*/false});
 }
 
-void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc) {
+void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc,
+ const IdentifierInfo *Namespace) {
   PragmaAttributeStack.emplace_back();
   PragmaAttributeStack.back().Loc = PragmaLoc;
+  PragmaAttributeStack.back().Namespace = Namespace;
 }
 
-void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc) {
+void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc,
+   const IdentifierInfo *Namespace) {
   if (PragmaAttributeStack.empty()) {
-Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch);
+Diag(PragmaLoc, 

[PATCH] D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/Basic/FixedPoint.h:98
  public:
+   APFixedPoint() = default;
APFixedPoint(const llvm::APInt , const FixedPointSemantics )

This should be documented to describe what it does.  Does it create a value 
with impossible semantics?  Some reasonable default value in some reasonable 
default semantics?



Comment at: clang/lib/AST/ExprConstant.cpp:9953
+  return false;
+APFixedPoint Result = Src.convert(DestFXSema);
+return Success(Result, E);

Can a fixed-point conversion ever have undefined behavior?  If so, you might 
need to flag that as a failed case, unless we want to give it defined semantics 
in Clang.



Comment at: clang/lib/AST/ExprConstant.cpp:9955
+return Success(Result, E);
+  }
+  default:

Do you not have a separate `CK_IntegralToFixedPoint`?  It's convenient here, 
but still, it's curious.

You also need a `CK_FloatingToFixedPoint`, I think.  (Obviously you don't need 
to implement that right now.)



Comment at: clang/lib/AST/ExprConstant.cpp:9959
+  }
+  llvm_unreachable("unknown cast resulting in fixed point value");
+}

This is obviously unreachable because of the `default` case in the `switch`.  
Should this be moved into the `default` case, and then you can put your `Error` 
call in cases for the known-missing fixed-point conversions?

You should go ahead and add cases for `CK_NoOp` and `CK_LValueToRValue`, they 
should be obvious from the other evaluators.

You should add tests for constant-evaluating fixed-point conversions.



Comment at: clang/lib/AST/ExprConstant.cpp:9982
+  }
+  llvm_unreachable("Should've exited before this");
+}

This `unreachable` seems more reasonable since you're probably never going to 
make this a covered `switch`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55868



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


[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

You're gating on OpenCL C++ in several places in this patch, but I don't think 
you need to.  This extension should be available to anyone using address spaces 
and C++.




Comment at: lib/Parse/ParseDecl.cpp:6162
+}
+  }
+

This is enforcing a restriction that users write `const __private`, which seems 
unreasonable.  It looks like `ParseTypeQualifierList` takes a flag saying not 
to parse attributes; try adding a new option to that enum allowing 
address-space attributes.

Collecting the attributes on the type-qualifiers `DeclSpec` rather than adding 
them as function attributes seems correct.



Comment at: lib/Sema/SemaOverload.cpp:1157
+NewMethod->getTypeQualifiers().getAddressSpace())
+  return true;
   }

If you leave `OldQuals` and `NewQuals` as `Qualifiers` and then just remove 
`restrict` from both sets, I think you don't need a special check for address 
spaces here.



Comment at: lib/Sema/SemaOverload.cpp:6671
+}
+  }
 }

I would expect this to fail in `TryObjectArgumentInitialization` above and not 
to need a special case here.



Comment at: lib/Sema/SemaOverload.cpp:9279
+(CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default))
+  return true;
+  }

This at least needs a comment explaining the rule you're trying to implement.


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

https://reviews.llvm.org/D55850



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


[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

2018-12-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 178836.
MaskRay added a comment.

.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55878

Files:
  lib/Driver/ToolChains/FreeBSD.cpp
  test/Driver/freebsd.c


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -73,38 +73,32 @@
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
+// CHECK-LDFLAGS8-NOT: --hash-style=gnu
 // CHECK-LDFLAGS8: --enable-new-dtags
-// CHECK-LDFLAGS9: --hash-style=both
+// CHECK-LDFLAGS9: --hash-style=gnu
 // CHECK-LDFLAGS9: --enable-new-dtags
 //
-// Check that we do not pass --hash-style=gnu and --hash-style=both to linker
-// and provide correct path to the dynamic linker for MIPS platforms.
-// Also verify that we tell the assembler to target the right ISA and ABI.
+// Verify that we tell the assembler to target the right ISA and ABI.
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: -target mips-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS %s
 // CHECK-MIPS: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPS: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS-NOT: "--hash-style={{gnu|both}}"
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: -target mipsel-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSEL %s
 // CHECK-MIPSEL: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPSEL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPSEL-NOT: "--hash-style={{gnu|both}}"
 // RUN: %clang %s -### 2>&1 \
 // RUN: -target mips64-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64 %s
 // CHECK-MIPS64: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPS64: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS64-NOT: "--hash-style={{gnu|both}}"
 // RUN: %clang %s -### 2>&1 \
 // RUN: -target mips64el-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64EL %s
 // CHECK-MIPS64EL: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPS64EL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS64EL-NOT: "--hash-style={{gnu|both}}"
 
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -static %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -156,12 +156,8 @@
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }
-if (ToolChain.getTriple().getOSMajorVersion() >= 9) {
-  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc ||
-  Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64) {
-CmdArgs.push_back("--hash-style=both");
-  }
-}
+if (ToolChain.getTriple().getOSMajorVersion() >= 9)
+  CmdArgs.push_back("--hash-style=gnu");
 CmdArgs.push_back("--enable-new-dtags");
   }
 


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -73,38 +73,32 @@
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
+// CHECK-LDFLAGS8-NOT: --hash-style=gnu
 // CHECK-LDFLAGS8: --enable-new-dtags
-// CHECK-LDFLAGS9: --hash-style=both
+// CHECK-LDFLAGS9: --hash-style=gnu
 // CHECK-LDFLAGS9: --enable-new-dtags
 //
-// Check that we do not pass --hash-style=gnu and --hash-style=both to linker
-// and provide correct path to the dynamic linker for MIPS platforms.
-// Also verify that we tell the assembler to target the right ISA and ABI.
+// Verify that we tell the assembler to target the right ISA and ABI.
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: -target mips-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS %s
 // CHECK-MIPS: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPS: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS-NOT: "--hash-style={{gnu|both}}"
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: -target mipsel-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSEL %s
 // CHECK-MIPSEL: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPSEL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPSEL-NOT: "--hash-style={{gnu|both}}"
 // RUN: %clang %s -### 2>&1 \
 // RUN: -target mips64-unknown-freebsd10.0 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64 %s
 // CHECK-MIPS64: "{{[^" ]*}}ld{{[^" ]*}}"
 // CHECK-MIPS64: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// 

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

So, once upon a time this was a problem because we were rewriting the method 
calls in the runtime itself.  Can you explain how that's not a problem now?  Do 
we just expect the runtime to be compiled with the right switch?




Comment at: include/clang/Basic/ObjCRuntime.h:191
+  ///   retain => objc_retain
+  ///   release => objc_release
+  bool shouldUseARCFunctionsForRetainRelease() const {

You're also rewriting `autorelease`.



Comment at: include/clang/Basic/ObjCRuntime.h:207
+  case ObjFW:
+return false;
+}

Case indentation.



Comment at: lib/CodeGen/CodeGenModule.h:170
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_retainRuntimeFunction;

Period (and on the other comments here)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55869



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


[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

2018-12-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: lib/Driver/ToolChains/FreeBSD.cpp:160
+if (ToolChain.getTriple().getOSMajorVersion() >= 9)
+  CmdArgs.push_back("--hash-style=gnu");
 CmdArgs.push_back("--enable-new-dtags");

I can't find rationale behind the MIPS discrepancy in the original commit. I 
can add the if branch back if you tell me why...


Repository:
  rC Clang

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

https://reviews.llvm.org/D55878



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


[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D55662#1335402 , @ahatanak wrote:

> Sorry, please ignore my previous comment. I was looking at the wrong place.
>
> The following code reaches `Sema::BuildDecltypeType` without going through 
> `ActOnDecltypeExpression`:
>
>   template 
>   void overloaded_fn(T);
>   decltype(auto) v5 = _fn;
>
>
> `Sema::BuildDecltypeType` is called from `Sema::DeduceAutoType`, so calling 
> `CheckPlaceholderExpr ` there should fix the assert when the test case above 
> is compiled.


Okay.  You may need to push an unevaluated context when doing that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55662



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


[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

2018-12-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: emaste, brooks, dim.
Herald added subscribers: cfe-commits, atanasyan, fedor.sergeev, arichardson, 
sdardis.

rtld started to support DT_GNU_HASH since rS234841 (2013).
libexec/rtld-elf/rtld.c:symlook_obj uses DT_GNU_HASH when it is present and 
ignores DT_HASH.
This saves a few hundreds bytes to a few kilobytes for typical executables. 
(DT_HASH is usually larger than DT_GNU_HASH because it does not skip undefined 
dynsym entries)

Also delete historical special case for MIPS.


Repository:
  rC Clang

https://reviews.llvm.org/D55878

Files:
  lib/Driver/ToolChains/FreeBSD.cpp
  test/Driver/freebsd.c


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -73,38 +73,10 @@
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
+// CHECK-LDFLAGS8-NOT: --hash-style=gnu
 // CHECK-LDFLAGS8: --enable-new-dtags
-// CHECK-LDFLAGS9: --hash-style=both
+// CHECK-LDFLAGS9: --hash-style=gnu
 // CHECK-LDFLAGS9: --enable-new-dtags
-//
-// Check that we do not pass --hash-style=gnu and --hash-style=both to linker
-// and provide correct path to the dynamic linker for MIPS platforms.
-// Also verify that we tell the assembler to target the right ISA and ABI.
-// RUN: %clang %s -### -o %t.o 2>&1 \
-// RUN: -target mips-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPS %s
-// CHECK-MIPS: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPS: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS-NOT: "--hash-style={{gnu|both}}"
-// RUN: %clang %s -### -o %t.o 2>&1 \
-// RUN: -target mipsel-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPSEL %s
-// CHECK-MIPSEL: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPSEL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPSEL-NOT: "--hash-style={{gnu|both}}"
-// RUN: %clang %s -### 2>&1 \
-// RUN: -target mips64-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPS64 %s
-// CHECK-MIPS64: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPS64: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS64-NOT: "--hash-style={{gnu|both}}"
-// RUN: %clang %s -### 2>&1 \
-// RUN: -target mips64el-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPS64EL %s
-// CHECK-MIPS64EL: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPS64EL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS64EL-NOT: "--hash-style={{gnu|both}}"
 
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -static %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -156,12 +156,8 @@
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }
-if (ToolChain.getTriple().getOSMajorVersion() >= 9) {
-  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc ||
-  Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64) {
-CmdArgs.push_back("--hash-style=both");
-  }
-}
+if (ToolChain.getTriple().getOSMajorVersion() >= 9)
+  CmdArgs.push_back("--hash-style=gnu");
 CmdArgs.push_back("--enable-new-dtags");
   }
 


Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -73,38 +73,10 @@
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
+// CHECK-LDFLAGS8-NOT: --hash-style=gnu
 // CHECK-LDFLAGS8: --enable-new-dtags
-// CHECK-LDFLAGS9: --hash-style=both
+// CHECK-LDFLAGS9: --hash-style=gnu
 // CHECK-LDFLAGS9: --enable-new-dtags
-//
-// Check that we do not pass --hash-style=gnu and --hash-style=both to linker
-// and provide correct path to the dynamic linker for MIPS platforms.
-// Also verify that we tell the assembler to target the right ISA and ABI.
-// RUN: %clang %s -### -o %t.o 2>&1 \
-// RUN: -target mips-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPS %s
-// CHECK-MIPS: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPS: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPS-NOT: "--hash-style={{gnu|both}}"
-// RUN: %clang %s -### -o %t.o 2>&1 \
-// RUN: -target mipsel-unknown-freebsd10.0 \
-// RUN:   | FileCheck --check-prefix=CHECK-MIPSEL %s
-// CHECK-MIPSEL: "{{[^" ]*}}ld{{[^" ]*}}"
-// CHECK-MIPSEL: "-dynamic-linker" "{{.*}}/libexec/ld-elf.so.1"
-// CHECK-MIPSEL-NOT: 

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

2018-12-18 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

You can't emit the type tests by default with `-flto=thin` because not all 
programs adhere to the conditions described in `LTOVisibility.rst` and we can't 
silently break those programs; it is something that they will need to 
explicitly opt in to. We should be able to emit the `!type` annotations by 
default, just not the type tests.




Comment at: lib/CodeGen/BackendUtil.cpp:831
+  *OS, CodeGenOpts.EmitLLVMUseLists, EmitLTOSummary,
+  /*EmitModuleHash=*/false));
 }

Why add this argument here (and below)?



Comment at: lib/Driver/ToolChains/Clang.cpp:5112
+  bool EnableSplitLTOUnit = Args.hasFlag(
+  options::OPT_fsplit_lto_unit, options::OPT_fno_split_lto_unit, false);
+  if (EnableSplitLTOUnit || WholeProgramVTables || Sanitize.needsLTO()) {

Should this default to `WholeProgramVTables || Sanitize.needsLTO()` and emit an 
error if the user passes the (for now) unsupported combinations 
`-fno-split-lto-unit -fwhole-program-vtables` or `-fno-split-lto-unit 
-fsanitize=cfi`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D53891



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


[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment.

In D55616#1335587 , @vitalybuka wrote:

> Looks like it's broken by this patch
>
>   clang: 
> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/AST/ExprConstant.cpp:11055:
>  llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext &, 
> SmallVectorImpl *) const: Assertion `Result && 
> "Could not evaluate expression"' failed.
>
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/9281/steps/check-clang%20msan/logs/stdio


This should be fixed now. Sorry about the breakage!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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


r349604 - Use "EvaluateAsRValue" instead of as a known int, because if it's not a known

2018-12-18 Thread Bill Wendling via cfe-commits
Author: void
Date: Tue Dec 18 20:54:29 2018
New Revision: 349604

URL: http://llvm.org/viewvc/llvm-project?rev=349604=rev
Log:
Use "EvaluateAsRValue" instead of as a known int, because if it's not a known
integer we want to emit a diagnostic instead of asserting.

Modified:
cfe/trunk/lib/Sema/SemaStmtAsm.cpp

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=349604=349603=349604=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Tue Dec 18 20:54:29 2018
@@ -379,16 +379,16 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
 llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {


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


r349603 - Revert accidentally included code.

2018-12-18 Thread Bill Wendling via cfe-commits
Author: void
Date: Tue Dec 18 20:36:42 2018
New Revision: 349603

URL: http://llvm.org/viewvc/llvm-project?rev=349603=rev
Log:
Revert accidentally included code.

Modified:
cfe/trunk/lib/Sema/SemaStmtAsm.cpp

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=349603=349602=349603=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Tue Dec 18 20:36:42 2018
@@ -378,17 +378,17 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-llvm::SmallVector Diags;
-llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
-if (!Diags.empty())
+Expr::EvalResult EVResult;
+if (!InputExpr->EvaluateAsInt(EVResult, Context))
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-if (!Info.isValidAsmImmediate(Result))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << Result.toString(10) << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
+llvm::APSInt Result = EVResult.Val.getInt();
+ if (!Info.isValidAsmImmediate(Result))
+   return StmtError(Diag(InputExpr->getBeginLoc(),
+ diag::err_invalid_asm_value_for_constraint)
+<< Result.toString(10) << Info.getConstraintStr()
+<< InputExpr->getSourceRange());
   }
 
 } else {


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


[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

2018-12-18 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment.

These seems reasonable, although it does also seem like there could be quite a 
few unintended consequences that we haven't discovered yet.

I'm also a bit worried about the change in the analyzer's behavior on copy(). 
Do you have a sense of how much of an effect this will have and how easy 
potential false positives from this will be to suppress?




Comment at: test/Analysis/bstring.cpp:47
 
+  // The TRUE warning shows up on the path on which the vector is empty.
   clang_analyzer_eval(i == 66); // expected-warning {{UNKNOWN}}

This seems like it will be a big analysis policy change from the user's 
perspective and is likely to generate a bunch of new reports.

Can the user add an assertion that v.size() > 0 to tell the analyzer that the 
path on which the vector is empty is not feasible?

What are the diagnostic notes look like? Can the user tell that the the 
analyzer is assuming that begin() == end() on that path?



Repository:
  rC Clang

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

https://reviews.llvm.org/D55875



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


[PATCH] D55873: [analyzer] CStringChecker: Fix a crash when an argument of a weird type is encountered.

2018-12-18 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: test/Analysis/string.cpp:23
+
+// A similarly weird override outside of the class.
+void *memcpy(void *, const S &, size_t);

I'm pretty sure you mean 'overload' instead of 'override' here and elsewhere.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55873



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


[PATCH] D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame.

2018-12-18 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

This seems reasonable to me, although I have a question inline about why you 
are using `makeZeroElementRegion()`.




Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:224
+bool IsArray = false;
+V = makeZeroElementRegion(State, V, ReturnTy, IsArray);
+assert(!IsArray && "Returning array from a function!");

I don't understand why you are using makeZeroElementRegion() here. Doesn't the 
assertion of !IsArray mean it must just return 'V'?


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

https://reviews.llvm.org/D55804



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


[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, 
rnkovacs, Szelethus, mikhail.ramalho, baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.

It is expected to have the same object (memory region) treated as if it has 
different types in different program points. Like, the object may be a union or 
contain a union, or a pointer to it can be reinterpreted as a pointer to a 
different type. The correct behavior for `RegionStore` when an object is stored 
as an object of type `T1` but loaded as an object of type `T2` is to store the 
object as if it has type `T1` but cast it to `T2` during load. This is what 
`CastRetrievedVal` is responsible for.

Note that the cast here is some sort of a "`reinterpret_cast`" (even in C). For 
instance, if you store a float and load an integer, you won't have your float 
rounded to an integer; instead, you will have garbage.

Therefore i propose to admit that we cannot perform the cast as long as types 
we're dealing with are non-trivial (neither integers, nor pointers). Our 
modeling would still be weird in this case when dealing with integer casts, but 
so is support for integer casts in general.

Of course, if the cast is not necessary (eg, `T1 == T2`), we can still load the 
value just fine. This trivial observation in fact improves our behavior on 
tests and addresses some old FIXMEs: previously we would have tried to perform 
the cast and fail. We should probably also teach `SValBuilder` to perform casts 
in this case.

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


Repository:
  rC Clang

https://reviews.llvm.org/D55875

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/bstring.cpp
  test/Analysis/casts.c
  test/Analysis/pointer-to-member.cpp

Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -253,11 +253,10 @@
   clang_analyzer_eval(::y); // expected-warning{{TRUE}}
   clang_analyzer_eval(::z); // expected-warning{{TRUE}}
 
-  // FIXME: These should be true.
   int A::*l = ::x, A::*m = ::y, A::*n = ::z;
-  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(l); // expected-warning{{TRUE}}
+  clang_analyzer_eval(m); // expected-warning{{TRUE}}
+  clang_analyzer_eval(n); // expected-warning{{TRUE}}
 
   // FIXME: These should be true as well.
   A a;
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,14 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f();
+}
Index: test/Analysis/bstring.cpp
===
--- test/Analysis/bstring.cpp
+++ test/Analysis/bstring.cpp
@@ -44,7 +44,9 @@
 
   int i = buf[0];
 
+  // The TRUE warning shows up on the path on which the vector is empty.
   clang_analyzer_eval(i == 66); // expected-warning {{UNKNOWN}}
+// expected-warning@-1 {{TRUE}}
 
   return buf;
 }
@@ -60,7 +62,9 @@
 
   int i = buf[0];
 
+  // The TRUE warning shows up on the path on which the vector is empty.
   clang_analyzer_eval(i == 66); // expected-warning {{UNKNOWN}}
+// expected-warning@-1 {{TRUE}}
 
   return buf;
 }
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -394,14 +394,28 @@
   return UnknownVal();
 }
 
+static bool isScalarEnoughToAttemptACast(QualType T) {
+  return T->isIntegralOrEnumerationType() || T->isAnyPointerType() ||
+ T->isReferenceType();
+}
+
 /// CastRetrievedVal - Used by subclasses of StoreManager to implement
 ///  implicit casts that arise from loads from regions that are reinterpreted
 ///  as another region.
 SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
+QualType CastTy) {
+  if (CastTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  QualType OrigTy = R->getValueType();
+
+  if (!isScalarEnoughToAttemptACast(OrigTy) ||
+  !isScalarEnoughToAttemptACast(CastTy)) {
+if (OrigTy == CastTy)
+  return V;
+return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, 

[PATCH] D55873: [analyzer] CStringChecker: Fix a crash when an argument of a weird type is encountered.

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, 
rnkovacs, Szelethus, mikhail.ramalho, baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.

It turns out that it's not all that uncommon to have a C++ override of, say, 
`memcpy` that receives a structure (or two) by reference (or by value, if it's 
being copied from) and copies memory from it (or into it, if it's passed by 
reference). In this case the argument will be of structure type (recall that 
expressions of reference type do not exist: instead, C++ classifies expressions 
into prvalues and lvalues and xvalues).

In this scenario we crash because we are trying to assume that, say, a memory 
region is equal to an empty `CompoundValue` (the non-lazy one; this is what 
`makeZeroVal()` return for compound types and it represents prvalue of an 
object that is initialized with an empty initializer list).

Add defensive checks. We should probably extend `CallDescription` so that it 
encapsulated these checks and we were always sure that this is the function 
we're looking for.


Repository:
  rC Clang

https://reviews.llvm.org/D55873

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.cpp


Index: test/Analysis/string.cpp
===
--- /dev/null
+++ test/Analysis/string.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
+
+// expected-no-diagnostics
+
+// Test functions that are called "memcpy" but aren't the memcpy
+// we're looking for. Unfortunately, this test cannot be put into
+// a namespace. The out-of-class weird memcpy needs to be recognized
+// as a normal C function for the test to make sense.
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *, const void *, size_t);
+
+struct S {
+  static S s1, s2;
+
+  // A weird override within the class that accepts a structure reference
+  // instead of a pointer.
+  void memcpy(void *, const S &, size_t);
+  void test_in_class_weird_memcpy() {
+memcpy(this, s2, 1); // no-crash
+  }
+};
+
+// A similarly weird override outside of the class.
+void *memcpy(void *, const S &, size_t);
+
+void test_out_of_class_weird_memcpy() {
+  memcpy(::s1, S::s2, 1); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2255,11 +2255,20 @@
 
//===--===//
 
 bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext ) const {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
 
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
   if (!FDecl)
 return false;
 
+  // Pro-actively check that argument types are safe to do arithmetic upon.
+  // We do not want to crash if someone accidentally passes a structure
+  // into, say, a C++ override of any of these functions.
+  for (auto I: CE->arguments()) {
+QualType T = I->getType();
+if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+  return false;
+  }
+
   // FIXME: Poorly-factored string switches are slow.
   FnCheck evalFunction = nullptr;
   if (C.isCLibraryFunction(FDecl, "memcpy"))


Index: test/Analysis/string.cpp
===
--- /dev/null
+++ test/Analysis/string.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
+
+// expected-no-diagnostics
+
+// Test functions that are called "memcpy" but aren't the memcpy
+// we're looking for. Unfortunately, this test cannot be put into
+// a namespace. The out-of-class weird memcpy needs to be recognized
+// as a normal C function for the test to make sense.
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *, const void *, size_t);
+
+struct S {
+  static S s1, s2;
+
+  // A weird override within the class that accepts a structure reference
+  // instead of a pointer.
+  void memcpy(void *, const S &, size_t);
+  void test_in_class_weird_memcpy() {
+memcpy(this, s2, 1); // no-crash
+  }
+};
+
+// A similarly weird override outside of the class.
+void *memcpy(void *, const S &, size_t);
+
+void test_out_of_class_weird_memcpy() {
+  memcpy(::s1, S::s2, 1); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2255,11 +2255,20 @@
 //===--===//
 
 bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext ) const {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
 
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
   if (!FDecl)
 return false;
 
+  

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

2018-12-18 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.

Minor typo noted below, but otherwise, LGTM (to avoid any misunderstanding: 
this should be committed after the LLVM change lands).




Comment at: lib/CodeGen/CGLoopInfo.cpp:341
+for (const LoopInfo  : Active) {
+  // Here we assume that ever loop that has an access group is parallel.
+  if (MDNode *Group = AL.getAccessGroup())

ever -> every


Repository:
  rC Clang

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

https://reviews.llvm.org/D52117



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


[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

Looks like it's broken by this patch

  clang: 
/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/AST/ExprConstant.cpp:11055:
 llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext &, 
SmallVectorImpl *) const: Assertion `Result && 
"Could not evaluate expression"' failed.

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/9281/steps/check-clang%20msan/logs/stdio


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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


[PATCH] D55823: [analyzer] Fix backward compatibility issue after D53280 'Emit an error for invalid -analyzer-config inputs'

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

These arguments seem to be available during the top-level call, 
`Driver::BuildCompilation`. I guess we need to stick this somewhere in between.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55823



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


[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-18 Thread Pete Cooper via Phabricator via cfe-commits
pete created this revision.
pete added reviewers: rjmccall, ahatanak, erik.pilkington.
Herald added a subscriber: cfe-commits.

It is faster to directly call the ObjC runtime for methods such as 
retain/release instead of sending a message to those functions.

This is an extension of the work we already do to convert alloc message sends 
to objc_alloc calls.


Repository:
  rC Clang

https://reviews.llvm.org/D55869

Files:
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  test/CodeGenObjC/convert-messages-to-runtime-calls.m

Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- test/CodeGenObjC/convert-messages-to-runtime-calls.m
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -14,16 +14,28 @@
 + (id)alloc;
 + (id)allocWithZone:(void*)zone;
 + (id)alloc2;
+- (id)retain;
+- (void)release;
+- (id)autorelease;
 @end
 
 // CHECK-LABEL: define {{.*}}void @test1
 void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // MSGS: {{call.*@objc_msgSend}}
   // CALLS: {{call.*@objc_alloc}}
   // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS: {{call.*@objc_release}}
+  // CALLS: {{call.*@objc_autorelease}}
   [NSObject alloc];
   [NSObject allocWithZone:nil];
+  [x retain];
+  [x release];
+  [x autorelease];
 }
 
 // CHECK-LABEL: define {{.*}}void @test2
@@ -43,6 +55,8 @@
 @interface B
 + (A*) alloc;
 + (A*)allocWithZone:(void*)zone;
+- (A*) retain;
+- (A*) autorelease;
 @end
 
 // Make sure we get a bitcast on the return type as the
@@ -65,9 +79,30 @@
   return [B allocWithZone:nil];
 }
 
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_retain_class_ptr
+A* test_retain_class_ptr(B *b) {
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b retain];
+}
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_autorelease_class_ptr
+A* test_autorelease_class_ptr(B *b) {
+  // CALLS: {{call.*@objc_autorelease}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b autorelease];
+}
+
 
 @interface C
 + (id)allocWithZone:(int)intArg;
+- (float) retain;
 @end
 
 // Make sure we only accept pointer types
@@ -78,3 +113,37 @@
   return [C allocWithZone:3];
 }
 
+// Make sure we use a message and not a call as the return type is
+// not a pointer type.
+// CHECK-LABEL: define {{.*}}void @test_cannot_message_return_float
+float test_cannot_message_return_float(C *c) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  return [c retain];
+}
+
+@interface NSString : NSObject
++ (void)retain_self;
+- (void)retain_super;
+@end
+
+@implementation NSString
+
+// Make sure we can convert a message to a dynamic receiver to a call
+// CHECK-LABEL: define {{.*}}void @retain_self
++ (void)retain_self {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_retain}}
+  [self retain];
+}
+
+// Make sure we never convert a message to super to a call
+// CHECK-LABEL: define {{.*}}void @retain_super
+- (void)retain_super {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [super retain];
+}
+
+@end
+
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -138,6 +138,10 @@
   /// id objc_autorelease(id);
   llvm::Constant *objc_autorelease;
 
+  /// id objc_autorelease(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_autoreleaseRuntimeFunction;
+
   /// id objc_autoreleaseReturnValue(id);
   llvm::Constant *objc_autoreleaseReturnValue;
 
@@ -162,6 +166,10 @@
   /// id objc_retain(id);
   llvm::Constant *objc_retain;
 
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_retainRuntimeFunction;
+
   /// id objc_retainAutorelease(id);
   llvm::Constant *objc_retainAutorelease;
 
@@ -177,6 +185,10 @@
   /// void objc_release(id);
   llvm::Constant *objc_release;
 
+  /// void objc_release(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_releaseRuntimeFunction;
+
   /// void objc_storeStrong(id*, id);
   llvm::Constant *objc_storeStrong;
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3798,6 +3798,11 @@
   llvm::Value *EmitARCRetainAutoreleasedReturnValue(llvm::Value *value);
   llvm::Value *EmitARCUnsafeClaimAutoreleasedReturnValue(llvm::Value *value);
 
+  

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2694
   while (true) {
 E = E->IgnoreParens();
 

Maybe `IgnoreParens` should be doing this itself?


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

https://reviews.llvm.org/D55853



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


[PATCH] D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

2018-12-18 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: ebevhan, bjope, rjmccall.
leonardchan added a project: clang.
leonardchan added a parent revision: D53738: [Fixed Point Arithmetic] Fixed 
Point Addition.

This patch includes logic for constant expression evaluation of fixed point 
additions.


Repository:
  rC Clang

https://reviews.llvm.org/D55868

Files:
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/test/Frontend/fixed_point_add.c

Index: clang/test/Frontend/fixed_point_add.c
===
--- clang/test/Frontend/fixed_point_add.c
+++ clang/test/Frontend/fixed_point_add.c
@@ -1,6 +1,49 @@
 // RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
 // RUN: %clang_cc1 -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
 
+// Addition between different fixed point types
+short _Accum sa_const = 1.0hk + 2.0hk;  // CHECK-DAG: @sa_const  = {{.*}}global i16 384, align 2
+_Accum a_const = 1.0hk + 2.0k;  // CHECK-DAG: @a_const   = {{.*}}global i32 98304, align 4
+long _Accum la_const = 1.0hk + 2.0lk;   // CHECK-DAG: @la_const  = {{.*}}global i64 6442450944, align 8
+short _Accum sa_const2 = 0.5hr + 2.0hk; // CHECK-DAG: @sa_const2  = {{.*}}global i16 320, align 2
+short _Accum sa_const3 = 0.5r + 2.0hk;  // CHECK-DAG: @sa_const3  = {{.*}}global i16 320, align 2
+short _Accum sa_const4 = 0.5lr + 2.0hk; // CHECK-DAG: @sa_const4  = {{.*}}global i16 320, align 2
+
+// Unsigned addition
+unsigned short _Accum usa_const = 1.0uhk + 2.0uhk;
+// CHECK-SIGNED-DAG:   @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned + signed
+short _Accum sa_const5 = 1.0uhk + 2.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 384, align 2
+
+// Addition with negative number
+short _Accum sa_const6 = 0.5hr + (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -192, align 2
+
+// Int addition
+unsigned short _Accum usa_const2 = 2 + 0.5uhk;
+// CHECK-SIGNED-DAG:   @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 2 + (-0.5hk);   // CHECK-DAG: @sa_const7 = {{.*}}global i16 192, align 2
+short _Accum sa_const8 = 257 + (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 32640, align 2
+long _Fract lf_const = -0.5lr + 1;   // CHECK-DAG: @lf_const  = {{.*}}global i32 1073741824, align 4
+
+// Saturated addition
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk + 128.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk + 128.0uhk;
+// CHECK-SIGNED-DAG:   @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk + 128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk + 128;
+// CHECK-SIGNED-DAG:   @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk + (-2);
+// CHECK-DAG:   @sat_usa_const3 = {{.*}}global i16 0, align 2
+
 void SignedAddition() {
   // CHECK-LABEL: SignedAddition
   short _Accum sa;
Index: clang/lib/Basic/FixedPoint.cpp
===
--- clang/lib/Basic/FixedPoint.cpp
+++ clang/lib/Basic/FixedPoint.cpp
@@ -39,7 +39,7 @@
 if (!(Masked == Mask || Masked == 0))
   NewVal = NewVal.isNegative() ? Mask : ~Mask;
 
-if (!DstSema.isSigned() && NewVal.isNegative())
+if (!DstSema.isSigned() && NewVal.isSigned() && NewVal.isNegative())
   NewVal = 0;
   }
 
@@ -137,4 +137,21 @@
  ResultIsSaturated, ResultHasUnsignedPadding);
 }
 
+APFixedPoint APFixedPoint::add(const APFixedPoint ) const {
+  auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+  APFixedPoint ConvertedThis = convert(CommonFXSema);
+  APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+  llvm::APSInt ThisVal = ConvertedThis.getValue();
+  llvm::APSInt OtherVal = ConvertedOther.getValue();
+
+  llvm::APInt Result;
+  if (CommonFXSema.isSaturated()) {
+Result = CommonFXSema.isSigned() ? ThisVal.sadd_sat(OtherVal)
+ : ThisVal.uadd_sat(OtherVal);
+  } else
+Result = ThisVal + OtherVal;
+
+  return APFixedPoint(Result, CommonFXSema);
+}
+
 }  // namespace clang
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ 

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2722-2725
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}

rnk wrote:
> majnemer wrote:
> > Just curious, why not go even further and use FullExpr rather than 
> > ConstantExpr?
> That would include MaterializeTemporaryExpr. Do you think we should consider 
> that a noop cast? You could make the argument that it is, but I was aiming to 
> match pre ConstantExpr behavior.
MaterializeTemporaryExpr is not a FullExpr. It would include ExprWithCleanups, 
though, and ExprWithCleanups is almost exclusively used by CodeGen, which 
probably shouldn't be calling this function.


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

https://reviews.llvm.org/D55853



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


[PATCH] D55823: [analyzer] Fix backward compatibility issue after D53280 'Emit an error for invalid -analyzer-config inputs'

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Ugh. No, it doesn't work. It seems that stuff after `-Xclang` is not included 
in `Args` (even at the beginning of the function, when no arguments are 
consumed), and it doesn't make it into `CmdArgs` (yet?).

Test passes because `-analyzer-config` is a sub-string of the //file name//. If 
you rename the test, it stops working (:

I'll be debugging this a bit more on my end, but help is welcome. And in any 
case, thanks a lot for looking into this!~ I'm even more surprised how hard it 
is to come up with a sensible solution to our weird problem.




Comment at: test/Analysis/invalid-analyzer-config-value.c:72
+// even if -analyze isn't specified.
+// RUN: %clang --analyze -Xclang -analyzer-config -Xclang remember=TheVasa %s
+

This run-line *does* have `-analyze` specified, because that's what `--analyze` 
expands to. I guess the test that we need would be along the lines of `clang 
-Xclang -analyzer-config ...` without `--analyze`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55823



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


[PATCH] D55862: [Sema] Don't try to account for the size of an incomplete type in CheckArrayAccess

2018-12-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:12381
+// It is possible that the base type is incomplete (see PR39746), even
+// though the effective type is complete. In this case we have no info
+// about the size of the base type and so skip the following adjustment.

This comment is really confusing; the meaning of "base type" and "effective 
type" in this context isn't clear.  Also, "effective type" is a term defined in 
the C standard to mean something else.  Better to explicitly say you're talking 
about the type of the base expression before/after the call to IgnoreParenCasts.



Comment at: lib/Sema/SemaChecking.cpp:12383
+// about the size of the base type and so skip the following adjustment.
+if ((BaseType != EffectiveType) && !BaseType->isIncompleteType(nullptr)) {
   // Make sure we're comparing apples to apples when comparing index to 
size

You don't need to explicitly pass nullptr here; there's a default argument.



Comment at: test/SemaCXX/array-bounds.cpp:294
+  C () { return reinterpret_cast(xxx)[1]; } // no-warning
+  C () { return reinterpret_cast(xxx)[2]; } // expected-warning {{array 
index 2 is past the end of the array (which contains 2 elements)}}
+}

Not sure it actually makes sense to print a diagnostic here... at least, not 
this diagnostic.  We don't know whether the result here is actually out of 
bounds.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55862



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


[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

2018-12-18 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, ahatanak, jordan_rose, aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.

This attribute, called "objc_externally_retained", exposes clang's notion of 
pseudo-`__strong` variables in ARC. Pseudo-strong variables "borrow" their 
initializer, meaning that they don't retain/release it, instead assuming that 
someone else is keeping their value alive. If a parameter is annotated with 
this attribute, it isn't implicitly retained/released in the function body, and 
is implicitly `const`. This is useful to expose for performance reasons, most 
functions don't need the extra safety of the retain/release, so programmers can 
opt-out as needed.

Thanks for taking a look!

rdar://20529015


Repository:
  rC Clang

https://reviews.llvm.org/D55865

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/externally-retained.m

Index: clang/test/SemaObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/SemaObjC/externally-retained.m
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc %s -verify
+
+#define EXT_RET __attribute__((objc_externally_retained))
+
+@interface ObjCTy
+@end
+
+void test1() {
+  EXT_RET int a; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak ObjCTy *b; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak int (^c)(); // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+  EXT_RET int (^d)();
+  EXT_RET ObjCTy *e;
+  EXT_RET __strong ObjCTy *f;
+
+  e = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  f = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  d = ^{ return 0; }; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+void test2(ObjCTy *a);
+
+void test2(EXT_RET ObjCTy *a) {
+  a = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+EXT_RET ObjCTy *test3; // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+@interface X // expected-warning{{defined without specifying a base class}} expected-note{{add a super class}}
+-(void)m: (ObjCTy *) p;
+@end
+@implementation X
+-(void)m: (ObjCTy *) EXT_RET p {
+  p = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -94,6 +94,7 @@
 // CHECK-NEXT: ObjCBridgeRelated (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
+// CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
Index: clang/test/CodeGenObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/externally-retained.m
@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-arc -fblocks -O0 %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-arc -fblocks -O0 -xobjective-c++ -std=c++11 %s -S -emit-llvm -o - | FileCheck %s --check-prefix CHECKXX
+
+#define EXT_RET __attribute__((objc_externally_retained))
+
+@interface ObjTy @end
+
+ObjTy *global;
+
+#if __cplusplus
+// Suppress name mangling in C++ mode for the sake of check lines.
+extern "C" void param(ObjTy *p);
+extern "C" void local();
+extern "C" void in_init();
+extern "C" void anchor();
+extern "C" void block_capture();
+extern "C" void esc(void (^)());
+extern "C" void escp(void (^)(ObjTy *));
+extern "C" void block_param();
+#endif
+
+void param(EXT_RET ObjTy *p) {
+  // CHECK-LABEL: define void @param
+  // CHECK-NOT: objc_
+  // CHECK ret
+}
+
+void local() {
+  

[PATCH] D55862: [Sema] Don't try to account for the size of an incomplete type in CheckArrayAccess

2018-12-18 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno created this revision.
riccibruno added reviewers: bkramer, efriedma.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

It is possible that the base type in CheckArrayAccess is incomplete even
though the effective type is complete. In this case don't try to account
for the size of the base type. This fixes PR39746.


Repository:
  rC Clang

https://reviews.llvm.org/D55862

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/array-bounds.cpp


Index: test/SemaCXX/array-bounds.cpp
===
--- test/SemaCXX/array-bounds.cpp
+++ test/SemaCXX/array-bounds.cpp
@@ -284,3 +284,12 @@
 int test_struct_multiarray() {
   return multi2[4].arr[0]; // expected-warning {{array index 4 is past the end 
of the array (which contains 4 elements)}}
 }
+
+namespace PR39746 {
+  struct S;
+  extern S xxx[2]; // expected-note {{array 'xxx' declared here}}
+  class C {};
+
+  C () { return reinterpret_cast(xxx)[1]; } // no-warning
+  C () { return reinterpret_cast(xxx)[2]; } // expected-warning {{array 
index 2 is past the end of the array (which contains 2 elements)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -12377,7 +12377,10 @@
   return;
 
 const Type *BaseType = BaseExpr->getType()->getPointeeOrArrayElementType();
-if (BaseType != EffectiveType) {
+// It is possible that the base type is incomplete (see PR39746), even
+// though the effective type is complete. In this case we have no info
+// about the size of the base type and so skip the following adjustment.
+if ((BaseType != EffectiveType) && !BaseType->isIncompleteType(nullptr)) {
   // Make sure we're comparing apples to apples when comparing index to 
size
   uint64_t ptrarith_typesize = Context.getTypeSize(EffectiveType);
   uint64_t array_typesize = Context.getTypeSize(BaseType);


Index: test/SemaCXX/array-bounds.cpp
===
--- test/SemaCXX/array-bounds.cpp
+++ test/SemaCXX/array-bounds.cpp
@@ -284,3 +284,12 @@
 int test_struct_multiarray() {
   return multi2[4].arr[0]; // expected-warning {{array index 4 is past the end of the array (which contains 4 elements)}}
 }
+
+namespace PR39746 {
+  struct S;
+  extern S xxx[2]; // expected-note {{array 'xxx' declared here}}
+  class C {};
+
+  C () { return reinterpret_cast(xxx)[1]; } // no-warning
+  C () { return reinterpret_cast(xxx)[2]; } // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -12377,7 +12377,10 @@
   return;
 
 const Type *BaseType = BaseExpr->getType()->getPointeeOrArrayElementType();
-if (BaseType != EffectiveType) {
+// It is possible that the base type is incomplete (see PR39746), even
+// though the effective type is complete. In this case we have no info
+// about the size of the base type and so skip the following adjustment.
+if ((BaseType != EffectiveType) && !BaseType->isIncompleteType(nullptr)) {
   // Make sure we're comparing apples to apples when comparing index to size
   uint64_t ptrarith_typesize = Context.getTypeSize(EffectiveType);
   uint64_t array_typesize = Context.getTypeSize(BaseType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55861: [OpenMP] Fix data sharing analysis in nested clause

2018-12-18 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny created this revision.
jdenny added a reviewer: ABataev.
Herald added a subscriber: guansong.

Without this patch, clang doesn't complain that X needs explicit data
sharing attributes in the following:

  #pragma omp target teams default(none)
  {
#pragma omp parallel num_threads(X)
  ;
  }

However, clang does produce that complaint after the braces are 
removed.  With this patch, clang complains in both cases.


Repository:
  rC Clang

https://reviews.llvm.org/D55861

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_teams_messages.cpp


Index: clang/test/OpenMP/target_teams_messages.cpp
===
--- clang/test/OpenMP/target_teams_messages.cpp
+++ clang/test/OpenMP/target_teams_messages.cpp
@@ -50,6 +50,16 @@
 #pragma omp target teams default(none)
   ++argc; // expected-error {{variable 'argc' must have explicitly specified 
data sharing attributes}}
 
+#pragma omp target teams default(none)
+#pragma omp parallel num_threads(argc) // expected-error {{variable 'argc' 
must have explicitly specified data sharing attributes}}
+  ;
+
+#pragma omp target teams default(none)
+  {
+#pragma omp parallel num_threads(argc) // expected-error {{variable 'argc' 
must have explicitly specified data sharing attributes}}
+;
+  }
+
   goto L2; // expected-error {{use of undeclared label 'L2'}}
 #pragma omp target teams
   L2:
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2390,13 +2390,9 @@
   void VisitStmt(Stmt *S) {
 for (Stmt *C : S->children()) {
   if (C) {
-if (auto *OED = dyn_cast(C)) {
-  // Check implicitly captured variables in the task-based directives 
to
-  // check if they must be firstprivatized.
-  VisitSubCaptures(OED);
-} else {
-  Visit(C);
-}
+// Check implicitly captured variables in the task-based directives to
+// check if they must be firstprivatized.
+Visit(C);
   }
 }
   }


Index: clang/test/OpenMP/target_teams_messages.cpp
===
--- clang/test/OpenMP/target_teams_messages.cpp
+++ clang/test/OpenMP/target_teams_messages.cpp
@@ -50,6 +50,16 @@
 #pragma omp target teams default(none)
   ++argc; // expected-error {{variable 'argc' must have explicitly specified data sharing attributes}}
 
+#pragma omp target teams default(none)
+#pragma omp parallel num_threads(argc) // expected-error {{variable 'argc' must have explicitly specified data sharing attributes}}
+  ;
+
+#pragma omp target teams default(none)
+  {
+#pragma omp parallel num_threads(argc) // expected-error {{variable 'argc' must have explicitly specified data sharing attributes}}
+;
+  }
+
   goto L2; // expected-error {{use of undeclared label 'L2'}}
 #pragma omp target teams
   L2:
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2390,13 +2390,9 @@
   void VisitStmt(Stmt *S) {
 for (Stmt *C : S->children()) {
   if (C) {
-if (auto *OED = dyn_cast(C)) {
-  // Check implicitly captured variables in the task-based directives to
-  // check if they must be firstprivatized.
-  VisitSubCaptures(OED);
-} else {
-  Visit(C);
-}
+// Check implicitly captured variables in the task-based directives to
+// check if they must be firstprivatized.
+Visit(C);
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55856: [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Dan Albert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349570: [Driver] Also obey -nostdlib++ when rewriting 
-lstdc++. (authored by danalbert, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55856

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/nostdlibxx.cpp


Index: cfe/trunk/test/Driver/nostdlibxx.cpp
===
--- cfe/trunk/test/Driver/nostdlibxx.cpp
+++ cfe/trunk/test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++ %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, 
Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }


Index: cfe/trunk/test/Driver/nostdlibxx.cpp
===
--- cfe/trunk/test/Driver/nostdlibxx.cpp
+++ cfe/trunk/test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++ %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r349570 - [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Dan Albert via cfe-commits
Author: danalbert
Date: Tue Dec 18 15:29:35 2018
New Revision: 349570

URL: http://llvm.org/viewvc/llvm-project?rev=349570=rev
Log:
[Driver] Also obey -nostdlib++ when rewriting -lstdc++.

Reviewers: pirama

Reviewed By: pirama

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/nostdlibxx.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=349570=349569=349570=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Dec 18 15:29:35 2018
@@ -303,6 +303,7 @@ DerivedArgList *Driver::TranslateInputAr
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@ DerivedArgList *Driver::TranslateInputAr
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, 
Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }

Modified: cfe/trunk/test/Driver/nostdlibxx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/nostdlibxx.cpp?rev=349570=349569=349570=diff
==
--- cfe/trunk/test/Driver/nostdlibxx.cpp (original)
+++ cfe/trunk/test/Driver/nostdlibxx.cpp Tue Dec 18 15:29:35 2018
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++ %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++


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


[PATCH] D55856: [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Dan Albert via Phabricator via cfe-commits
danalbert updated this revision to Diff 178805.

Repository:
  rC Clang

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

https://reviews.llvm.org/D55856

Files:
  lib/Driver/Driver.cpp
  test/Driver/nostdlibxx.cpp


Index: test/Driver/nostdlibxx.cpp
===
--- test/Driver/nostdlibxx.cpp
+++ test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++ %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, 
Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }


Index: test/Driver/nostdlibxx.cpp
===
--- test/Driver/nostdlibxx.cpp
+++ test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++ %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame.

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 178802.
NoQ added a comment.

Wait a minute.

In D55804#1334957 , @NoQ wrote:

> we would also need to introduce a separate liveness hack to keep it alive


No, we wouldn't! With a symbolic region it is fine if it dies too early, 
because putting anything into a symbolic region is an immediate escape anyways. 
It is still scary to keep it dying too early, but i don't see any actual 
problem arise from it, and in any case it's better than before, and the leaks 
do indeed go away.

Additionally, this is still a fairly safe change because there's nothing new in 
constructing into symbolic regions.


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

https://reviews.llvm.org/D55804

Files:
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/temporaries.cpp

Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1,9 +1,25 @@
-// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-config eagerly-assume=false %s -std=c++11
-// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-config eagerly-assume=false %s -std=c++17
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\
+// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\
+// RUN: -analyzer-config eagerly-assume=false -verify %s\
+// RUN: -std=c++03 -analyzer-config cfg-temporary-dtors=false
+
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\
+// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\
+// RUN: -analyzer-config eagerly-assume=false -verify %s\
+// RUN: -std=c++11 -analyzer-config cfg-temporary-dtors=false
+
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\
+// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\
+// RUN: -analyzer-config eagerly-assume=false -verify %s\
+// RUN: -std=c++11 -analyzer-config cfg-temporary-dtors=true\
+// RUN: -DTEMPORARY_DTORS
+
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\
+// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\
+// RUN: -analyzer-config eagerly-assume=false -verify %s\
+// RUN: -std=c++17 -analyzer-config cfg-temporary-dtors=true\
+// RUN: -DTEMPORARY_DTORS
 
-// Note: The C++17 run-line doesn't -verify yet - it is a no-crash test.
 
 extern bool clang_analyzer_eval(bool);
 extern bool clang_analyzer_warnIfReached();
@@ -450,7 +466,16 @@
   }
 
 #if __cplusplus >= 201103L
-  CtorWithNoReturnDtor returnNoReturnDtor() {
+  struct CtorWithNoReturnDtor2 {
+CtorWithNoReturnDtor2() = default;
+
+CtorWithNoReturnDtor2(int x) {
+  clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+}
+
+~CtorWithNoReturnDtor2() __attribute__((noreturn));
+  };
+  CtorWithNoReturnDtor2 returnNoReturnDtor() {
 return {1}; // no-crash
   }
 #endif
@@ -805,7 +830,12 @@
   // On each branch the variable is constructed directly.
   if (coin) {
 clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+#if __cplusplus < 201703L
 clang_analyzer_eval(y == 1); // expected-warning{{TRUE}}
+#else
+// FIXME: Destructor called twice in C++17?
+clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+#endif
 clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
 clang_analyzer_eval(w == 0); // expected-warning{{TRUE}}
 
@@ -813,7 +843,12 @@
 clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
 clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
 clang_analyzer_eval(z == 1); // expected-warning{{TRUE}}
+#if __cplusplus < 201703L
 clang_analyzer_eval(w == 1); // expected-warning{{TRUE}}
+#else
+// FIXME: Destructor called twice in C++17?
+clang_analyzer_eval(w == 2); // expected-warning{{TRUE}}
+#endif
   }
 }
 } // namespace test_match_constructors_and_destructors
@@ -865,9 +900,12 @@
 public:
   ~C() {
 glob = 1;
+// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS
-// expected-warning@-2{{TRUE}}
+#if __cplusplus < 201703L
+// expected-warning@-3{{TRUE}}
+#endif
 #endif
   

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

2018-12-18 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Sorry, please ignore my previous comment. I was looking at the wrong place.

The following code reaches `Sema::BuildDecltypeType` without going through 
`ActOnDecltypeExpression`:

  template 
  void overloaded_fn(T);
  decltype(auto) v5 = _fn;

`Sema::BuildDecltypeType` is called from `Sema::DeduceAutoType`, so calling 
`CheckPlaceholderExpr ` there should fix the assert when the test case above is 
compiled.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55662



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


[PATCH] D55856: [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama added inline comments.



Comment at: test/Driver/nostdlibxx.cpp:14
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++%s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s

Missing space here?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55856



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


[PATCH] D55856: [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama accepted this revision.
pirama added inline comments.
This revision is now accepted and ready to land.



Comment at: test/Driver/nostdlibxx.cpp:14
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++%s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s

pirama wrote:
> Missing space here?
Between -lstdc++ and the %s.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55856



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


[PATCH] D55856: [Driver] Also obey -nostdlib++ when rewriting -lstdc++.

2018-12-18 Thread Dan Albert via Phabricator via cfe-commits
danalbert created this revision.
danalbert added a reviewer: pirama.

Repository:
  rC Clang

https://reviews.llvm.org/D55856

Files:
  lib/Driver/Driver.cpp
  test/Driver/nostdlibxx.cpp


Index: test/Driver/nostdlibxx.cpp
===
--- test/Driver/nostdlibxx.cpp
+++ test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++%s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, 
Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }


Index: test/Driver/nostdlibxx.cpp
===
--- test/Driver/nostdlibxx.cpp
+++ test/Driver/nostdlibxx.cpp
@@ -6,3 +6,12 @@
 // CHECK-NOT: -lstdc++
 // CHECK-NOT: -lc++
 // CHECK: -lm
+
+// Make sure -lstdc++ isn't rewritten to the default stdlib when -nostdlib++ is
+// used.
+//
+// RUN: %clangxx -target i686-pc-linux-gnu -### \
+// RUN: -nostdlib++ -stdlib=libc++ -lstdc++%s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-RESERVED-LIB-REWRITE < %t %s
+// CHECK-RESERVED-LIB-REWRITE: -lstdc++
+// CHECK-RESERVED-LIB-REWRITE-NOT: -lc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -303,6 +303,7 @@
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
+  bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
   for (Arg *A : Args) {
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
@@ -347,7 +348,8 @@
   StringRef Value = A->getValue();
 
   // Rewrite unless -nostdlib is present.
-  if (!HasNostdlib && !HasNodefaultlib && Value == "stdc++") {
+  if (!HasNostdlib && !HasNodefaultlib && !HasNostdlibxx &&
+  Value == "stdc++") {
 DAL->AddFlagArg(A, Opts->getOption(options::OPT_Z_reserved_lib_stdcxx));
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done.
rnk added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2722-2725
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}

majnemer wrote:
> Just curious, why not go even further and use FullExpr rather than 
> ConstantExpr?
That would include MaterializeTemporaryExpr. Do you think we should consider 
that a noop cast? You could make the argument that it is, but I was aiming to 
match pre ConstantExpr behavior.


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

https://reviews.llvm.org/D55853



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


[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 178793.
void added a comment.

Fix how prefixes are used in the testcase.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : 
-1));
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -685,7 +685,9 @@
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/Sema/SemaStmtAsm.cpp

[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Bill Wendling via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC349561: Emit ASM input in a constant context (authored by 
void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55616?vs=178793=178794#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -685,7 +685,9 @@
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':
Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : 
-1));
+}


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-

r349561 - Emit ASM input in a constant context

2018-12-18 Thread Bill Wendling via cfe-commits
Author: void
Date: Tue Dec 18 14:54:03 2018
New Revision: 349561

URL: http://llvm.org/viewvc/llvm-project?rev=349561=rev
Log:
Emit ASM input in a constant context

Summary:
Some ASM input constraints (e.g., "i" and "n") require immediate values. At O0,
very few code transformations are performed. So if we cannot resolve to an
immediate when emitting the ASM input we shouldn't delay its processing.

Reviewers: rsmith, efriedma

Reviewed By: efriedma

Subscribers: rehana, efriedma, craig.topper, jyknight, cfe-commits

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

Modified:
cfe/trunk/lib/Basic/TargetInfo.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/CodeGen/builtin-constant-p.c

Modified: cfe/trunk/lib/Basic/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=349561=349560=349561=diff
==
--- cfe/trunk/lib/Basic/TargetInfo.cpp (original)
+++ cfe/trunk/lib/Basic/TargetInfo.cpp Tue Dec 18 14:54:03 2018
@@ -685,7 +685,9 @@ bool TargetInfo::validateInputConstraint
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=349561=349560=349561=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Tue Dec 18 14:54:03 2018
@@ -1820,11 +1820,14 @@ llvm::Value* CodeGenFunction::EmitAsmInp
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=349561=349560=349561=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Tue Dec 18 14:54:03 2018
@@ -378,17 +378,17 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {

Modified: cfe/trunk/test/CodeGen/builtin-constant-p.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-constant-p.c?rev=349561=349560=349561=diff
==
--- cfe/trunk/test/CodeGen/builtin-constant-p.c (original)
+++ cfe/trunk/test/CodeGen/builtin-constant-p.c Tue Dec 18 14:54:03 2018
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@ struct { const 

[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Bill Wendling via Phabricator via cfe-commits
void marked an inline comment as done.
void added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

efriedma wrote:
> efriedma wrote:
> > void wrote:
> > > efriedma wrote:
> > > > Given this code doesn't check the optimization level anymore, do you 
> > > > still need separate check prefixes for `O2` and `O0`.  Or if that 
> > > > doesn't work for everything, maybe you could share a check prefix for 
> > > > some of the tests? (Maybe it would make sense to check IR generated 
> > > > using `-disable-llvm-passes`.)
> > > The bug only triggered at `O0`, so I still want to test it without 
> > > optimizations. Note that we do check optimization levels during code 
> > > generation to determine if we should generate an `is.constant` intrinsic.
> > You can use something like "-check-prefixes=CHECK,O0" to reduce the 
> > duplication.
> That doesn't pass.
> 
> More specifically, I was thinking something more along the lines of using 
> "--check-prefixes=CHECK,O2" for the first run, and 
> "--check-prefixes=CHECK,O0" for the second run; then you can use "CHECK:" for 
> the common lines and "O2:"/"O0:" for the lines that are different.
I think think that works. The thing is that there should rarely be common lines 
between `O2` and `O0`. I understand what you're going for here, but I don't 
think it's beneficial to this testcase. However, I don't need to change all of 
the `CHECK`s either. I came up with a compromise...


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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


[PATCH] D55719: [OpenMP] parsing and sema support for 'close' map-type-modifier

2018-12-18 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC349551: [OPENMP] parsing and sema support for 
close map-type-modifier (authored by kli, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55719?vs=178336=178785#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55719

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/target_ast_print.cpp
  test/OpenMP/target_data_ast_print.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/target_parallel_for_map_messages.cpp
  test/OpenMP/target_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_parallel_map_messages.cpp
  test/OpenMP/target_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_simd_map_messages.cpp
  test/OpenMP/target_teams_map_messages.cpp

Index: include/clang/Basic/OpenMPKinds.def
===
--- include/clang/Basic/OpenMPKinds.def
+++ include/clang/Basic/OpenMPKinds.def
@@ -120,6 +120,9 @@
 #ifndef OPENMP_MAP_KIND
 #define OPENMP_MAP_KIND(Name)
 #endif
+#ifndef OPENMP_MAP_MODIFIER_KIND
+#define OPENMP_MAP_MODIFIER_KIND(Name)
+#endif
 #ifndef OPENMP_DIST_SCHEDULE_KIND
 #define OPENMP_DIST_SCHEDULE_KIND(Name)
 #endif
@@ -559,14 +562,17 @@
 OPENMP_ORDERED_CLAUSE(simd)
 OPENMP_ORDERED_CLAUSE(depend)
 
-// Map types and map type modifier for 'map' clause.
+// Map types for 'map' clause.
 OPENMP_MAP_KIND(alloc)
 OPENMP_MAP_KIND(to)
 OPENMP_MAP_KIND(from)
 OPENMP_MAP_KIND(tofrom)
 OPENMP_MAP_KIND(delete)
 OPENMP_MAP_KIND(release)
-OPENMP_MAP_KIND(always)
+
+// Map-type-modifiers for 'map' clause.
+OPENMP_MAP_MODIFIER_KIND(always)
+OPENMP_MAP_MODIFIER_KIND(close)
 
 // Clauses allowed for OpenMP directive 'taskloop'.
 OPENMP_TASKLOOP_CLAUSE(if)
@@ -919,6 +925,7 @@
 #undef OPENMP_FOR_CLAUSE
 #undef OPENMP_FOR_SIMD_CLAUSE
 #undef OPENMP_MAP_KIND
+#undef OPENMP_MAP_MODIFIER_KIND
 #undef OPENMP_DISTRIBUTE_CLAUSE
 #undef OPENMP_DIST_SCHEDULE_KIND
 #undef OPENMP_DEFAULTMAP_KIND
Index: include/clang/Basic/DiagnosticParseKinds.td
===
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -1157,9 +1157,11 @@
 def err_omp_unknown_map_type : Error<
   "incorrect map type, expected one of 'to', 'from', 'tofrom', 'alloc', 'release', or 'delete'">;
 def err_omp_unknown_map_type_modifier : Error<
-  "incorrect map type modifier, expected 'always'">;
+  "incorrect map type modifier, expected 'always' or 'close'">;
 def err_omp_map_type_missing : Error<
   "missing map type">;
+def err_omp_map_type_modifier_missing : Error<
+  "missing map type modifier">;
 def err_omp_declare_simd_inbranch_notinbranch : Error<
   "unexpected '%0' clause, '%1' is specified already">;
 def err_expected_end_declare_target : Error<
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8725,6 +8725,8 @@
   "expected access to data field">;
 def err_omp_multiple_array_items_in_map_clause : Error<
   "multiple array elements associated with the same variable are not allowed in map clauses of the same construct">;
+def err_omp_duplicate_map_type_modifier : Error<
+  "same map type modifier has been specified more than once">;
 def err_omp_pointer_mapped_along_with_derived_section : Error<
   "pointer cannot be mapped along with a section derived from itself">;
 def err_omp_original_storage_is_shared_and_does_not_contain : Error<
Index: include/clang/Basic/OpenMPKinds.h
===
--- include/clang/Basic/OpenMPKinds.h
+++ include/clang/Basic/OpenMPKinds.h
@@ -96,6 +96,15 @@
   OMPC_MAP_unknown
 };
 
+/// OpenMP modifier kind for 'map' clause.
+enum OpenMPMapModifierKind {
+  OMPC_MAP_MODIFIER_unknown = OMPC_MAP_unknown,
+#define OPENMP_MAP_MODIFIER_KIND(Name) \
+  OMPC_MAP_MODIFIER_##Name,
+#include "clang/Basic/OpenMPKinds.def"
+  OMPC_MAP_MODIFIER_last
+};
+
 /// OpenMP attributes for 'dist_schedule' clause.
 enum OpenMPDistScheduleClauseKind {
 #define OPENMP_DIST_SCHEDULE_KIND(Name) OMPC_DIST_SCHEDULE_##Name,
Index: 

r349551 - [OPENMP] parsing and sema support for 'close' map-type-modifier

2018-12-18 Thread Kelvin Li via cfe-commits
Author: kli
Date: Tue Dec 18 14:18:41 2018
New Revision: 349551

URL: http://llvm.org/viewvc/llvm-project?rev=349551=rev
Log:
[OPENMP] parsing and sema support for 'close' map-type-modifier

A map clause with the close map-type-modifier is a hint to 
prefer that the variables are mapped using a copy into faster 
memory.

Patch by Ahsan Saghir (saghir)

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

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Basic/OpenMPKinds.h
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/target_ast_print.cpp
cfe/trunk/test/OpenMP/target_data_ast_print.cpp
cfe/trunk/test/OpenMP/target_map_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_map_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_map_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_map_messages.cpp
cfe/trunk/test/OpenMP/target_simd_map_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_map_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_map_messages.cpp
cfe/trunk/test/OpenMP/target_teams_map_messages.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=349551=349550=349551=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Tue Dec 18 14:18:41 2018
@@ -4061,8 +4061,19 @@ class OMPMapClause final : public OMPMap
 return getUniqueDeclarationsNum() + getTotalComponentListNum();
   }
 
-  /// Map type modifier for the 'map' clause.
-  OpenMPMapClauseKind MapTypeModifier = OMPC_MAP_unknown;
+public:
+  /// Number of allowed map-type-modifiers.
+  static constexpr unsigned NumberOfModifiers =
+  OMPC_MAP_MODIFIER_last - OMPC_MAP_MODIFIER_unknown - 1;
+
+private:
+  /// Map-type-modifiers for the 'map' clause.
+  OpenMPMapModifierKind MapTypeModifiers[NumberOfModifiers] = {
+OMPC_MAP_MODIFIER_unknown, OMPC_MAP_MODIFIER_unknown
+  };
+  
+  /// Location of map-type-modifiers for the 'map' clause.
+  SourceLocation MapTypeModifiersLoc[NumberOfModifiers];
 
   /// Map type for the 'map' clause.
   OpenMPMapClauseKind MapType = OMPC_MAP_unknown;
@@ -4080,7 +4091,8 @@ class OMPMapClause final : public OMPMap
   /// NumUniqueDeclarations declarations, \a NumComponentLists total component
   /// lists, and \a NumComponents total expression components.
   ///
-  /// \param MapTypeModifier Map type modifier.
+  /// \param MapModifiers Map-type-modifiers.
+  /// \param MapModifiersLoc Locations of map-type-modifiers.
   /// \param MapType Map type.
   /// \param MapTypeIsImplicit Map type is inferred implicitly.
   /// \param MapLoc Location of the map type.
@@ -4091,7 +4103,8 @@ class OMPMapClause final : public OMPMap
   /// clause.
   /// \param NumComponentLists Number of component lists in this clause.
   /// \param NumComponents Total number of expression components in the clause.
-  explicit OMPMapClause(OpenMPMapClauseKind MapTypeModifier,
+  explicit OMPMapClause(ArrayRef MapModifiers,
+ArrayRef MapModifiersLoc,
 OpenMPMapClauseKind MapType, bool MapTypeIsImplicit,
 SourceLocation MapLoc, SourceLocation StartLoc,
 SourceLocation LParenLoc, SourceLocation EndLoc,
@@ -4100,8 +4113,17 @@ class OMPMapClause final : public OMPMap
   : OMPMappableExprListClause(OMPC_map, StartLoc, LParenLoc, EndLoc,
   NumVars, NumUniqueDeclarations,
   NumComponentLists, NumComponents),
-MapTypeModifier(MapTypeModifier), MapType(MapType),
-MapTypeIsImplicit(MapTypeIsImplicit), MapLoc(MapLoc) {}
+MapType(MapType), MapTypeIsImplicit(MapTypeIsImplicit),
+MapLoc(MapLoc) {
+  assert(llvm::array_lengthof(MapTypeModifiers) == MapModifiers.size()
+ && "Unexpected number of map type modifiers.");
+  llvm::copy(MapModifiers, std::begin(MapTypeModifiers));
+
+  assert(llvm::array_lengthof(MapTypeModifiersLoc) ==
+ MapModifiersLoc.size() &&

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread David Majnemer via Phabricator via cfe-commits
majnemer added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2722-2725
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}

Just curious, why not go even further and use FullExpr rather than ConstantExpr?


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

https://reviews.llvm.org/D55853



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


[PATCH] D55737: [Utils] Attempt to fix clang.natvis following "[AST] Various optimizations in DeclarationName(Table)"

2018-12-18 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno abandoned this revision.
riccibruno added a comment.

In D55737#1335245 , @aaron.ballman 
wrote:

> There was a bit more work to be done here, but this got me started. I commit 
> extra fixes on top of your work in r349547 since I was able to test the 
> behavior locally. Thank you for the help!


Nice ! I see that you fixed the `FunctionProtoType` case too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55737



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


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2018-12-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D55793#1335249 , @m4tx wrote:

> In D55793#1333661 , @lebedev.ri 
> wrote:
>
> > Please add tests with preprocessor (`#if ...`) that will show that it 
> > ignores disabled code. e.g.:
> >
> >   class ProbablyValid {
> >   private:
> > int a;
> >   #if defined(ZZ)
> >   public:
> > int b;
> >   #endif
> >   private:
> > int c;
> >   protected:
> > int d;
> >   public:
> > int e;
> >   };
> >
>
>
> Is this actually possible?
>  It seems that macros are ran through the preprocessor before one can fiddle 
> with them in clang-tidy.
>  In other words, `int b` is not at all present in the AST.


.. and by "ignores" i meant that it **will** be diagnosing this code, since it 
did not know anything about the code within the preprocessor-disabled section.

> However, I added a code to detect macro expansions, so duplicated access 
> specifiers are ignored if at least one of them comes from a macro. If there 
> is a way to cover your case as well, please let me know, because even after 
> looking at the code of other checks I haven't found out a solution for this.


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

https://reviews.llvm.org/D55793



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


[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: void, rsmith.

It is ignored by IgnoreParenImpCasts, so it seems reasonable to ignore
it here as well. Besides, this preserves behavior from before this AST
node was added.

Fixes PR39881


https://reviews.llvm.org/D55853

Files:
  clang/lib/AST/Expr.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp


Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - 
-fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - 
-fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck 
-check-prefix X64 %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - 
-fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - 
-fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck 
-check-prefix X64 %s
 
 template
 class Class {
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2719,6 +2719,11 @@
   continue;
 }
 
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}
+
 return E;
   }
 }


Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
 
 template
 class Class {
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2719,6 +2719,11 @@
   continue;
 }
 
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}
+
 return E;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2018-12-18 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx added a comment.

In D55793#1333661 , @lebedev.ri wrote:

> Please add tests with preprocessor (`#if ...`) that will show that it ignores 
> disabled code. e.g.:
>
>   class ProbablyValid {
>   private:
> int a;
>   #if defined(ZZ)
>   public:
> int b;
>   #endif
>   private:
> int c;
>   protected:
> int d;
>   public:
> int e;
>   };
>


Is this actually possible? It seems that macros are ran through the 
preprocessor before one can fiddle with them in clang-tidy. In other words, 
`int b` is not at all present in the AST. However, I added a code to detect 
macro expansions, so duplicated access specifiers are ignored if at least one 
of them comes from a macro. If there is a way to cover your case as well, 
please let me know, because even after looking at the code of other checks I 
haven't found out a solution for this.


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

https://reviews.llvm.org/D55793



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


[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D55662#1335146 , @ahatanak wrote:

> Here are a couple of examples I found running the regression tests:
>
>   int f0(int);
>   float f0(float);
>   decltype(f0) f0_a; // this is not valid.
>
>
>
>
>   template 
>   int var_expr(Ts... ts);
>  
>   template 
>   auto a_function(Ts... ts) -> decltype(var_expr(ts...));
>  
>   template 
>   using partial = decltype(a_function);
>  
>   int use_partial() { partial n; }
>
>
>
>
>   template void oneT() { }
>   int main()
>   {
> decltype(oneT)* fun = 0;
>   }
>
>
> If the call to `CheckPlaceholderExpr` in `Sema::BuildDecltypeType` is moved 
> to `TreeTransform::TransformDecltypeType` and 
> `Parser::ParseDecltypeSpecifier`, we can assert at the beginning of 
> `Sema::BuildDecltypeType`.


Okay, I'm not really understanding what path goes through those two places but 
not through `ActOnDecltypeExpression`, since both of them seem to 
unconditionally call the latter (at least, in the expression-parsing path).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55662



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


[PATCH] D55737: [Utils] Attempt to fix clang.natvis following "[AST] Various optimizations in DeclarationName(Table)"

2018-12-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

There was a bit more work to be done here, but this got me started. I commit 
extra fixes on top of your work in r349547 since I was able to test the 
behavior locally. Thank you for the help!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55737



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


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2018-12-18 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx marked an inline comment as done.
m4tx added a comment.

In D55793#1333723 , @riccibruno wrote:

> In D55793#1333691 , @m4tx wrote:
>
> > Don't use `CXXRecordDecl::accessSpecs()`, use unique comments in tests.
>
>
> Thanks! `CXXRecordDecl` is already huge and so adding iterators for a single 
> checker is in my opinion not a good idea (especially if the alternative is 
> actually less code).
>  Would it make sense to also issue a diagnostic where the first access 
> specifier is redundant (ie `public` in a `struct`, and `private` in a 
> `class`) ?


Yes, I was thinking about the same thing! However, I believe that some people 
may find this kind of "redundant" access specs actually more readable, so maybe 
it makes sense to add a check option for this to allow users to disable it?


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

https://reviews.llvm.org/D55793



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


r349547 - Fix errors with the Clang natvis file.

2018-12-18 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Tue Dec 18 13:42:20 2018
New Revision: 349547

URL: http://llvm.org/viewvc/llvm-project?rev=349547=rev
Log:
Fix errors with the Clang natvis file.

This updates the FunctionProtoType visualizer to use the proper bits for 
determining parameter information and the DeclarationName visualizer to use the 
detail namespace. It also adds support for viewing newer special declaration 
names (like deduction guides).

Patch with help of Bruno Ricci.

Modified:
cfe/trunk/utils/ClangVisualizers/clang.natvis

Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=349547=349546=349547=diff
==
--- cfe/trunk/utils/ClangVisualizers/clang.natvis (original)
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis Tue Dec 18 13:42:20 2018
@@ -171,17 +171,17 @@ For later versions of Visual Studio, no
but the expansion has all parameters -->
   
 {ResultType,view(cpp)}
-
+
 {*(clang::QualType 
*)(this+1),view(cpp)}{*this,view(parm1)}
-
+
 , {*((clang::QualType 
*)(this+1)+1),view(cpp)}{*this,view(parm2)}
-
+
 , {*((clang::QualType 
*)(this+1)+2),view(cpp)}{*this,view(parm3)}
-
+
 , {*((clang::QualType 
*)(this+1)+3),view(cpp)}{*this,view(parm4)}
-
+
 , {*((clang::QualType 
*)(this+1)+4),view(cpp)}{*this,view(parm5)}
-
+
 , /* expand for more params 
*/
 {*this,view(retType)}({*this,view(parm0)})
 
@@ -190,7 +190,7 @@ For later versions of Visual Studio, no
 {*this,view(parm0)}
 
   
-NumParams
+FunctionTypeBits.NumParams
 (clang::QualType *)(this+1)
   
 
@@ -395,22 +395,29 @@ For later versions of Visual Studio, no
 {{Identifier ({*(clang::IdentifierInfo *)(Ptr  
~PtrMask)})}}
 {{ObjC Zero Arg Selector (*{(clang::IdentifierInfo 
*)(Ptr  ~PtrMask)})}}
 {{ObjC One Arg Selector (*{(clang::IdentifierInfo 
*)(Ptr  ~PtrMask)})}}
+C++ Constructor 
{{*(clang::detail::CXXSpecialNameExtra *)(Ptr  ~PtrMask)}}
+C++ Destructor {{*(clang::detail::CXXSpecialNameExtra 
*)(Ptr  ~PtrMask)}}
+C++ Conversion function 
{{*(clang::detail::CXXSpecialNameExtra *)(Ptr  ~PtrMask)}}
+C++ Operator {{*(clang::detail::CXXOperatorIdName *)(Ptr 
 ~PtrMask)}}
 {*(clang::DeclarationNameExtra *)(Ptr 
 ~PtrMask),view(cpp)}
-{{Extra ({*(clang::DeclarationNameExtra *)(Ptr 
 ~PtrMask)})}}
+   IncludeView="cpp">{*(clang::detail::DeclarationNameExtra 
*)(Ptr  ~PtrMask),view(cpp)}
+{{Extra ({*(clang::detail::DeclarationNameExtra 
*)(Ptr  ~PtrMask)})}}
 
   *(clang::IdentifierInfo *)(Ptr  ~PtrMask)
   *(clang::IdentifierInfo *)(Ptr  
~PtrMask)
   *(clang::IdentifierInfo *)(Ptr  
~PtrMask)
-  (clang::DeclarationNameExtra *)(Ptr  ~PtrMask)
+  *(clang::detail::CXXSpecialNameExtra *)(Ptr  
~PtrMask)
+  *(clang::detail::CXXSpecialNameExtra *)(Ptr  
~PtrMask)
+  *(clang::detail::CXXSpecialNameExtra *)(Ptr 
 ~PtrMask)
+  *(clang::detail::CXXOperatorIdName *)(Ptr  
~PtrMask)  
+  (clang::detail::DeclarationNameExtra *)(Ptr  
~PtrMask)
 
   
-  
-{((clang::CXXSpecialName 
*)this)-Type,view(cpp)}
-
{(clang::DeclarationNameExtra::ExtraKind)ExtraKindOrNumArgs,en}{"
  ",sb}{*this,view(cpp)}
+  
+C++ 
Deduction guide
+C++ Literal operator
+C++ 
Using directive  
+
{(clang::detail::DeclarationNameExtra::ExtraKind)ExtraKindOrNumArgs,en}{"
  ",sb}{*this,view(cpp)}
   
   
 {(clang::tok::TokenKind)Kind,en}
@@ -458,17 +465,17 @@ For later versions of Visual Studio, no
   
   
 {*(clang::FunctionProtoType 
*)((clang::ExtQualsTypeCommonBase *)(((uintptr_t)DeclType.Value.Value)  
~15))-BaseType,view(retType)}
-
+
 {*ParamInfo[0]}{*this,view(parm1)nd}
-
+
 , 
{*ParamInfo[1]}{*this,view(parm2)nd}
-
+
 , 
{*ParamInfo[2]}{*this,view(parm3)nd}
-
+
 , 
{*ParamInfo[3]}{*this,view(parm4)nd}
-
+
 , 
{*ParamInfo[4]}{*this,view(parm5)nd}
-
+
 , /* expand for more params 
*/
 {*this,view(retType)nd} 
{Name,view(cpp)nd}({*this,view(parm0)nd})
 
@@ -477,7 +484,7 @@ For later versions of Visual Studio, no
 {*this,view(parm0)nd}
 
   
-((clang::FunctionProtoType *)((clang::ExtQualsTypeCommonBase 
*)(((uintptr_t)DeclType.Value.Value)  
~15))-BaseType)-NumParams
+((clang::FunctionProtoType *)((clang::ExtQualsTypeCommonBase 
*)(((uintptr_t)DeclType.Value.Value)  
~15))-BaseType)-FunctionTypeBits.NumParams
 ParamInfo
   
 


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


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2018-12-18 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx marked 6 inline comments as done.
m4tx added inline comments.



Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:33
+  for (DeclContext::specific_decl_iterator
+   NS(MatchedDecl->decls_begin()),
+   NSEnd(MatchedDecl->decls_end());

aaron.ballman wrote:
> Why `NS` -- that seems like a strange naming choice.
Sorry, this small piece of code was copied from another place and I forgot to 
change the variable name. I switched to `AS`, hopefully that's more meaningful 
here.


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

https://reviews.llvm.org/D55793



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


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2018-12-18 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx updated this revision to Diff 178777.
m4tx added a comment.

Fix variable names, add macro expansion checking as well as macro and inner 
class tests.


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

https://reviews.llvm.org/D55793

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp
  clang-tidy/readability/DuplicatedAccessSpecifiersCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-duplicated-access-specifiers.rst
  test/clang-tidy/readability-duplicated-access-specifiers.cpp

Index: test/clang-tidy/readability-duplicated-access-specifiers.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-duplicated-access-specifiers.cpp
@@ -0,0 +1,98 @@
+// RUN: %check_clang_tidy %s readability-duplicated-access-specifiers %t
+
+class FooPublic {
+public:
+  int a;
+public: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-duplicated-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int b;
+private:
+  int c;
+};
+
+struct StructPublic {
+public:
+  int a;
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-duplicated-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int b;
+private:
+  int c;
+};
+
+union UnionPublic {
+public:
+  int a;
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-duplicated-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooProtected {
+protected:
+  int a;
+protected: // comment-3
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-duplicated-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-3{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooPrivate {
+private:
+  int a;
+private: // comment-4
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-duplicated-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-4{{$}}
+  int b;
+public:
+  int c;
+};
+
+class Valid {
+private:
+  int a;
+public:
+  int b;
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class ValidInnerClass {
+public:
+  int a;
+
+  class Inner {
+  public:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+private:
+  int a;
+MIXIN
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
Index: docs/clang-tidy/checks/readability-duplicated-access-specifiers.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-duplicated-access-specifiers.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - readability-duplicated-access-specifiers
+
+readability-duplicated-access-specifiers
+
+
+Finds classes, structs and unions containing duplicated member access specifiers
+that can be removed.
+
+Examples:
+
+.. code-block:: c++
+
+  class Foo {
+  public:
+int x;
+int y;
+  public:
+int z;
+  protected:
+int a;
+  public:
+int c;
+  }
+
+In the example above, the second ``public`` declaration can be removed without
+any changes of behavior.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -238,6 +238,7 @@
readability-container-size-empty
readability-delete-null-pointer
readability-deleted-default
+   readability-duplicated-access-specifiers
readability-else-after-return
readability-function-size
readability-identifier-naming
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -192,6 +192,12 @@
   Checks for functions with a ``const``-qualified return type and recommends
   removal of the ``const`` keyword.
 
+- New :doc:`readability-duplicated-access-specifiers
+  ` check.
+
+  Finds classes, structs, and unions that contain duplicated member
+  access specifiers.
+
 - New :doc:`readability-magic-numbers
   ` check.
 
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -16,6 +16,7 @@
 #include "ContainerSizeEmptyCheck.h"
 

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

Alex, would you mind submitting this since I don't yet have submit privileges? 
I'll request them shortly, but I'd rather this go one in the meantime.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55765



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


[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349542: [CodeGen] Handle mixed-width ops in mixed-sign 
mul-with-overflow lowering (authored by vedantk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55843?vs=178752=178774#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55843

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/builtins-overflow.c


Index: cfe/trunk/test/CodeGen/builtins-overflow.c
===
--- cfe/trunk/test/CodeGen/builtins-overflow.c
+++ cfe/trunk/test/CodeGen/builtins-overflow.c
@@ -339,6 +339,27 @@
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: @test_mixed_sign_mul_overflow_sext_signed_op
+// CHECK: [[SignedOp:%.*]] = sext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 [[SignedOp]], 0
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
+int test_mixed_sign_mul_overflow_zext_unsigned_op(long long x, unsigned y) {
+// CHECK: @test_mixed_sign_mul_overflow_zext_unsigned_op
+// CHECK: [[UnsignedOp:%.*]] = zext i32 %1 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 %0, 0
+// CHECK: @llvm.umul.with.overflow.i64({{.*}}, i64 [[UnsignedOp]])
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
 int test_mixed_sign_mull_overflow(int x, unsigned y) {
 // CHECK: @test_mixed_sign_mull_overflow
 // CHECK:   [[IsNeg:%.*]] = icmp slt i32 [[Op1:%.*]], 0
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -1235,7 +1235,7 @@
WidthAndSignedness Op2Info,
WidthAndSignedness ResultInfo) {
   return BuiltinID == Builtin::BI__builtin_mul_overflow &&
- Op1Info.Width == Op2Info.Width && Op1Info.Width >= ResultInfo.Width &&
+ std::max(Op1Info.Width, Op2Info.Width) >= ResultInfo.Width &&
  Op1Info.Signed != Op2Info.Signed;
 }
 
@@ -1256,11 +1256,20 @@
   const clang::Expr *UnsignedOp = Op1Info.Signed ? Op2 : Op1;
   llvm::Value *Signed = CGF.EmitScalarExpr(SignedOp);
   llvm::Value *Unsigned = CGF.EmitScalarExpr(UnsignedOp);
+  unsigned SignedOpWidth = Op1Info.Signed ? Op1Info.Width : Op2Info.Width;
+  unsigned UnsignedOpWidth = Op1Info.Signed ? Op2Info.Width : Op1Info.Width;
+
+  // One of the operands may be smaller than the other. If so, [s|z]ext it.
+  if (SignedOpWidth < UnsignedOpWidth)
+Signed = CGF.Builder.CreateSExt(Signed, Unsigned->getType(), "op.sext");
+  if (UnsignedOpWidth < SignedOpWidth)
+Unsigned = CGF.Builder.CreateZExt(Unsigned, Signed->getType(), "op.zext");
 
   llvm::Type *OpTy = Signed->getType();
   llvm::Value *Zero = llvm::Constant::getNullValue(OpTy);
   Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
   llvm::Type *ResTy = ResultPtr.getElementType();
+  unsigned OpWidth = std::max(Op1Info.Width, Op2Info.Width);
 
   // Take the absolute value of the signed operand.
   llvm::Value *IsNegative = CGF.Builder.CreateICmpSLT(Signed, Zero);
@@ -1278,8 +1287,8 @@
   if (ResultInfo.Signed) {
 // Signed overflow occurs if the result is greater than INT_MAX or lesser
 // than INT_MIN, i.e when |Result| > (INT_MAX + IsNegative).
-auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width)
-  .zextOrSelf(Op1Info.Width);
+auto IntMax =
+llvm::APInt::getSignedMaxValue(ResultInfo.Width).zextOrSelf(OpWidth);
 llvm::Value *MaxResult =
 CGF.Builder.CreateAdd(llvm::ConstantInt::get(OpTy, IntMax),
   CGF.Builder.CreateZExt(IsNegative, OpTy));
@@ -1297,9 +1306,9 @@
 llvm::Value *Underflow = CGF.Builder.CreateAnd(
 IsNegative, CGF.Builder.CreateIsNotNull(UnsignedResult));
 Overflow = CGF.Builder.CreateOr(UnsignedOverflow, Underflow);
-if (ResultInfo.Width < Op1Info.Width) {
+if (ResultInfo.Width < OpWidth) {
   auto IntMax =
-  llvm::APInt::getMaxValue(ResultInfo.Width).zext(Op1Info.Width);
+  llvm::APInt::getMaxValue(ResultInfo.Width).zext(OpWidth);
   llvm::Value *TruncOverflow = CGF.Builder.CreateICmpUGT(
   UnsignedResult, llvm::ConstantInt::get(OpTy, IntMax));
   Overflow = CGF.Builder.CreateOr(Overflow, TruncOverflow);


Index: cfe/trunk/test/CodeGen/builtins-overflow.c
===
--- cfe/trunk/test/CodeGen/builtins-overflow.c
+++ cfe/trunk/test/CodeGen/builtins-overflow.c
@@ -339,6 +339,27 @@
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: 

r349542 - [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Tue Dec 18 13:05:03 2018
New Revision: 349542

URL: http://llvm.org/viewvc/llvm-project?rev=349542=rev
Log:
[CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

The special lowering for __builtin_mul_overflow introduced in r320902
fixed an ICE seen when passing mixed-sign operands to the builtin.

This patch extends the special lowering to cover mixed-width, mixed-sign
operands. In a few common scenarios, calls to muloti4 will no longer be
emitted.

This should address the latest comments in PR34920 and work around the
link failure seen in:

  https://bugzilla.redhat.com/show_bug.cgi?id=1657544

Testing:
- check-clang
- A/B output comparison with: 
https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins-overflow.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=349542=349541=349542=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Dec 18 13:05:03 2018
@@ -1235,7 +1235,7 @@ static bool isSpecialMixedSignMultiply(u
WidthAndSignedness Op2Info,
WidthAndSignedness ResultInfo) {
   return BuiltinID == Builtin::BI__builtin_mul_overflow &&
- Op1Info.Width == Op2Info.Width && Op1Info.Width >= ResultInfo.Width &&
+ std::max(Op1Info.Width, Op2Info.Width) >= ResultInfo.Width &&
  Op1Info.Signed != Op2Info.Signed;
 }
 
@@ -1256,11 +1256,20 @@ EmitCheckedMixedSignMultiply(CodeGenFunc
   const clang::Expr *UnsignedOp = Op1Info.Signed ? Op2 : Op1;
   llvm::Value *Signed = CGF.EmitScalarExpr(SignedOp);
   llvm::Value *Unsigned = CGF.EmitScalarExpr(UnsignedOp);
+  unsigned SignedOpWidth = Op1Info.Signed ? Op1Info.Width : Op2Info.Width;
+  unsigned UnsignedOpWidth = Op1Info.Signed ? Op2Info.Width : Op1Info.Width;
+
+  // One of the operands may be smaller than the other. If so, [s|z]ext it.
+  if (SignedOpWidth < UnsignedOpWidth)
+Signed = CGF.Builder.CreateSExt(Signed, Unsigned->getType(), "op.sext");
+  if (UnsignedOpWidth < SignedOpWidth)
+Unsigned = CGF.Builder.CreateZExt(Unsigned, Signed->getType(), "op.zext");
 
   llvm::Type *OpTy = Signed->getType();
   llvm::Value *Zero = llvm::Constant::getNullValue(OpTy);
   Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
   llvm::Type *ResTy = ResultPtr.getElementType();
+  unsigned OpWidth = std::max(Op1Info.Width, Op2Info.Width);
 
   // Take the absolute value of the signed operand.
   llvm::Value *IsNegative = CGF.Builder.CreateICmpSLT(Signed, Zero);
@@ -1278,8 +1287,8 @@ EmitCheckedMixedSignMultiply(CodeGenFunc
   if (ResultInfo.Signed) {
 // Signed overflow occurs if the result is greater than INT_MAX or lesser
 // than INT_MIN, i.e when |Result| > (INT_MAX + IsNegative).
-auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width)
-  .zextOrSelf(Op1Info.Width);
+auto IntMax =
+llvm::APInt::getSignedMaxValue(ResultInfo.Width).zextOrSelf(OpWidth);
 llvm::Value *MaxResult =
 CGF.Builder.CreateAdd(llvm::ConstantInt::get(OpTy, IntMax),
   CGF.Builder.CreateZExt(IsNegative, OpTy));
@@ -1297,9 +1306,9 @@ EmitCheckedMixedSignMultiply(CodeGenFunc
 llvm::Value *Underflow = CGF.Builder.CreateAnd(
 IsNegative, CGF.Builder.CreateIsNotNull(UnsignedResult));
 Overflow = CGF.Builder.CreateOr(UnsignedOverflow, Underflow);
-if (ResultInfo.Width < Op1Info.Width) {
+if (ResultInfo.Width < OpWidth) {
   auto IntMax =
-  llvm::APInt::getMaxValue(ResultInfo.Width).zext(Op1Info.Width);
+  llvm::APInt::getMaxValue(ResultInfo.Width).zext(OpWidth);
   llvm::Value *TruncOverflow = CGF.Builder.CreateICmpUGT(
   UnsignedResult, llvm::ConstantInt::get(OpTy, IntMax));
   Overflow = CGF.Builder.CreateOr(Overflow, TruncOverflow);

Modified: cfe/trunk/test/CodeGen/builtins-overflow.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-overflow.c?rev=349542=349541=349542=diff
==
--- cfe/trunk/test/CodeGen/builtins-overflow.c (original)
+++ cfe/trunk/test/CodeGen/builtins-overflow.c Tue Dec 18 13:05:03 2018
@@ -339,6 +339,27 @@ long long test_smulll_overflow(long long
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: @test_mixed_sign_mul_overflow_sext_signed_op
+// CHECK: [[SignedOp:%.*]] = sext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 [[SignedOp]], 0
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
+int 

r349540 - [OPENMP][NVPTX]Emit shared memory buffer for reduction as 128 bytes

2018-12-18 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Dec 18 13:01:42 2018
New Revision: 349540

URL: http://llvm.org/viewvc/llvm-project?rev=349540=rev
Log:
[OPENMP][NVPTX]Emit shared memory buffer for reduction as 128 bytes
buffer.

Seems to me, nvlink has a bug with the proper support of the weakly
linked symbols. It does not allow to define several shared memory buffer
with the different sizes even with the weak linkage. Instead we always
use 128 bytes buffer to prevent nvlink from the error message emission.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
cfe/trunk/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=349540=349539=349540=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Dec 18 13:01:42 2018
@@ -4518,6 +4518,22 @@ void CGOpenMPRuntimeNVPTX::clear() {
   Records.UseSharedMemory->setInitializer(
   llvm::ConstantInt::get(CGM.Int16Ty, UseSharedMemory ? 1 : 0));
 }
+// Allocate SharedMemorySize buffer for the shared memory.
+// FIXME: nvlink does not handle weak linkage correctly (object with the
+// different size are reported as erroneous).
+// Restore this code as sson as nvlink is fixed.
+if (!SharedStaticRD->field_empty()) {
+  llvm::APInt ArySize(/*numBits=*/64, SharedMemorySize);
+  QualType SubTy = C.getConstantArrayType(
+  C.CharTy, ArySize, ArrayType::Normal, /*IndexTypeQuals=*/0);
+  auto *Field = FieldDecl::Create(
+  C, SharedStaticRD, SourceLocation(), SourceLocation(), nullptr, 
SubTy,
+  C.getTrivialTypeSourceInfo(SubTy, SourceLocation()),
+  /*BW=*/nullptr, /*Mutable=*/false,
+  /*InitStyle=*/ICIS_NoInit);
+  Field->setAccess(AS_public);
+  SharedStaticRD->addDecl(Field);
+}
 SharedStaticRD->completeDefinition();
 if (!SharedStaticRD->field_empty()) {
   QualType StaticTy = C.getRecordType(SharedStaticRD);

Modified: cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp?rev=349540=349539=349540=diff
==
--- cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp Tue Dec 18 13:01:42 2018
@@ -26,7 +26,7 @@ void test_ds(){
 }
   }
 }
-// CK1: [[MEM_TY:%.+]] = type { [8 x i8] }
+// CK1: [[MEM_TY:%.+]] = type { [128 x i8] }
 // CK1-DAG: [[SHARED_GLOBAL_RD:@.+]] = common addrspace(3) global [[MEM_TY]] 
zeroinitializer
 // CK1-DAG: [[KERNEL_PTR:@.+]] = internal addrspace(3) global i8* null
 // CK1-DAG: [[KERNEL_SIZE:@.+]] = internal unnamed_addr constant i64 8

Modified: 
cfe/trunk/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp?rev=349540=349539=349540=diff
==
--- cfe/trunk/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp 
Tue Dec 18 13:01:42 2018
@@ -21,7 +21,7 @@ int main(int argc, char **argv) {
   return 0;
 }
 
-// CHECK: [[MEM_TY:%.+]] = type { [84 x i8] }
+// CHECK: [[MEM_TY:%.+]] = type { [128 x i8] }
 // CHECK-DAG: [[SHARED_GLOBAL_RD:@.+]] = common addrspace(3) global [[MEM_TY]] 
zeroinitializer
 // CHECK-DAG: [[KERNEL_PTR:@.+]] = internal addrspace(3) global i8* null
 // CHECK-DAG: [[KERNEL_SIZE:@.+]] = internal unnamed_addr constant i{{64|32}} 
84

Modified: cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp?rev=349540=349539=349540=diff
==
--- cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp Tue Dec 18 13:01:42 2018
@@ -72,7 +72,7 @@ int bar(int n){
   return a;
 }
 
-// CHECK: [[MEM_TY:%.+]] = type { [4 x i8] }
+// CHECK: [[MEM_TY:%.+]] = type { [128 x i8] }
 // CHECK-DAG: 

[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D55843



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


[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

2018-12-18 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Here are a couple of examples I found running the regression tests:

  int f0(int);
  float f0(float);
  decltype(f0) f0_a; // this is not valid.



  template 
  int var_expr(Ts... ts);
  
  template 
  auto a_function(Ts... ts) -> decltype(var_expr(ts...));
  
  template 
  using partial = decltype(a_function);
  
  int use_partial() { partial n; }



  template void oneT() { }
  int main()
  {
decltype(oneT)* fun = 0;
  }

If the call to `CheckPlaceholderExpr` in `Sema::BuildDecltypeType` is moved to 
`TreeTransform::TransformDecltypeType` and 
`Parser::ParseDecltypeSpecifier`, we can assert at the beginning of 
`Sema::BuildDecltypeType`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55662



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


[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

2018-12-18 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:2828
 
   // FIXME: OpenCL: Need to consider address spaces
   unsigned FromQuals = FromFunction->getTypeQuals().getCVRUQualifiers();

I am still missing something here.


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

https://reviews.llvm.org/D55850



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


[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

2018-12-18 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 178767.
Anastasia added a comment.

Removed FIXME that has been addressed


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

https://reviews.llvm.org/D55850

Files:
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl

Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,20 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
+  //expected-note@-1{{candidate function}}
+  void m1() __global;
+  //expected-note@-1{{candidate function}}
+  void m2() __global __local; //expected-error{{multiple address spaces specified for type}}
+};
+
+__global C c_glob;
+
+__kernel void bar() {
+  __local C c_loc;
+  C c_priv;
+
+  c_glob.m1();
+  c_loc.m1();
+  c_priv.m1(); //expected-error{{no matching member function for call to 'm1'}}
+}
Index: test/CodeGenOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,35 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct C {
+  void foo() __local;
+  void foo() __global;
+  void foo();
+  void bar();
+};
+
+__global C c1;
+
+__kernel void k() {
+  __local C c2;
+  C c3;
+  __global C _ref = c1;
+  __global C *c_ptr;
+
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c1.foo();
+  // CHECK: call void @_ZNU3AS31C3fooEv(%struct.C addrspace(3)*
+  c2.foo();
+  // CHECK: call void @_ZNU3AS41C3fooEv(%struct.C addrspace(4)*
+  c3.foo();
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ptr->foo();
+  // CHECK: void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ref.foo();
+
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c1.bar();
+  //FIXME: Doesn't compile yet
+  //c_ptr->bar();
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c_ref.bar();
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -3921,6 +3921,14 @@
   llvm_unreachable("unknown NullabilityKind");
 }
 
+/// IsClassMember - Determines whether a state belongs to a class member.
+static bool IsClassMember(TypeProcessingState ) {
+  return (!State.getDeclarator().getCXXScopeSpec().isEmpty() &&
+  State.getDeclarator().getCXXScopeSpec().getScopeRep()->getKind() ==
+  NestedNameSpecifier::TypeSpec) ||
+ State.getDeclarator().getContext() == DeclaratorContext::MemberContext;
+}
+
 static TypeSourceInfo *
 GetTypeSourceInfoForDeclarator(TypeProcessingState ,
QualType T, TypeSourceInfo *ReturnTypeInfo);
@@ -4825,18 +4833,45 @@
   Exceptions,
   EPI.ExceptionSpec);
 
-const auto  = D.getCXXScopeSpec();
+// FIXME: Set address space from attrs for C++ mode here.
 // OpenCLCPlusPlus: A class member function has an address space.
 if (state.getSema().getLangOpts().OpenCLCPlusPlus &&
-((!Spec.isEmpty() &&
-  Spec.getScopeRep()->getKind() == NestedNameSpecifier::TypeSpec) ||
- state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext)) {
-  LangAS CurAS = EPI.TypeQuals.getAddressSpace();
+IsClassMember(state)) {
+  LangAS ASIdx = LangAS::Default;
+  // Take address space attr if any and mark as invalid to avoid adding
+  // them later while creating QualType.
+  for (ParsedAttr  : DeclType.getAttrs()) {
+switch (attr.getKind()) {
+case ParsedAttr::AT_OpenCLConstantAddressSpace:
+  ASIdx = LangAS::opencl_constant;
+  attr.setInvalid();
+  break;
+case ParsedAttr::AT_OpenCLLocalAddressSpace:
+  ASIdx = LangAS::opencl_local;
+  attr.setInvalid();
+  break;
+case ParsedAttr::AT_OpenCLGlobalAddressSpace:
+  ASIdx = LangAS::opencl_global;
+  attr.setInvalid();
+  break;
+case ParsedAttr::AT_OpenCLPrivateAddressSpace:
+  ASIdx = LangAS::opencl_private;
+  attr.setInvalid();
+  break;
+case ParsedAttr::AT_OpenCLGenericAddressSpace:
+  ASIdx = 

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

2018-12-18 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia created this revision.
Anastasia added a reviewer: rjmccall.
Herald added a subscriber: yaxunl.

This patch allows qualifying methods with address spaces and extends some 
overloading rules to use those qualifiers.

The main use case is to prevent conversions to generic/default address space 
where it's requested by the programmer. More details can be found in: 
http://lists.llvm.org/pipermail/cfe-dev/2018-December/060470.html

This patch doesn't enable the feature for C++ yet but it can easily be 
generalized if the overall flow is right.


https://reviews.llvm.org/D55850

Files:
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl

Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,20 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
+  //expected-note@-1{{candidate function}}
+  void m1() __global;
+  //expected-note@-1{{candidate function}}
+  void m2() __global __local; //expected-error{{multiple address spaces specified for type}}
+};
+
+__global C c_glob;
+
+__kernel void bar() {
+  __local C c_loc;
+  C c_priv;
+
+  c_glob.m1();
+  c_loc.m1();
+  c_priv.m1(); //expected-error{{no matching member function for call to 'm1'}}
+}
Index: test/CodeGenOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,35 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct C {
+  void foo() __local;
+  void foo() __global;
+  void foo();
+  void bar();
+};
+
+__global C c1;
+
+__kernel void k() {
+  __local C c2;
+  C c3;
+  __global C _ref = c1;
+  __global C *c_ptr;
+
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c1.foo();
+  // CHECK: call void @_ZNU3AS31C3fooEv(%struct.C addrspace(3)*
+  c2.foo();
+  // CHECK: call void @_ZNU3AS41C3fooEv(%struct.C addrspace(4)*
+  c3.foo();
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ptr->foo();
+  // CHECK: void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ref.foo();
+
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c1.bar();
+  //FIXME: Doesn't compile yet
+  //c_ptr->bar();
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c_ref.bar();
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -3921,6 +3921,14 @@
   llvm_unreachable("unknown NullabilityKind");
 }
 
+/// IsClassMember - Determines whether a state belongs to a class member.
+static bool IsClassMember(TypeProcessingState ) {
+  return (!State.getDeclarator().getCXXScopeSpec().isEmpty() &&
+  State.getDeclarator().getCXXScopeSpec().getScopeRep()->getKind() ==
+  NestedNameSpecifier::TypeSpec) ||
+ State.getDeclarator().getContext() == DeclaratorContext::MemberContext;
+}
+
 static TypeSourceInfo *
 GetTypeSourceInfoForDeclarator(TypeProcessingState ,
QualType T, TypeSourceInfo *ReturnTypeInfo);
@@ -4825,18 +4833,45 @@
   Exceptions,
   EPI.ExceptionSpec);
 
-const auto  = D.getCXXScopeSpec();
+// FIXME: Set address space from attrs for C++ mode here.
 // OpenCLCPlusPlus: A class member function has an address space.
 if (state.getSema().getLangOpts().OpenCLCPlusPlus &&
-((!Spec.isEmpty() &&
-  Spec.getScopeRep()->getKind() == NestedNameSpecifier::TypeSpec) ||
- state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext)) {
-  LangAS CurAS = EPI.TypeQuals.getAddressSpace();
+IsClassMember(state)) {
+  LangAS ASIdx = LangAS::Default;
+  // Take address space attr if any and mark as invalid to avoid adding
+  // them later while creating QualType.
+  for (ParsedAttr  : DeclType.getAttrs()) {
+switch (attr.getKind()) {
+case ParsedAttr::AT_OpenCLConstantAddressSpace:
+  ASIdx = LangAS::opencl_constant;
+  attr.setInvalid();
+  break;
+case ParsedAttr::AT_OpenCLLocalAddressSpace:
+  ASIdx = LangAS::opencl_local;
+  attr.setInvalid();
+  break;
+case 

[PATCH] D55848: [clang-tidy] Add export-fixes flag to clang-tidy-diff

2018-12-18 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, alexfh.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

So that you can export the fixes generated.


https://reviews.llvm.org/D55848

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py


Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:


Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55847: [gn build] Add build file for clang/lib/Basic and dependencies

2018-12-18 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision.
Herald added subscribers: fedor.sergeev, aheejin, dschuff.

Adds a build file for clang-tblgen and an action for running it, and uses that 
to process all the .td files in include/clang/Basic.

Also adds an action to write include/clang/Config/config.h and 
include/clang/Basic/Version.inc.


https://reviews.llvm.org/D55847

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/clang/include/clang/Basic/BUILD.gn
  llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
  llvm/utils/gn/secondary/clang/lib/ARCMigrate/enable.gni
  llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
  llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Frontend/enable.gni
  llvm/utils/gn/secondary/clang/utils/TableGen/BUILD.gn
  llvm/utils/gn/secondary/clang/utils/TableGen/clang_tablegen.gni
  llvm/utils/gn/secondary/lld/include/lld/Common/BUILD.gn
  llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni

Index: llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
===
--- llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
+++ llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
@@ -36,8 +36,12 @@
   action(target_name) {
 forward_variables_from(invoker, [ "visibility" ])
 
-# FIXME: In cross builds, this should depend on the host binary.
-tblgen_target = "//llvm/utils/TableGen:llvm-tblgen"
+if (defined(invoker.tblgen_target)) {
+  tblgen_target = invoker.tblgen_target
+} else {
+  # FIXME: In cross builds, this should depend on the host binary.
+  tblgen_target = "//llvm/utils/TableGen:llvm-tblgen"
+}
 tblgen_executable = get_label_info(tblgen_target, "root_out_dir") +
 "/bin/" + get_label_info(tblgen_target, "name")
 deps = [
Index: llvm/utils/gn/secondary/lld/include/lld/Common/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/include/lld/Common/BUILD.gn
+++ llvm/utils/gn/secondary/lld/include/lld/Common/BUILD.gn
@@ -12,7 +12,6 @@
   args = [
 "-o",
 rebase_path(outputs[0], root_out_dir),
-
 rebase_path(sources[0], root_out_dir),
 
 "LLD_VERSION=$llvm_version",
Index: llvm/utils/gn/secondary/clang/utils/TableGen/clang_tablegen.gni
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang/utils/TableGen/clang_tablegen.gni
@@ -0,0 +1,41 @@
+# This file introduces a templates for running clang-tblgen.
+#
+# Parameters:
+#
+#   args (required)
+#   [list of strings] Flags to pass to llvm-tblgen.
+#
+#   output_name (optional)
+#   Basename of the generated output file.
+#   Defaults to target name with ".inc" appended.
+#
+#   td_file (optional)
+#   The .td file to pass to llvm-tblgen.
+#   Defaults to target name with ".td" appended.
+#
+#   visibility (optional)
+#   GN's regular visibility attribute, see `gn help visibility`.
+#
+# Example of usage:
+#
+#   clang_tablegen("DiagnosticGroups") {
+# args = [ "-gen-clang-diag-groups" ]
+# td_file = "Diagnostic.td"
+#   }
+
+import("//llvm/utils/TableGen/tablegen.gni")
+
+template("clang_tablegen") {
+  tablegen(target_name) {
+forward_variables_from(invoker,
+   [
+ "args",
+ "output_name",
+ "td_file",
+ "visibility",
+   ])
+
+# FIXME: In cross builds, this should depend on the host binary.
+tblgen_target = "//clang/utils/TableGen:clang-tblgen"
+  }
+}
Index: llvm/utils/gn/secondary/clang/utils/TableGen/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang/utils/TableGen/BUILD.gn
@@ -0,0 +1,19 @@
+executable("clang-tblgen") {
+  deps = [
+"//llvm/lib/Support",
+"//llvm/lib/TableGen",
+  ]
+  sources = [
+"ClangASTNodesEmitter.cpp",
+"ClangAttrEmitter.cpp",
+"ClangCommentCommandInfoEmitter.cpp",
+"ClangCommentHTMLNamedCharacterReferenceEmitter.cpp",
+"ClangCommentHTMLTagsEmitter.cpp",
+"ClangDataCollectorsEmitter.cpp",
+"ClangDiagnosticsEmitter.cpp",
+"ClangOptionDocEmitter.cpp",
+"ClangSACheckersEmitter.cpp",
+"NeonEmitter.cpp",
+"TableGen.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Frontend/enable.gni
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Frontend/enable.gni
@@ -0,0 +1,4 @@
+declare_args() {
+  # Whether to include the static analyzer in the clang binary.
+  clang_enable_static_analyzer = true
+}
Index: llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
@@ -0,0 +1,80 @@

[PATCH] D55616: Emit ASM input in a constant context

2018-12-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

efriedma wrote:
> void wrote:
> > efriedma wrote:
> > > Given this code doesn't check the optimization level anymore, do you 
> > > still need separate check prefixes for `O2` and `O0`.  Or if that doesn't 
> > > work for everything, maybe you could share a check prefix for some of the 
> > > tests? (Maybe it would make sense to check IR generated using 
> > > `-disable-llvm-passes`.)
> > The bug only triggered at `O0`, so I still want to test it without 
> > optimizations. Note that we do check optimization levels during code 
> > generation to determine if we should generate an `is.constant` intrinsic.
> You can use something like "-check-prefixes=CHECK,O0" to reduce the 
> duplication.
That doesn't pass.

More specifically, I was thinking something more along the lines of using 
"--check-prefixes=CHECK,O2" for the first run, and "--check-prefixes=CHECK,O0" 
for the second run; then you can use "CHECK:" for the common lines and 
"O2:"/"O0:" for the lines that are different.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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


[libunwind] r349532 - [SEH] Add initial support for AArch64

2018-12-18 Thread Martin Storsjo via cfe-commits
Author: mstorsjo
Date: Tue Dec 18 12:05:59 2018
New Revision: 349532

URL: http://llvm.org/viewvc/llvm-project?rev=349532=rev
Log:
[SEH] Add initial support for AArch64

This doesn't yet implement inspecting the .pdata/.xdata to find the
LSDA pointer (in UnwindCursor::getInfoFromSEH), but normal C++
exception handling seems to run just fine without it. (The only
place I can see where it's even referenced is in
unwind_phase2_forced, and I can't find a codepath where libcxxabi
would end up calling that.)

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

Modified:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/src/Unwind-seh.cpp
libunwind/trunk/src/UnwindCursor.hpp

Modified: libunwind/trunk/include/__libunwind_config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=349532=349531=349532=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Tue Dec 18 12:05:59 2018
@@ -57,7 +57,11 @@
 # elif defined(__aarch64__)
 #  define _LIBUNWIND_TARGET_AARCH64 1
 #  define _LIBUNWIND_CONTEXT_SIZE 66
-#  define _LIBUNWIND_CURSOR_SIZE 78
+#  if defined(__SEH__)
+#define _LIBUNWIND_CURSOR_SIZE 164
+#  else
+#define _LIBUNWIND_CURSOR_SIZE 78
+#  endif
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64
 # elif defined(__arm__)
 #  define _LIBUNWIND_TARGET_ARM 1

Modified: libunwind/trunk/src/Unwind-seh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-seh.cpp?rev=349532=349531=349532=diff
==
--- libunwind/trunk/src/Unwind-seh.cpp (original)
+++ libunwind/trunk/src/Unwind-seh.cpp Tue Dec 18 12:05:59 2018
@@ -87,6 +87,8 @@ _GCC_specific_handler(PEXCEPTION_RECORD
   disp->ContextRecord->Rdx = ms_exc->ExceptionInformation[3];
 #elif defined(__arm__)
   disp->ContextRecord->R1 = ms_exc->ExceptionInformation[3];
+#elif defined(__aarch64__)
+  disp->ContextRecord->X1 = ms_exc->ExceptionInformation[3];
 #endif
 }
 // This is the collided unwind to the landing pad. Nothing to do.
@@ -172,12 +174,16 @@ _GCC_specific_handler(PEXCEPTION_RECORD
 exc->private_[2] = disp->TargetPc;
 unw_get_reg(, UNW_ARM_R0, );
 unw_get_reg(, UNW_ARM_R1, >private_[3]);
+#elif defined(__aarch64__)
+exc->private_[2] = disp->TargetPc;
+unw_get_reg(, UNW_ARM64_X0, );
+unw_get_reg(, UNW_ARM64_X1, >private_[3]);
 #endif
 unw_get_reg(, UNW_REG_IP, );
 ms_exc->ExceptionCode = STATUS_GCC_UNWIND;
 #ifdef __x86_64__
 ms_exc->ExceptionInformation[2] = disp->TargetIp;
-#elif defined(__arm__)
+#elif defined(__arm__) || defined(__aarch64__)
 ms_exc->ExceptionInformation[2] = disp->TargetPc;
 #endif
 ms_exc->ExceptionInformation[3] = exc->private_[3];
@@ -447,6 +453,12 @@ _unw_init_seh(unw_cursor_t *cursor, CONT
   auto *co = reinterpret_cast(cursor);
   co->setInfoBasedOnIPRegister();
   return UNW_ESUCCESS;
+#elif defined(_LIBUNWIND_TARGET_AARCH64)
+  new ((void *)cursor) UnwindCursor(
+  context, LocalAddressSpace::sThisAddressSpace);
+  auto *co = reinterpret_cast(cursor);
+  co->setInfoBasedOnIPRegister();
+  return UNW_ESUCCESS;
 #else
   return UNW_EINVAL;
 #endif
@@ -458,6 +470,8 @@ _unw_seh_get_disp_ctx(unw_cursor_t *curs
   return reinterpret_cast 
*>(cursor)->getDispatcherContext();
 #elif defined(_LIBUNWIND_TARGET_ARM)
   return reinterpret_cast 
*>(cursor)->getDispatcherContext();
+#elif defined(_LIBUNWIND_TARGET_AARCH64)
+  return reinterpret_cast 
*>(cursor)->getDispatcherContext();
 #else
   return nullptr;
 #endif
@@ -469,6 +483,8 @@ _unw_seh_set_disp_ctx(unw_cursor_t *curs
   reinterpret_cast 
*>(cursor)->setDispatcherContext(disp);
 #elif defined(_LIBUNWIND_TARGET_ARM)
   reinterpret_cast 
*>(cursor)->setDispatcherContext(disp);
+#elif defined(_LIBUNWIND_TARGET_AARCH64)
+  reinterpret_cast 
*>(cursor)->setDispatcherContext(disp);
 #endif
 }
 

Modified: libunwind/trunk/src/UnwindCursor.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindCursor.hpp?rev=349532=349531=349532=diff
==
--- libunwind/trunk/src/UnwindCursor.hpp (original)
+++ libunwind/trunk/src/UnwindCursor.hpp Tue Dec 18 12:05:59 2018
@@ -615,6 +615,13 @@ UnwindCursor::UnwindCursor(unw_con
 d.d = r.getFloatRegister(i);
 _msContext.D[i - UNW_ARM_D0] = d.w;
   }
+#elif defined(_LIBUNWIND_TARGET_AARCH64)
+  for (int i = UNW_ARM64_X0; i <= UNW_ARM64_X30; ++i)
+_msContext.X[i - UNW_ARM64_X0] = r.getRegister(i);
+  _msContext.Sp = r.getRegister(UNW_REG_SP);
+  _msContext.Pc = r.getRegister(UNW_REG_IP);
+  for (int i = UNW_ARM64_D0; i <= UNW_ARM64_D31; ++i)
+_msContext.V[i - UNW_ARM64_D0].D[0] = r.getFloatRegister(i);
 #endif
 }
 
@@ -638,6 +645,8 @@ bool 

[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 178752.
vsk marked an inline comment as done.
vsk added a comment.

- Simplify logic per Eli's review feedback.


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

https://reviews.llvm.org/D55843

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-overflow.c


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -339,6 +339,27 @@
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: @test_mixed_sign_mul_overflow_sext_signed_op
+// CHECK: [[SignedOp:%.*]] = sext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 [[SignedOp]], 0
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
+int test_mixed_sign_mul_overflow_zext_unsigned_op(long long x, unsigned y) {
+// CHECK: @test_mixed_sign_mul_overflow_zext_unsigned_op
+// CHECK: [[UnsignedOp:%.*]] = zext i32 %1 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 %0, 0
+// CHECK: @llvm.umul.with.overflow.i64({{.*}}, i64 [[UnsignedOp]])
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
 int test_mixed_sign_mull_overflow(int x, unsigned y) {
 // CHECK: @test_mixed_sign_mull_overflow
 // CHECK:   [[IsNeg:%.*]] = icmp slt i32 [[Op1:%.*]], 0
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1235,7 +1235,7 @@
WidthAndSignedness Op2Info,
WidthAndSignedness ResultInfo) {
   return BuiltinID == Builtin::BI__builtin_mul_overflow &&
- Op1Info.Width == Op2Info.Width && Op1Info.Width >= ResultInfo.Width &&
+ std::max(Op1Info.Width, Op2Info.Width) >= ResultInfo.Width &&
  Op1Info.Signed != Op2Info.Signed;
 }
 
@@ -1256,11 +1256,20 @@
   const clang::Expr *UnsignedOp = Op1Info.Signed ? Op2 : Op1;
   llvm::Value *Signed = CGF.EmitScalarExpr(SignedOp);
   llvm::Value *Unsigned = CGF.EmitScalarExpr(UnsignedOp);
+  unsigned SignedOpWidth = Op1Info.Signed ? Op1Info.Width : Op2Info.Width;
+  unsigned UnsignedOpWidth = Op1Info.Signed ? Op2Info.Width : Op1Info.Width;
+
+  // One of the operands may be smaller than the other. If so, [s|z]ext it.
+  if (SignedOpWidth < UnsignedOpWidth)
+Signed = CGF.Builder.CreateSExt(Signed, Unsigned->getType(), "op.sext");
+  if (UnsignedOpWidth < SignedOpWidth)
+Unsigned = CGF.Builder.CreateZExt(Unsigned, Signed->getType(), "op.zext");
 
   llvm::Type *OpTy = Signed->getType();
   llvm::Value *Zero = llvm::Constant::getNullValue(OpTy);
   Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
   llvm::Type *ResTy = ResultPtr.getElementType();
+  unsigned OpWidth = std::max(Op1Info.Width, Op2Info.Width);
 
   // Take the absolute value of the signed operand.
   llvm::Value *IsNegative = CGF.Builder.CreateICmpSLT(Signed, Zero);
@@ -1278,8 +1287,8 @@
   if (ResultInfo.Signed) {
 // Signed overflow occurs if the result is greater than INT_MAX or lesser
 // than INT_MIN, i.e when |Result| > (INT_MAX + IsNegative).
-auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width)
-  .zextOrSelf(Op1Info.Width);
+auto IntMax =
+llvm::APInt::getSignedMaxValue(ResultInfo.Width).zextOrSelf(OpWidth);
 llvm::Value *MaxResult =
 CGF.Builder.CreateAdd(llvm::ConstantInt::get(OpTy, IntMax),
   CGF.Builder.CreateZExt(IsNegative, OpTy));
@@ -1297,9 +1306,9 @@
 llvm::Value *Underflow = CGF.Builder.CreateAnd(
 IsNegative, CGF.Builder.CreateIsNotNull(UnsignedResult));
 Overflow = CGF.Builder.CreateOr(UnsignedOverflow, Underflow);
-if (ResultInfo.Width < Op1Info.Width) {
+if (ResultInfo.Width < OpWidth) {
   auto IntMax =
-  llvm::APInt::getMaxValue(ResultInfo.Width).zext(Op1Info.Width);
+  llvm::APInt::getMaxValue(ResultInfo.Width).zext(OpWidth);
   llvm::Value *TruncOverflow = CGF.Builder.CreateICmpUGT(
   UnsignedResult, llvm::ConstantInt::get(OpTy, IntMax));
   Overflow = CGF.Builder.CreateOr(Overflow, TruncOverflow);


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -339,6 +339,27 @@
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: @test_mixed_sign_mul_overflow_sext_signed_op
+// CHECK: [[SignedOp:%.*]] = sext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 [[SignedOp]], 0
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
+int 

[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/test/CodeGenObjCXX/inheriting-constructor-cleanup.mm:25
+// CHECK-LABEL: define void @_Z1fv
+// CHECK: call void (%struct.Base*, i8*, ...) 
@_ZN4BaseC2E6Strongz(%struct.Base* {{.*}}, i8* {{.*}})
+// CHECK-NEXT: call void @_ZN9InheritorD1Ev(%struct.Inheritor* {{.*}})

Can you include checks for all the calls in this function, with CHECK-NOTs to 
prove that there aren't any other calls?


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

https://reviews.llvm.org/D55543



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


[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

This looks reasonable.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1265
+NeedSExt = Op1Info.Signed;
+NeedZExt = !Op1Info.Signed;
+  } else if (Op1Info.Width > Op2Info.Width) {

This is a weird way to express this... could you instead compute the width of 
the signed operand and the unsigned operand, and compare those?


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

https://reviews.llvm.org/D55843



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


[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

2018-12-18 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 178746.
vsapsai added a comment.

- [ObjC++] Pass struct with `__strong` field as a value, not as a reference.


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

https://reviews.llvm.org/D55543

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenCXX/inheriting-constructor-cleanup.cpp
  clang/test/CodeGenObjCXX/inheriting-constructor-cleanup.mm

Index: clang/test/CodeGenObjCXX/inheriting-constructor-cleanup.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/inheriting-constructor-cleanup.mm
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-darwin -std=c++11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s
+// rdar://problem/45805151
+
+struct Strong {
+  __strong id x;
+};
+
+struct Base {
+  // Use variadic args to cause inlining the inherited constructor.
+  Base(Strong s, ...) {}
+};
+
+struct NonTrivialDtor {
+  ~NonTrivialDtor() {}
+};
+struct Inheritor : public NonTrivialDtor, public Base {
+  using Base::Base;
+};
+
+id g(void);
+void f() {
+  Inheritor({g()});
+}
+// CHECK-LABEL: define void @_Z1fv
+// CHECK: call void (%struct.Base*, i8*, ...) @_ZN4BaseC2E6Strongz(%struct.Base* {{.*}}, i8* {{.*}})
+// CHECK-NEXT: call void @_ZN9InheritorD1Ev(%struct.Inheritor* {{.*}})
+
+// CHECK-LABEL: define linkonce_odr void @_ZN4BaseC2E6Strongz(%struct.Base* {{.*}}, i8* {{.*}}, ...)
+// CHECK: call void @_ZN6StrongD1Ev(%struct.Strong* {{.*}})
+
+// CHECK-LABEL: define linkonce_odr void @_ZN6StrongD1Ev(%struct.Strong* {{.*}})
+// CHECK: call void @_ZN6StrongD2Ev(%struct.Strong* {{.*}})
+
+// CHECK-LABEL: define linkonce_odr void @_ZN6StrongD2Ev(%struct.Strong* {{.*}})
+// CHECK: call void @objc_storeStrong(i8** {{.*}}, i8* null)
Index: clang/test/CodeGenCXX/inheriting-constructor-cleanup.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/inheriting-constructor-cleanup.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -triple x86_64-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-darwin -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s --check-prefix=EXCEPTIONS
+
+// PR36748
+// rdar://problem/45805151
+
+// Classes to verify order of destroying function parameters.
+struct S1 {
+  ~S1();
+};
+struct S2 {
+  ~S2();
+};
+
+struct Base {
+  // Use variadic args to cause inlining the inherited constructor.
+  Base(const S1&, const S2&, const char *fmt, ...) {}
+};
+
+struct NonTrivialDtor {
+  ~NonTrivialDtor() {}
+};
+struct Inheritor : public NonTrivialDtor, public Base {
+  using Base::Base;
+};
+
+void f() {
+  Inheritor(S1(), S2(), "foo");
+  // CHECK-LABEL: define void @_Z1fv
+  // CHECK: %[[TMP1:.*]] = alloca %struct.S1
+  // CHECK: %[[TMP2:.*]] = alloca %struct.S2
+  // CHECK: call void (%struct.Base*, %struct.S1*, %struct.S2*, i8*, ...) @_ZN4BaseC2ERK2S1RK2S2PKcz(%struct.Base* {{.*}}, %struct.S1* dereferenceable(1) %[[TMP1]], %struct.S2* dereferenceable(1) %[[TMP2]], i8* {{.*}})
+  // CHECK-NEXT: call void @_ZN9InheritorD1Ev(%struct.Inheritor* {{.*}})
+  // CHECK-NEXT: call void @_ZN2S2D1Ev(%struct.S2* %[[TMP2]])
+  // CHECK-NEXT: call void @_ZN2S1D1Ev(%struct.S1* %[[TMP1]])
+
+  // EXCEPTIONS-LABEL: define void @_Z1fv
+  // EXCEPTIONS: %[[TMP1:.*]] = alloca %struct.S1
+  // EXCEPTIONS: %[[TMP2:.*]] = alloca %struct.S2
+  // EXCEPTIONS: invoke void (%struct.Base*, %struct.S1*, %struct.S2*, i8*, ...) @_ZN4BaseC2ERK2S1RK2S2PKcz(%struct.Base* {{.*}}, %struct.S1* dereferenceable(1) %[[TMP1]], %struct.S2* dereferenceable(1) %[[TMP2]], i8* {{.*}})
+  // EXCEPTIONS-NEXT: to label %[[CONT:.*]] unwind label %[[LPAD:.*]]
+
+  // EXCEPTIONS: [[CONT]]:
+  // EXCEPTIONS-NEXT: call void @_ZN9InheritorD1Ev(%struct.Inheritor* {{.*}})
+  // EXCEPTIONS-NEXT: call void @_ZN2S2D1Ev(%struct.S2* %[[TMP2]])
+  // EXCEPTIONS-NEXT: call void @_ZN2S1D1Ev(%struct.S1* %[[TMP1]])
+
+  // EXCEPTIONS: [[LPAD]]:
+  // EXCEPTIONS: call void @_ZN14NonTrivialDtorD2Ev(%struct.NonTrivialDtor* {{.*}})
+  // EXCEPTIONS-NEXT: call void @_ZN2S2D1Ev(%struct.S2* %[[TMP2]])
+  // EXCEPTIONS-NEXT: call void @_ZN2S1D1Ev(%struct.S1* %[[TMP1]])
+}
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2196,6 +2196,7 @@
   GlobalDecl GD(Ctor, CtorType);
   InlinedInheritingConstructorScope Scope(*this, GD);
   ApplyInlineDebugLocation DebugScope(*this, GD);
+  RunCleanupsScope RunCleanups(*this);
 
   // Save the arguments to be passed to the inherited constructor.
   CXXInheritedCtorInitExprArgs = Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55844: [Fixed Point Arithmetic] Fixed Point Subtraction

2018-12-18 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: ebevhan, bjope, rjmccall.
leonardchan added a project: clang.

This patch covers subtraction between fixed point types and other fixed point 
types or integers, using the conversion rules described in 4.1.4 of N1169.


Repository:
  rC Clang

https://reviews.llvm.org/D55844

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/Frontend/fixed_point_sub.c

Index: clang/test/Frontend/fixed_point_sub.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_sub.c
@@ -0,0 +1,390 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
+void SignedSubtraction() {
+  // CHECK-LABEL: SignedSubtraction
+  short _Accum sa;
+  _Accum a, b, c, d;
+  long _Accum la;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+  unsigned long _Accum ula;
+
+  short _Fract sf;
+  _Fract f;
+  long _Fract lf;
+  unsigned short _Fract usf;
+  unsigned _Fract uf;
+  unsigned long _Fract ulf;
+
+  // Same type
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[SA2:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i16 [[SA]], [[SA2]]
+  // CHECK-NEXT: store i16 [[SUM]], i16* %sa, align 2
+  sa = sa - sa;
+
+  // To larger scale and larger width
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[EXT_SA:%[a-z0-9]+]] = sext i16 [[SA]] to i32
+  // CHECK-NEXT: [[SA:%[a-z0-9]+]] = shl i32 [[EXT_SA]], 8
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i32 [[SA]], [[A]]
+  // CHECK-NEXT: store i32 [[SUM]], i32* %a, align 4
+  a = sa - a;
+
+  // To same scale and smaller width
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[SF:%[0-9]+]] = load i8, i8* %sf, align 1
+  // CHECK-NEXT: [[EXT_SF:%[a-z0-9]+]] = sext i8 [[SF]] to i16
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i16 [[SA]], [[EXT_SF]]
+  // CHECK-NEXT: store i16 [[SUM]], i16* %sa, align 2
+  sa = sa - sf;
+
+  // To smaller scale and same width.
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[F:%[0-9]+]] = load i16, i16* %f, align 2
+  // CHECK-NEXT: [[EXT_SA:%[a-z0-9]+]] = sext i16 [[SA]] to i24
+  // CHECK-NEXT: [[SA:%[a-z0-9]+]] = shl i24 [[EXT_SA]], 8
+  // CHECK-NEXT: [[EXT_F:%[a-z0-9]+]] = sext i16 [[F]] to i24
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i24 [[SA]], [[EXT_F]]
+  // CHECK-NEXT: [[RES:%[a-z0-9]+]] = ashr i24 [[SUM]], 8
+  // CHECK-NEXT: [[TRUNC_RES:%[a-z0-9]+]] = trunc i24 [[RES]] to i16
+  // CHECK-NEXT: store i16 [[TRUNC_RES]], i16* %sa, align 2
+  sa = sa - f;
+
+  // To smaller scale and smaller width
+  // CHECK:  [[A:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[SF:%[0-9]+]] = load i8, i8* %sf, align 1
+  // CHECK-NEXT: [[EXT_SF:%[a-z0-9]+]] = sext i8 [[SF]] to i32
+  // CHECK-NEXT: [[SF:%[a-z0-9]+]] = shl i32 [[EXT_SF]], 8
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i32 [[A]], [[SF]]
+  // CHECK-NEXT: store i32 [[SUM]], i32* %a, align 4
+  a = a - sf;
+
+  // To larger scale and same width
+  // CHECK:  [[A:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[LF:%[0-9]+]] = load i32, i32* %lf, align 4
+  // CHECK-NEXT: [[EXT_A:%[a-z0-9]+]] = sext i32 [[A]] to i48
+  // CHECK-NEXT: [[A:%[a-z0-9]+]] = shl i48 [[EXT_A]], 16
+  // CHECK-NEXT: [[EXT_LF:%[a-z0-9]+]] = sext i32 [[LF]] to i48
+  // CHECK-NEXT: [[SUM:%[0-9]+]] = sub i48 [[A]], [[EXT_LF]]
+  // CHECK-NEXT: [[RES:%[a-z0-9]+]] = ashr i48 [[SUM]], 16
+  // CHECK-NEXT: [[TRUNC_RES:%[a-z0-9]+]] = trunc i48 [[RES]] to i32
+  // CHECK-NEXT: store i32 [[TRUNC_RES]], i32* %a, align 4
+  a = a - lf;
+
+  // With corresponding unsigned type
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[USA:%[0-9]+]] = load i16, i16* %usa, align 2
+  // SIGNED-NEXT: [[SA_EXT:%[a-z0-9]+]] = sext i16 [[SA]] to i17
+  // SIGNED-NEXT: [[SA:%[a-z0-9]+]] = shl i17 [[SA_EXT]], 1
+  // SIGNED-NEXT: [[USA_EXT:%[a-z0-9]+]] = zext i16 [[USA]] to i17
+  // SIGNED-NEXT: [[SUM:%[0-9]+]] = sub i17 [[SA]], [[USA_EXT]]
+  // SIGNED-NEXT: [[RESULT:%[a-z0-9]+]] = ashr i17 [[SUM]], 1
+  // SIGNED-NEXT: [[SUM:%[a-z0-9]+]] = trunc i17 [[RESULT]] to i16
+  // UNSIGNED-NEXT: [[SUM:%[0-9]+]] = sub i16 [[SA]], [[USA]]
+  // CHECK-NEXT: store i16 [[SUM]], i16* %sa, align 2
+  sa = sa - usa;
+
+  // With unsigned of larger scale
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[USA:%[0-9]+]] = load i32, i32* %ua, align 4
+  // SIGNED-NEXT: [[SA_EXT:%[a-z0-9]+]] = sext i16 [[SA]] to i33
+  // SIGNED-NEXT: [[SA:%[a-z0-9]+]] = shl i33 [[SA_EXT]], 9
+  // SIGNED-NEXT: [[USA_EXT:%[a-z0-9]+]] = zext i32 [[USA]] to i33
+  // SIGNED-NEXT: [[SUM:%[0-9]+]] 

r349525 - [OPENMP][NVPTX]Added extra sync point to the inter-warp copy function.

2018-12-18 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Dec 18 11:20:15 2018
New Revision: 349525

URL: http://llvm.org/viewvc/llvm-project?rev=349525=rev
Log:
[OPENMP][NVPTX]Added extra sync point to the inter-warp copy function.

The parallel reduction operation requires an extra synchronization point
in the inter-warp copy function to avoid divergence.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=349525=349524=349525=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Dec 18 11:20:15 2018
@@ -3089,6 +3089,7 @@ static void emitReductionListCopy(
 /// void inter_warp_copy_func(void* reduce_data, num_warps)
 ///   shared smem[warp_size];
 ///   For all data entries D in reduce_data:
+/// sync
 /// If (I am the first lane in each warp)
 ///   Copy my local D to smem[warp_id]
 /// sync
@@ -3203,6 +3204,10 @@ static llvm::Value *emitInterWarpCopyFun
 Bld.CreateCondBr(Cmp, BodyBB, ExitBB);
 CGF.EmitBlock(BodyBB);
   }
+  // kmpc_barrier.
+  CGM.getOpenMPRuntime().emitBarrierCall(CGF, Loc, OMPD_unknown,
+ /*EmitChecks=*/false,
+ /*ForceSimpleCall=*/true);
   llvm::BasicBlock *ThenBB = CGF.createBasicBlock("then");
   llvm::BasicBlock *ElseBB = CGF.createBasicBlock("else");
   llvm::BasicBlock *MergeBB = CGF.createBasicBlock("ifcont");

Modified: cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp?rev=349525=349524=349525=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp Tue Dec 
18 11:20:15 2018
@@ -190,6 +190,7 @@ int bar(int n){
   // CHECK: [[CNT:%.+]] = load i32, i32* [[CNT_ADDR]],
   // CHECK: [[DONE_COPY:%.+]] = icmp ult i32 [[CNT]], 2
   // CHECK: br i1 [[DONE_COPY]], label
+  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[IS_WARP_MASTER:%.+]] = icmp eq i32 [[LANEID]], 0
   // CHECK: br i1 [[IS_WARP_MASTER]], label {{%?}}[[DO_COPY:.+]], label 
{{%?}}[[COPY_ELSE:.+]]
   //
@@ -427,6 +428,7 @@ int bar(int n){
   // CHECK-DAG: [[LANEID:%.+]] = and i32 {{.+}}, 31
   // CHECK-DAG: [[WARPID:%.+]] = ashr i32 {{.+}}, 5
   // CHECK-DAG: [[RED_LIST:%.+]] = bitcast i8* {{.+}} to [[RLT]]*
+  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[IS_WARP_MASTER:%.+]] = icmp eq i32 [[LANEID]], 0
   // CHECK: br i1 [[IS_WARP_MASTER]], label {{%?}}[[DO_COPY:.+]], label 
{{%?}}[[COPY_ELSE:.+]]
   //
@@ -466,6 +468,7 @@ int bar(int n){
   //
   // CHECK: [[READ_CONT]]
   // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
+  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[IS_WARP_MASTER:%.+]] = icmp eq i32 [[LANEID]], 0
   // CHECK: br i1 [[IS_WARP_MASTER]], label {{%?}}[[DO_COPY:.+]], label 
{{%?}}[[COPY_ELSE:.+]]
   //
@@ -740,6 +743,7 @@ int bar(int n){
   // CHECK-DAG: [[LANEID:%.+]] = and i32 {{.+}}, 31
   // CHECK-DAG: [[WARPID:%.+]] = ashr i32 {{.+}}, 5
   // CHECK-DAG: [[RED_LIST:%.+]] = bitcast i8* {{.+}} to [[RLT]]*
+  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[IS_WARP_MASTER:%.+]] = icmp eq i32 [[LANEID]], 0
   // CHECK: br i1 [[IS_WARP_MASTER]], label {{%?}}[[DO_COPY:.+]], label 
{{%?}}[[COPY_ELSE:.+]]
   //


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


[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

2018-12-18 Thread Vedant Kumar via Phabricator via cfe-commits
vsk created this revision.
vsk added reviewers: efriedma, tstellar, dtzWill.

The special lowering for __builtin_mul_overflow introduced in r320902
fixed an ICE seen when passing mixed-sign operands to the builtin.

This patch extends the special lowering to cover mixed-width, mixed-sign
operands. In a few common scenarios, calls to muloti4 will no longer be
emitted.

This should address the latest comments in PR34920 and work around the
link failure seen in:

https://bugzilla.redhat.com/show_bug.cgi?id=1657544.

Testing:

- check-clang
- A/B output comparison with: 
https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081


https://reviews.llvm.org/D55843

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-overflow.c


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -339,6 +339,27 @@
   return result;
 }
 
+int test_mixed_sign_mul_overflow_sext_signed_op(int x, unsigned long long y) {
+// CHECK: @test_mixed_sign_mul_overflow_sext_signed_op
+// CHECK: [[SignedOp:%.*]] = sext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 [[SignedOp]], 0
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
+int test_mixed_sign_mul_overflow_zext_unsigned_op(long long x, unsigned y) {
+// CHECK: @test_mixed_sign_mul_overflow_zext_unsigned_op
+// CHECK: [[UnsignedOp:%.*]] = zext i32 %0 to i64
+// CHECK: [[IsNeg:%.*]] = icmp slt i64 %0, 0
+// CHECK: @llvm.umul.with.overflow.i64({{.*}}, i64 [[UnsignedOp]])
+  int result;
+  if (__builtin_mul_overflow(x, y, ))
+return LongErrorCode;
+  return result;
+}
+
 int test_mixed_sign_mull_overflow(int x, unsigned y) {
 // CHECK: @test_mixed_sign_mull_overflow
 // CHECK:   [[IsNeg:%.*]] = icmp slt i32 [[Op1:%.*]], 0
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1235,7 +1235,7 @@
WidthAndSignedness Op2Info,
WidthAndSignedness ResultInfo) {
   return BuiltinID == Builtin::BI__builtin_mul_overflow &&
- Op1Info.Width == Op2Info.Width && Op1Info.Width >= ResultInfo.Width &&
+ std::max(Op1Info.Width, Op2Info.Width) >= ResultInfo.Width &&
  Op1Info.Signed != Op2Info.Signed;
 }
 
@@ -1257,10 +1257,26 @@
   llvm::Value *Signed = CGF.EmitScalarExpr(SignedOp);
   llvm::Value *Unsigned = CGF.EmitScalarExpr(UnsignedOp);
 
+  // One of the operands may be smaller than the other. If so, [s|z]ext it.
+  bool NeedSExt = false;
+  bool NeedZExt = false;
+  if (Op1Info.Width < Op2Info.Width) {
+NeedSExt = Op1Info.Signed;
+NeedZExt = !Op1Info.Signed;
+  } else if (Op1Info.Width > Op2Info.Width) {
+NeedSExt = Op2Info.Signed;
+NeedZExt = !Op2Info.Signed;
+  }
+  if (NeedSExt)
+Signed = CGF.Builder.CreateSExt(Signed, Unsigned->getType(), "op.sext");
+  if (NeedZExt)
+Unsigned = CGF.Builder.CreateZExt(Unsigned, Signed->getType(), "op.zext");
+
   llvm::Type *OpTy = Signed->getType();
   llvm::Value *Zero = llvm::Constant::getNullValue(OpTy);
   Address ResultPtr = CGF.EmitPointerWithAlignment(ResultArg);
   llvm::Type *ResTy = ResultPtr.getElementType();
+  unsigned OpWidth = std::max(Op1Info.Width, Op2Info.Width);
 
   // Take the absolute value of the signed operand.
   llvm::Value *IsNegative = CGF.Builder.CreateICmpSLT(Signed, Zero);
@@ -1278,8 +1294,8 @@
   if (ResultInfo.Signed) {
 // Signed overflow occurs if the result is greater than INT_MAX or lesser
 // than INT_MIN, i.e when |Result| > (INT_MAX + IsNegative).
-auto IntMax = llvm::APInt::getSignedMaxValue(ResultInfo.Width)
-  .zextOrSelf(Op1Info.Width);
+auto IntMax =
+llvm::APInt::getSignedMaxValue(ResultInfo.Width).zextOrSelf(OpWidth);
 llvm::Value *MaxResult =
 CGF.Builder.CreateAdd(llvm::ConstantInt::get(OpTy, IntMax),
   CGF.Builder.CreateZExt(IsNegative, OpTy));
@@ -1297,9 +1313,9 @@
 llvm::Value *Underflow = CGF.Builder.CreateAnd(
 IsNegative, CGF.Builder.CreateIsNotNull(UnsignedResult));
 Overflow = CGF.Builder.CreateOr(UnsignedOverflow, Underflow);
-if (ResultInfo.Width < Op1Info.Width) {
+if (ResultInfo.Width < OpWidth) {
   auto IntMax =
-  llvm::APInt::getMaxValue(ResultInfo.Width).zext(Op1Info.Width);
+  llvm::APInt::getMaxValue(ResultInfo.Width).zext(OpWidth);
   llvm::Value *TruncOverflow = CGF.Builder.CreateICmpUGT(
   UnsignedResult, llvm::ConstantInt::get(OpTy, IntMax));
   Overflow = CGF.Builder.CreateOr(Overflow, TruncOverflow);


Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c

[PATCH] D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame.

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Actually, `LiveVariables` analysis is not the right place to deal with this 
problem; these changes would go into `SymbolReaper` (or directly into 
`ExprEngine`), because we're dealing with something that depends on the stack 
frame, while `LiveVariables` analysis only depends on the CFG.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55804



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


[PATCH] D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame.

2018-12-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D55804#1334106 , @xazax.hun wrote:

> Is there any downsides for using symbolic region for the construction target? 
> For me that would make perfect sense, since this is often modelled by passing 
> the address of the target into the callee. The programmer could do RVO like 
> thing by hand, so modeling automatic and manual RVO the same way would be the 
> least surprising in my opinion.


Hmm, let me actually see how hard it is. The main reason why i don't like it is 
that we still need to come up with a symbol to stuff into it, so we're kinda 
just delaying the inevitable. If we had, for instance, a region for the 
"implicit variable" that stores the return address, we could announce that this 
region is live for as long as the stack frame is on the stack, and its 
`SymbolRegionValue` (together with the `SymbolicRegion` around it) would be 
automatically kept alive. But if we make an anonymous `SymbolConjured` instead, 
we would also need to introduce a separate liveness hack to keep it alive, 
which should ideally be removed later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55804



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


[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

2018-12-18 Thread Hyrum Wright via Phabricator via cfe-commits
hwright added a comment.

@aaron.ballman I am both grateful and sad that I don't possess the same macro 
creativity as you do. :)

(I'm also sad that the various clang APIs aren't as well documented as I hoped. 
 The current solution works well, but it took a while and a bit of consultation 
with some other llvm'ers to get it right.)




Comment at: test/clang-tidy/abseil-duration-comparison.cpp:146
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 0)
+  int a3 = VALUE_IF(1, d1, Double);

aaron.ballman wrote:
> What if this is changed to:
> ```
> #define VALUE_IF(v, e, type) (v ? (5 > 
> VALUE_IF_2(absl::To##type##Seconds(e))) : 0)
> ```
This also doesn't transform.


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

https://reviews.llvm.org/D55784



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


[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

2018-12-18 Thread Hyrum Wright via Phabricator via cfe-commits
hwright updated this revision to Diff 178733.
hwright marked 3 inline comments as done.
hwright added a comment.

Another test.


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

https://reviews.llvm.org/D55784

Files:
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  test/clang-tidy/abseil-duration-comparison.cpp

Index: test/clang-tidy/abseil-duration-comparison.cpp
===
--- test/clang-tidy/abseil-duration-comparison.cpp
+++ test/clang-tidy/abseil-duration-comparison.cpp
@@ -127,6 +127,33 @@
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison]
   // CHECK-FIXES: absl::Milliseconds((y + 5) * 10) > d1;
 
+  // We should still transform the expression inside this macro invocation
+#define VALUE_IF(v, e) v ? (e) : 0
+  int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform comparison in the duration domain [abseil-duration-comparison]
+  // CHECK-FIXES: VALUE_IF(1, absl::Seconds(5) > d1);
+#undef VALUE_IF
+
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e) v ? VALUE_IF_2(e) : VALUE_IF_2(0)
+  int a2 = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: perform comparison in the duration domain [abseil-duration-comparison]
+  // CHECK-FIXES: VALUE_IF(1, absl::Seconds(5) > d1);
+#undef VALUE_IF
+#undef VALUE_IF_2
+
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 0)
+  int a3 = VALUE_IF(1, d1, Double);
+#undef VALUE_IF
+#undef VALUE_IF_2
+
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e, type) (v ? (5 > VALUE_IF_2(absl::To##type##Seconds(e))) : 0)
+  int a4 = VALUE_IF(1, d1, Double);
+#undef VALUE_IF
+#undef VALUE_IF_2
+
   // These should not match
   b = 6 < 4;
 
Index: clang-tidy/abseil/DurationSubtractionCheck.cpp
===
--- clang-tidy/abseil/DurationSubtractionCheck.cpp
+++ clang-tidy/abseil/DurationSubtractionCheck.cpp
@@ -42,10 +42,8 @@
   if (!Scale)
 return;
 
-  llvm::Optional RhsReplacement =
+  std::string RhsReplacement =
   rewriteExprFromNumberToDuration(Result, *Scale, Binop->getRHS());
-  if (!RhsReplacement)
-return;
 
   const Expr *LhsArg = Result.Nodes.getNodeAs("lhs_arg");
 
@@ -54,7 +52,7 @@
  Binop->getSourceRange(),
  (llvm::Twine("absl::") + FuncDecl->getName() + "(" +
   tooling::fixit::getText(*LhsArg, *Result.Context) + " - " +
-  *RhsReplacement + ")")
+  RhsReplacement + ")")
  .str());
 }
 
Index: clang-tidy/abseil/DurationRewriter.h
===
--- clang-tidy/abseil/DurationRewriter.h
+++ clang-tidy/abseil/DurationRewriter.h
@@ -65,7 +65,7 @@
 
 /// Assuming `Node` has type `double` or `int` representing a time interval of
 /// `Scale`, return the expression to make it a suitable `Duration`.
-llvm::Optional rewriteExprFromNumberToDuration(
+std::string rewriteExprFromNumberToDuration(
 const ast_matchers::MatchFinder::MatchResult , DurationScale Scale,
 const Expr *Node);
 
Index: clang-tidy/abseil/DurationRewriter.cpp
===
--- clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tidy/abseil/DurationRewriter.cpp
@@ -183,14 +183,11 @@
   return ScaleIter->second;
 }
 
-llvm::Optional rewriteExprFromNumberToDuration(
+std::string rewriteExprFromNumberToDuration(
 const ast_matchers::MatchFinder::MatchResult , DurationScale Scale,
 const Expr *Node) {
   const Expr  = *Node->IgnoreParenImpCasts();
 
-  if (RootNode.getBeginLoc().isMacroID())
-return llvm::None;
-
   // First check to see if we can undo a complimentary function call.
   if (llvm::Optional MaybeRewrite =
   rewriteInverseDurationCall(Result, Scale, RootNode))
Index: clang-tidy/abseil/DurationComparisonCheck.cpp
===
--- clang-tidy/abseil/DurationComparisonCheck.cpp
+++ clang-tidy/abseil/DurationComparisonCheck.cpp
@@ -19,6 +19,26 @@
 namespace tidy {
 namespace abseil {
 
+/// Return `true` if `E` is a either: not a macro at all; or an argument to
+/// one. In the latter case, we should still transform it.
+static bool IsValidMacro(const MatchFinder::MatchResult ,
+ const Expr *E) {
+  if (!E->getBeginLoc().isMacroID())
+return true;
+
+  SourceLocation Loc = E->getBeginLoc();
+  // We want to get closer towards the initial macro typed into the source only
+  // if the location is being expanded as a macro argument.
+  while (Result.SourceManager->isMacroArgExpansion(Loc)) {
+// We are calling getImmediateMacroCallerLoc, 

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

2018-12-18 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

This is awesome. Please wait for either @NoQ or @george.karpenkov to have the 
final say.


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

https://reviews.llvm.org/D55792



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


[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

2018-12-18 Thread Michał Górny via Phabricator via cfe-commits
mgorny marked 2 inline comments as done.
mgorny added inline comments.



Comment at: lib/Driver/ToolChains/NetBSD.cpp:465
+  const SanitizerArgs  = getSanitizerArgs();
+  if (SanArgs.needsAsanRt() || SanArgs.needsHwasanRt() ||
+  SanArgs.needsDfsanRt() || SanArgs.needsLsanRt() ||

krytarowski wrote:
> There should be some way to express any sanitizer (-fsanitize) and we want 
> _REENTRANT for all of them.
I've added it in dependent commit.


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

https://reviews.llvm.org/D55654



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


[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

2018-12-18 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 178701.
mgorny edited the summary of this revision.

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

https://reviews.llvm.org/D55654

Files:
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/NetBSD.h


Index: lib/Driver/ToolChains/NetBSD.h
===
--- lib/Driver/ToolChains/NetBSD.h
+++ lib/Driver/ToolChains/NetBSD.h
@@ -76,6 +76,10 @@
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  void addClangTargetOptions(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const 
override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -457,3 +457,11 @@
   }
   return Res;
 }
+
+void NetBSD::addClangTargetOptions(const ArgList &,
+   ArgStringList ,
+   Action::OffloadKind) const {
+  const SanitizerArgs  = getSanitizerArgs();
+  if (SanArgs.hasAnySanitizer())
+CC1Args.push_back("-D_REENTRANT");
+}


Index: lib/Driver/ToolChains/NetBSD.h
===
--- lib/Driver/ToolChains/NetBSD.h
+++ lib/Driver/ToolChains/NetBSD.h
@@ -76,6 +76,10 @@
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  void addClangTargetOptions(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -457,3 +457,11 @@
   }
   return Res;
 }
+
+void NetBSD::addClangTargetOptions(const ArgList &,
+   ArgStringList ,
+   Action::OffloadKind) const {
+  const SanitizerArgs  = getSanitizerArgs();
+  if (SanArgs.hasAnySanitizer())
+CC1Args.push_back("-D_REENTRANT");
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55832: [clang] [Driver] Add .hasAnySanitizer() to SanitizerArgs

2018-12-18 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: krytarowski, vitalybuka, samsonov, pcc.

Add a simple method to query whether any sanitizer was enabled,
via SanitizerArgs.  This will be used in the NetBSD driver to pass
additional definitions that are required by all sanitizers.


Repository:
  rC Clang

https://reviews.llvm.org/D55832

Files:
  include/clang/Driver/SanitizerArgs.h


Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -82,6 +82,7 @@
   bool needsUnwindTables() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }
   bool hasCrossDsoCfi() const { return CfiCrossDso; }
+  bool hasAnySanitizer() const { return !Sanitizers.empty(); }
   void addArgs(const ToolChain , const llvm::opt::ArgList ,
llvm::opt::ArgStringList , types::ID InputType) const;
 };


Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -82,6 +82,7 @@
   bool needsUnwindTables() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }
   bool hasCrossDsoCfi() const { return CfiCrossDso; }
+  bool hasAnySanitizer() const { return !Sanitizers.empty(); }
   void addArgs(const ToolChain , const llvm::opt::ArgList ,
llvm::opt::ArgStringList , types::ID InputType) const;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

2018-12-18 Thread pierre gousseau via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC349508: [Driver][PS4] Do not implicitly link against asan or 
ubsan if -nostdlib or… (authored by pgousseau, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D55712

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/PS4CPU.cpp
  test/Driver/fsanitize.c
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -673,6 +673,13 @@
 // CHECK-AUBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
 // CHECK-AUBSAN-PS4: -lSceDbgAddressSanitizer_stub_weak
 
+// RUN: %clang -fsanitize=address,undefined %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
+// RUN: -shared \
+// RUN: -nostdlib \
+// RUN:   | FileCheck --check-prefix=CHECK-NOLIB-PS4 %s
+// CHECK-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
+
 // RUN: %clang -fsanitize=efficiency-cache-frag %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-ESAN-LINUX %s
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -753,6 +753,10 @@
 // CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
 // CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
 // CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nostdlib %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs 
-nostdlib  %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// CHECK-ASAN-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-MINIMAL
 // CHECK-ASAN-MINIMAL: error: invalid argument '-fsanitize-minimal-runtime' 
not allowed with '-fsanitize=address'
Index: lib/Driver/ToolChains/PS4CPU.cpp
===
--- lib/Driver/ToolChains/PS4CPU.cpp
+++ lib/Driver/ToolChains/PS4CPU.cpp
@@ -121,7 +121,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
@@ -190,7 +191,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 const char *crt1 = nullptr;
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4097,7 +4097,8 @@
 ABICompatArg->render(Args, CmdArgs);
 
   // Add runtime flag for PS4 when PGO, coverage, or sanitizers are enabled.
-  if (RawTriple.isPS4CPU()) {
+  if (RawTriple.isPS4CPU() &&
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 PS4cpu::addProfileRTArgs(TC, Args, CmdArgs);
 PS4cpu::addSanitizerArgs(TC, CmdArgs);
   }


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -673,6 +673,13 @@
 // CHECK-AUBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
 // CHECK-AUBSAN-PS4: -lSceDbgAddressSanitizer_stub_weak
 
+// RUN: %clang -fsanitize=address,undefined %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
+// RUN: -shared \
+// RUN: -nostdlib \
+// RUN:   | FileCheck --check-prefix=CHECK-NOLIB-PS4 %s
+// CHECK-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
+
 // RUN: %clang -fsanitize=efficiency-cache-frag %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-ESAN-LINUX %s
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -753,6 +753,10 @@
 // CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
 // CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
 // CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
+// RUN: %clang -target 

r349508 - [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

2018-12-18 Thread Pierre Gousseau via cfe-commits
Author: pgousseau
Date: Tue Dec 18 09:03:35 2018
New Revision: 349508

URL: http://llvm.org/viewvc/llvm-project?rev=349508=rev
Log:
[Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or 
-nodefaultlibs on PS4.

NFC for targets other than PS4.

Respect -nostdlib and -nodefaultlibs when enabling asan or ubsan.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=349508=349507=349508=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Dec 18 09:03:35 2018
@@ -4097,7 +4097,8 @@ void Clang::ConstructJob(Compilation ,
 ABICompatArg->render(Args, CmdArgs);
 
   // Add runtime flag for PS4 when PGO, coverage, or sanitizers are enabled.
-  if (RawTriple.isPS4CPU()) {
+  if (RawTriple.isPS4CPU() &&
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 PS4cpu::addProfileRTArgs(TC, Args, CmdArgs);
 PS4cpu::addSanitizerArgs(TC, CmdArgs);
   }

Modified: cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp?rev=349508=349507=349508=diff
==
--- cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp Tue Dec 18 09:03:35 2018
@@ -121,7 +121,8 @@ static void ConstructPS4LinkJob(const To
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
@@ -190,7 +191,8 @@ static void ConstructGoldLinkJob(const T
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 const char *crt1 = nullptr;

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=349508=349507=349508=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Dec 18 09:03:35 2018
@@ -753,6 +753,10 @@
 // CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
 // CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
 // CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nostdlib %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs 
-nostdlib  %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// CHECK-ASAN-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-MINIMAL
 // CHECK-ASAN-MINIMAL: error: invalid argument '-fsanitize-minimal-runtime' 
not allowed with '-fsanitize=address'

Modified: cfe/trunk/test/Driver/sanitizer-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sanitizer-ld.c?rev=349508=349507=349508=diff
==
--- cfe/trunk/test/Driver/sanitizer-ld.c (original)
+++ cfe/trunk/test/Driver/sanitizer-ld.c Tue Dec 18 09:03:35 2018
@@ -673,6 +673,13 @@
 // CHECK-AUBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
 // CHECK-AUBSAN-PS4: -lSceDbgAddressSanitizer_stub_weak
 
+// RUN: %clang -fsanitize=address,undefined %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
+// RUN: -shared \
+// RUN: -nostdlib \
+// RUN:   | FileCheck --check-prefix=CHECK-NOLIB-PS4 %s
+// CHECK-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
+
 // RUN: %clang -fsanitize=efficiency-cache-frag %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-ESAN-LINUX %s


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


[PATCH] D55830: [clang] [Basic] Correct description of SanitizerSet.empty()

2018-12-18 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: bkramer, samsonov, krytarowski.

It seems to have gotten inverted.


Repository:
  rC Clang

https://reviews.llvm.org/D55830

Files:
  include/clang/Basic/Sanitizers.h


Index: include/clang/Basic/Sanitizers.h
===
--- include/clang/Basic/Sanitizers.h
+++ include/clang/Basic/Sanitizers.h
@@ -66,7 +66,7 @@
   /// Disable the sanitizers specified in \p K.
   void clear(SanitizerMask K = SanitizerKind::All) { Mask &= ~K; }
 
-  /// Returns true if at least one sanitizer is enabled.
+  /// Returns true if no sanitizers are enabled.
   bool empty() const { return Mask == 0; }
 
   /// Bitmask of enabled sanitizers.


Index: include/clang/Basic/Sanitizers.h
===
--- include/clang/Basic/Sanitizers.h
+++ include/clang/Basic/Sanitizers.h
@@ -66,7 +66,7 @@
   /// Disable the sanitizers specified in \p K.
   void clear(SanitizerMask K = SanitizerKind::All) { Mask &= ~K; }
 
-  /// Returns true if at least one sanitizer is enabled.
+  /// Returns true if no sanitizers are enabled.
   bool empty() const { return Mask == 0; }
 
   /// Bitmask of enabled sanitizers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

2018-12-18 Thread pierre gousseau via Phabricator via cfe-commits
pgousseau added a comment.

In D55712#1334603 , @probinson wrote:

> I missed that filcab had said ok internally.  Go ahead Pierre.


Will do! Thanks for reviewing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55712



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


[PATCH] D55828: [clang] [Driver] Disable -faddrsig by default on NetBSD

2018-12-18 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: krytarowski, pcc.

Avoid passing -faddrsig by default on NetBSD.  This platform is still
using old GNU binutils that crashes on executables containing those
sections.


Repository:
  rC Clang

https://reviews.llvm.org/D55828

Files:
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5272,7 +5272,8 @@
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
(TC.getTriple().isOSBinFormatELF() ||
 TC.getTriple().isOSBinFormatCOFF()) &&
-   TC.useIntegratedAs()))
+   TC.useIntegratedAs() &&
+   RawTriple.getOS() != llvm::Triple::NetBSD))
 CmdArgs.push_back("-faddrsig");
 
   // Finally add the compile command to the compilation.


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5272,7 +5272,8 @@
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
(TC.getTriple().isOSBinFormatELF() ||
 TC.getTriple().isOSBinFormatCOFF()) &&
-   TC.useIntegratedAs()))
+   TC.useIntegratedAs() &&
+   RawTriple.getOS() != llvm::Triple::NetBSD))
 CmdArgs.push_back("-faddrsig");
 
   // Finally add the compile command to the compilation.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55437: [Index] Index declarations in lambda expression.

2018-12-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 178688.
hokein marked an inline comment as done.
hokein added a comment.

Fix a nit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55437

Files:
  lib/Index/IndexBody.cpp
  test/Index/cxx11-lambdas.cpp


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -7,6 +7,7 @@
 auto lambda = [, localB] (Integer x) -> Integer {
   return localA + localB + x;
 };
+auto lambda2 = [](Integer y) {};
   }
 };
 
@@ -26,8 +27,10 @@
 // RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file -std=c++11 
%s | FileCheck -check-prefix=CHECK-INDEX %s
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: 
VariableRef=localA:6:9 | loc: 7:21
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: 
VariableRef=localB:6:17 | loc: 7:29
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
ParmDecl=x:7:46 (Definition) | loc: 7:46
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:52
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:38
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: 
DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: 
DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
DeclRefExpr=x:7:46 | loc: 8:32
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: y | USR: 
c:cxx11-lambdas.cpp@244@S@X@F@f#@Sa@F@operator()#I#1@y | lang: C | cursor: 
ParmDecl=y:10:31 (Definition) | loc: 10:31
\ No newline at end of file
Index: lib/Index/IndexBody.cpp
===
--- lib/Index/IndexBody.cpp
+++ lib/Index/IndexBody.cpp
@@ -9,6 +9,7 @@
 
 #include "IndexingContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/ASTLambda.h"
 
 using namespace clang;
 using namespace clang::index;
@@ -454,6 +455,16 @@
 }
 return true;
   }
+
+  bool VisitParmVarDecl(ParmVarDecl* D) {
+// Index the parameters of lambda expression.
+if (IndexCtx.shouldIndexFunctionLocalSymbols()) {
+  const auto *DC = D->getDeclContext();
+  if (DC && isLambdaCallOperator(DC))
+IndexCtx.handleDecl(D);
+}
+return true;
+  }
 };
 
 } // anonymous namespace


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -7,6 +7,7 @@
 auto lambda = [, localB] (Integer x) -> Integer {
   return localA + localB + x;
 };
+auto lambda2 = [](Integer y) {};
   }
 };
 
@@ -26,8 +27,10 @@
 // RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file -std=c++11 %s | FileCheck -check-prefix=CHECK-INDEX %s
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: VariableRef=localA:6:9 | loc: 7:21
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: VariableRef=localB:6:17 | loc: 7:29
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: ParmDecl=x:7:46 (Definition) | loc: 7:46
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 7:52
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 7:38
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: DeclRefExpr=x:7:46 | loc: 8:32
+// 

[PATCH] D55437: [Index] Index declarations in lambda expression.

2018-12-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 178687.
hokein added a comment.

Rebase and simplify the test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55437

Files:
  lib/Index/IndexBody.cpp
  test/Index/cxx11-lambdas.cpp


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -7,6 +7,7 @@
 auto lambda = [, localB] (Integer x) -> Integer {
   return localA + localB + x;
 };
+auto lambda2 = [](Integer y) {};
   }
 };
 
@@ -26,8 +27,10 @@
 // RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file -std=c++11 
%s | FileCheck -check-prefix=CHECK-INDEX %s
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: 
VariableRef=localA:6:9 | loc: 7:21
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: 
VariableRef=localB:6:17 | loc: 7:29
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
ParmDecl=x:7:46 (Definition) | loc: 7:46
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:52
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:38
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: 
DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: 
DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
DeclRefExpr=x:7:46 | loc: 8:32
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: y | USR: 
c:cxx11-lambdas.cpp@244@S@X@F@f#@Sa@F@operator()#I#1@y | lang: C | cursor: 
ParmDecl=y:10:31 (Definition) | loc: 10:31
\ No newline at end of file
Index: lib/Index/IndexBody.cpp
===
--- lib/Index/IndexBody.cpp
+++ lib/Index/IndexBody.cpp
@@ -9,6 +9,7 @@
 
 #include "IndexingContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/ASTLambda.h"
 
 using namespace clang;
 using namespace clang::index;
@@ -454,6 +455,17 @@
 }
 return true;
   }
+
+  bool VisitParmVarDecl(ParmVarDecl* D) {
+// Index the parameters of lambda expression.
+if (IndexCtx.shouldIndexFunctionLocalSymbols()) {
+  const auto *DC = D->getDeclContext();
+  if (DC && isLambdaCallOperator(DC)) {
+IndexCtx.handleDecl(D);
+  }
+}
+return true;
+  }
 };
 
 } // anonymous namespace


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -7,6 +7,7 @@
 auto lambda = [, localB] (Integer x) -> Integer {
   return localA + localB + x;
 };
+auto lambda2 = [](Integer y) {};
   }
 };
 
@@ -26,8 +27,10 @@
 // RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file -std=c++11 %s | FileCheck -check-prefix=CHECK-INDEX %s
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: VariableRef=localA:6:9 | loc: 7:21
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: VariableRef=localB:6:17 | loc: 7:29
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: ParmDecl=x:7:46 (Definition) | loc: 7:46
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 7:52
 // CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: c:cxx11-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 7:38
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: DeclRefExpr=x:7:46 | loc: 8:32
+// CHECK-INDEX: 

r349506 - [NFC] Fix usage of Builder.insert(new Bitcast...)in CodeGenFunction

2018-12-18 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Tue Dec 18 08:22:21 2018
New Revision: 349506

URL: http://llvm.org/viewvc/llvm-project?rev=349506=rev
Log:
[NFC] Fix usage of Builder.insert(new Bitcast...)in CodeGenFunction

This is exactly a "CreateBitCast", so refactor this to get rid of a
'new'.

Note that this slightly changes the test, as the Builder is now
seemingly smart enough to fold one of the bitcasts into the annotation
call.

Change-Id: I1733fb1fdf91f5c9d88651067130b9a4e7b5ab67

Modified:
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/CodeGen/annotations-field.c

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=349506=349505=349506=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Tue Dec 18 08:22:21 2018
@@ -2243,7 +2243,7 @@ Address CodeGenFunction::EmitFieldAnnota
 // annotation on the first field of a struct and annotation on the struct
 // itself.
 if (VTy != CGM.Int8PtrTy)
-  V = Builder.Insert(new llvm::BitCastInst(V, CGM.Int8PtrTy));
+  V = Builder.CreateBitCast(V, CGM.Int8PtrTy);
 V = EmitAnnotationCall(F, V, I->getAnnotation(), D->getLocation());
 V = Builder.CreateBitCast(V, VTy);
   }

Modified: cfe/trunk/test/CodeGen/annotations-field.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/annotations-field.c?rev=349506=349505=349506=diff
==
--- cfe/trunk/test/CodeGen/annotations-field.c (original)
+++ cfe/trunk/test/CodeGen/annotations-field.c Tue Dec 18 08:22:21 2018
@@ -21,7 +21,6 @@ int main(int argc, char **argv) {
 // CHECK-NEXT: call i8* @llvm.ptr.annotation.p0i8({{.*}}str{{.*}}str{{.*}}i32 
8)
 // CHECK-NEXT: bitcast i8* {{.*}} to i32*
 gf.v = argc;
-// CHECK: bitcast i32* getelementptr inbounds (%struct.foo, %struct.foo* @gf, 
i32 0, i32 0) to i8*
-// CHECK-NEXT: call i8* @llvm.ptr.annotation.p0i8({{.*}}str{{.*}}str{{.*}}i32 
8)
+// CHECK: call i8* @llvm.ptr.annotation.p0i8(i8* bitcast (%struct.foo* @gf to 
i8*), {{.*}}str{{.*}}str{{.*}}i32 8)
 return 0;
 }


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


[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2018-12-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: docs/clang-tidy/index.rst:262
+
+Clang-tidy integrated
+-

So how about moving this chapter to a separate page and adding a link to it at 
the top of this document? While certainly useful, the information in this 
section has a different focus from the rest of the documentation. Given that 
the document has grown significantly, I suggest to split out this section and 
the "Getting involved" section. (The latter can be done separately)


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

https://reviews.llvm.org/D54945



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


[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

2018-12-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: test/clang-tidy/abseil-duration-comparison.cpp:131
+  // We should still transform the expression inside this macro invocation
+#define VALUE_IF(v, e) v ? (e) : 0
+  int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));

hwright wrote:
> aaron.ballman wrote:
> > Can you add another example that has one more level of macro arg expansion? 
> > e.g.,
> > ```
> > #define VALUE_IF_2(e) (e)
> > #define VALUE_IF(v, e) v ? VALUE_IF_2(e) : VALUE_IF_2(0)
> > int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));
> > ```
> > Which makes me wonder -- is this transformation valid in all cases? For 
> > instance, how do the fixes look with this abomination?
> > ```
> > #define VALUE_IF_2(e) (e)
> > #define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 
> > 0)
> > int a = VALUE_IF(1, d1, Double);
> > ```
> > I would expect these shenanigans to be vanishingly rare, so if this turns 
> > out to be a bad FIXIT you may want to document it as a known deficiency if 
> > you don't feel like handling such a case.
> I've added both these as test cases.
> 
> The first one works as expected, and I'm pleasantly surprised to see that the 
> second one does as well (which is to say that it doesn't transform).
Great, thank you!



Comment at: test/clang-tidy/abseil-duration-comparison.cpp:146
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 0)
+  int a3 = VALUE_IF(1, d1, Double);

What if this is changed to:
```
#define VALUE_IF(v, e, type) (v ? (5 > VALUE_IF_2(absl::To##type##Seconds(e))) 
: 0)
```


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

https://reviews.llvm.org/D55784



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


[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

2018-12-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D55792#1334644 , @Szelethus wrote:

> Global nit: I guess having both DESC and DOCS as variable/macro arg names is 
> confusing. Can we instead use DOCSURI/DocsUri?


Sure, I'll make that change.

> In D55792#1334339 , @aaron.ballman 
> wrote:
> 
>> In D55792#1334291 , @Szelethus 
>> wrote:
>>
>> > Thank you so much for working on this! The lack of a 
>> >  I guess it'd be better to generate the entire URI in the tblgen file, and 
>> > also allow plugins to register their own through `CheckerRegistry`. Sarif 
>> > lacking the support for them is one thing, but adding a new field to the 
>> > `Checker` class in `CheckerBase.td` I think should be accompanied by 
>> > extending `CheckerRegistry::CheckerInfo` and `CheckerRegistry::getChecker`.
>>
>>
>> Good point about the checker registry, I'll make those changes. However, I'm 
>> a bit less certain about generating the entire URI in the tablegen file -- 
>> that adds a lot of string literals to the binary when I think we don't have 
>> to. However, perhaps that's not a huge concern and having the information in 
>> its final form in the .inc file is a better approach. WDYT?
> 
> 
> Well, we already store checker- and the quite lengthy -analyzer-config 
> descriptions... but I'm not sure thats a good point.
>  However, plugin checkers being able to specify uris such as 
> "mycompany.internal.supersecret.idk/dontshare/checkers/tradesecrets.html" 
> would be neat.
> 
> One more thing to consider is that if Sphinx lands after 8.0.0., the uri will 
> most probably change. We can always redirect, but thats a thing to consider. 
> I'm not sure whether there's any likelihood of it being completed until the 
> new release branch is created. But I don't think there's any change needed 
> for this patch just because of this.

Yeah, I wound up pushing all of the data into the table generator as suggested. 
The plugin registration was what did it for me -- when registering a checker, 
we should have the full link to the help URI rather than try to build it from 
pieces that may not be correct for plugins.


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

https://reviews.llvm.org/D55792



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


[PATCH] D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array.

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, looks great.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55771



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


[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

2018-12-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clangd/index/Background.cpp:259
+  // if this thread sees the older version but finishes later. This should
+  // be rare in practice.
+  DigestIt.first->second = Hash;

ilya-biryukov wrote:
> > "should be rare in practice"
> And yet, can we avoid this altogether?
> 
> Also, I believe it won't be rare. When processing multiple different TUs, we 
> can easily run into the same header multiple times, possibly with the 
> different contents.
> E.g. imagine the index for the whole of clang is being built and the user is 
> editing `Sema.h` at the same time. We'll definitely be running into this case 
> over and over again.
Well I am open to ideas, but currently there is no way of knowing whether this 
is the newer version for the file. Because only information we have is the 
digest is different than what we currently have and this doesn't yield any 
chronological information.



Comment at: clangd/index/Background.cpp:338
 std::lock_guard Lock(DigestsMu);
-if (IndexedFileDigests.lookup(AbsolutePath) == Hash) {
-  vlog("No need to index {0}, already up to date", AbsolutePath);

ilya-biryukov wrote:
> This looks like an important optimization. Did we move it to some other 
> place? Why should we remove it?
We kind of moved it into loadShards logic by not running indexing for same file 
multiple times. I needed to delete this check since we might wanna run indexing 
on a TU, even if it is up-to-date, to get new symbols coming from one of it's 
includes.



Comment at: clangd/index/Background.cpp:365
   StringMap FilesToUpdate;
+  // Main file always needs to be updated even if it has no symbols.
+  FilesToUpdate[AbsolutePath] = Hash;

ilya-biryukov wrote:
> The comment does not mention **why** should we update it. Is it because of 
> the dependencies?
Nice catch, that was the case in one of my experiments but currently during the 
update we store shards for every source file that exists in include graph even 
if it has no symbols So it is no longer necessary.



Comment at: clangd/index/Background.cpp:386
   Action->EndSourceFile();
+  if (!Index.Symbols)
+return createStringError(inconvertibleErrorCode(), "Uncompilable errors");

ilya-biryukov wrote:
> Was this error possible before this change too?
Yes and it got fixed by D55650, just deleting the check it will arrive once I 
rebase.



Comment at: clangd/index/Background.cpp:481
+  if (!Buf)
+continue;
+  // If digests match then dependency doesn't need re-indexing.

ilya-biryukov wrote:
> maybe log the error? would be nice to somehow recover too, but not sure what 
> we can do here.
Well, if this happens we don't have access to file's contents for some reason. 
I don't think we can do anything. We might actually want to skip loading these 
files into index, since they are most likely gone and symbols coming from them 
won't be accessible. WDYT?



Comment at: clangd/index/Background.cpp:508
+BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
+auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);
+for (const auto  : Dependencies) {

ilya-biryukov wrote:
> Maybe return a `vector` with dependencies that need reindexing?  We 
> ignore the dependencies that don't need to be reindexed anyway.
Yes we ignore them for now, but they might change in one of the consecutive 
checks. And if that happens we don't need to schedule another TU for 
re-indexing that dependency, since we mark it as already indexed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55224



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


[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

2018-12-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 178685.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55818

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  clangd/index/Background.cpp
  clangd/index/SymbolCollector.cpp

Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -51,37 +51,17 @@
 //
 // The Path can be a path relative to the build directory, or retrieved from
 // the SourceManager.
-Optional toURI(const SourceManager , StringRef Path,
-const SymbolCollector::Options ) {
-  SmallString<128> AbsolutePath(Path);
-  if (std::error_code EC =
-  SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
-  AbsolutePath))
-log("Warning: could not make absolute file: {0}", EC.message());
-  if (sys::path::is_absolute(AbsolutePath)) {
-// Handle the symbolic link path case where the current working directory
-// (getCurrentWorkingDirectory) is a symlink./ We always want to the real
-// file path (instead of the symlink path) for the  C++ symbols.
-//
-// Consider the following example:
-//
-//   src dir: /project/src/foo.h
-//   current working directory (symlink): /tmp/build -> /project/src/
-//
-// The file path of Symbol is "/project/src/foo.h" instead of
-// "/tmp/build/foo.h"
-if (const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
-sys::path::parent_path(AbsolutePath.str( {
-  StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
-  SmallString<128> AbsoluteFilename;
-  sys::path::append(AbsoluteFilename, DirName,
-sys::path::filename(AbsolutePath.str()));
-  AbsolutePath = AbsoluteFilename;
-}
-  } else if (!Opts.FallbackDir.empty()) {
-sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
+std::string toURI(const SourceManager , llvm::StringRef Path,
+  const SymbolCollector::Options ) {
+  llvm::SmallString<128> AbsolutePath(Path);
+  if (auto CanonPath =
+  getCanonicalPath(SM.getFileManager().getFile(Path), SM)) {
+AbsolutePath = *CanonPath;
   }
-
+  // We don't perform is_absolute check in an else branch because makeAbsolute
+  // might return a relative path on some InMemoryFileSystems.
+  if (!sys::path::is_absolute(AbsolutePath) && !Opts.FallbackDir.empty())
+sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
   sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
   return URI::create(AbsolutePath).toString();
 }
@@ -211,17 +191,17 @@
   const SymbolCollector::Options ,
   const clang::LangOptions ,
   std::string ) {
-  auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
-  if (!U)
+  auto Path = SM.getFilename(TokLoc);
+  if (Path.empty())
 return None;
-  FileURIStorage = std::move(*U);
+  FileURIStorage = toURI(SM, Path, Opts);
   SymbolLocation Result;
   Result.FileURI = FileURIStorage.c_str();
   auto Range = getTokenRange(TokLoc, SM, LangOpts);
   Result.Start = Range.first;
   Result.End = Range.second;
 
-  return std::move(Result);
+  return Result;
 }
 
 // Checks whether \p ND is a definition of a TagDecl (class/struct/enum/union)
@@ -487,11 +467,7 @@
 if (Found == URICache.end()) {
   if (auto *FileEntry = SM.getFileEntryForID(FID)) {
 auto FileURI = toURI(SM, FileEntry->getName(), Opts);
-if (!FileURI) {
-  log("Failed to create URI for file: {0}\n", FileEntry);
-  FileURI = ""; // reset to empty as we also want to cache this case.
-}
-Found = URICache.insert({FID, *FileURI}).first;
+Found = URICache.insert({FID, FileURI}).first;
   } else {
 // Ignore cases where we can not find a corresponding file entry
 // for the loc, thoses are not interesting, e.g. symbols formed
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -96,26 +96,20 @@
 createFileFilter(const llvm::StringMap ,
  llvm::StringMap ) {
   return [, ](const SourceManager , FileID FID) {
-StringRef Path;
-if (const auto *F = SM.getFileEntryForID(FID))
-  Path = F->getName();
-if (Path.empty())
+const auto *F = SM.getFileEntryForID(FID);
+if (!F)
   return false; // Skip invalid files.
-SmallString<128> AbsPath(Path);
-if (std::error_code EC =
-SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsPath)) {
-  elog("Warning: could not make absolute file: {0}", EC.message());
+ 

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

2018-12-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 7 inline comments as done.
kadircet added inline comments.



Comment at: clangd/SourceCode.h:91
+/// possible.
+llvm::Expected getCanonicalPath(const FileEntry *F,
+ const SourceManager );

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > A different name proposal: `canonicalizePath`? WDYT?
> Maybe leave it `Optional`?
> Semantic differences make it very hard to verify the code after rewrite is 
> correct, as `Expected` requires checking for errors.
but it rather gets the canonical path for a FileEntry rather than 
canonicalizing a path ?



Comment at: clangd/index/SymbolCollector.cpp:58
+  if (auto CanonPath =
+  getCanonicalPath(SM.getFileManager().getFile(Path), SM)) {
+AbsolutePath = *CanonPath;

ilya-biryukov wrote:
> This looks like the only change that might subtly change the behavior of our 
> program. I believe things might break because of this.
> Could we keep this review closer to an NFC and avoid introducing this subtle 
> difference here? Happy to take a look at a separate patch with it.
As discussed offline this is not introducing a behavior change.



Comment at: clangd/index/SymbolCollector.cpp:64
   }
-
+  if (!sys::path::is_absolute(AbsolutePath) && !Opts.FallbackDir.empty())
+sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);

ilya-biryukov wrote:
> Why not do this only in the `else` branch?
Adding a comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55818



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


  1   2   >