[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

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



Comment at: llvm/docs/LangRef.rst:1659-1661
+that hardening. It should also be possible to *not* harden a hot and/or 
safe
+function and have code inlined there *not* be hardened (even if the generic
+form is hardened).

kristof.beyls wrote:
> It feels wrong to me to have source code that is annotated to get hardened, 
> but that actually will not get hardened (whether it is due to it being 
> inlined somewhere or due to any other automatic 
> behind-the-back-of-the-developer transformation). I fear this may lower trust 
> in the protection this attribute provides.
> I assume there is a use case where the developer wants to indicate "no 
> hardening in this function nor in any functions inlined here". If that needs 
> to be supported, my feel is that we may need to support that in another way.
> I guess that there must be some cases where just duplicating the function to 
> be inlined in the source code into a hardened and a non-hardened version 
> could be too hard to do for some programs.
> So, in short, I don't know what the best solution here is. I just want to 
> raise my concern that I don't think it's a good idea that functions that are 
> marked to be hardened end up not getting hardened under some circumstances.
I actually started out with this, so I'm very sympathetic to the perspective.

Given that you prefer it, let's start with this very conservative stance. We 
can always add specific mechanisms to be less conservative later, but I agree 
that it seems much better to start with a safe position.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 162325.
chandlerc added a comment.

Move to a more conservative model suggested by Kristof.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-speculative-load-hardening.c
  clang/test/Driver/x86-target-features.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-gather.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
  llvm/test/CodeGen/X86/speculative-load-hardening.ll
  llvm/test/Transforms/Inline/attributes.ll

Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -26,6 +26,10 @@
   ret i32 %i
 }
 
+define i32 @slh_callee(i32 %i) speculative_load_hardening {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_callee(i32 %i) alwaysinline {
   ret i32 %i
 }
@@ -161,6 +165,28 @@
 ; CHECK-NEXT: ret i32
 }
 
+; Can inline a normal function into an SLH'ed function.
+define i32 @test_caller_slh(i32 %i) speculative_load_hardening {
+; CHECK-LABEL: @test_caller_slh(
+; CHECK-SAME: ) [[SLH:.*]] {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @noattr_callee(i32 %i)
+  ret i32 %callee
+}
+
+; Can inline a SLH'ed function into a normal one, propagating SLH.
+define i32 @test_callee_slh(i32 %i) {
+; CHECK-LABEL: @test_callee_slh(
+; CHECK-SAME: ) [[SLH:.*]] {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @slh_callee(i32 %i)
+  ret i32 %callee
+}
+
 ; Check that a function doesn't get inlined if target-cpu strings don't match
 ; exactly.
 define i32 @test_target_cpu_callee0(i32 %i) "target-cpu"="corei7" {
@@ -384,6 +410,7 @@
 ; CHECK-NEXT: ret i32
 }
 
+; CHECK: attributes [[SLH]] = { speculative_load_hardening }
 ; CHECK: attributes [[FPMAD_FALSE]] = { "less-precise-fpmad"="false" }
 ; CHECK: attributes [[FPMAD_TRUE]] = { "less-precise-fpmad"="true" }
 ; CHECK: attributes [[NOIMPLICITFLOAT]] = { noimplicitfloat }
Index: llvm/test/CodeGen/X86/speculative-load-hardening.ll
===
--- llvm/test/CodeGen/X86/speculative-load-hardening.ll
+++ llvm/test/CodeGen/X86/speculative-load-hardening.ll
@@ -1,14 +1,14 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening | FileCheck %s --check-prefix=X64
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
 ;
 ; FIXME: Add support for 32-bit and other EH ABIs.
 
 declare void @leak(i32 %v1, i32 %v2)
 
 declare void @sink(i32)
 
-define i32 @test_trivial_entry_load(i32* %ptr) {
+define i32 @test_trivial_entry_load(i32* %ptr) speculative_load_hardening {
 ; X64-LABEL: test_trivial_entry_load:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:movq %rsp, %rcx
@@ -29,7 +29,7 @@
   ret i32 %v
 }
 
-define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) {
+define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) speculative_load_hardening {
 ; X64-LABEL: test_basic_conditions:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %r15
@@ -189,7 +189,7 @@
   ret void
 }
 
-define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_loop:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %rbp
@@ -293,7 +293,7 @@
   ret void
 }
 
-define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_nested_loop:
 ; 

[PATCH] D51178: [ASTImporter] Add test for importing anonymous namespaces.

[Aleksei Sidorin via Phabricator via cfe-commits]

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



Comment at: test/Import/cxx-anon-namespace/test.cpp:10
+// This is for the nested anonymous namespace.
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''

Could you please change a comment a bit and point that implicit using 
directives are created by Sema for anonymous namespaces?


Repository:
  rC Clang

https://reviews.llvm.org/D51178



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added inline comments.



Comment at: llvm/docs/LangRef.rst:1659-1661
+that hardening. It should also be possible to *not* harden a hot and/or 
safe
+function and have code inlined there *not* be hardened (even if the generic
+form is hardened).

It feels wrong to me to have source code that is annotated to get hardened, but 
that actually will not get hardened (whether it is due to it being inlined 
somewhere or due to any other automatic behind-the-back-of-the-developer 
transformation). I fear this may lower trust in the protection this attribute 
provides.
I assume there is a use case where the developer wants to indicate "no 
hardening in this function nor in any functions inlined here". If that needs to 
be supported, my feel is that we may need to support that in another way.
I guess that there must be some cases where just duplicating the function to be 
inlined in the source code into a hardened and a non-hardened version could be 
too hard to do for some programs.
So, in short, I don't know what the best solution here is. I just want to raise 
my concern that I don't think it's a good idea that functions that are marked 
to be hardened end up not getting hardened under some circumstances.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I'd say LGTM since it's an introduction of any sort of **runtime** within the 
LLVM project scope that deals with SEH specifically. So far all the published 
code is pretty much exclusively related to Clang/LLVM IR and MC support for 
codegen of SEH related code, but with an implicit reliance on core Windows 
runtime and other goodies being around. Even if not super polished, as long as 
it does not cause regressions in other parts of libunwind, it's a start 
nontheless.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

Re: r294176 - [AVR] Add support for the full set of inline asm constraints

[Chandler Carruth via cfe-commits]

This was due to r340519. I've fixed it in r340596 to clean things up.

On Thu, Aug 23, 2018 at 8:20 PM Chandler Carruth 
wrote:

> Trying new address again...
>
>
> On Thu, Aug 23, 2018 at 8:17 PM Chandler Carruth 
> wrote:
>
>> Sorry for ancient thread revival, but...
>>
>> On Mon, Feb 6, 2017 at 2:10 AM Dylan McKay via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: dylanmckay
>>> Date: Mon Feb  6 03:01:59 2017
>>> New Revision: 294176
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=294176&view=rev
>>> Log:
>>> [AVR] Add support for the full set of inline asm constraints
>>>
>>> Summary:
>>> Previously the method would simply return false, causing every single
>>> inline assembly constraint to trigger a compile error.
>>>
>>> This adds inline assembly constraint support for the AVR target.
>>>
>>> This patch is derived from the code in
>>> AVRISelLowering::getConstraintType.
>>>
>>> More details can be found on the AVR-GCC reference wiki
>>> http://www.nongnu.org/avr-libc/user-manual/inline_asm.html
>>>
>>> Reviewers: jroelofs, asl
>>>
>>> Reviewed By: asl
>>>
>>> Subscribers: asl, ahatanak, saaadhu, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D28344
>>>
>>> Added:
>>> cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
>>>
>>
>> This test is currently failing. Can you look at fixing it?
>>
>>
>>
>>> cfe/trunk/test/CodeGen/avr-unsupported-inline-asm-constraints.c
>>> Modified:
>>> cfe/trunk/lib/Basic/Targets.cpp
>>>
>>> Modified: cfe/trunk/lib/Basic/Targets.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=294176&r1=294175&r2=294176&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Basic/Targets.cpp (original)
>>> +++ cfe/trunk/lib/Basic/Targets.cpp Mon Feb  6 03:01:59 2017
>>> @@ -8517,6 +8517,57 @@ public:
>>>
>>>bool validateAsmConstraint(const char *&Name,
>>>   TargetInfo::ConstraintInfo &Info) const
>>> override {
>>> +// There aren't any multi-character AVR specific constraints.
>>> +if (StringRef(Name).size() > 1) return false;
>>> +
>>> +switch (*Name) {
>>> +  default: return false;
>>> +  case 'a': // Simple upper registers
>>> +  case 'b': // Base pointer registers pairs
>>> +  case 'd': // Upper register
>>> +  case 'l': // Lower registers
>>> +  case 'e': // Pointer register pairs
>>> +  case 'q': // Stack pointer register
>>> +  case 'r': // Any register
>>> +  case 'w': // Special upper register pairs
>>> +  case 't': // Temporary register
>>> +  case 'x': case 'X': // Pointer register pair X
>>> +  case 'y': case 'Y': // Pointer register pair Y
>>> +  case 'z': case 'Z': // Pointer register pair Z
>>> +Info.setAllowsRegister();
>>> +return true;
>>> +  case 'I': // 6-bit positive integer constant
>>> +Info.setRequiresImmediate(0, 63);
>>> +return true;
>>> +  case 'J': // 6-bit negative integer constant
>>> +Info.setRequiresImmediate(-63, 0);
>>> +return true;
>>> +  case 'K': // Integer constant (Range: 2)
>>> +Info.setRequiresImmediate(2);
>>> +return true;
>>> +  case 'L': // Integer constant (Range: 0)
>>> +Info.setRequiresImmediate(0);
>>> +return true;
>>> +  case 'M': // 8-bit integer constant
>>> +Info.setRequiresImmediate(0, 0xff);
>>> +return true;
>>> +  case 'N': // Integer constant (Range: -1)
>>> +Info.setRequiresImmediate(-1);
>>> +return true;
>>> +  case 'O': // Integer constant (Range: 8, 16, 24)
>>> +Info.setRequiresImmediate({8, 16, 24});
>>> +return true;
>>> +  case 'P': // Integer constant (Range: 1)
>>> +Info.setRequiresImmediate(1);
>>> +return true;
>>> +  case 'R': // Integer constant (Range: -6 to 5)
>>> +Info.setRequiresImmediate(-6, 5);
>>> +return true;
>>> +  case 'G': // Floating point constant
>>> +  case 'Q': // A memory address based on Y or Z pointer with
>>> displacement.
>>> +return true;
>>> +}
>>> +
>>>  return false;
>>>}
>>>
>>>
>>> Added: cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c?rev=294176&view=auto
>>>
>>> ==
>>> --- cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c (added)
>>> +++ cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c Mon Feb  6
>>> 03:01:59 2017
>>> @@ -0,0 +1,124 @@
>>> +// REQUIRES: avr-registered-target
>>> +// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s |
>>> FileCheck %s
>>> +
>>> +int data;
>>> +
>>> +void a() {
>>> +  // CHECK: call void asm sideeffect "add r5, $0", "a"(i16 %0)
>>> +  asm("add r5, %0" :: "a"(data));
>>> +}
>>> +
>>> +voi

r340596 - [AVR] Fix inline asm calls now that the addrspace(0) there is explicit.

[Chandler Carruth via cfe-commits]

Author: chandlerc
Date: Thu Aug 23 21:45:04 2018
New Revision: 340596

URL: http://llvm.org/viewvc/llvm-project?rev=340596&view=rev
Log:
[AVR] Fix inline asm calls now that the addrspace(0) there is explicit.

This updates the test case for r340519 so it should pass again. r340522
only got some of the AVR tests that needed an update.

Modified:
cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c

Modified: cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c?rev=340596&r1=340595&r2=340596&view=diff
==
--- cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c (original)
+++ cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c Thu Aug 23 21:45:04 2018
@@ -4,121 +4,121 @@
 int data;
 
 void a() {
-  // CHECK: call void asm sideeffect "add r5, $0", "a"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "a"(i16 %0)
   asm("add r5, %0" :: "a"(data));
 }
 
 void b() {
-  // CHECK: call void asm sideeffect "add r5, $0", "b"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "b"(i16 %0)
   asm("add r5, %0" :: "b"(data));
 }
 
 void d() {
-  // CHECK: call void asm sideeffect "add r5, $0", "d"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "d"(i16 %0)
   asm("add r5, %0" :: "d"(data));
 }
 
 void l() {
-  // CHECK: call void asm sideeffect "add r5, $0", "l"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "l"(i16 %0)
   asm("add r5, %0" :: "l"(data));
 }
 
 void e() {
-  // CHECK: call void asm sideeffect "add r5, $0", "e"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "e"(i16 %0)
   asm("add r5, %0" :: "e"(data));
 }
 
 void q() {
-  // CHECK: call void asm sideeffect "add r5, $0", "q"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "q"(i16 %0)
   asm("add r5, %0" :: "q"(data));
 }
 
 void r() {
-  // CHECK: call void asm sideeffect "add r5, $0", "r"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "r"(i16 %0)
   asm("add r5, %0" :: "r"(data));
 }
 
 void w() {
-  // CHECK: call void asm sideeffect "add r5, $0", "w"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "w"(i16 %0)
   asm("add r5, %0" :: "w"(data));
 }
 
 void t() {
-  // CHECK: call void asm sideeffect "add r5, $0", "t"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "t"(i16 %0)
   asm("add r5, %0" :: "t"(data));
 }
 
 void x() {
-  // CHECK: call void asm sideeffect "add r5, $0", "x"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "x"(i16 %0)
   asm("add r5, %0" :: "x"(data));
 }
 
 void y() {
-  // CHECK: call void asm sideeffect "add r5, $0", "y"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "y"(i16 %0)
   asm("add r5, %0" :: "y"(data));
 }
 
 void z() {
-  // CHECK: call void asm sideeffect "add r5, $0", "z"(i16 %0)
+  // CHECK: call addrspace(0) void asm sideeffect "add r5, $0", "z"(i16 %0)
   asm("add r5, %0" :: "z"(data));
 }
 
 void I() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "I"(i16 50)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "I"(i16 50)
   asm("subi r30, %0" :: "I"(50));
 }
 
 void J() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "J"(i16 -50)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "J"(i16 -50)
   asm("subi r30, %0" :: "J"(-50));
 }
 
 void K() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "K"(i16 2)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "K"(i16 2)
   asm("subi r30, %0" :: "K"(2));
 }
 
 void L() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "L"(i16 0)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "L"(i16 0)
   asm("subi r30, %0" :: "L"(0));
 }
 
 void M() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "M"(i16 255)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "M"(i16 255)
   asm("subi r30, %0" :: "M"(255));
 }
 
 void O() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "O"(i16 16)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "O"(i16 16)
   asm("subi r30, %0" :: "O"(16));
 }
 
 void P() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "P"(i16 1)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "P"(i16 1)
   asm("subi r30, %0" :: "P"(1));
 }
 
 void R() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "R"(i16 -3)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "R"(i16 -3)
   asm("subi r30, %0" :: "R"(-3));
 }
 
 void G() {
-  // CHECK: call void asm sideeffect "subi r30, $0", "G"(i16 50)
+  // CHECK: call addrspace(0) void asm sideeffect "subi r30, $0", "G"(i16 50)
   asm("subi r30, %0" :: "G"(50));
 }
 
 void Q() {
-  // CHECK: call void asm sideeffect "s

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

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

rsmith accepted this revision.
rsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1150
 
+  Opts.SpeculativeLoadHardening =
+  Args.hasFlag(OPT_mspeculative_load_hardening,

You can just use `hasArg(OPT_mspeculative_load_hardening)` here.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

Re: r294176 - [AVR] Add support for the full set of inline asm constraints

[Chandler Carruth via cfe-commits]

Trying new address again...

On Thu, Aug 23, 2018 at 8:17 PM Chandler Carruth 
wrote:

> Sorry for ancient thread revival, but...
>
> On Mon, Feb 6, 2017 at 2:10 AM Dylan McKay via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: dylanmckay
>> Date: Mon Feb  6 03:01:59 2017
>> New Revision: 294176
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=294176&view=rev
>> Log:
>> [AVR] Add support for the full set of inline asm constraints
>>
>> Summary:
>> Previously the method would simply return false, causing every single
>> inline assembly constraint to trigger a compile error.
>>
>> This adds inline assembly constraint support for the AVR target.
>>
>> This patch is derived from the code in
>> AVRISelLowering::getConstraintType.
>>
>> More details can be found on the AVR-GCC reference wiki
>> http://www.nongnu.org/avr-libc/user-manual/inline_asm.html
>>
>> Reviewers: jroelofs, asl
>>
>> Reviewed By: asl
>>
>> Subscribers: asl, ahatanak, saaadhu, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D28344
>>
>> Added:
>> cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
>>
>
> This test is currently failing. Can you look at fixing it?
>
>
>
>> cfe/trunk/test/CodeGen/avr-unsupported-inline-asm-constraints.c
>> Modified:
>> cfe/trunk/lib/Basic/Targets.cpp
>>
>> Modified: cfe/trunk/lib/Basic/Targets.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=294176&r1=294175&r2=294176&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Basic/Targets.cpp (original)
>> +++ cfe/trunk/lib/Basic/Targets.cpp Mon Feb  6 03:01:59 2017
>> @@ -8517,6 +8517,57 @@ public:
>>
>>bool validateAsmConstraint(const char *&Name,
>>   TargetInfo::ConstraintInfo &Info) const
>> override {
>> +// There aren't any multi-character AVR specific constraints.
>> +if (StringRef(Name).size() > 1) return false;
>> +
>> +switch (*Name) {
>> +  default: return false;
>> +  case 'a': // Simple upper registers
>> +  case 'b': // Base pointer registers pairs
>> +  case 'd': // Upper register
>> +  case 'l': // Lower registers
>> +  case 'e': // Pointer register pairs
>> +  case 'q': // Stack pointer register
>> +  case 'r': // Any register
>> +  case 'w': // Special upper register pairs
>> +  case 't': // Temporary register
>> +  case 'x': case 'X': // Pointer register pair X
>> +  case 'y': case 'Y': // Pointer register pair Y
>> +  case 'z': case 'Z': // Pointer register pair Z
>> +Info.setAllowsRegister();
>> +return true;
>> +  case 'I': // 6-bit positive integer constant
>> +Info.setRequiresImmediate(0, 63);
>> +return true;
>> +  case 'J': // 6-bit negative integer constant
>> +Info.setRequiresImmediate(-63, 0);
>> +return true;
>> +  case 'K': // Integer constant (Range: 2)
>> +Info.setRequiresImmediate(2);
>> +return true;
>> +  case 'L': // Integer constant (Range: 0)
>> +Info.setRequiresImmediate(0);
>> +return true;
>> +  case 'M': // 8-bit integer constant
>> +Info.setRequiresImmediate(0, 0xff);
>> +return true;
>> +  case 'N': // Integer constant (Range: -1)
>> +Info.setRequiresImmediate(-1);
>> +return true;
>> +  case 'O': // Integer constant (Range: 8, 16, 24)
>> +Info.setRequiresImmediate({8, 16, 24});
>> +return true;
>> +  case 'P': // Integer constant (Range: 1)
>> +Info.setRequiresImmediate(1);
>> +return true;
>> +  case 'R': // Integer constant (Range: -6 to 5)
>> +Info.setRequiresImmediate(-6, 5);
>> +return true;
>> +  case 'G': // Floating point constant
>> +  case 'Q': // A memory address based on Y or Z pointer with
>> displacement.
>> +return true;
>> +}
>> +
>>  return false;
>>}
>>
>>
>> Added: cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c?rev=294176&view=auto
>>
>> ==
>> --- cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c (added)
>> +++ cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c Mon Feb  6
>> 03:01:59 2017
>> @@ -0,0 +1,124 @@
>> +// REQUIRES: avr-registered-target
>> +// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s |
>> FileCheck %s
>> +
>> +int data;
>> +
>> +void a() {
>> +  // CHECK: call void asm sideeffect "add r5, $0", "a"(i16 %0)
>> +  asm("add r5, %0" :: "a"(data));
>> +}
>> +
>> +void b() {
>> +  // CHECK: call void asm sideeffect "add r5, $0", "b"(i16 %0)
>> +  asm("add r5, %0" :: "b"(data));
>> +}
>> +
>> +void d() {
>> +  // CHECK: call void asm sideeffect "add r5, $0", "d"(i16 %0)
>> +  asm("add r5, %0" :: "d"(data));
>> +}
>> +
>> +void l() {
>> 

Re: r294176 - [AVR] Add support for the full set of inline asm constraints

[Chandler Carruth via cfe-commits]

Sorry for ancient thread revival, but...

On Mon, Feb 6, 2017 at 2:10 AM Dylan McKay via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: dylanmckay
> Date: Mon Feb  6 03:01:59 2017
> New Revision: 294176
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294176&view=rev
> Log:
> [AVR] Add support for the full set of inline asm constraints
>
> Summary:
> Previously the method would simply return false, causing every single
> inline assembly constraint to trigger a compile error.
>
> This adds inline assembly constraint support for the AVR target.
>
> This patch is derived from the code in
> AVRISelLowering::getConstraintType.
>
> More details can be found on the AVR-GCC reference wiki
> http://www.nongnu.org/avr-libc/user-manual/inline_asm.html
>
> Reviewers: jroelofs, asl
>
> Reviewed By: asl
>
> Subscribers: asl, ahatanak, saaadhu, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D28344
>
> Added:
> cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
>

This test is currently failing. Can you look at fixing it?



> cfe/trunk/test/CodeGen/avr-unsupported-inline-asm-constraints.c
> Modified:
> cfe/trunk/lib/Basic/Targets.cpp
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=294176&r1=294175&r2=294176&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Mon Feb  6 03:01:59 2017
> @@ -8517,6 +8517,57 @@ public:
>
>bool validateAsmConstraint(const char *&Name,
>   TargetInfo::ConstraintInfo &Info) const
> override {
> +// There aren't any multi-character AVR specific constraints.
> +if (StringRef(Name).size() > 1) return false;
> +
> +switch (*Name) {
> +  default: return false;
> +  case 'a': // Simple upper registers
> +  case 'b': // Base pointer registers pairs
> +  case 'd': // Upper register
> +  case 'l': // Lower registers
> +  case 'e': // Pointer register pairs
> +  case 'q': // Stack pointer register
> +  case 'r': // Any register
> +  case 'w': // Special upper register pairs
> +  case 't': // Temporary register
> +  case 'x': case 'X': // Pointer register pair X
> +  case 'y': case 'Y': // Pointer register pair Y
> +  case 'z': case 'Z': // Pointer register pair Z
> +Info.setAllowsRegister();
> +return true;
> +  case 'I': // 6-bit positive integer constant
> +Info.setRequiresImmediate(0, 63);
> +return true;
> +  case 'J': // 6-bit negative integer constant
> +Info.setRequiresImmediate(-63, 0);
> +return true;
> +  case 'K': // Integer constant (Range: 2)
> +Info.setRequiresImmediate(2);
> +return true;
> +  case 'L': // Integer constant (Range: 0)
> +Info.setRequiresImmediate(0);
> +return true;
> +  case 'M': // 8-bit integer constant
> +Info.setRequiresImmediate(0, 0xff);
> +return true;
> +  case 'N': // Integer constant (Range: -1)
> +Info.setRequiresImmediate(-1);
> +return true;
> +  case 'O': // Integer constant (Range: 8, 16, 24)
> +Info.setRequiresImmediate({8, 16, 24});
> +return true;
> +  case 'P': // Integer constant (Range: 1)
> +Info.setRequiresImmediate(1);
> +return true;
> +  case 'R': // Integer constant (Range: -6 to 5)
> +Info.setRequiresImmediate(-6, 5);
> +return true;
> +  case 'G': // Floating point constant
> +  case 'Q': // A memory address based on Y or Z pointer with
> displacement.
> +return true;
> +}
> +
>  return false;
>}
>
>
> Added: cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c?rev=294176&view=auto
>
> ==
> --- cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c (added)
> +++ cfe/trunk/test/CodeGen/avr-inline-asm-constraints.c Mon Feb  6
> 03:01:59 2017
> @@ -0,0 +1,124 @@
> +// REQUIRES: avr-registered-target
> +// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s
> +
> +int data;
> +
> +void a() {
> +  // CHECK: call void asm sideeffect "add r5, $0", "a"(i16 %0)
> +  asm("add r5, %0" :: "a"(data));
> +}
> +
> +void b() {
> +  // CHECK: call void asm sideeffect "add r5, $0", "b"(i16 %0)
> +  asm("add r5, %0" :: "b"(data));
> +}
> +
> +void d() {
> +  // CHECK: call void asm sideeffect "add r5, $0", "d"(i16 %0)
> +  asm("add r5, %0" :: "d"(data));
> +}
> +
> +void l() {
> +  // CHECK: call void asm sideeffect "add r5, $0", "l"(i16 %0)
> +  asm("add r5, %0" :: "l"(data));
> +}
> +
> +void e() {
> +  // CHECK: call void asm sideeffect "add r5, $0", "e"(i16 %0)
> +  asm("add r5, %0" :: "e"(data));
> +}
> +

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 162322.
chandlerc added a comment.

Add a test file that I somehow missed earlier (sorry about that).


Repository:
  rL LLVM

https://reviews.llvm.org/D51157

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-speculative-load-hardening.c
  clang/test/Driver/x86-target-features.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-gather.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
  llvm/test/CodeGen/X86/speculative-load-hardening.ll
  llvm/test/Transforms/Inline/attributes.ll

Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -26,6 +26,10 @@
   ret i32 %i
 }
 
+define i32 @slh_callee(i32 %i) speculative_load_hardening {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_callee(i32 %i) alwaysinline {
   ret i32 %i
 }
@@ -161,6 +165,27 @@
 ; CHECK-NEXT: ret i32
 }
 
+; Can inline a normal function into an SLH'ed function.
+define i32 @test_caller_slh(i32 %i) speculative_load_hardening {
+; CHECK-LABEL: @test_caller_slh(
+; CHECK-SAME: ) [[SLH:.*]] {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @noattr_callee(i32 %i)
+  ret i32 %callee
+}
+
+; Can inline a SLH'ed function into a normal one, dropping the SLH.
+define i32 @test_callee_slh(i32 %i) {
+; CHECK-LABEL: @test_callee_slh(i32 %i) {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @slh_callee(i32 %i)
+  ret i32 %callee
+}
+
 ; Check that a function doesn't get inlined if target-cpu strings don't match
 ; exactly.
 define i32 @test_target_cpu_callee0(i32 %i) "target-cpu"="corei7" {
@@ -384,6 +409,7 @@
 ; CHECK-NEXT: ret i32
 }
 
+; CHECK: attributes [[SLH]] = { speculative_load_hardening }
 ; CHECK: attributes [[FPMAD_FALSE]] = { "less-precise-fpmad"="false" }
 ; CHECK: attributes [[FPMAD_TRUE]] = { "less-precise-fpmad"="true" }
 ; CHECK: attributes [[NOIMPLICITFLOAT]] = { noimplicitfloat }
Index: llvm/test/CodeGen/X86/speculative-load-hardening.ll
===
--- llvm/test/CodeGen/X86/speculative-load-hardening.ll
+++ llvm/test/CodeGen/X86/speculative-load-hardening.ll
@@ -1,14 +1,14 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening | FileCheck %s --check-prefix=X64
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
 ;
 ; FIXME: Add support for 32-bit and other EH ABIs.
 
 declare void @leak(i32 %v1, i32 %v2)
 
 declare void @sink(i32)
 
-define i32 @test_trivial_entry_load(i32* %ptr) {
+define i32 @test_trivial_entry_load(i32* %ptr) speculative_load_hardening {
 ; X64-LABEL: test_trivial_entry_load:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:movq %rsp, %rcx
@@ -29,7 +29,7 @@
   ret i32 %v
 }
 
-define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) {
+define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) speculative_load_hardening {
 ; X64-LABEL: test_basic_conditions:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %r15
@@ -189,7 +189,7 @@
   ret void
 }
 
-define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_loop:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %rbp
@@ -293,7 +293,7 @@
   ret void
 }
 
-define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_nested_loop:
 ; X64:  

[PATCH] D50043: [RISCV] RISC-V using -fuse-init-array by default

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340595: [RISCV] RISC-V using -fuse-init-array by default 
(authored by kito, committed by ).
Herald added subscribers: llvm-commits, jrtc27.

Changed prior to commit:
  https://reviews.llvm.org/D50043?vs=161990&id=162321#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50043

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/riscv32-toolchain.c


Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -2554,7 +2554,9 @@
   getTriple().getOS() == llvm::Triple::NaCl ||
   (getTriple().getVendor() == llvm::Triple::MipsTechnologies &&
!getTriple().hasEnvironment()) ||
-  getTriple().getOS() == llvm::Triple::Solaris;
+  getTriple().getOS() == llvm::Triple::Solaris ||
+  getTriple().getArch() == llvm::Triple::riscv32 ||
+  getTriple().getArch() == llvm::Triple::riscv64;
 
   if (DriverArgs.hasFlag(options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, UseInitArrayDefault))
Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -9,6 +9,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-ILP32 %s
 
+// C-RV32-BAREMETAL-ILP32: "-fuse-init-array"
 // C-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // C-RV32-BAREMETAL-ILP32: 
"--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // C-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
@@ -24,6 +25,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
+// CXX-RV32-BAREMETAL-ILP32: "-fuse-init-array"
 // CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: 
"--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
@@ -40,6 +42,7 @@
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32 %s
 
+// C-RV32-LINUX-MULTI-ILP32: "-fuse-init-array"
 // C-RV32-LINUX-MULTI-ILP32: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}ld"
 // C-RV32-LINUX-MULTI-ILP32: 
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
 // C-RV32-LINUX-MULTI-ILP32: "-m" "elf32lriscv"
@@ -55,6 +58,7 @@
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32D %s
 
+// C-RV32-LINUX-MULTI-ILP32D: "-fuse-init-array"
 // C-RV32-LINUX-MULTI-ILP32D: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}ld"
 // C-RV32-LINUX-MULTI-ILP32D: 
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
 // C-RV32-LINUX-MULTI-ILP32D: "-m" "elf32lriscv"


Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -2554,7 +2554,9 @@
   getTriple().getOS() == llvm::Triple::NaCl ||
   (getTriple().getVendor() == llvm::Triple::MipsTechnologies &&
!getTriple().hasEnvironment()) ||
-  getTriple().getOS() == llvm::Triple::Solaris;
+  getTriple().getOS() == llvm::Triple::Solaris ||
+  getTriple().getArch() == llvm::Triple::riscv32 ||
+  getTriple().getArch() == llvm::Triple::riscv64;
 
   if (DriverArgs.hasFlag(options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, UseInitArrayDefault))
Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -9,6 +9,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-ILP32 %s
 
+// C-RV32-BAREMETAL-ILP32: "-fuse-init-array"
 // C-RV32-BAREMETAL-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // C-RV32-BAREMETAL-ILP32: "--sysroot={{.*}}/Inputs/basic_ri

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

Thanks, should all be addressed now.




Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:169-170
   options::OPT_mno_retpoline_external_thunk, false)) {
 // FIXME: Add a warning about failing to specify `-mretpoline` and
 // eventually switch to an error here.
 Features.push_back("+retpoline-indirect-calls");

rsmith wrote:
> `-mspeculative-load-hardening -mretpoline-external-thunk` does not enable 
> `+retpoline-indirect-branches` (but `-mretpoline-external-thunk` by itself 
> does). Is that intentional?
Yes, as that's is specifically useful (and that is one thing that motivated my 
above comment that we should remove the ability to use 
`-mretpoline-external-thunk` by itself eventually).

You might only need SLH's minimal retpolines, but you might need to provide 
your own thunks for them in the Kernel for example.



Comment at: llvm/include/llvm/IR/Attributes.td:249
 def : MergeRule<"adjustNullPointerValidAttr">;
+

rnk wrote:
> I can't tell if this is just vim fixing the lack of a proper trailing 
> newline, but revert any whitespace change if possible.
Soryr, my annoying editor. Will fix before submit.



Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1169
+  case Attribute::SpeculativeLoadHardening:
+return 1ULL << 60;
   case Attribute::Dereferenceable:

rsmith wrote:
> rnk wrote:
> > These appear to repeat LLVMBitcodes.h, unless I am mistaken. Weird. Anyway, 
> > fixing that is out of scope.
> The values "above the fold" in Phabricator aren't all the same as the enum 
> value plus one. *shrug*
Yeah, this isn't factorable easily. Anyways


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 162319.
chandlerc marked 3 inline comments as done.
chandlerc added a comment.

Address review comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/x86-target-features.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-gather.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
  llvm/test/CodeGen/X86/speculative-load-hardening.ll
  llvm/test/Transforms/Inline/attributes.ll

Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -26,6 +26,10 @@
   ret i32 %i
 }
 
+define i32 @slh_callee(i32 %i) speculative_load_hardening {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_callee(i32 %i) alwaysinline {
   ret i32 %i
 }
@@ -161,6 +165,27 @@
 ; CHECK-NEXT: ret i32
 }
 
+; Can inline a normal function into an SLH'ed function.
+define i32 @test_caller_slh(i32 %i) speculative_load_hardening {
+; CHECK-LABEL: @test_caller_slh(
+; CHECK-SAME: ) [[SLH:.*]] {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @noattr_callee(i32 %i)
+  ret i32 %callee
+}
+
+; Can inline a SLH'ed function into a normal one, dropping the SLH.
+define i32 @test_callee_slh(i32 %i) {
+; CHECK-LABEL: @test_callee_slh(i32 %i) {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @slh_callee(i32 %i)
+  ret i32 %callee
+}
+
 ; Check that a function doesn't get inlined if target-cpu strings don't match
 ; exactly.
 define i32 @test_target_cpu_callee0(i32 %i) "target-cpu"="corei7" {
@@ -384,6 +409,7 @@
 ; CHECK-NEXT: ret i32
 }
 
+; CHECK: attributes [[SLH]] = { speculative_load_hardening }
 ; CHECK: attributes [[FPMAD_FALSE]] = { "less-precise-fpmad"="false" }
 ; CHECK: attributes [[FPMAD_TRUE]] = { "less-precise-fpmad"="true" }
 ; CHECK: attributes [[NOIMPLICITFLOAT]] = { noimplicitfloat }
Index: llvm/test/CodeGen/X86/speculative-load-hardening.ll
===
--- llvm/test/CodeGen/X86/speculative-load-hardening.ll
+++ llvm/test/CodeGen/X86/speculative-load-hardening.ll
@@ -1,14 +1,14 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening | FileCheck %s --check-prefix=X64
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
 ;
 ; FIXME: Add support for 32-bit and other EH ABIs.
 
 declare void @leak(i32 %v1, i32 %v2)
 
 declare void @sink(i32)
 
-define i32 @test_trivial_entry_load(i32* %ptr) {
+define i32 @test_trivial_entry_load(i32* %ptr) speculative_load_hardening {
 ; X64-LABEL: test_trivial_entry_load:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:movq %rsp, %rcx
@@ -29,7 +29,7 @@
   ret i32 %v
 }
 
-define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) {
+define void @test_basic_conditions(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2, i32** %ptr3) speculative_load_hardening {
 ; X64-LABEL: test_basic_conditions:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %r15
@@ -189,7 +189,7 @@
   ret void
 }
 
-define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_loop(i32 %a, i32 %b, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_loop:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %rbp
@@ -293,7 +293,7 @@
   ret void
 }
 
-define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind {
+define void @test_basic_nested_loop(i32 %a, i32 %b, i32 %c, i32* %ptr1, i32* %ptr2) nounwind speculative_load_hardening {
 ; X64-LABEL: test_basic_nested_loop:
 ; X64:   # %bb.0: # %entry
 ; X64-NEXT:pushq %rbp
@@ -48

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

In https://reviews.llvm.org/D48027#1209844, @NoQ wrote:

> So i believe that one of the important remaining problems with 
> `CallDescription` is to teach it to discriminate between global functions and 
> methods. We can do it in a number of ways:
>
> 1. Make a special sub-class for `CallDescription` depending on the sort of 
> the function (too verbose),
> 2. Tag all items in the list as "record" vs. "namespace" (too many tags),
> 3. Dunno, tons of other boring and verbose approaches,
> 4. Change our `PreCall`/`PostCall` callbacks to callback templates that let 
> allow the user subscribe to specific sub-classes of `CallEvent` similarly to 
> how he can subscribe to different kinds of statements in 
> `PreStmt`/`PostStmt`, and then the user doesn't need to write 
> any code to check it dynamically.


Personally, I prefer `4`. It is more checker developer friendly!


Repository:
  rC Clang

https://reviews.llvm.org/D48027



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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I feel like this is a much tricky situation than just new and init. Following 
example is the same situation.

  __attribute__((objc_root_class))
  @interface NSObject
  - (void) foo;
  - (void) bar;
  @end
  
  @implementation NSObject
  - (void) foo {}
  - (void) bar { [self foo]; }
  @end
  
  @interface MyObject : NSObject
  - (void) foo __attribute__((unavailable));
  @end
  
  void test(MyObject *obj) {
[obj bar];
  }

We can do something about [NSObject new] because we know it's implementation 
but we have to live with more general cases.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

TAPI mostly cares about linkable symbols, so this shouldn't be a problem.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51204: [COFF, ARM64] Add MS intrinsics: __getReg, _ReadStatusReg, _WriteStatusReg

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rnk, compnerd, mstorsjo, haripul, TomTan.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.

Added declarations for the intrinsics in arm64intr.h. These are defined in MSVC 
libs and are needed for
certain spec2000 benchmarks.


Repository:
  rC Clang

https://reviews.llvm.org/D51204

Files:
  lib/Headers/arm64intr.h
  test/CodeGen/arm64-microsoft-intrinsics.c


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -59,3 +59,19 @@
 
 // CHECK-MSVC: @llvm.aarch64.hint(i32 5)
 // CHECK-LINUX: error: implicit declaration of function '__sevl'
+
+unsigned __int64 check__getReg(void) {
+  return __getReg(1);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @__getReg to i32 (i32)*)(i32 1)
+
+__int64 check_ReadStatusReg(void) {
+  return _ReadStatusReg(1);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @_ReadStatusReg to i32 (i32)*)(i32 
1)
+
+void check_WriteStatusReg(void) {
+  __int64 x;
+  _WriteStatusReg(1, x);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @_WriteStatusReg to i32 (i32, 
i64)*)(i32 1, i64 %0)
Index: lib/Headers/arm64intr.h
===
--- lib/Headers/arm64intr.h
+++ lib/Headers/arm64intr.h
@@ -45,5 +45,9 @@
   _ARM64_BARRIER_OSHLD = 0x1
 } _ARM64INTR_BARRIER_TYPE;
 
+unsigned __int64 __getReg(int);
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
 #endif /* __ARM64INTR_H */
 #endif /* _MSC_VER */


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -59,3 +59,19 @@
 
 // CHECK-MSVC: @llvm.aarch64.hint(i32 5)
 // CHECK-LINUX: error: implicit declaration of function '__sevl'
+
+unsigned __int64 check__getReg(void) {
+  return __getReg(1);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @__getReg to i32 (i32)*)(i32 1)
+
+__int64 check_ReadStatusReg(void) {
+  return _ReadStatusReg(1);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @_ReadStatusReg to i32 (i32)*)(i32 1)
+
+void check_WriteStatusReg(void) {
+  __int64 x;
+  _WriteStatusReg(1, x);
+}
+// CHECK-MSVC: call i32 bitcast (i32 (...)* @_WriteStatusReg to i32 (i32, i64)*)(i32 1, i64 %0)
Index: lib/Headers/arm64intr.h
===
--- lib/Headers/arm64intr.h
+++ lib/Headers/arm64intr.h
@@ -45,5 +45,9 @@
   _ARM64_BARRIER_OSHLD = 0x1
 } _ARM64INTR_BARRIER_TYPE;
 
+unsigned __int64 __getReg(int);
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
 #endif /* __ARM64INTR_H */
 #endif /* _MSC_VER */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

[Teresa Johnson via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D51198



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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a subscriber: ributzka.
arphaman added a comment.

In https://reviews.llvm.org/D51189#1211763, @erik.pilkington wrote:

> In https://reviews.llvm.org/D51189#1211754, @arphaman wrote:
>
> > Hmm, I don't think this solution is ideal, we'd rather have an attribute 
> > somewhere for other consumers of availability annotations. Does MyObject 
> > have an implicit decl of `new`, or are we referring to `NSObject`s `new`? 
> > Ideally we would an attribute on a particular `new` instead, but that might 
> > not work.
>
>
> We're referring to NSObject's new. I don't think it's unreasonable to ask 
> users who override init to be unavailable also override new with the same 
> annotation, but it seems like extra boilerplate for something that we can 
> easily infer in clang. What other consumers are you concerned about?


+ @ributzka 
One consumer is TAPI. It looks at the declarations present in the header file, 
so it won't be able to reason about the availability of `new` with the current 
implementation. We could potentially implicitly declare unavailable `new` in 
the interface if `init` is unavailable, but that wouldn't really too work with 
class categories (since `new` might be explicitly declared there). Maybe it's 
worth it though.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51200: Introduce per-callsite inline intrinsics

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Disclaimer: I'm a Clang newbie and I'm not sure if that's a good way to 
implement these intrinsics. I'm not sure about the following things:

- The new enum CallInlineKind may not be in the right place
- Not sure if adding the extra parameter to EmitSomething methods is the 
cleanest thing possible -- some functions have it now, some don't, and it makes 
argument lists longer
- Not sure if just adding an extra attribute at each callsite is enough -- I'm 
not sure what is supposed to happen when there are conflicting inline 
attributes at call sites and function declarations (e.g. 
`__builtin_always_inline` and `__attribute__ ((always_inline))`)
- I don't know how to handle constexpr functions
- I don't know how to implement it for constructor calls

I would really appreciate any tips here :).


Repository:
  rC Clang

https://reviews.llvm.org/D51200



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

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

[Eli Friedman via Phabricator via cfe-commits]

efriedma updated this revision to Diff 162308.
efriedma added a comment.

Fix new pass manager.


Repository:
  rC Clang

https://reviews.llvm.org/D51198

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/summary-index-unnamed-global.ll


Index: test/CodeGen/summary-index-unnamed-global.ll
===
--- /dev/null
+++ test/CodeGen/summary-index-unnamed-global.ll
@@ -0,0 +1,10 @@
+; RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc 
-disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -flto=thin -triple x86_64-pc-linux-gnu -emit-llvm-bc 
-disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -triple 
x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | 
llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -fexperimental-new-pass-manager -flto=thin -triple 
x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | 
llvm-bcanalyzer -dump | FileCheck %s
+
+; CHECK-NOT:GLOBALVAL_SUMMARY_BLOCK
+
+; Make sure this doesn't crash, and we don't try to emit a module summary.
+; (The command is roughly emulating what -save-temps would do.)
+@0 = global i32 0
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -783,7 +783,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -796,6 +796,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO &&
+   !CodeGenOpts.DisableLLVMPasses &&
llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
llvm::Triple::Apple);
   if (EmitLTOSummary && !TheModule->getModuleFlag("ThinLTO"))
@@ -1014,7 +1015,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -1027,6 +1028,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO &&
+   !CodeGenOpts.DisableLLVMPasses &&
llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
llvm::Triple::Apple);
   if (EmitLTOSummary && !TheModule->getModuleFlag("ThinLTO"))


Index: test/CodeGen/summary-index-unnamed-global.ll
===
--- /dev/null
+++ test/CodeGen/summary-index-unnamed-global.ll
@@ -0,0 +1,10 @@
+; RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -flto=thin -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -fexperimental-new-pass-manager -flto=thin -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+
+; CHECK-NOT:GLOBALVAL_SUMMARY_BLOCK
+
+; Make sure this doesn't crash, and we don't try to emit a module summary.
+; (The command is roughly emulating what -save-temps would do.)
+@0 = global i32 0
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -783,7 +783,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -796,6 +796,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO &&
+   !CodeGenOpts.DisableLLVMPasses &&
llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
llvm::Triple::Apple);
   if (EmitLTOSummary && !TheModule->getModuleFlag("ThinLTO"))
@@ -1014,7 +1015,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -1027,6 +1028,

[PATCH] D51200: Introduce per-callsite inline intrinsics

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar created this revision.
kuhar added reviewers: rsmith, pcc, Prazek, sanjoy.
kuhar added a project: clang.
Herald added a subscriber: eraman.
Herald added a reviewer: grosser.

Traditionally, to force some inlining decisions one has to annotate function 
declarations with `__attribute__((always_inline))` or 
`__attribute__((noinline))`. One problem with these attributes is that they 
affect every call site. Always inlining or forbidding inlining may not be 
desirable in every context, and a workaround for that is creating a few copies 
of functions, each with a different inline attribute. Furthermore, it's not 
always feasible (in every project) to modify library code and introduce new 
function attributes there.

This patch introduces a new way of forcing inlining decisions on a per-callsite 
basis. This allows for more fine-grained control over inlining, without 
creating any duplicate functions. The two new intrinsics for controlling 
inlining are:

- `__builtin_always_inline(Foo())` -- inlines the function `Foo` at the 
callsite, if possible. Internally, this applies the `alwaysinline` attribute to 
the generated call instruction.
- `__builtin_no_inline(Foo())` -- forbids the function `Foo` to be inlined at 
the callsite. Internally, this applies the `noinline` attribute to the 
generated call instruction.

The inline intrinsics support function, function pointer, member function, 
member function pointer, virutal function, and operator calls. Support for 
constructor calls (`CXXTemporaryExpr`) should also be possible, but is not the 
part of this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D51200

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenTypes.h
  lib/Sema/SemaChecking.cpp
  test/CodeGenCXX/inline_builtin_call.cpp
  test/Sema/inline-builtins.c
  test/SemaCXX/inline-builtins.cpp

Index: test/SemaCXX/inline-builtins.cpp
===
--- /dev/null
+++ test/SemaCXX/inline-builtins.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+
+struct S {
+  S();
+  void foo();
+  void bar(int);
+  int baz(float, char);
+  static void lol();
+
+  virtual void virt();
+  void operator++();
+  friend S operator+(const S &, const S &);
+};
+
+S operator"" _s(unsigned long long);
+
+S &getS();
+
+void test_positive() {
+  S &s = getS();
+  __builtin_always_inline(s.foo());
+  __builtin_no_inline(s.foo());
+
+  __builtin_always_inline(s.bar(1));
+  __builtin_no_inline(s.bar(1));
+
+  int a = __builtin_always_inline(s.baz(3.14, 'a'));
+  int b = __builtin_no_inline(s.baz(3.14, 'a'));
+
+  __builtin_always_inline(s.lol());
+  __builtin_no_inline(s.lol());
+
+  __builtin_always_inline(S::lol());
+  __builtin_no_inline(S::lol());
+
+  __builtin_always_inline(s.virt());
+  __builtin_no_inline(s.virt());
+
+  __builtin_always_inline(++s);
+  __builtin_no_inline(++s);
+
+  __builtin_always_inline(s.operator++());
+  __builtin_no_inline(s.operator++());
+
+  S s1;
+  __builtin_always_inline(s + s1);
+  __builtin_no_inline(s + s1);
+
+  auto mptr = &S::foo;
+  __builtin_always_inline((s.*mptr)());
+  __builtin_no_inline((s.*mptr)());
+
+  __builtin_always_inline(s.~S());
+  __builtin_no_inline(s.~S());
+
+  __builtin_always_inline([] {}());
+  __builtin_no_inline([] {}());
+
+  auto lam = [&a, b](S &ss) { return ++a + b; };
+  int c = __builtin_always_inline(lam(s1));
+  int d = __builtin_no_inline(lam(s1));
+}
+
+void test_errors() {
+  using I = int;
+  I e = 1;
+  __builtin_always_inline(e.~I()); // expected-error {{argument to __builtin_always_inline must not be a pseudo-destructor call}}
+  __builtin_no_inline(e.~I()); // expected-error {{argument to __builtin_no_inline must not be a pseudo-destructor call}}
+
+  S s2 = __builtin_always_inline(1_s); // expected-error {{argument to __builtin_always_inline must be a function, member function, or operator call}}
+  s2 = __builtin_no_inline(1_s);   // expected-error {{argument to __builtin_no_inline must be a function, member function, or operator call}}
+
+  // TODO: This should be handled as well.
+  S s3 = __builtin_always_inline(S()); // expected-error {{argument to __builtin_always_inline must be a function, member function, or operator call}}
+  S s4 = __builtin_no_inline(S()); // expected-error {{argument to __builtin_no_inline must be a function, member function, or operator call}}
+}
+
+constexpr int f1() { return 1; }
+
+// TODO: It should be possible to make the inline intrinsics constexpr-friendly.
+constexpr int f2(int x) {   // expected-error {{constexpr function never produces a constant expression}}
+  return x + __builtin_always_inline(f1()); // expected-note {{subexpression not valid in a consta

[PATCH] D50318: Support Swift in platform availability attribute

[Michael Wu via Phabricator via cfe-commits]

michaelwu added a comment.

Review ping


Repository:
  rC Clang

https://reviews.llvm.org/D50318



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

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Thanks. Can you fix the same code in EmitAssemblyWithNewPassManager?


Repository:
  rC Clang

https://reviews.llvm.org/D51198



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

https://reviews.llvm.org/D51199 fixes the above breakage as well as 
crbug.com/877235. Once it lands, I'll reland this change.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: tejohnson, tobiasvk.
Herald added subscribers: dexonsmith, inglorion.

If all LLVM passes are disabled, we can't emit a summary because there could be 
unnamed globals in the IR.


Repository:
  rC Clang

https://reviews.llvm.org/D51198

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/summary-index-unnamed-global.ll


Index: test/CodeGen/summary-index-unnamed-global.ll
===
--- /dev/null
+++ test/CodeGen/summary-index-unnamed-global.ll
@@ -0,0 +1,8 @@
+; RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc 
-disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -flto=thin -triple x86_64-pc-linux-gnu -emit-llvm-bc 
-disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+
+; CHECK-NOT:GLOBALVAL_SUMMARY_BLOCK
+
+; Make sure this doesn't crash, and we don't try to emit a module summary.
+; (The command is roughly emulating what -save-temps would do.)
+@0 = global i32 0
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -783,7 +783,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -796,6 +796,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO &&
+   !CodeGenOpts.DisableLLVMPasses &&
llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
llvm::Triple::Apple);
   if (EmitLTOSummary && !TheModule->getModuleFlag("ThinLTO"))


Index: test/CodeGen/summary-index-unnamed-global.ll
===
--- /dev/null
+++ test/CodeGen/summary-index-unnamed-global.ll
@@ -0,0 +1,8 @@
+; RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+; RUN: %clang_cc1 -flto=thin -triple x86_64-pc-linux-gnu -emit-llvm-bc -disable-llvm-passes -x ir < %s -o - | llvm-bcanalyzer -dump | FileCheck %s
+
+; CHECK-NOT:GLOBALVAL_SUMMARY_BLOCK
+
+; Make sure this doesn't crash, and we don't try to emit a module summary.
+; (The command is roughly emulating what -save-temps would do.)
+@0 = global i32 0
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -783,7 +783,7 @@
 break;
 
   case Backend_EmitBC:
-if (CodeGenOpts.PrepareForThinLTO) {
+if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
   if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
 ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
 if (!ThinLinkOS)
@@ -796,6 +796,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO &&
+   !CodeGenOpts.DisableLLVMPasses &&
llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
llvm::Triple::Apple);
   if (EmitLTOSummary && !TheModule->getModuleFlag("ThinLTO"))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In https://reviews.llvm.org/D51189#1211754, @arphaman wrote:

> Hmm, I don't think this solution is ideal, we'd rather have an attribute 
> somewhere for other consumers of availability annotations. Does MyObject have 
> an implicit decl of `new`, or are we referring to `NSObject`s `new`? Ideally 
> we would an attribute on a particular `new` instead, but that might not work.


We're referring to NSObject's new. I don't think it's unreasonable to ask users 
who override init to be unavailable also override new with the same annotation, 
but it seems like extra boilerplate for something that we can easily infer in 
clang. What other consumers are you concerned about?


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Hmm, I don't think this solution is ideal, we'd rather have an attribute 
somewhere for other consumers of availability annotations. Does MyObject have 
an implicit decl of `new`, or are we referring to `NSObject`s `new`? Ideally we 
would an attribute on a particular `new` instead, but that might not work.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51170: [libc++] Remove race condition in std::async

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: libcxx/include/future:556
 bool __has_value() const
 {return (__state_ & __constructed) || (__exception_ != nullptr);}
 

I'm not auditing everything, but it seems like code above can still access 
__state_ without holding __mut_? Like in the dtor.

Generally this patch lgtm because it's a step forward, but maybe we should 
separately refactor the code to make it so that accesses to __state_ require 
passing in a reference to lock_guard to show we actually hold __mut_. It would 
ignore that reference, but that's a way to enforce, in the type system, that 
__state_ is only touched when the lock is held.

WDYT?


Repository:
  rCXX libc++

https://reviews.llvm.org/D51170



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list= to match gcc options.

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.
Herald added a subscriber: erik.pilkington.

Hello folks, is there a plan to merge this feature still?


https://reviews.llvm.org/D37624



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

[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D47757#1211276, @tra wrote:

> I've confirmed that the patch does not break anything in our CUDA code, so 
> it's good to go as far as CUDA is concerned.


Thanks. @rsmith, do you have any other comments about the patch?


Repository:
  rC Clang

https://reviews.llvm.org/D47757



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

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162291.
nickdesaulniers added a comment.

- link to correct doc


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51192: Fix reported range of partial token replacement

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 162288.
steveire added a comment.

Fix reported range of partial token replacement


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51192

Files:
  clang-tidy/ClangTidy.cpp


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -125,7 +125,6 @@
   // Retrieve the source range for applicable fixes. Macro definitions
   // on the command line have locations in a virtual buffer and don't
   // have valid file paths and are therefore not applicable.
-  SourceRange Range;
   SourceLocation FixLoc;
   ++TotalFixes;
   bool CanBeApplied = false;
@@ -166,7 +165,7 @@
 FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
 SourceLocation FixEndLoc =
 FixLoc.getLocWithOffset(Repl.getLength());
-Range = SourceRange(FixLoc, FixEndLoc);
+CharSourceRange Range = 
CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
 Diag << FixItHint::CreateReplacement(Range,
  Repl.getReplacementText());
   }


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -125,7 +125,6 @@
   // Retrieve the source range for applicable fixes. Macro definitions
   // on the command line have locations in a virtual buffer and don't
   // have valid file paths and are therefore not applicable.
-  SourceRange Range;
   SourceLocation FixLoc;
   ++TotalFixes;
   bool CanBeApplied = false;
@@ -166,7 +165,7 @@
 FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
 SourceLocation FixEndLoc =
 FixLoc.getLocWithOffset(Repl.getLength());
-Range = SourceRange(FixLoc, FixEndLoc);
+CharSourceRange Range = CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
 Diag << FixItHint::CreateReplacement(Range,
  Repl.getReplacementText());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51192: Fix reported range of partial token replacement

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: klimek, rsmith.
Herald added a subscriber: cfe-commits.

Fixes bug: 38678


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51192

Files:
  clang-tidy/ClangTidy.cpp


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -125,7 +125,6 @@
   // Retrieve the source range for applicable fixes. Macro definitions
   // on the command line have locations in a virtual buffer and don't
   // have valid file paths and are therefore not applicable.
-  SourceRange Range;
   SourceLocation FixLoc;
   ++TotalFixes;
   bool CanBeApplied = false;
@@ -166,7 +165,7 @@
 FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
 SourceLocation FixEndLoc =
 FixLoc.getLocWithOffset(Repl.getLength());
-Range = SourceRange(FixLoc, FixEndLoc);
+CharSourceRange Range = CharSourceRange::getCharRange(FixLoc, 
FixEndLoc);
 Diag << FixItHint::CreateReplacement(Range,
  Repl.getReplacementText());
   }


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -125,7 +125,6 @@
   // Retrieve the source range for applicable fixes. Macro definitions
   // on the command line have locations in a virtual buffer and don't
   // have valid file paths and are therefore not applicable.
-  SourceRange Range;
   SourceLocation FixLoc;
   ++TotalFixes;
   bool CanBeApplied = false;
@@ -166,7 +165,7 @@
 FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
 SourceLocation FixEndLoc =
 FixLoc.getLocWithOffset(Repl.getLength());
-Range = SourceRange(FixLoc, FixEndLoc);
+CharSourceRange Range = CharSourceRange::getCharRange(FixLoc, FixEndLoc);
 Diag << FixItHint::CreateReplacement(Range,
  Repl.getReplacementText());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340586 - Fix typo

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Thu Aug 23 15:41:52 2018
New Revision: 340586

URL: http://llvm.org/viewvc/llvm-project?rev=340586&view=rev
Log:
Fix typo

Modified:
cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp

Modified: 
cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp?rev=340586&r1=340585&r2=340586&view=diff
==
--- cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp 
(original)
+++ cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp 
Thu Aug 23 15:41:52 2018
@@ -7,7 +7,7 @@
 //
 
//===--===//
 //
-// This tablegen backend emits an fficient function to translate HTML named
+// This tablegen backend emits an efficient function to translate HTML named
 // character references to UTF-8 sequences.
 //
 
//===--===//


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

[PATCH] D50738: Remove vestiges of configure buildsystem

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 162283.
steveire added a comment.

Remove vestiges of configure buildsystem


Repository:
  rC Clang

https://reviews.llvm.org/D50738

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -285,26 +285,14 @@
 set(CLANG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(CLANG_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
-if( CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE )
-  message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite 
"
-"the makefiles distributed with LLVM. Please create a directory and run cmake "
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE)
+  message(FATAL_ERROR "In-source builds are not allowed. "
+"Please create a directory and run cmake "
 "from there, passing the path to this source directory as the last argument. "
 "This process created the file `CMakeCache.txt' and the directory "
 "`CMakeFiles'. Please delete them.")
 endif()
 
-if( NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR )
-  file(GLOB_RECURSE
-tablegenned_files_on_include_dir
-"${CLANG_SOURCE_DIR}/include/clang/*.inc")
-  if( tablegenned_files_on_include_dir )
-message(FATAL_ERROR "Apparently there is a previous in-source build, "
-"probably as the result of running `configure' and `make' on "
-"${CLANG_SOURCE_DIR}. This may cause problems. The suspicious files are:\n"
-"${tablegenned_files_on_include_dir}\nPlease clean the source directory.")
-  endif()
-endif()
-
 # If CLANG_VERSION_* is specified, use it, if not use LLVM_VERSION_*.
 if(NOT DEFINED CLANG_VERSION_MAJOR)
   set(CLANG_VERSION_MAJOR ${LLVM_VERSION_MAJOR})


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -285,26 +285,14 @@
 set(CLANG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(CLANG_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
-if( CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE )
-  message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite "
-"the makefiles distributed with LLVM. Please create a directory and run cmake "
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE)
+  message(FATAL_ERROR "In-source builds are not allowed. "
+"Please create a directory and run cmake "
 "from there, passing the path to this source directory as the last argument. "
 "This process created the file `CMakeCache.txt' and the directory "
 "`CMakeFiles'. Please delete them.")
 endif()
 
-if( NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR )
-  file(GLOB_RECURSE
-tablegenned_files_on_include_dir
-"${CLANG_SOURCE_DIR}/include/clang/*.inc")
-  if( tablegenned_files_on_include_dir )
-message(FATAL_ERROR "Apparently there is a previous in-source build, "
-"probably as the result of running `configure' and `make' on "
-"${CLANG_SOURCE_DIR}. This may cause problems. The suspicious files are:\n"
-"${tablegenned_files_on_include_dir}\nPlease clean the source directory.")
-  endif()
-endif()
-
 # If CLANG_VERSION_* is specified, use it, if not use LLVM_VERSION_*.
 if(NOT DEFINED CLANG_VERSION_MAJOR)
   set(CLANG_VERSION_MAJOR ${LLVM_VERSION_MAJOR})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50662: Add dump() method for SourceRange

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: unittests/Basic/SourceManagerTest.cpp:189
+
+  // TODO: How do I get a loc in another file?
+  auto headerLoc = 
SourceMgr.getSpellingLoc(SourceMgr.translateLineCol(MainFileID, 3, 5));

I don't know how to resolve this TODO, and somehow test a range between two 
files.


Repository:
  rC Clang

https://reviews.llvm.org/D50662



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

[PATCH] D50662: Add dump() method for SourceRange

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 162282.
steveire marked 3 inline comments as done.
steveire added a comment.

Add dump() and supporting methods to SourceRange


Repository:
  rC Clang

https://reviews.llvm.org/D50662

Files:
  include/clang/Basic/SourceLocation.h
  lib/Basic/SourceLocation.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -155,6 +155,52 @@
   EXPECT_EQ(1U, SourceMgr.getColumnNumber(MainFileID, 0, nullptr));
 }
 
+TEST_F(SourceManagerTest, locationPrintTest) {
+  const char *header =
+"#define IDENTITY(x) x\n";
+
+  const char *Source =
+"int x;\n"
+"include \"test-header.h\"\n"
+"IDENTITY(int y);\n"
+"int z;";
+
+  std::unique_ptr HeaderBuf =
+  llvm::MemoryBuffer::getMemBuffer(header);
+  std::unique_ptr Buf =
+  llvm::MemoryBuffer::getMemBuffer(Source);
+
+  const FileEntry *sourceFile = FileMgr.getVirtualFile("/mainFile.cpp",
+ Buf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(sourceFile, std::move(Buf));
+
+  const FileEntry *headerFile = FileMgr.getVirtualFile("/test-header.h",
+ HeaderBuf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
+
+  FileID MainFileID = SourceMgr.getOrCreateFileID(sourceFile, SrcMgr::C_User);
+  SourceMgr.setMainFileID(MainFileID);
+
+  auto beginLoc = SourceMgr.getLocForStartOfFile(MainFileID);
+  auto endLoc = SourceMgr.getLocForEndOfFile(MainFileID);
+
+  auto beginEOLLoc = SourceMgr.translateLineCol(MainFileID, 1, 7);
+
+  // TODO: How do I get a loc in another file?
+  auto headerLoc = SourceMgr.getSpellingLoc(SourceMgr.translateLineCol(MainFileID, 3, 5));
+
+  EXPECT_EQ(beginLoc.printToString(SourceMgr), "/mainFile.cpp:1:1");
+  EXPECT_EQ(endLoc.printToString(SourceMgr), "/mainFile.cpp:4:7");
+
+  EXPECT_EQ(beginEOLLoc.printToString(SourceMgr), "/mainFile.cpp:1:7");
+  EXPECT_EQ(headerLoc.printToString(SourceMgr), "TODO");
+
+  EXPECT_EQ(SourceRange(beginLoc, beginLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, beginEOLLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, endLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, headerLoc).printToString(SourceMgr), "");
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: lib/Basic/SourceLocation.cpp
===
--- lib/Basic/SourceLocation.cpp
+++ lib/Basic/SourceLocation.cpp
@@ -80,6 +80,60 @@
   llvm::errs() << '\n';
 }
 
+LLVM_DUMP_METHOD void SourceRange::dump(const SourceManager &SM) const {
+  print(llvm::errs(), SM);
+  llvm::errs() << '\n';
+}
+
+static PresumedLoc printDifference(raw_ostream &OS, const SourceManager &SM, SourceLocation Loc, PresumedLoc Previous)
+{
+  if (Loc.isFileID()) {
+
+PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+
+if (PLoc.isInvalid()) {
+  OS << "";
+  return Previous;
+}
+
+if (Previous.isInvalid() || strcmp(PLoc.getFilename(), Previous.getFilename()) != 0) {
+  OS << PLoc.getFilename() << ':' << PLoc.getLine()
+ << ':' << PLoc.getColumn();
+} else if (Previous.isInvalid() || PLoc.getLine() != Previous.getLine()) {
+  OS << "line" << ':' << PLoc.getLine()
+ << ':' << PLoc.getColumn();
+} else {
+  OS << "col" << ':' << PLoc.getColumn();
+}
+return PLoc;
+  }
+  auto printedLoc = printDifference(OS, SM, SM.getExpansionLoc(Loc), Previous);
+
+  OS << " ';
+  return printedLoc;
+}
+
+void SourceRange::print(raw_ostream &OS, const SourceManager &SM) const {
+
+  OS << '<';
+  auto printedLoc = printDifference(OS, SM, B, {});
+  if (B != E) {
+OS << ", ";
+printDifference(OS, SM, E, printedLoc);
+  }
+  OS << '>';
+}
+
+LLVM_DUMP_METHOD std::string
+SourceRange::printToString(const SourceManager &SM) const {
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  print(OS, SM);
+  return OS.str();
+}
+
 //===--===//
 // FullSourceLoc
 //===--===//
Index: include/clang/Basic/SourceLocation.h
===
--- include/clang/Basic/SourceLocation.h
+++ include/clang/Basic/SourceLocation.h
@@ -220,6 +220,10 @@
   bool operator!=(const SourceRange &X) const {
 return B != X.B || E != X.E;
   }
+
+  void print(raw_ostream &OS, const SourceManager &SM) const;
+  std::string printToString(const SourceManager &SM) const;
+  void dump(const SourceManager &SM) const;
 };
 
 /// Represents a character-granular source range.
___
cfe-commits mailing list

[PATCH] D50662: Add dump() method for SourceRange

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 162280.
steveire added a comment.

Add dump() and supporting methods to SourceRange


Repository:
  rC Clang

https://reviews.llvm.org/D50662

Files:
  include/clang/Basic/SourceLocation.h
  lib/Basic/SourceLocation.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -155,6 +155,53 @@
   EXPECT_EQ(1U, SourceMgr.getColumnNumber(MainFileID, 0, nullptr));
 }
 
+TEST_F(SourceManagerTest, locationPrintTest) {
+  const char *header =
+"#define IDENTITY(x) x\n";
+
+  const char *Source =
+"int x;\n"
+"include \"test-header.h\"\n"
+"IDENTITY(int y);\n"
+"int z;";
+
+  std::unique_ptr HeaderBuf =
+  llvm::MemoryBuffer::getMemBuffer(header);
+  std::unique_ptr Buf =
+  llvm::MemoryBuffer::getMemBuffer(Source);
+
+  const FileEntry *sourceFile = FileMgr.getVirtualFile("/mainFile.cpp",
+ Buf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(sourceFile, std::move(Buf));
+
+
+  const FileEntry *headerFile = FileMgr.getVirtualFile("/test-header.h",
+ HeaderBuf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
+
+  FileID MainFileID = SourceMgr.getOrCreateFileID(sourceFile, SrcMgr::C_User);
+  SourceMgr.setMainFileID(MainFileID);
+
+  auto beginLoc = SourceMgr.getLocForStartOfFile(MainFileID);
+  auto endLoc = SourceMgr.getLocForEndOfFile(MainFileID);
+
+  auto beginEOLLoc = SourceMgr.translateLineCol(MainFileID, 1, 7);
+
+  // TODO: How do I get a loc in another file?
+  auto headerLoc = SourceMgr.getSpellingLoc(SourceMgr.translateLineCol(MainFileID, 3, 5));
+
+  EXPECT_EQ(beginLoc.printToString(SourceMgr), "/mainFile.cpp:1:1");
+  EXPECT_EQ(endLoc.printToString(SourceMgr), "/mainFile.cpp:4:7");
+
+  EXPECT_EQ(beginEOLLoc.printToString(SourceMgr), "/mainFile.cpp:1:7");
+  EXPECT_EQ(headerLoc.printToString(SourceMgr), "TODO");
+
+  EXPECT_EQ(SourceRange(beginLoc, beginLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, beginEOLLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, endLoc).printToString(SourceMgr), "");
+  EXPECT_EQ(SourceRange(beginLoc, headerLoc).printToString(SourceMgr), "");
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: lib/Basic/SourceLocation.cpp
===
--- lib/Basic/SourceLocation.cpp
+++ lib/Basic/SourceLocation.cpp
@@ -80,6 +80,60 @@
   llvm::errs() << '\n';
 }
 
+LLVM_DUMP_METHOD void SourceRange::dump(const SourceManager &SM) const {
+  print(llvm::errs(), SM);
+  llvm::errs() << '\n';
+}
+
+static PresumedLoc printDifference(raw_ostream &OS, const SourceManager &SM, SourceLocation Loc, PresumedLoc Previous)
+{
+  if (Loc.isFileID()) {
+
+PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+
+if (PLoc.isInvalid()) {
+  OS << "";
+  return Previous;
+}
+
+if (Previous.isInvalid() || strcmp(PLoc.getFilename(), Previous.getFilename()) != 0) {
+  OS << PLoc.getFilename() << ':' << PLoc.getLine()
+ << ':' << PLoc.getColumn();
+} else if (Previous.isInvalid() || PLoc.getLine() != Previous.getLine()) {
+  OS << "line" << ':' << PLoc.getLine()
+ << ':' << PLoc.getColumn();
+} else {
+  OS << "col" << ':' << PLoc.getColumn();
+}
+return PLoc;
+  }
+  auto printedLoc = printDifference(OS, SM, SM.getExpansionLoc(Loc), Previous);
+
+  OS << " ';
+  return printedLoc;
+}
+
+void SourceRange::print(raw_ostream &OS, const SourceManager &SM) const {
+
+  OS << '<';
+  auto printedLoc = printDifference(OS, SM, B, {});
+  if (B != E) {
+OS << ", ";
+printDifference(OS, SM, E, printedLoc);
+  }
+  OS << '>';
+}
+
+LLVM_DUMP_METHOD std::string
+SourceRange::printToString(const SourceManager &SM) const {
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  print(OS, SM);
+  return OS.str();
+}
+
 //===--===//
 // FullSourceLoc
 //===--===//
Index: include/clang/Basic/SourceLocation.h
===
--- include/clang/Basic/SourceLocation.h
+++ include/clang/Basic/SourceLocation.h
@@ -220,6 +220,10 @@
   bool operator!=(const SourceRange &X) const {
 return B != X.B || E != X.E;
   }
+
+  void print(raw_ostream &OS, const SourceManager &SM) const;
+  std::string printToString(const SourceManager &SM) const;
+  void dump(const SourceManager &SM) const;
 };
 
 /// Represents a character-granular source range.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

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

This wasn't documented https://clang.llvm.org/docs/AttributeReference.html, and 
briefly mentioned 
https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes.


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added a subscriber: dexonsmith.

rdar://18335828

Thanks!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D51189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/NSAPI.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/infer-availability-from-init.m

Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- /dev/null
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+
+__attribute__((objc_root_class))
+@interface NSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface MyObject : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note{{'init' has been explicitly marked unavailable here}}
+@end
+
+void usemyobject() {
+  [MyObject new]; // expected-error{{'new' is unavailable}}
+}
+
+@interface MyOtherObject : NSObject
++(instancetype)init __attribute__((unavailable));
++(instancetype)new;
+@end
+
+void usemyotherobject() {
+  [MyOtherObject new]; // no error; new is overrideen.
+}
+
+@interface NotGoodOverride : NSObject
++(instancetype)init __attribute__((unavailable));
+-(instancetype)new;
++(instancetype)new: (int)x;
+@end
+
+void usenotgoodoverride() {
+  [NotGoodOverride new]; // no error
+}
+
+@interface NotNSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface NotMyObject : NotNSObject
+-(instancetype)init __attribute__((unavailable));
+@end
+
+void usenotmyobject() {
+  [NotMyObject new]; // no error; this isn't NSObject
+}
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2471,7 +2471,8 @@
 if (!Method)
   Method = Class->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs,
+nullptr, false, false, Class))
   return ExprError();
   }
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -206,7 +206,8 @@
 bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
  const ObjCInterfaceDecl *UnknownObjCClass,
  bool ObjCPropertyAccess,
- bool AvoidPartialAvailabilityChecks) {
+ bool AvoidPartialAvailabilityChecks,
+ ObjCInterfaceDecl *CD) {
   SourceLocation Loc = Locs.front();
   if (getLangOpts().CPlusPlus && isa(D)) {
 // If there were any diagnostics suppressed by template argument deduction,
@@ -292,7 +293,7 @@
   }
 
   DiagnoseAvailabilityOfDecl(D, Locs, UnknownObjCClass, ObjCPropertyAccess,
- AvoidPartialAvailabilityChecks);
+ AvoidPartialAvailabilityChecks, CD);
 
   DiagnoseUnusedOfDecl(*this, D, Loc);
 
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6943,8 +6943,12 @@
 /// \param D The declaration to check.
 /// \param Message If non-null, this will be populated with the message from
 /// the availability attribute that is selected.
+/// \param ClassMessageReceiver If we're checking the the method of a class
+/// message send, the class. Otherwise nullptr.
 static std::pair
-ShouldDiagnoseAvailabilityOfDecl(const NamedDecl *D, std::string *Message) {
+ShouldDiagnoseAvailabilityOfDecl(Sema &S, const NamedDecl *D,
+ std::string *Message,
+ ObjCInterfaceDecl *ClassMessageReceiver) {
   AvailabilityResult Result = D->getAvailability(Message);
 
   // For typedefs, if the typedef declaration appears available look
@@ -6977,6 +6981,20 @@
   }
 }
 
+  // For +new, infer availability from -init.
+  if (const auto *MD = dyn_cast(D)) {
+if (S.NSAPIObj != nullptr && ClassMessageReceiver != nullptr) {
+  ObjCMethodDecl *Init = ClassMessageReceiver->lookupInstanceMethod(
+  S.NSAPIObj->getInitSelector());
+  if (Init != nullptr && Result == AR_Available && MD->isClassMethod() &&
+  MD->getSelector() == S.NSAPIObj->getNewSelector() &&
+  MD->definedInNSObject(S.getASTContext())) {
+Result = Init->getAvailability(Message);
+D = Init;
+  }
+}
+  }
+
   return {Result, D};
 }
 
@@ -75

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Kernel patch: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=815f0ddb346c196018d4d8f8f55c12b83da1de3f

Thanks Eli and Richard, I appreciate it.


Repository:
  rC Clang

https://reviews.llvm.org/D51011



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

[PATCH] D51187: Thread safety analysis: Warn on double (un-)lock of scoped capability

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: delesley, aaron.ballman.
Herald added a subscriber: cfe-commits.

We now warn when acquiring or releasing a scoped capability a second
time, not just if the underlying mutexes have been acquired or released
a second time. It's debatable whether that should really be a warning,
but there seem to be more advantages.


Repository:
  rC Clang

https://reviews.llvm.org/D51187

Files:
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2605,16 +2605,16 @@
 void Foo::test4() {
   ReleasableMutexLock rlock(&mu_);
   rlock.Release();
-  rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not held}}
+  rlock.Release();  // expected-warning {{releasing mutex 'rlock' that was not held}}
 }
 
 void Foo::test5() {
   ReleasableMutexLock rlock(&mu_);
   if (c) {
 rlock.Release();
   }
   // no warning on join point for managed lock.
-  rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not held}}
+  rlock.Release();  // expected-warning {{releasing mutex 'rlock' that was not held}}
 }
 
 
@@ -2704,36 +2704,32 @@
 void doubleUnlock() {
   RelockableExclusiveMutexLock scope(&mu);
   scope.Unlock();
-  scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
+  scope.Unlock(); // expected-warning {{releasing mutex 'scope' that was not held}}
 }
 
 void doubleLock1() {
   RelockableExclusiveMutexLock scope(&mu);
-  scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
+  scope.Lock(); // expected-warning {{acquiring mutex 'scope' that is already held}}
 }
 
 void doubleLock2() {
   RelockableExclusiveMutexLock scope(&mu);
   scope.Unlock();
   scope.Lock();
-  scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
+  scope.Lock(); // expected-warning {{acquiring mutex 'scope' that is already held}}
 }
 
 void directUnlock() {
   RelockableExclusiveMutexLock scope(&mu);
   mu.Unlock();
-  // Debatable that there is no warning. Currently we don't track in the scoped
-  // object whether it is active, but just check if the contained locks can be
-  // reacquired. Here they can, because mu has been unlocked manually.
-  scope.Lock();
+  scope.Lock(); // expected-warning {{acquiring mutex 'scope' that is already held}}
 }
 
 void directRelock() {
   RelockableExclusiveMutexLock scope(&mu);
   scope.Unlock();
   mu.Lock();
-  // Similarly debatable that there is no warning.
-  scope.Unlock();
+  scope.Unlock(); // expected-warning {{releasing mutex 'scope' that was not held}}
 }
 
 // Doesn't make a lot of sense, just making sure there is no crash.
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -144,11 +144,11 @@
 ThreadSafetyHandler &Handler) const = 0;
   virtual void handleLock(FactSet &FSet, FactManager &FactMan,
   const FactEntry &entry, ThreadSafetyHandler &Handler,
-  StringRef DiagKind) const = 0;
+  StringRef DiagKind) = 0;
   virtual void handleUnlock(FactSet &FSet, FactManager &FactMan,
 const CapabilityExpr &Cp, SourceLocation UnlockLoc,
 bool FullyRemove, ThreadSafetyHandler &Handler,
-StringRef DiagKind) const = 0;
+StringRef DiagKind) = 0;
 
   // Return true if LKind >= LK, where exclusive > shared
   bool isAtLeast(LockKind LK) {
@@ -877,15 +877,14 @@
   }
 
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
-  ThreadSafetyHandler &Handler,
-  StringRef DiagKind) const override {
+  ThreadSafetyHandler &Handler, StringRef DiagKind) override {
 Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc());
   }
 
   void handleUnlock(FactSet &FSet, FactManager &FactMan,
 const CapabilityExpr &Cp, SourceLocation UnlockLoc,
 bool FullyRemove, ThreadSafetyHandler &Handler,
-StringRef DiagKind) const override {
+StringRef DiagKind) override {
 FSet.removeLock(FactMan, Cp);
 if (!Cp.negative()) {
   FSet.addLock(FactMan, llvm::make_unique(
@@ -897,6 +896,7 @@
 class ScopedLockableFactEntry : public FactEntry {
 private:
   SmallVector UnderlyingMutexes;
+  bool Locked = true; // Are the UnderlyingMutexes currently locked?
 
 public:
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
@@ -923,8 +923,11 @@
   }
 
   void handleLock(FactSet &FSet, FactManager &FactM

r340580 - Remove more const_casts by using ConstStmtVisitor [NFC]

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Thu Aug 23 14:53:04 2018
New Revision: 340580

URL: http://llvm.org/viewvc/llvm-project?rev=340580&view=rev
Log:
Remove more const_casts by using ConstStmtVisitor [NFC]

Again, this required adding some const specifiers.

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340580&r1=340579&r2=340580&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 23 14:53:04 2018
@@ -422,7 +422,7 @@ private:
   Context::Factory ContextFactory;
   std::vector VarDefinitions;
   std::vector CtxIndices;
-  std::vector> SavedContexts;
+  std::vector> SavedContexts;
 
 public:
   LocalVariableMap() {
@@ -463,7 +463,7 @@ public:
   /// Return the next context after processing S.  This function is used by
   /// clients of the class to get the appropriate context when traversing the
   /// CFG.  It must be called for every assignment or DeclStmt.
-  Context getNextContext(unsigned &CtxIndex, Stmt *S, Context C) {
+  Context getNextContext(unsigned &CtxIndex, const Stmt *S, Context C) {
 if (SavedContexts[CtxIndex+1].first == S) {
   CtxIndex++;
   Context Result = SavedContexts[CtxIndex].second;
@@ -525,7 +525,7 @@ protected:
   unsigned getContextIndex() { return SavedContexts.size()-1; }
 
   // Save the current context for later replay
-  void saveContext(Stmt *S, Context C) {
+  void saveContext(const Stmt *S, Context C) {
 SavedContexts.push_back(std::make_pair(S, C));
   }
 
@@ -595,7 +595,7 @@ CFGBlockInfo CFGBlockInfo::getEmptyBlock
 namespace {
 
 /// Visitor which builds a LocalVariableMap
-class VarMapBuilder : public StmtVisitor {
+class VarMapBuilder : public ConstStmtVisitor {
 public:
   LocalVariableMap* VMap;
   LocalVariableMap::Context Ctx;
@@ -603,16 +603,16 @@ public:
   VarMapBuilder(LocalVariableMap *VM, LocalVariableMap::Context C)
   : VMap(VM), Ctx(C) {}
 
-  void VisitDeclStmt(DeclStmt *S);
-  void VisitBinaryOperator(BinaryOperator *BO);
+  void VisitDeclStmt(const DeclStmt *S);
+  void VisitBinaryOperator(const BinaryOperator *BO);
 };
 
 } // namespace
 
 // Add new local variables to the variable map
-void VarMapBuilder::VisitDeclStmt(DeclStmt *S) {
+void VarMapBuilder::VisitDeclStmt(const DeclStmt *S) {
   bool modifiedCtx = false;
-  DeclGroupRef DGrp = S->getDeclGroup();
+  const DeclGroupRef DGrp = S->getDeclGroup();
   for (const auto *D : DGrp) {
 if (const auto *VD = dyn_cast_or_null(D)) {
   const Expr *E = VD->getInit();
@@ -630,7 +630,7 @@ void VarMapBuilder::VisitDeclStmt(DeclSt
 }
 
 // Update local variable definitions in variable map
-void VarMapBuilder::VisitBinaryOperator(BinaryOperator *BO) {
+void VarMapBuilder::VisitBinaryOperator(const BinaryOperator *BO) {
   if (!BO->isAssignmentOp())
 return;
 
@@ -783,7 +783,7 @@ void LocalVariableMap::traverseCFG(CFG *
   switch (BI.getKind()) {
 case CFGElement::Statement: {
   CFGStmt CS = BI.castAs();
-  VMapBuilder.Visit(const_cast(CS.getStmt()));
+  VMapBuilder.Visit(CS.getStmt());
   break;
 }
 default:
@@ -1520,7 +1520,7 @@ namespace {
 /// An expression may cause us to add or remove locks from the lockset, or else
 /// output error messages related to missing locks.
 /// FIXME: In future, we may be able to not inherit from a visitor.
-class BuildLockset : public StmtVisitor {
+class BuildLockset : public ConstStmtVisitor {
   friend class ThreadSafetyAnalyzer;
 
   ThreadSafetyAnalyzer *Analyzer;
@@ -1540,19 +1540,19 @@ class BuildLockset : public StmtVisitor<
   void checkPtAccess(const Expr *Exp, AccessKind AK,
  ProtectedOperationKind POK = POK_VarAccess);
 
-  void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
+  void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
 
 public:
   BuildLockset(ThreadSafetyAnalyzer *Anlzr, CFGBlockInfo &Info)
-  : StmtVisitor(), Analyzer(Anlzr), FSet(Info.EntrySet),
+  : ConstStmtVisitor(), Analyzer(Anlzr), FSet(Info.EntrySet),
 LVarCtx(Info.EntryContext), CtxIndex(Info.EntryIndex) {}
 
-  void VisitUnaryOperator(UnaryOperator *UO);
-  void VisitBinaryOperator(BinaryOperator *BO);
-  void VisitCastExpr(CastExpr *CE);
-  void VisitCallExpr(CallExpr *Exp);
-  void VisitCXXConstructExpr(CXXConstructExpr *Exp);
-  void VisitDeclStmt(DeclStmt *S);
+  void VisitUnaryOperator(const UnaryOperator *UO);
+  void VisitBinaryOperator(const BinaryOperator *BO);
+  void VisitCastExpr(const CastExpr *CE);
+  void VisitCallExpr(const CallExpr *Exp);
+  void VisitCXXConstructExpr(const CXXConstructExpr *Exp);
+  void VisitDeclStmt(const DeclStmt *S);
 };
 
 } // namespace
@@ -1744,7 +1744,8 @@ void BuildLockset::checkPtAccess(const E
 /// 

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

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

I received another report of breakage so I reverted in 
https://reviews.llvm.org/rC340579.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

r340579 - Revert r340552, "Driver: Enable address-significance tables by default when targeting COFF."

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Thu Aug 23 14:34:57 2018
New Revision: 340579

URL: http://llvm.org/viewvc/llvm-project?rev=340579&view=rev
Log:
Revert r340552, "Driver: Enable address-significance tables by default when 
targeting COFF."

Received multiple reports of breakage due to undefined symbols
suspected to be caused by this change.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/addrsig.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=340579&r1=340578&r2=340579&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Aug 23 14:34:57 2018
@@ -4857,8 +4857,7 @@ void Clang::ConstructJob(Compilation &C,
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   (getToolChain().getTriple().isOSBinFormatELF() ||
-getToolChain().getTriple().isOSBinFormatCOFF()) &&
+   getToolChain().getTriple().isOSBinFormatELF() &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 

Modified: cfe/trunk/test/Driver/addrsig.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/addrsig.c?rev=340579&r1=340578&r2=340579&view=diff
==
--- cfe/trunk/test/Driver/addrsig.c (original)
+++ cfe/trunk/test/Driver/addrsig.c Thu Aug 23 14:34:57 2018
@@ -1,5 +1,4 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
-// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s


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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Thanks, I'll take a look.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

This breaks Windows bot
 
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/33846/steps/run%20check-asan/logs/stdio

  742542.065 [0/1/37] Running the AddressSanitizer tests
  -- Testing: 607 tests, 16 threads --
  Testing: 0 .. 10.. 20.. 30.. 40.. 50
  FAIL: AddressSanitizer-i386-windows :: TestCases/Windows/coverage-basic.cc 
(332 of 607)
   TEST 'AddressSanitizer-i386-windows :: 
TestCases/Windows/coverage-basic.cc' FAILED 
  Script:
  --
  : 'RUN: at line 1';   rm -rf 
C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\asan\I386WindowsConfig\TestCases\Windows\Output\coverage-basic.cc.tmp-dir
  : 'RUN: at line 2';   mkdir 
C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\asan\I386WindowsConfig\TestCases\Windows\Output\coverage-basic.cc.tmp-dir
 && cd 
C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\asan\I386WindowsConfig\TestCases\Windows\Output\coverage-basic.cc.tmp-dir
  : 'RUN: at line 3';  C:/b/slave/sanitizer-windows/build/./bin/clang.exe  
-fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer 
-fno-optimize-sibling-calls -gline-tables-only -gcodeview -gcolumn-info   
-fms-compatibility-version=19.00.24215.1  -fsanitize-coverage=func 
C:\b\slave\sanitizer-windows\llvm\projects\compiler-rt\test\asan\TestCases\Windows\coverage-basic.cc
 -o test.exe
  : 'RUN: at line 4';   env ASAN_OPTIONS=coverage=1  ./test.exe
  : 'RUN: at line 6';   C:/Python27/python.exe 
C:/b/slave/sanitizer-windows/llvm/projects/compiler-rt\lib\sanitizer_common\scripts\sancov.py
 print *.sancov | FileCheck 
C:\b\slave\sanitizer-windows\llvm\projects\compiler-rt\test\asan\TestCases\Windows\coverage-basic.cc
  --
  Exit Code: 1120
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "rm" "-rf" 
"C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\asan\I386WindowsConfig\TestCases\Windows\Output\coverage-basic.cc.tmp-dir"
  $ ":" "RUN: at line 2"
  $ "mkdir" 
"C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\asan\I386WindowsConfig\TestCases\Windows\Output\coverage-basic.cc.tmp-dir"
  $ ":" "RUN: at line 3"
  $ "C:/b/slave/sanitizer-windows/build/./bin/clang.exe" "-fsanitize=address" 
"-mno-omit-leaf-frame-pointer" "-fno-omit-frame-pointer" 
"-fno-optimize-sibling-calls" "-gline-tables-only" "-gcodeview" "-gcolumn-info" 
"-fms-compatibility-version=19.00.24215.1" "-fsanitize-coverage=func" 
"C:\b\slave\sanitizer-windows\llvm\projects\compiler-rt\test\asan\TestCases\Windows\coverage-basic.cc"
 "-o" "test.exe"
  # command output:
 Creating library test.lib and object test.exp
  
  coverage-basic-f4c01a.o : error LNK2001: unresolved external symbol 
___sanitizer_cov_trace_pc
  
  test.exe : fatal error LNK1120: 1 unresolved externals


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

r340575 - Remove unnecessary const_cast [NFC]

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Thu Aug 23 14:13:32 2018
New Revision: 340575

URL: http://llvm.org/viewvc/llvm-project?rev=340575&view=rev
Log:
Remove unnecessary const_cast [NFC]

This required adding a few const specifiers on functions.

Also a minor formatting fix suggested in D49885.

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340575&r1=340574&r2=340575&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 23 14:13:32 2018
@@ -929,9 +929,9 @@ public:
   CapabilityExpr UnderCp(UnderlyingMutex, false);
 
   // We're relocking the underlying mutexes. Warn on double locking.
-  if (FSet.findLock(FactMan, UnderCp))
+  if (FSet.findLock(FactMan, UnderCp)) {
 Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
-  else {
+  } else {
 FSet.removeLock(FactMan, !UnderCp);
 FSet.addLock(FactMan, llvm::make_unique(
   UnderCp, entry.kind(), entry.loc()));
@@ -1002,11 +1002,11 @@ public:
   StringRef DiagKind);
 
   template 
-  void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
+  void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
const NamedDecl *D, VarDecl *SelfDecl = nullptr);
 
   template 
-  void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
+  void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
const NamedDecl *D,
const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
Expr *BrE, bool Neg);
@@ -1315,7 +1315,7 @@ void ThreadSafetyAnalyzer::removeLock(Fa
 /// and push them onto Mtxs, discarding any duplicates.
 template 
 void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
-   Expr *Exp, const NamedDecl *D,
+   const Expr *Exp, const NamedDecl *D,
VarDecl *SelfDecl) {
   if (Attr->args_size() == 0) {
 // The mutex held is the "this" object.
@@ -1347,7 +1347,7 @@ void ThreadSafetyAnalyzer::getMutexIDs(C
 /// any duplicates.
 template 
 void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
-   Expr *Exp, const NamedDecl *D,
+   const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
Expr *BrE, bool Neg) {
@@ -1460,7 +1460,7 @@ void ThreadSafetyAnalyzer::getEdgeLockse
   const LocalVarContext &LVarCtx = PredBlockInfo->ExitContext;
   StringRef CapDiagKind = "mutex";
 
-  auto *Exp = const_cast(getTrylockCallExpr(Cond, LVarCtx, 
Negate));
+  const auto *Exp = getTrylockCallExpr(Cond, LVarCtx, Negate);
   if (!Exp)
 return;
 


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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Not much more comments from me. The implementation seems reasonable, and works 
for one simple test I did (with an earlier revision of the patch at least), and 
further refinement can happen in-tree I guess.

I'd like to have someone else (@rnk @compnerd?) give it a more proper approval 
though, at least a general ack for the style/structure.




Comment at: include/__libunwind_config.h:66
+#if defined(__ARM_WMMX)
+#  define _LIBUNWIND_CONTEXT_SIZE 61
+#else

I don't think the `__ARM_WMMX` case here is relevant; there are no ARM CPUs 
with WMMX running modern windows on arm, afaik (and the size number here I 
presume only is a copy from the one below); sorry for not pointing it out 
earlier.



Comment at: src/UnwindCursor.hpp:54
 #include "EHHeaderParser.hpp"
-#include "libunwind.h"
+#include "libunwind_ext.h"
 #include "Registers.hpp"

mstorsjo wrote:
> This looks like another leftover; there's no `libunwind_ext.h` any longer 
> here.
Sorry I think I misread and looked for `libunwind_ext.h` in the `include` dir 
only. But in case it isn't really needed here, keep the old `libunwind.h` 
include at least, in order not to break other potential build configurations 
that might need it.



Comment at: src/UnwindCursor.hpp:1157
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  pint_t getLastPC() const { /* FIXME: Implement */ return 0; }

cdavis5x wrote:
> mstorsjo wrote:
> > What does this whole block do here? Isn't this within an !SEH ifdef block? 
> > The same methods are declared for the SEH version of this struct further 
> > above.
> Right now, it doesn't do anything. I added it so @kristina has someplace to 
> put the code for unwinding with SEH data when we're //not// on Windows and we 
> //don't// have the `RtlUnwindEx()` function available. You'll note that the 
> '!SEH' `ifdef` now says:
> 
> ```lang=c++
> #if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
> ```
Ah, I see. Maybe a comment clarifying that here as well?


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld planned changes to this revision.
Hahnfeld added a comment.

This patch breaks C++ and CUDA compilation at the moment, sorry. I need to find 
and add more macros that turn out to be needed.


Repository:
  rC Clang

https://reviews.llvm.org/D50845



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

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D50845#1211463, @gregrodgers wrote:

> What am I missing?


As discussed above this patch doesn't fix this problem. However we need 
`__x86_64__` because `bits/wordsize.h` will use it to determine if we are 64- 
or 32-bit.


Repository:
  rC Clang

https://reviews.llvm.org/D50845



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

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

[Greg Rodgers via Phabricator via cfe-commits]

gregrodgers added a comment.

I have a longer comment on header files,  but let me first understand this 
patch.

IIUC,the concept of this patch is to fake the macros to think it is seeing
a host on the device patch.

if ((LangOpts.CUDA || LangOpts.OpenMPIsDevice) && PP.getAuxTargetInfo())

  InitializePredefinedAuxMacros(*PP.getAuxTargetInfo(), Builder);

That would be counterproductive because well-behaved headers that only
provide optized asm definitions would wrap that asm with

#ifdef __x86_64__

  do some x86 asm function definition

#else

  just provide the function declaration to be implemented somewhere else.

#endif

What am I missing?


Repository:
  rC Clang

https://reviews.llvm.org/D50845



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

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

rsmith added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2004-2007
+def mspeculative_load_hardening : Flag<["-"], "mspeculative-load-hardening">,
+  Group, Flags<[CoreOption,CC1Option]>;
+def mno_speculative_load_hardening : Flag<["-"], 
"mno-speculative-load-hardening">,
+  Group, Flags<[CoreOption,CC1Option]>;

We usually only provide `-cc1` flags to switch into the non-default state.



Comment at: clang/include/clang/Frontend/CodeGenOptions.def:214
 CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.
+CODEGENOPT(SpeculativeLoadHardening, 1, 0) ///< Enable 
speculative_load_hardening.
 CODEGENOPT(FineGrainedBitfieldAccesses, 1, 0) ///< Enable fine-grained 
bitfield accesses.

Spaces rather than underscores here.



Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:169-170
   options::OPT_mno_retpoline_external_thunk, false)) {
 // FIXME: Add a warning about failing to specify `-mretpoline` and
 // eventually switch to an error here.
 Features.push_back("+retpoline-indirect-calls");

`-mspeculative-load-hardening -mretpoline-external-thunk` does not enable 
`+retpoline-indirect-branches` (but `-mretpoline-external-thunk` by itself 
does). Is that intentional?



Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1169
+  case Attribute::SpeculativeLoadHardening:
+return 1ULL << 60;
   case Attribute::Dereferenceable:

rnk wrote:
> These appear to repeat LLVMBitcodes.h, unless I am mistaken. Weird. Anyway, 
> fixing that is out of scope.
The values "above the fold" in Phabricator aren't all the same as the enum 
value plus one. *shrug*


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162256.
hugoeg added a comment.

minor fixes and style improvement


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSA

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

simark wrote:
> ioeric wrote:
> > simark wrote:
> > > ioeric wrote:
> > > > simark wrote:
> > > > > ioeric wrote:
> > > > > > simark wrote:
> > > > > > > What's the rationale for only computing the field if `UFE.File` 
> > > > > > > is non-null?
> > > > > > > 
> > > > > > > Previously, if you looked up the file with `openFile == false` 
> > > > > > > and then later `openFile == true`, the `RealPathName` field would 
> > > > > > > not be set because of this.  That doesn't seem right.
> > > > > > There has been no guarantee that RealFilePath is always set. I 
> > > > > > think that's the reason why the acceasor is called 
> > > > > > tryGetRealPathName.
> > > > > The way I understood it was that it could be empty because computing 
> > > > > the real path can fail.  Not just because we didn't skipped computing 
> > > > > it.
> > > > I agree that the API is confusing and lack of documentation (and we 
> > > > should fix), but the previous implementation did skip the computation 
> > > > if File is not available though. 
> > > > I agree that the API is confusing and lack of documentation (and we 
> > > > should fix), but the previous implementation did skip the computation 
> > > > if File is not available though.
> > > 
> > > Did it have a reason not to?  What is the `RealPathName` field useful 
> > > for, if it's unreliable?
> > I think the biggest concern is the performance.
> >  
> > For example, clangd code completion latency increased dramatically with 
> > `real_path`:
> > With `real_path`:
> > {F7039629}
> > {F7041680}
> > 
> > Wihtou `real_path`:
> > {F7039630}
> But with this patch, we are not using real_path anymore, so it can't be the 
> reason for not computing `RealPathName` when not opening the file.  Since the 
> non-real_path method is much more lightweight, would it make a significant 
> difference if we always computed the field?
> 
> In the end I don't really mind, I am just looking for a rational explanation 
> to why it's done this way.
Ohh, sorry that I was confused with the other thread...

Honestly, I also don't know where all these came from (it was implemented many 
years ago...). For now, I'm just trying to fix the problem with the safest 
change, as the old behavior, although confusing, should be pretty stable.  
Changing behavior further would likely cause more problems. I would prefer 
making further change when we are actually cleaning up or redesigning 
`RealPathName`/`tryGetRealPath`.


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Reid Kleckner via Phabricator via cfe-commits]

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

It makes sense to me to do this all in one go and defer the clang 
`__attribute__` until later.




Comment at: llvm/include/llvm/IR/Attributes.td:249
 def : MergeRule<"adjustNullPointerValidAttr">;
+

I can't tell if this is just vim fixing the lack of a proper trailing newline, 
but revert any whitespace change if possible.



Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1169
+  case Attribute::SpeculativeLoadHardening:
+return 1ULL << 60;
   case Attribute::Dereferenceable:

These appear to repeat LLVMBitcodes.h, unless I am mistaken. Weird. Anyway, 
fixing that is out of scope.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case.

[Richard Smith via cfe-commits]

On Thu, 23 Aug 2018 at 02:52, Anastasia Stulova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Richard,
>
>
> There was a change in the spec to disallow unprototyped functions, which
> was made this year. Unfortunately it seems it didn't make into the Khronos
> registry yet to appear publicly. I will chase it up with Khronos.
>

Thanks!

Reiterating what I said below, I think it is reasonable and appropriate to
disallow implicit function declarations in languages that don't have
unprototyped functions -- implicitly declaring something that could not be
declared explicitly doesn't seem appropriate. So feel free to
revert r314872, but please update the comment to explain that we don't
allow this as an extension in OpenCL because OpenCL does not permit
unprototyped functions. Apologies for the round-trip time here.


> I would like to highlight that OpenCL C was based on C99 originally and
> therefore in contrast to C doesn't have backwards compatibility with the
> old C standards. I don't think it's common to either write or port old C
> code to OpenCL simply because it's not practical to run regular C code on
> OpenCL targets efficiently but also in many cases it won't even compile due
> to many restrictions.
>
>
> Anastasia
>
>
> --
> *From:* Richard Smith 
> *Sent:* 22 August 2018 21:16
> *To:* Anastasia Stulova
> *Cc:* nd; cfe-commits
> *Subject:* Re: r314872 - We allow implicit function declarations as an
> extension in all C dialects. Remove OpenCL special case.
>
> On Wed, 22 Aug 2018 at 06:55, Anastasia Stulova via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Hi Richard,
>
> > This is incorrect. Implicit function declarations declare unprototyped
> functions, which are *not* variadic, and are in fact supported by Clang's
> OpenCL language mode.
>
> We have removed the support for the unprototyped functions from the OpenCL
> as well. See commit: https://reviews.llvm.org/D33681. This is the reason
> why in the OpenCL mode we now generated empty parameter list instead of
> unprototyped function like for C in the examples I provided before (that is
> not governed now by any standard or any compiler extension).
>
>
> That's interesting. Do I understand from that review thread that we're
> diverging from the OpenCL specification in treating () as (void) rather
> than as an unprototyped function declaration? If so, that seems like a
> surprising and concerning decision, unless we're confident that the OpenCL
> language really did mean to change this aspect of the C semantics and
> omitted the wording change by oversight. (And I've checked, and can find
> nothing in the OpenCL specification that justifies this: it looks like a
> Clang bug that we reject
>
>   int f();
>   void g() { f(1, 2, 3); }
>   int f(int a, int b, int c) { return 0; }
>
> ... for example, unless Khronos really did mean to use the C++ rule.)
>
> If it is indeed the intent of the OpenCL specification to treat () as
> (void) like in C++ and not have unprototyped functions, then I think it
> does make sense to also disable our implicitly declared functions extension
> in OpenCL. But, conversely, if the OpenCL specification instead intends to
> follow C here and allow unprototyped functions, then it is a bug in our
> OpenCL support that we don't follow that intent, and when that bug is fixed
> it makes sense to continue to accept implicit function declarations as an
> extension.
>
> > I would have sympathy for your position if we did not produce an
> extension warning on this construct by default. But we do, and it says the
> construct is invalid in OpenCL; moreover, in our strict conformance mode
> (-pedantic-errors), we reject the code.
>
> I understand the motivation for C to maintain the compatibility with the
> previous standards and other compilers (like gcc) to be able to support the
> legacy code. However, for OpenCL we don't have this requirement wrt older C
> standards. And therefore it is desirable to take advantage of this and
> remove problematic features that are generally confusing for developers or
> that can't be efficiently supported by the targets (especially if there is
> a little cost to that).
>
>
> For this "can't be efficiently supported by the target" claim, I think
> you're conflating the target and the language mode. If some target can't
> reasonably support variadic functions, we should disable variadic functions
> for that target. That has *nothing* to do with the language mode; targets
> and language modes can be combined arbitrarily. [If I want to use Clang to
> compile OpenCL code for my PDP-11, then I should be allowed to do that
> (assuming I have a suitable LLVM backend), and that target presumably would
> support variadic functions just fine.] Likewise, if the target doesn't
> support variadic functions, we should not be generating variadic function
> types when producing IR (particularly in calls to non-variadic functions
> like in your example elsewhere 

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

ioeric wrote:
> simark wrote:
> > ioeric wrote:
> > > simark wrote:
> > > > ioeric wrote:
> > > > > simark wrote:
> > > > > > What's the rationale for only computing the field if `UFE.File` is 
> > > > > > non-null?
> > > > > > 
> > > > > > Previously, if you looked up the file with `openFile == false` and 
> > > > > > then later `openFile == true`, the `RealPathName` field would not 
> > > > > > be set because of this.  That doesn't seem right.
> > > > > There has been no guarantee that RealFilePath is always set. I think 
> > > > > that's the reason why the acceasor is called tryGetRealPathName.
> > > > The way I understood it was that it could be empty because computing 
> > > > the real path can fail.  Not just because we didn't skipped computing 
> > > > it.
> > > I agree that the API is confusing and lack of documentation (and we 
> > > should fix), but the previous implementation did skip the computation if 
> > > File is not available though. 
> > > I agree that the API is confusing and lack of documentation (and we 
> > > should fix), but the previous implementation did skip the computation if 
> > > File is not available though.
> > 
> > Did it have a reason not to?  What is the `RealPathName` field useful for, 
> > if it's unreliable?
> I think the biggest concern is the performance.
>  
> For example, clangd code completion latency increased dramatically with 
> `real_path`:
> With `real_path`:
> {F7039629}
> {F7041680}
> 
> Wihtou `real_path`:
> {F7039630}
But with this patch, we are not using real_path anymore, so it can't be the 
reason for not computing `RealPathName` when not opening the file.  Since the 
non-real_path method is much more lightweight, would it make a significant 
difference if we always computed the field?

In the end I don't really mind, I am just looking for a rational explanation to 
why it's done this way.



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

ioeric wrote:
> simark wrote:
> > ioeric wrote:
> > > simark wrote:
> > > > ioeric wrote:
> > > > > simark wrote:
> > > > > > If the path contains symlinks, doesn't this put a non-real path in 
> > > > > > the RealPathName field?  Won't users (e.g. clangd) use this value 
> > > > > > thinking it is a real path, when it is actually not?
> > > > > This was the original behavior. In general, File Manager should never 
> > > > > call real_path for users because it can be very expensive. Users 
> > > > > should call real_path if they want to resolve symlinks. That said, 
> > > > > it's fair to say that "RealPathName" is just a wrong name, and we 
> > > > > should clean it up at some point.
> > > > Ok, then if the goal is not to actually have a real path (in the 
> > > > realpath(3) sense), that's fine.  But I think we should rename the 
> > > > field sooner than later, it's really confusing.
> > > > 
> > > > That also means that it's kind of useless for us in clangd, so we 
> > > > should always call real_path there and not rely on that field.
> > > > Ok, then if the goal is not to actually have a real path (in the 
> > > > realpath(3) sense), that's fine. But I think we should rename the field 
> > > > sooner than later, it's really confusing.
> > > +1
> > > 
> > > > That also means that it's kind of useless for us in clangd, so we 
> > > > should always call real_path there and not rely on that field.
> > > I guess it depends on whether you want symlink resolution or not. I know 
> > > that clangd's index symbol collector resolves symlink explicitly, but I 
> > > don't think clangd enforces symlink resolution in general?
> > > I guess it depends on whether you want symlink resolution or not. I know 
> > > that clangd's index symbol collector resolves symlink explicitly, but I 
> > > don't think clangd enforces symlink resolution in general?
> > 
> > If we don't, we probably risk having duplicate results similar to what
> > 
> >   https://reviews.llvm.org/D48687
> > 
> > fixed, by with paths differing because of symlinks instead of dot-dots.
> Was the bug addressed in D48687 actually caused by symlinks though? If want 
> we want is absolute paths with dotdot cleaned, it should be much cheaper to 
> call `VFS::makeAbsolutePath` with `remove_dot_dot`.
> 
> In general, it's unclear whether clangd should resolve symlink (it might not 
> always be what users want), and it should probably be a decision made by the 
> build system integration. I think we would need a much more careful design if 
> we decide to handle symlinks in clangd. 
> Was the bug addressed in D48687 actually cau

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.
Herald added a subscriber: kadircet.



Comment at: clangd/SourceCode.cpp:209
+  llvm::SmallString<128> RealPath;
+  if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath(
+  Path, RealPath)) {

With the recent performance regression due to `getRealPath()` mentioned in 
D51159, I think we should re-evaluate whether `VFS::getRealPath()`, which calls 
expensive `real_path` system call on real FS, was necessary to fix the bug 
mentioned in the patch summary. I think `vfs::makeAbsolutePath` with 
`remove_dot_dot` (without symlink resolution) should be sufficient to address 
the issue. The code completion latency has increased dramatically with the 
`real_path` call, so I would also expect `Xrefs`/`findDefinition` to slow down 
due to this.  As `SymbolCollector` is not in a latency sensitive code paths, it 
might be OK for it to call `real_path`, but I think we should try to avoid 
using `real_path` elsewhere.

In general, it's unclear whether clangd should always resolve symlink (it might 
not always be what users want), and it should probably be a decision made by 
the build system integration. I think we would need a more careful design if we 
decide to handle symlinks in clangd. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162253.
hugoeg added a comment.

minor fixes, style improvements


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void Foo(A& a) {
+  a = StrCat(a);
+}
+
+void Bar() {
+  std::string A, B;
+  Foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to app

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

simark wrote:
> ioeric wrote:
> > simark wrote:
> > > ioeric wrote:
> > > > simark wrote:
> > > > > What's the rationale for only computing the field if `UFE.File` is 
> > > > > non-null?
> > > > > 
> > > > > Previously, if you looked up the file with `openFile == false` and 
> > > > > then later `openFile == true`, the `RealPathName` field would not be 
> > > > > set because of this.  That doesn't seem right.
> > > > There has been no guarantee that RealFilePath is always set. I think 
> > > > that's the reason why the acceasor is called tryGetRealPathName.
> > > The way I understood it was that it could be empty because computing the 
> > > real path can fail.  Not just because we didn't skipped computing it.
> > I agree that the API is confusing and lack of documentation (and we should 
> > fix), but the previous implementation did skip the computation if File is 
> > not available though. 
> > I agree that the API is confusing and lack of documentation (and we should 
> > fix), but the previous implementation did skip the computation if File is 
> > not available though.
> 
> Did it have a reason not to?  What is the `RealPathName` field useful for, if 
> it's unreliable?
I think the biggest concern is the performance.
 
For example, clangd code completion latency increased dramatically with 
`real_path`:
With `real_path`:
{F7039629}
{F7041680}

Wihtou `real_path`:
{F7039630}



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

simark wrote:
> ioeric wrote:
> > simark wrote:
> > > ioeric wrote:
> > > > simark wrote:
> > > > > If the path contains symlinks, doesn't this put a non-real path in 
> > > > > the RealPathName field?  Won't users (e.g. clangd) use this value 
> > > > > thinking it is a real path, when it is actually not?
> > > > This was the original behavior. In general, File Manager should never 
> > > > call real_path for users because it can be very expensive. Users should 
> > > > call real_path if they want to resolve symlinks. That said, it's fair 
> > > > to say that "RealPathName" is just a wrong name, and we should clean it 
> > > > up at some point.
> > > Ok, then if the goal is not to actually have a real path (in the 
> > > realpath(3) sense), that's fine.  But I think we should rename the field 
> > > sooner than later, it's really confusing.
> > > 
> > > That also means that it's kind of useless for us in clangd, so we should 
> > > always call real_path there and not rely on that field.
> > > Ok, then if the goal is not to actually have a real path (in the 
> > > realpath(3) sense), that's fine. But I think we should rename the field 
> > > sooner than later, it's really confusing.
> > +1
> > 
> > > That also means that it's kind of useless for us in clangd, so we should 
> > > always call real_path there and not rely on that field.
> > I guess it depends on whether you want symlink resolution or not. I know 
> > that clangd's index symbol collector resolves symlink explicitly, but I 
> > don't think clangd enforces symlink resolution in general?
> > I guess it depends on whether you want symlink resolution or not. I know 
> > that clangd's index symbol collector resolves symlink explicitly, but I 
> > don't think clangd enforces symlink resolution in general?
> 
> If we don't, we probably risk having duplicate results similar to what
> 
>   https://reviews.llvm.org/D48687
> 
> fixed, by with paths differing because of symlinks instead of dot-dots.
Was the bug addressed in D48687 actually caused by symlinks though? If want we 
want is absolute paths with dotdot cleaned, it should be much cheaper to call 
`VFS::makeAbsolutePath` with `remove_dot_dot`.

In general, it's unclear whether clangd should resolve symlink (it might not 
always be what users want), and it should probably be a decision made by the 
build system integration. I think we would need a much more careful design if 
we decide to handle symlinks in clangd. 


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x marked 2 inline comments as done.
cdavis5x added inline comments.



Comment at: src/Unwind-seh.cpp:163
+#ifdef __x86_64__
+unw_get_reg(&cursor, UNW_X86_64_RAX, &retval);
+unw_get_reg(&cursor, UNW_X86_64_RDX, &exc->private_[3]);

mstorsjo wrote:
> Without understanding the code flow completely - is there a risk that this 
> tries to use an uninitialized cursor, in case we hit `ctx = (struct 
> _Unwind_Context *)ms_exc->ExceptionInformation[1];` above and never 
> initialized the local cursor?
We should only hit this point if we're in phase 2 (i.e. 
`IS_UNWINDING(exc->ExceptionFlags)` is true). But you've now got me worried 
about the potential for a rogue personality function surreptitiously returning 
this to exploit libunwind. I've added a check here.



Comment at: src/Unwind-seh.cpp:174
+CONTEXT new_ctx;
+RtlUnwindEx(frame, (PVOID)target, ms_exc, (PVOID)retval, &new_ctx, 
disp->HistoryTable);
+_LIBUNWIND_ABORT("RtlUnwindEx() failed");

mstorsjo wrote:
> Who will get this new uninitialized CONTEXT here, and will they try to use it 
> for something?
It won't be uninitialized. `RtlUnwindEx()` always starts unwinding from the 
current frame; it fills in the passed in `CONTEXT` and passes its address to 
any handlers it encounters along the way. In other words, it's there so 
`RtlUnwindEx()` has a place to keep track of the unwind context.



Comment at: src/UnwindCursor.hpp:1157
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  pint_t getLastPC() const { /* FIXME: Implement */ return 0; }

mstorsjo wrote:
> What does this whole block do here? Isn't this within an !SEH ifdef block? 
> The same methods are declared for the SEH version of this struct further 
> above.
Right now, it doesn't do anything. I added it so @kristina has someplace to put 
the code for unwinding with SEH data when we're //not// on Windows and we 
//don't// have the `RtlUnwindEx()` function available. You'll note that the 
'!SEH' `ifdef` now says:

```lang=c++
#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
```



Comment at: src/UnwindCursor.hpp:1801
+  if (pc != getLastPC()) {
+UNWIND_INFO *xdata = reinterpret_cast(base + 
unwindEntry->UnwindData);
+if (xdata->Flags & (UNW_FLAG_EHANDLER|UNW_FLAG_UHANDLER)) {

kristina wrote:
> mstorsjo wrote:
> > I can't say I understand all of this yet, but I'm slowly getting there, and 
> > I'm trying to compare this to what libgcc does.
> > 
> > libgcc doesn't use any definition of UNWIND_INFO and doesn't need to do the 
> > equivalent of `getInfoFromSEH`, used by `step()`, anywhere. `unw_step()` is 
> > used in `_Unwind_ForcedUnwind`, which in libgcc is implemented using 
> > `RaiseException (STATUS_GCC_FORCED, ...`.
> > 
> > I guess if you happen to have all of the `unw_step` API available, it's 
> > easier to just do it like this, in custom code without relying on the NTDLL 
> > functions for it, while the libgcc version relies more on the NTDLL API.
> This primarily deals with the SEH exceptions re-purposed as a C++ exception 
> mechanism on x86_64 (if I understood this right), it's possible to set a 
> custom filter using a runtime call so I suspect GCC does that or defines a 
> translation function (also via a runtime call) which acts as a filter for 
> "true" SEH exceptions behind the scenes deep within the runtime. Typially 
> "true" SEH exceptions don't, outside of runtime magic, play nicely with C++ 
> exceptions, with the `__C_specific_handler` ones being a completely different 
> paradigm that falls far outside the scope of libunwind (those ones being the 
> "true"/explicit SEH exceptions).
> 
> (Don't take my word for it, it's been a while and I only implemented the 
> "true" variation for 64-bit Linux by reserving some RT signals and using that 
> to invoke additional runtime glue that would then do the unwinding, 
> completely ignoring DWARF since CFI exceptions and SEH exceptions really 
> don't mix especially on platforms that are not Windows-like)
Actually, it's just to implement the lower-level `libunwind` 
APIs--specifically, `unw_get_info()`. The code @kristina is talking about is 
all in Unwind-seh.cpp.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x updated this revision to Diff 162250.
cdavis5x marked 7 inline comments as done.
cdavis5x added a comment.

- Remove unnecessary code.
- Guard against rogue personality functions returning wrong values.
- Add a comment clarifying the purpose of the `new_ctx` variable.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564

Files:
  include/__libunwind_config.h
  include/unwind.h
  src/AddressSpace.hpp
  src/CMakeLists.txt
  src/Unwind-seh.cpp
  src/UnwindCursor.hpp
  src/UnwindLevel1-gcc-ext.c
  src/UnwindLevel1.c
  src/config.h
  src/libunwind_ext.h

Index: src/libunwind_ext.h
===
--- src/libunwind_ext.h
+++ src/libunwind_ext.h
@@ -17,6 +17,12 @@
 #include 
 #include 
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
+  #include 
+  #include 
+  #include 
+#endif
+
 #define UNW_STEP_SUCCESS 1
 #define UNW_STEP_END 0
 
@@ -33,6 +39,30 @@
 extern void _unw_add_dynamic_fde(unw_word_t fde);
 extern void _unw_remove_dynamic_fde(unw_word_t fde);
 
+// Provide a definition for the DISPATCHER_CONTEXT struct for old (Win7 and
+// earlier) SDKs.
+// MinGW-w64 has always provided this struct.
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && !defined(__MINGW32__) && VER_PRODUCTBUILD < 8000
+  #ifdef _WIN32
+#if defined(__x86_64__)
+struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;
+  ULONG64 ImageBase;
+  PRUNTIME_FUNCTION FunctionEntry;
+  ULONG64 EstablisherFrame;
+  ULONG64 TargetIp;
+  PCONTEXT ContextRecord;
+  PEXCEPTION_ROUTINE LanguageHandler;
+  PVOID HandlerData;
+  PUNWIND_HISTORY_TABLE HistoryTable;
+  ULONG ScopeIndex;
+  ULONG Fill0;
+};
+#endif
+  #endif
+typedef DISPATCHER_CONTEXT* PDISPATCHER_CONTEXT;
+#endif
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -38,7 +38,11 @@
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND   1
   #endif
 #elif defined(_WIN32)
-  #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #ifdef __SEH__
+#define _LIBUNWIND_SUPPORT_SEH_UNWIND 1
+  #else
+#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #endif
 #else
   #if defined(__ARM_DWARF_EH__) || !defined(__arm__)
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -32,6 +32,8 @@
 
 #if !defined(_LIBUNWIND_ARM_EHABI) && !defined(__USING_SJLJ_EXCEPTIONS__)
 
+#ifndef _LIBUNWIND_SUPPORT_SEH_UNWIND
+
 static _Unwind_Reason_Code
 unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *exception_object) {
   unw_init_local(cursor, uc);
@@ -449,6 +451,7 @@
   return result;
 }
 
+#endif // !_LIBUNWIND_SUPPORT_SEH_UNWIND
 
 /// Called by personality handler during phase 2 if a foreign exception
 // is caught.
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -25,6 +25,10 @@
 
 #if defined(_LIBUNWIND_BUILD_ZERO_COST_APIS)
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+#define private_1 private_[0]
+#endif
+
 ///  Called by __cxa_rethrow().
 _LIBUNWIND_EXPORT _Unwind_Reason_Code
 _Unwind_Resume_or_Rethrow(_Unwind_Exception *exception_object) {
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -18,18 +18,37 @@
 #include 
 #include 
 
+#ifdef _WIN32
+  #include 
+#endif
 #ifdef __APPLE__
   #include 
 #endif
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+struct UNWIND_INFO {
+  uint8_t Version : 3;
+  uint8_t Flags : 5;
+  uint8_t SizeOfProlog;
+  uint8_t CountOfCodes;
+  uint8_t FrameRegister : 4;
+  uint8_t FrameOffset : 4;
+  uint16_t UnwindCodes[2];
+};
+
+extern "C" _Unwind_Reason_Code __libunwind_seh_personality(
+int, _Unwind_Action, uint64_t, _Unwind_Exception *,
+struct _Unwind_Context *);
+
+#endif
+
 #include "config.h"
 
 #include "AddressSpace.hpp"
 #include "CompactUnwinder.hpp"
 #include "config.h"
 #include "DwarfInstructions.hpp"
 #include "EHHeaderParser.hpp"
-#include "libunwind.h"
 #include "Registers.hpp"
 #include "RWMutex.hpp"
 #include "Unwind-EHABI.h"
@@ -412,6 +431,472 @@
 #endif
 };
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
+
+/// \c UnwindCursor contains all state (including all register values) during
+/// an unwind.  This is normally stack-allocated inside a unw_cursor_t.
+template 
+class UnwindCursor : public AbstractUnwindCursor {
+  typedef typename A::pint_t pint_t;
+public:
+  UnwindCursor(unw_context_t *context, A &as);
+  UnwindCursor(CONTEXT *context, A &as);
+  UnwindCursor(A &as, 

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Do we plan to expose an API in `ClangdServer` to allow C++ API users to track 
index memory usages?




Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:123
+  size_t Bytes = Index.estimateMemoryUsage();
+  for (const auto &Scheme : URISchemes) {
+// std::string contains chars with sizeof(char) == 1.

I think the URI scheme names should be negligible.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:180
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+  {

kbobyrev wrote:
> sammccall wrote:
> > I think you're not counting the size of the actual symbols.
> > This is difficult to do precisely, but avoiding it seems misleading (we 
> > have "shared ownership" but it's basically exclusive). What's the plan here?
> As discussed offline: I should put a FIXME and leave it for later.
SymbolSlab provides a pretty good estimation of memory usage, and I think we 
should try to use if possible (as Sam suggested, it could be misleading without 
the size of index corpus). IIUC, the challenge here is that SymbolSlabs can be 
hiden behind `shared_ptr`, and index might not have access to the underlying 
slabs. If that's the case, I think it would be reasonable to ask callers of 
`build` to also pass in an estimated size of the hidden slabs?


https://reviews.llvm.org/D51154



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

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

ioeric wrote:
> simark wrote:
> > ioeric wrote:
> > > simark wrote:
> > > > What's the rationale for only computing the field if `UFE.File` is 
> > > > non-null?
> > > > 
> > > > Previously, if you looked up the file with `openFile == false` and then 
> > > > later `openFile == true`, the `RealPathName` field would not be set 
> > > > because of this.  That doesn't seem right.
> > > There has been no guarantee that RealFilePath is always set. I think 
> > > that's the reason why the acceasor is called tryGetRealPathName.
> > The way I understood it was that it could be empty because computing the 
> > real path can fail.  Not just because we didn't skipped computing it.
> I agree that the API is confusing and lack of documentation (and we should 
> fix), but the previous implementation did skip the computation if File is not 
> available though. 
> I agree that the API is confusing and lack of documentation (and we should 
> fix), but the previous implementation did skip the computation if File is not 
> available though.

Did it have a reason not to?  What is the `RealPathName` field useful for, if 
it's unreliable?



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

ioeric wrote:
> simark wrote:
> > ioeric wrote:
> > > simark wrote:
> > > > If the path contains symlinks, doesn't this put a non-real path in the 
> > > > RealPathName field?  Won't users (e.g. clangd) use this value thinking 
> > > > it is a real path, when it is actually not?
> > > This was the original behavior. In general, File Manager should never 
> > > call real_path for users because it can be very expensive. Users should 
> > > call real_path if they want to resolve symlinks. That said, it's fair to 
> > > say that "RealPathName" is just a wrong name, and we should clean it up 
> > > at some point.
> > Ok, then if the goal is not to actually have a real path (in the 
> > realpath(3) sense), that's fine.  But I think we should rename the field 
> > sooner than later, it's really confusing.
> > 
> > That also means that it's kind of useless for us in clangd, so we should 
> > always call real_path there and not rely on that field.
> > Ok, then if the goal is not to actually have a real path (in the 
> > realpath(3) sense), that's fine. But I think we should rename the field 
> > sooner than later, it's really confusing.
> +1
> 
> > That also means that it's kind of useless for us in clangd, so we should 
> > always call real_path there and not rely on that field.
> I guess it depends on whether you want symlink resolution or not. I know that 
> clangd's index symbol collector resolves symlink explicitly, but I don't 
> think clangd enforces symlink resolution in general?
> I guess it depends on whether you want symlink resolution or not. I know that 
> clangd's index symbol collector resolves symlink explicitly, but I don't 
> think clangd enforces symlink resolution in general?

If we don't, we probably risk having duplicate results similar to what

  https://reviews.llvm.org/D48687

fixed, by with paths differing because of symlinks instead of dot-dots.


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

simark wrote:
> ioeric wrote:
> > simark wrote:
> > > What's the rationale for only computing the field if `UFE.File` is 
> > > non-null?
> > > 
> > > Previously, if you looked up the file with `openFile == false` and then 
> > > later `openFile == true`, the `RealPathName` field would not be set 
> > > because of this.  That doesn't seem right.
> > There has been no guarantee that RealFilePath is always set. I think that's 
> > the reason why the acceasor is called tryGetRealPathName.
> The way I understood it was that it could be empty because computing the real 
> path can fail.  Not just because we didn't skipped computing it.
I agree that the API is confusing and lack of documentation (and we should 
fix), but the previous implementation did skip the computation if File is not 
available though. 



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

simark wrote:
> ioeric wrote:
> > simark wrote:
> > > If the path contains symlinks, doesn't this put a non-real path in the 
> > > RealPathName field?  Won't users (e.g. clangd) use this value thinking it 
> > > is a real path, when it is actually not?
> > This was the original behavior. In general, File Manager should never call 
> > real_path for users because it can be very expensive. Users should call 
> > real_path if they want to resolve symlinks. That said, it's fair to say 
> > that "RealPathName" is just a wrong name, and we should clean it up at some 
> > point.
> Ok, then if the goal is not to actually have a real path (in the realpath(3) 
> sense), that's fine.  But I think we should rename the field sooner than 
> later, it's really confusing.
> 
> That also means that it's kind of useless for us in clangd, so we should 
> always call real_path there and not rely on that field.
> Ok, then if the goal is not to actually have a real path (in the realpath(3) 
> sense), that's fine. But I think we should rename the field sooner than 
> later, it's really confusing.
+1

> That also means that it's kind of useless for us in clangd, so we should 
> always call real_path there and not rely on that field.
I guess it depends on whether you want symlink resolution or not. I know that 
clangd's index symbol collector resolves symlink explicitly, but I don't think 
clangd enforces symlink resolution in general?


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D51178: [ASTImporter] Add test for importing anonymous namespaces.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.

Repository:
  rC Clang

https://reviews.llvm.org/D51178

Files:
  test/Import/cxx-anon-namespace/Inputs/F.cpp
  test/Import/cxx-anon-namespace/test.cpp


Index: test/Import/cxx-anon-namespace/test.cpp
===
--- /dev/null
+++ test/Import/cxx-anon-namespace/test.cpp
@@ -0,0 +1,43 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: NamespaceDecl
+// The nested anonymous namespace.
+// CHECK-NEXT: NamespaceDecl
+// CHECK: FunctionDecl
+// CHECK-SAME: func4
+// CHECK-NEXT: CompoundStmt
+// This is for the nested anonymous namespace.
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+// CHECK: FunctionDecl
+// CHECK-SAME: func1
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+// CHECK: NamespaceDecl
+// CHECK-SAME: test_namespace1
+// CHECK-NEXT: NamespaceDecl
+// CHECK: FunctionDecl
+// CHECK-SAME: func2
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+// CHECK-NEXT: NamespaceDecl
+// CHECK-SAME: test_namespace2
+// CHECK-NEXT: NamespaceDecl
+// CHECK-NEXT: NamespaceDecl
+// CHECK-SAME: test_namespace3
+// CHECK: FunctionDecl
+// CHECK-SAME: func3
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+void expr() {
+  func1();
+  test_namespace1::func2();
+  test_namespace2::test_namespace3::func3();
+  func4();
+}
Index: test/Import/cxx-anon-namespace/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-anon-namespace/Inputs/F.cpp
@@ -0,0 +1,25 @@
+namespace {
+void func1() {
+}
+} // namespace
+
+namespace test_namespace1 {
+namespace {
+void func2() {}
+} // namespace
+} // namespace test_namespace1
+
+namespace test_namespace2 {
+namespace {
+namespace test_namespace3 {
+void func3() {}
+} // namespace test_namespace3
+} // namespace
+} // namespace test_namespace2
+
+namespace {
+namespace {
+void func4() {
+}
+} // namespace
+} // namespace


Index: test/Import/cxx-anon-namespace/test.cpp
===
--- /dev/null
+++ test/Import/cxx-anon-namespace/test.cpp
@@ -0,0 +1,43 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+
+// CHECK: NamespaceDecl
+// The nested anonymous namespace.
+// CHECK-NEXT: NamespaceDecl
+// CHECK: FunctionDecl
+// CHECK-SAME: func4
+// CHECK-NEXT: CompoundStmt
+// This is for the nested anonymous namespace.
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+// CHECK: FunctionDecl
+// CHECK-SAME: func1
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+// CHECK: NamespaceDecl
+// CHECK-SAME: test_namespace1
+// CHECK-NEXT: NamespaceDecl
+// CHECK: FunctionDecl
+// CHECK-SAME: func2
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+// CHECK-NEXT: NamespaceDecl
+// CHECK-SAME: test_namespace2
+// CHECK-NEXT: NamespaceDecl
+// CHECK-NEXT: NamespaceDecl
+// CHECK-SAME: test_namespace3
+// CHECK: FunctionDecl
+// CHECK-SAME: func3
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: UsingDirectiveDecl
+// CHECK-SAME: ''
+
+void expr() {
+  func1();
+  test_namespace1::func2();
+  test_namespace2::test_namespace3::func3();
+  func4();
+}
Index: test/Import/cxx-anon-namespace/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-anon-namespace/Inputs/F.cpp
@@ -0,0 +1,25 @@
+namespace {
+void func1() {
+}
+} // namespace
+
+namespace test_namespace1 {
+namespace {
+void func2() {}
+} // namespace
+} // namespace test_namespace1
+
+namespace test_namespace2 {
+namespace {
+namespace test_namespace3 {
+void func3() {}
+} // namespace test_namespace3
+} // namespace
+} // namespace test_namespace2
+
+namespace {
+namespace {
+void func4() {
+}
+} // namespace
+} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: echristo.
Herald added subscribers: JDevlieghere, fedor.sergeev, aprantl.

Added option -glineinfo-only to support emission of the debug directives
only. It behaves very similar to -gline-tables-only, except that it sets
llvm debug info emission kind to
llvm::DICompileUnit::DebugDirectivesOnly.


Repository:
  rC Clang

https://reviews.llvm.org/D51177

Files:
  include/clang/Basic/DebugInfoOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/CGDebugInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/debug-info-gline-tables-only.c
  test/CodeGen/debug-info-gline-tables-only2.c
  test/CodeGen/debug-info-line.c
  test/CodeGen/debug-info-macro.c
  test/CodeGen/debug-info-scope.c
  test/CodeGen/lifetime-debuginfo-1.c
  test/CodeGen/lifetime-debuginfo-2.c
  test/CodeGenCXX/crash.cpp
  test/CodeGenCXX/debug-info-blocks.cpp
  test/CodeGenCXX/debug-info-gline-tables-only.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  test/CodeGenCXX/debug-info-namespace.cpp
  test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  test/CodeGenCXX/debug-info-windows-dtor.cpp
  test/CodeGenCXX/linetable-virtual-variadic.cpp
  test/CodeGenObjCXX/debug-info-line.mm
  test/CodeGenObjCXX/pr14474-gline-tables-only.mm
  test/Driver/debug-options.c

Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -119,6 +119,25 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
+// RUN: %clang -### -c -glineinfo-only %s -target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY %s
+// RUN: %clang -### -c -glineinfo-only %s -target i686-pc-openbsd 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY_DWARF2 %s
+// RUN: %clang -### -c -glineinfo-only %s -target x86_64-pc-freebsd10.0 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY_DWARF2 %s
+// RUN: %clang -### -c -glineinfo-only -g %s -target x86_64-linux-gnu 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY %s
+// RUN: %clang -### -c -glineinfo-only -g %s -target x86_64-apple-darwin16 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -glineinfo-only -g %s -target i686-pc-openbsd 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -glineinfo-only -g %s -target x86_64-pc-freebsd10.0 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -glineinfo-only -g %s -target i386-pc-solaris 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -glineinfo-only -g0 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_NO %s
+//
 // RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
 // | FileCheck -check-prefix=GRECORD %s
 // RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
@@ -178,6 +197,9 @@
 // RUN: %clang -### -gmodules -gline-tables-only %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GLTO_ONLY %s
 //
+// RUN: %clang -### -gmodules -glineinfo-only %s 2>&1 \
+// RUN:| FileCheck -check-prefix=GLIO_ONLY %s
+//
 // G: "-cc1"
 // G: "-debug-info-kind=limited"
 //
@@ -204,6 +226,15 @@
 // GLTO_ONLY_DWARF2: "-debug-info-kind=line-tables-only"
 // GLTO_ONLY_DWARF2: "-dwarf-version=2"
 //
+// GLIO_ONLY: "-cc1"
+// GLIO_ONLY-NOT: "-dwarf-ext-refs"
+// GLIO_ONLY: "-debug-info-kind=lineinfo-directives-only"
+// GLIO_ONLY-NOT: "-dwarf-ext-refs"
+//
+// GLIO_ONLY_DWARF2: "-cc1"
+// GLIO_ONLY_DWARF2: "-debug-info-kind=lineinfo-directives-only"
+// GLIO_ONLY_DWARF2: "-dwarf-version=2"
+//
 // G_ONLY: "-cc1"
 // G_ONLY: "-debug-info-kind=limited"
 //
@@ -225,6 +256,10 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// This tests asserts that "-glineinfo-only" "-g0" disables debug info.
+// GLIO_NO: "-cc1"
+// GLIO_NO-NOT: -debug-info-kind=
+//
 // GRECORD: "-dwarf-debug-flags"
 // GRECORD: -### -c -grecord-gcc-switches
 //
Index: test/CodeGenObjCXX/pr14474-gline-tables-only.mm
===
--- test/CodeGenObjCXX/pr14474-gline-tables-only.mm
+++ test/CodeGenObjCXX/pr14474-gline-tables-only.mm
@@ -1,6 +1,8 @@
 // PR 14474
 // RUN: %clang_cc1 -triple i386-apple-macosx10.6.0 -emit-llvm \
 // RUN:   -debug-info-kind=line-tables-only -x objective-c++ -o /dev/null %s
+// RUN: %clang_cc1 -triple i386-apple-macosx10.6.0 -emit-llvm \
+// RUN:   -debug-info-kind=lineinfo-directives-only -x objective-c++ -o /dev/null %s
 
 typedef signed char BOOL;
 @class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
Index: test/CodeGenObjCXX/debug-info-line.mm
===
--

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

ioeric wrote:
> simark wrote:
> > What's the rationale for only computing the field if `UFE.File` is non-null?
> > 
> > Previously, if you looked up the file with `openFile == false` and then 
> > later `openFile == true`, the `RealPathName` field would not be set because 
> > of this.  That doesn't seem right.
> There has been no guarantee that RealFilePath is always set. I think that's 
> the reason why the acceasor is called tryGetRealPathName.
The way I understood it was that it could be empty because computing the real 
path can fail.  Not just because we didn't skipped computing it.



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

ioeric wrote:
> simark wrote:
> > If the path contains symlinks, doesn't this put a non-real path in the 
> > RealPathName field?  Won't users (e.g. clangd) use this value thinking it 
> > is a real path, when it is actually not?
> This was the original behavior. In general, File Manager should never call 
> real_path for users because it can be very expensive. Users should call 
> real_path if they want to resolve symlinks. That said, it's fair to say that 
> "RealPathName" is just a wrong name, and we should clean it up at some point.
Ok, then if the goal is not to actually have a real path (in the realpath(3) 
sense), that's fine.  But I think we should rename the field sooner than later, 
it's really confusing.

That also means that it's kind of useless for us in clangd, so we should always 
call real_path there and not rely on that field.


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I've confirmed that the patch does not break anything in our CUDA code, so it's 
good to go as far as CUDA is concerned.

I'll fix the exposed CUDA issue in a separate patch.


Repository:
  rC Clang

https://reviews.llvm.org/D47757



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

r340559 - [docs] Regenerate ClangCommandLineReference.rst

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Thu Aug 23 10:55:03 2018
New Revision: 340559

URL: http://llvm.org/viewvc/llvm-project?rev=340559&view=rev
Log:
[docs] Regenerate ClangCommandLineReference.rst

Modified:
cfe/trunk/docs/ClangCommandLineReference.rst

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=340559&r1=340558&r2=340559&view=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Thu Aug 23 10:55:03 2018
@@ -36,7 +36,7 @@ Treat source input files as Objective-C
 
 Treat source input files as Objective-C++ inputs
 
-.. option:: -Qn
+.. option:: -Qn, -fno-ident
 
 Do not emit metadata containing compiler name and version
 
@@ -44,7 +44,7 @@ Do not emit metadata containing compiler
 
 Don't emit warning for unused driver arguments
 
-.. option:: -Qy
+.. option:: -Qy, -fident
 
 Emit metadata containing compiler name and version
 
@@ -214,9 +214,13 @@ Flush denormal floating point values to
 
 Generate relocatable device code, also known as separate compilation mode.
 
+.. option:: -fcuda-short-ptr, -fno-cuda-short-ptr
+
+Use 32-bit pointers for accessing const/local/shared address spaces.
+
 .. option:: -ffixed-r19
 
-Reserve the r19 register (Hexagon only)
+Reserve register r19 (Hexagon only)
 
 .. option:: -fheinous-gnu-extensions
 
@@ -260,6 +264,10 @@ Display available options
 
 Display help for hidden options
 
+.. option:: --hip-link
+
+Link clang-offload-bundler bundles for HIP
+
 .. option:: -image\_base 
 
 .. option:: -index-header-map
@@ -452,6 +460,10 @@ Use pipes between commands, when possibl
 
 .. option:: --print-diagnostic-categories
 
+.. option:: -print-effective-triple, --print-effective-triple
+
+Print the effective target triple
+
 .. option:: -print-file-name=, --print-file-name=, 
--print-file-name 
 
 Print the full library path of 
@@ -480,6 +492,10 @@ Print the resource directory pathname
 
 Print the paths used for finding libraries and programs
 
+.. option:: -print-target-triple, --print-target-triple
+
+Print the normalized target triple
+
 .. option:: -private\_bundle
 
 .. option:: -pthread, -no-pthread
@@ -558,6 +574,8 @@ Serialize compiler diagnostics to a file
 
 .. option:: -shared-libsan, -shared-libasan
 
+Dynamically link the sanitizer runtime
+
 .. option:: -single\_module
 
 .. option:: -specs=, --specs=
@@ -568,6 +586,8 @@ Serialize compiler diagnostics to a file
 
 .. option:: -static-libsan
 
+Statically link the sanitizer runtime
+
 .. option:: -static-libstdc++
 
 .. option:: -std-default=
@@ -712,6 +732,12 @@ Attempt to match the ABI of Clang  as a documentation comment block 
command
 
+.. option:: -fcomplete-member-pointers, -fno-complete-member-pointers
+
+Require member pointer base types to be complete if they would be significant 
under the Microsoft ABI
+
+.. option:: -fcrash-diagnostics-dir=
+
 .. option:: -fdeclspec, -fno-declspec
 
 Allow \_\_declspec as a keyword
@@ -746,7 +772,7 @@ Enables an experimental new pass manager
 
 .. option:: -ffine-grained-bitfield-accesses, 
-fno-fine-grained-bitfield-accesses
 
-Use separate accesses for bitfields with legal widths and alignments.
+Use separate accesses for consecutive bitfield runs with legal widths and 
alignments.
 
 .. option:: -finline-functions, -fno-inline-functions
 
@@ -854,6 +880,10 @@ Strip (or keep only, if negative) a give
 
 Turn on runtime checks for various forms of undefined or suspicious behavior. 
See user manual for available checks
 
+.. option:: -moutline, -mno-outline
+
+Enable function outlining (AArch64 only)
+
 .. option:: --param , --param=
 
 .. option:: -std=, --std=, --std 
@@ -1151,6 +1181,10 @@ Target-independent compilation options
 
 .. option:: -faccess-control, -fno-access-control
 
+.. option:: -faddrsig, -fno-addrsig
+
+Emit an address-significance table
+
 .. option:: -falign-functions, -fno-align-functions
 
 .. program:: clang1
@@ -1223,12 +1257,20 @@ Accept non-standard constructs supported
 
 Load the clang builtins module map file.
 
+.. option:: -fc++-static-destructors, -fno-c++-static-destructors
+
+Enable C++ static destructor registration (the default)
+
 .. option:: -fcaret-diagnostics, -fno-caret-diagnostics
 
 .. option:: -fcf-protection=, -fcf-protection (equivalent to 
-fcf-protection=full)
 
 Instrument control-flow architecture protection. Options: return, branch, 
full, none.
 
+.. option:: -fchar8\_t, -fno-char8\_t
+
+Enable C++ builtin type char8\_t
+
 .. option:: -fclasspath=, --CLASSPATH , --CLASSPATH=, 
--classpath , --classpath=
 
 .. option:: -fcolor-diagnostics, -fno-color-diagnostics
@@ -1293,6 +1335,10 @@ Place debug types in their own section (
 
 Parse templated function definitions at the end of the translation unit
 
+.. option:: -fdelete-null-pointer-checks, -fno-delete-null-pointer-checks
+
+Treat usage

r340558 - [options] Document -(static|shared)-libsan

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Thu Aug 23 10:54:48 2018
New Revision: 340558

URL: http://llvm.org/viewvc/llvm-project?rev=340558&view=rev
Log:
[options] Document -(static|shared)-libsan

Modified:
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=340558&r1=340557&r2=340558&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 23 10:54:48 2018
@@ -639,8 +639,10 @@ def fapple_kext : Flag<["-"], "fapple-ke
   HelpText<"Use Apple's kernel extensions ABI">;
 def fapple_pragma_pack : Flag<["-"], "fapple-pragma-pack">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable Apple gcc-compatible #pragma pack handling">;
-def shared_libsan : Flag<["-"], "shared-libsan">;
-def static_libsan : Flag<["-"], "static-libsan">;
+def shared_libsan : Flag<["-"], "shared-libsan">,
+  HelpText<"Dynamically link the sanitizer runtime">;
+def static_libsan : Flag<["-"], "static-libsan">,
+  HelpText<"Statically link the sanitizer runtime">;
 def : Flag<["-"], "shared-libasan">, Alias;
 def fasm : Flag<["-"], "fasm">, Group;
 


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

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

simark wrote:
> What's the rationale for only computing the field if `UFE.File` is non-null?
> 
> Previously, if you looked up the file with `openFile == false` and then later 
> `openFile == true`, the `RealPathName` field would not be set because of 
> this.  That doesn't seem right.
There has been no guarantee that RealFilePath is always set. I think that's the 
reason why the acceasor is called tryGetRealPathName.



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

simark wrote:
> If the path contains symlinks, doesn't this put a non-real path in the 
> RealPathName field?  Won't users (e.g. clangd) use this value thinking it is 
> a real path, when it is actually not?
This was the original behavior. In general, File Manager should never call 
real_path for users because it can be very expensive. Users should call 
real_path if they want to resolve symlinks. That said, it's fair to say that 
"RealPathName" is just a wrong name, and we should clean it up at some point.


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

r340552 - Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Thu Aug 23 10:43:29 2018
New Revision: 340552

URL: http://llvm.org/viewvc/llvm-project?rev=340552&view=rev
Log:
Driver: Enable address-significance tables by default when targeting COFF.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/addrsig.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=340552&r1=340551&r2=340552&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Aug 23 10:43:29 2018
@@ -4857,7 +4857,8 @@ void Clang::ConstructJob(Compilation &C,
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 

Modified: cfe/trunk/test/Driver/addrsig.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/addrsig.c?rev=340552&r1=340551&r2=340552&view=diff
==
--- cfe/trunk/test/Driver/addrsig.c (original)
+++ cfe/trunk/test/Driver/addrsig.c Thu Aug 23 10:43:29 2018
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s


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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340552: Driver: Enable address-significance tables by 
default when targeting COFF. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51049?vs=161763&id=162232#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51049

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/addrsig.c


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162231.

https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string A, B;
+  foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
+``absl::StrAppend()``

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-str-cat-append.rst:11
+
+  a = StrCat(a, b); // Use StrAppend(&a, b) instead.
+

Please add namespace.


https://reviews.llvm.org/D51061



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

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Let me know if there's anything else I can fix to move the process along.


https://reviews.llvm.org/D51061



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

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

What's the rationale for only computing the field if `UFE.File` is non-null?

Previously, if you looked up the file with `openFile == false` and then later 
`openFile == true`, the `RealPathName` field would not be set because of this.  
That doesn't seem right.



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

If the path contains symlinks, doesn't this put a non-real path in the 
RealPathName field?  Won't users (e.g. clangd) use this value thinking it is a 
real path, when it is actually not?


Repository:
  rC Clang

https://reviews.llvm.org/D51159



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

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162224.
ioeric added a comment.

- another minior cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -20,6 +20,7 @@
 #include "llvm/Support/StringSaver.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +344,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -14,6 +14,7 @@
 #include "Function.h"
 #inclu

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdServer.cpp:159
+  }
+  if (SpecFuzzyReq) {
+if (auto Filter = speculateCompletionFilter(Content, Pos)) {

ilya-biryukov wrote:
> NIT: maybe invert condition to reduce nesting?
It would become something like:
```
if (!SpecFuzzyReq)
  return SpecFuzzyReq;
 // some work
return SpecFuzzyReq;
```

Having to return the same thing twice seems a bit worse IMO.



Comment at: clangd/CodeComplete.cpp:1388
+  if (Len > 0)
+St++; // Shift to the first valid character.
+  return Content.substr(St, Len);

ilya-biryukov wrote:
> This looks incorrect in case of the identifiers starting at offset 0. A 
> corner case, but still.
`Offset == 0` is handled above.



Comment at: clangd/tool/ClangdMain.cpp:179
+should be effective for a number of code completions.)"),
+llvm::cl::init(true));
+

ilya-biryukov wrote:
> Maybe remove this option?
> Doing a speculative request seems like the right thing to do in all cases. It 
> never hurts completion latency. Any reasons to avoid it?
Sure, was trying to be conservative :)

Removed the command-line option but still keeping the code completion option. 
As async call has some overhead, I think we should only enable it when static 
index is provided.



Comment at: unittests/clangd/CodeCompleteTests.cpp:926
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);

ilya-biryukov wrote:
> Why do we want to change this? To avoid tests getting requests from the 
> previous tests?
Yes. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962



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

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 16.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -20,6 +20,7 @@
 #include "llvm/Support/StringSaver.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +344,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -8,6 +8,9 @@
 //===

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162223.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162221.
ioeric marked 5 inline comments as done.
ioeric added a comment.
Herald added subscribers: jfb, javed.absar.

- Moved most logic into CodeComplete.cc


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -19,7 +19,9 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/StringSaver.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +345,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/index/In

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16
+
+   string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", 
"D")));
+   string s = absl::StrCat("A", "B", "C", "D");

std::string. Please insert empty line.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:17
+   string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", 
"D")));
+   string s = absl::StrCat("A", "B", "C", "D");
+   absl::StrAppend(&s, absl::StrCat("E", "F", "G"));

std::string. Same indentation as in previous line. Same for second example.


https://reviews.llvm.org/D51132



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

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162219.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional RecordV =
   DerefdV.getAs()) {
 
 const TypedValueRegion *R = RecordV->getRegion();

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > This looks like one more situation where we dereference a location to get 
> > > a value and then struggle to get back to the location that we've 
> > > dereferenced by looking at the value. Can we just use `V`?
> > I've struggled with derefencing for months now -- I'm afraid I just don't 
> > really get what you'd like to see here.
> > 
> > Here's what I attempted to implement:
> > I'd like to obtain the pointee's region of a `Loc` region, even if it has 
> > to be casted to another type, like through void pointers and 
> > `nonloc::LocAsInteger`, and continue analysis on said region as usual.
> > 
> > The trickiest part I can't seem to get right is the acquisition of the 
> > pointee region. For the problem this patch attempts to solve, even though 
> > `DynT` correctly says that the dynamic type is `DynTDerived2 *`, `DerefdV` 
> > contains a region for `DynTBase`.
> > 
> > I uploaded a new patch, D51057, which hopefully settles derefence related 
> > issues. Please note that it **does not **replace this diff, as the acquired 
> > region is still of type `DynTBase`.
> > 
> > I find understanding these intricate details of the analyzer very 
> > difficult, as I found very little documentation about how this works, which 
> > often left me guessing what the proper way to do this is. Can you recommend 
> > some literature for me on this field?
> > Can you recommend some literature for me on this field?
> 
> This is pretty specific to our analyzer. `SVal`/`SymExpr`/`MemRegion` 
> hierarchy is tightly coupled to implementation details of the `RegionStore`, 
> which is our memory model. There's a paper on it [1]. We have some in-tree 
> documentation of the `RegionStore` [2] (other docs there are also interesting 
> to read). And there's my old workbook [3]. And i guess that's it.
> 
> [1] Xu, Zhongxing & Kremenek, Ted & Zhang, Jian. (2010). A Memory Model for 
> Static Analysis of C Programs. 535-548. 10.1007/978-3-642-16558-0_44.
> [2] 
> https://github.com/llvm-mirror/clang/blob/master/docs/analyzer/RegionStore.txt
> [3] 
> https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf
Thank you! I'm aware of these works, and have read them already.

The actual implementation of the analyzer is most described in your book, and 
I've often got the best ideas from that. However, some very well described 
things in there have absolutely no documentation in the actual code -- for 
example, it isn't obvious at all to me what `CompundValue` is, and I was 
surprised to learn what it does from your guide. Other examples are the 
difference between `TypedValueRegion`s `getLocationType` and `getValueType`, 
`SymExpr` in general, and so on.

I think it would also be very valuable to have a link in `docs/analyzer` to 
your book. On another note, would you mind if I were to put some of the 
information from there into the actual code?



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:787
   // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
   int x; // no-note
 };

NoQ wrote:
> Szelethus wrote:
> > The checker should be able to catch this one -- for some reason it is 
> > regarded as an unknown region. Odd, as the test case right after this one 
> > works perfectly.
> There's a variety of problems we have with empty base classes, might be one 
> of those, and they are usually easy to fix because, well, yes it's a special 
> case, but it's also an extremely simple case.
> 
> I encourage you to open up the Exploded Graph and study it carefully to see 
> what and where goes wrong (not for this revision).
I'd love to learn about other parts of the analyzer (besides chasing pointers), 
but for now, this seems to have fixed itself ;)


https://reviews.llvm.org/D50892



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

[PATCH] D50617: [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

[Shuai Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340547: [ASTMatchers] Let hasObjectExpression also support 
UnresolvedMemberExpr… (authored by shuaiwang, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50617?vs=161830&id=162218#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50617

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1517,6 +1517,26 @@
 "struct X { int m; }; void f(X* x) { x->m; }",
 memberExpr(hasObjectExpression(
   hasType(pointsTo(recordDecl(hasName("X";
+  EXPECT_TRUE(matches("template  struct X { void f() { T t; t.m; } };",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+  EXPECT_TRUE(
+  matches("template  struct X { void f() { T t; t->m; } };",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+}
+
+TEST(HasObjectExpression, MatchesBaseOfMemberFunc) {
+  EXPECT_TRUE(matches(
+  "struct X { void f(); }; void g(X x) { x.f(); }",
+  memberExpr(hasObjectExpression(hasType(recordDecl(hasName("X")));
+  EXPECT_TRUE(matches("struct X { template  void f(); };"
+  "template  void g(X x) { x.f(); }",
+  unresolvedMemberExpr(hasObjectExpression(
+  hasType(recordDecl(hasName("X")));
+  EXPECT_TRUE(matches("template  void f(T t) { t.g(); }",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
 }
 
 TEST(HasObjectExpression,
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4668,6 +4668,20 @@
 
 
 
+MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
+Matches a member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> InnerMatcher
 Matches a 'for', 'while', 'do while' statement or a function
 definition that has a given body.
@@ -6692,6 +6706,20 @@
 
 
 
+MatcherUnresolvedMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
+Matches a member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherUnresolvedUsingType>hasDeclarationconst MatcherDecl>  InnerMatcher
 Matches a node if the declaration associated with that node
 matches the given matcher.
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4825,8 +4825,17 @@
 ///   matches "x.m" and "m"
 /// with hasObjectExpression(...)
 ///   matching "x" and the implicit object expression of "m" which has type X*.
-AST_MATCHER_P(MemberExpr, hasObjectExpression,
-  internal::Matcher, InnerMatcher) {
+AST_POLYMORPHIC_MATCHER_P(
+hasObjectExpression,
+AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
+CXXDependentScopeMemberExpr),
+internal::Matcher, InnerMatcher) {
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
   return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
 }
 
___
cfe

r340547 - [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

[Shuai Wang via cfe-commits]

Author: shuaiwang
Date: Thu Aug 23 10:16:06 2018
New Revision: 340547

URL: http://llvm.org/viewvc/llvm-project?rev=340547&view=rev
Log:
[ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, 
CXXDependentScopeMemberExpr

Reviewers: aaron.ballman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Thu Aug 23 10:16:06 2018
@@ -4668,6 +4668,20 @@ with withInitializer matching (1)
 
 
 
+MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> 
InnerMatcher
+Matches a 
member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> 
InnerMatcher
 Matches a 'for', 'while', 
'do while' statement or a function
 definition that has a given body.
@@ -6692,6 +6706,20 @@ Example matches true (matcher = hasUnary
 
 
 
+MatcherUnresolvedMemberExpr>hasObjectExpressionMatcherExpr> 
InnerMatcher
+Matches a 
member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherUnresolvedUsingType>hasDeclarationconst MatcherDecl>  
InnerMatcher
 Matches a node if 
the declaration associated with that node
 matches the given matcher.

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Thu Aug 23 10:16:06 2018
@@ -4825,8 +4825,17 @@ AST_MATCHER_P(MemberExpr, member,
 ///   matches "x.m" and "m"
 /// with hasObjectExpression(...)
 ///   matching "x" and the implicit object expression of "m" which has type X*.
-AST_MATCHER_P(MemberExpr, hasObjectExpression,
-  internal::Matcher, InnerMatcher) {
+AST_POLYMORPHIC_MATCHER_P(
+hasObjectExpression,
+AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
+CXXDependentScopeMemberExpr),
+internal::Matcher, InnerMatcher) {
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
   return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
 }
 

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Thu Aug 23 
10:16:06 2018
@@ -1517,6 +1517,26 @@ TEST(HasObjectExpression, MatchesBaseOfV
 "struct X { int m; }; void f(X* x) { x->m; }",
 memberExpr(hasObjectExpression(
   hasType(pointsTo(recordDecl(hasName("X";
+  EXPECT_TRUE(matches("template  struct X { void f() { T t; t.m; } 
};",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+  EXPECT_TRUE(
+  matches("template  struct X { void f() { T t; t->m; } };",
+  cxxDependentScopeMemberExpr(has

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16
+
+   string s = StrCat("A", StrCat("B", StrCat("C", "D")));
+   ==> string s = StrCat("A", "B", "C", "D");

Please add namespaces and use empty line instead of ==>


https://reviews.llvm.org/D51132



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

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Landed as r340544. @hans: Can you cherry-pick?


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172



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

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:4924
+<< Callee->getSourceRange();
+  }
+

Why is the diagnostic at the end location?  And why are you separately checking 
whether it's ignored at the begin location?



Comment at: lib/Sema/SemaChecking.cpp:10003
+static void DiagnoseImplicitAtomicSeqCst(Sema &S, Expr *E) {
+  if (S.Diags.isIgnored(diag::warn_atomic_implicit_seq_cst, E->getBeginLoc()))
+return;

`isIgnored` is not actually a cheap operation; you should *definitely* be 
checking for atomic types first.  And usually we just fire off the diagnostic 
without checking `isIgnored` because the setup cost of a diagnostic is not 
assumed to be that high.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX340544: Comment out #define __cpp_lib_node_extract, we 
only support half of that… (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51172?vs=162208&id=162215#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D51172

Files:
  include/__node_handle


Index: include/__node_handle
===
--- include/__node_handle
+++ include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


Index: include/__node_handle
===
--- include/__node_handle
+++ include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r340544 - Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Aug 23 10:08:02 2018
New Revision: 340544

URL: http://llvm.org/viewvc/llvm-project?rev=340544&view=rev
Log:
Comment out #define __cpp_lib_node_extract, we only support half of that 
functionality

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

Modified:
libcxx/trunk/include/__node_handle

Modified: libcxx/trunk/include/__node_handle
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__node_handle?rev=340544&r1=340543&r2=340544&view=diff
==
--- libcxx/trunk/include/__node_handle (original)
+++ libcxx/trunk/include/__node_handle Thu Aug 23 10:08:02 2018
@@ -26,7 +26,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


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

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Louis Dionne via Phabricator via cfe-commits]

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

LGTM. Don't forget to update https://reviews.llvm.org/D48896 so it uncomments 
this. Also, this should be merged into LLVM 7.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172



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

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: ldionne, mclow.lists, hans.
Herald added a reviewer: EricWF.
Herald added subscribers: dexonsmith, christof.

The other half of this is in https://reviews.llvm.org/D48896, so we shouldn't 
claim that we support this feature. This should be cherry-picked into the 7.0 
release, since I believe the first half landed before the branch point.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172

Files:
  libcxx/include/__node_handle


Index: libcxx/include/__node_handle
===
--- libcxx/include/__node_handle
+++ libcxx/include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


Index: libcxx/include/__node_handle
===
--- libcxx/include/__node_handle
+++ libcxx/include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

The discussion kind of moved away from the original patch, probably because the 
problem is larger than the defition of some host macros. However I still think 
that this patch improves the situation.


Repository:
  rC Clang

https://reviews.llvm.org/D50845



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

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 162205.
Szelethus added a comment.

Rebased to https://reviews.llvm.org/D51057.


https://reviews.llvm.org/D50892

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-inheritance.cpp


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 
//===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 
uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'this->bptr->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 
uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,9 @@
   break;
   }
 
+  while (R->getAs()) {
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 //===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'this->bptr->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,9 @@
   break;
   }
 
+  while (R->getAs()) {
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits