[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 387851.
qiucf marked an inline comment as done.
qiucf added a comment.

Fix C++ tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109751

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/Sema/float128-ld-incompatibility.cpp

Index: clang/test/Sema/float128-ld-incompatibility.cpp
===
--- clang/test/Sema/float128-ld-incompatibility.cpp
+++ clang/test/Sema/float128-ld-incompatibility.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 \
 // RUN: -triple powerpc64le-unknown-linux-gnu -target-cpu pwr9 \
-// RUN: -target-feature +float128 %s
+// RUN: -Wno-unused-value -Wno-parentheses -target-feature +float128 %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -Wno-parentheses %s
 
 __float128 qf();
@@ -9,8 +9,8 @@
 #ifdef __PPC__
 // FIXME: once operations between long double and __float128 are implemented for
 //targets where the types are different, these next two will change
-long double ld{qf()}; // expected-error {{cannot initialize a variable of type 'long double' with an rvalue of type '__float128'}}
-__float128 q{ldf()};  // expected-error {{cannot initialize a variable of type '__float128' with an rvalue of type 'long double'}}
+long double ld{qf()}; // expected-error {{non-constant-expression cannot be narrowed from type '__float128' to 'long double' in initializer list}} expected-note {{insert an explicit cast to silence this issue}}
+__float128 q{ldf()}; // expected-no-error
 
 auto test1(__float128 q, long double ld) -> decltype(q + ld) { // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
   return q + ld;  // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
@@ -33,7 +33,9 @@
   q * ld; // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
   ld / q; // expected-error {{invalid operands to binary expression ('long double' and '__float128')}}
   q / ld; // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
-  ld = q; // expected-error {{assigning to 'long double' from incompatible type '__float128'}}
-  q = ld; // expected-error {{assigning to '__float128' from incompatible type 'long double'}}
+  ld = q; // expected-no-error {{assigning to 'long double' from incompatible type '__float128'}}
+  q = ld; // expected-no-error {{assigning to '__float128' from incompatible type 'long double'}}
   q + b ? q : ld; // expected-error {{incompatible operand types ('__float128' and 'long double')}}
+  q += ld; // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
+  ld /= q; // expected-error {{invalid operands to binary expression ('long double' and '__float128')}}
 }
Index: clang/test/CodeGen/ibm128-cast.c
===
--- clang/test/CodeGen/ibm128-cast.c
+++ clang/test/CodeGen/ibm128-cast.c
@@ -2,11 +2,18 @@
 // RUN:   -target-feature +float128 -mabi=ieeelongdouble -fsyntax-only -Wno-unused %s
 // RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown -verify \
 // RUN:   -target-feature +float128 -fsyntax-only -Wno-unused %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown -DCODEGEN \
+// RUN:   -target-feature +float128 -mabi=ieeelongdouble %s -o - | FileCheck %s
 
-__float128 cast1(__ibm128 x) { return x; } // expected-error {{returning '__ibm128' from a function with incompatible result type '__float128'}}
+// CHECK: define dso_local fp128 @cast1(ppc_fp128 %{{.+}})
+// CHECK: %{{.+}} = call fp128 @llvm.ppc.convert.ppcf128.to.f128(ppc_fp128 %{{.+}})
+__float128 cast1(__ibm128 x) { return x; } // expected-no-error
 
-__ibm128 cast2(__float128 x) { return x; } // expected-error {{returning '__float128' from a function with incompatible result type '__ibm128'}}
+// IEEE: define dso_local ppc_fp128 @cast2(fp128 %{{.+}})
+// IEEE: %{{.+}} = call ppc_fp128 @llvm.ppc.convert.f128.to.ppcf128(fp128 %{{.+}})
+__ibm128 cast2(__float128 x) { return x; } // expected-no-error
 
+#ifndef CODEGEN
 __ibm128 gf;
 
 void narrow(double *d, float *f) {
@@ -17,9 +24,9 @@
 }
 
 #ifdef __LONG_DOUBLE_IEEE128__
-long double cast3(__ibm128 x) { return x; } // expected-error {{returning '__ibm128' from a function with incompatible result type 'long double'}}
+long double cast3(__ibm128 x) { return x; } // expected-no-error
 
-__ibm128 cast4(long double x) { return x; } // expected-error {{returning 'long double' from a function with incompatible result type '__ibm128'}}
+__ibm128 cast4(long double x) { return x; } // expected-no-error
 
 void imp_cast(__ibm128 w, __float128 q, long dou

[PATCH] D113977: [Coroutine] Warn deprecated 'std::experimental::coro' uses

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

The CI fails. But I guess it would be irrelevant with this diff:

  /var/lib/buildkite-agent/builds/llvm-project/libcxx/.clang-format:6:17: 
error: invalid boolean
  SpacesInAngles: Leave
  ^
  Error reading 
/var/lib/buildkite-agent/builds/llvm-project/libcxx/.clang-format: Invalid 
argument


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

https://reviews.llvm.org/D113977

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

[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> Disabled double or float return for x86 targets

Sorry, I think we still need to match GCC's behavior. I.e., we shouldn't 
diagnosticate any FP type (float, double, long double) on 32-bits. 
https://godbolt.org/z/KrbhfWc9o
We have users who require of that. Noticed this problem during D112143 
.
Could you have a look again?




Comment at: clang/test/Sema/x86-no-x87.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu 
-target-feature -x87 -DRET_ERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -DNOERROR

This seems not used?



Comment at: clang/test/Sema/x86-no-x87.cpp:22-24
+// expected-error@+4{{'def' requires  'long_double' (aka 'long double') type 
support, but target 'i686-unknown-linux-gnu' does not support it}}
+// expected-note@+3{{'def' defined here}}
+// expected-note@+2{{'x' defined here}}

I'd think we can't emit error diagnostics on 32-bits now. We have users who 
rely on such behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

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

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 387855.
pengfei added a comment.

1. Add support for f80.
2. Add test case for 3 return values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112143

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/x86-target-features.c
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/no-ret-in-x87-reg.ll

Index: llvm/test/CodeGen/X86/no-ret-in-x87-reg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/no-ret-in-x87-reg.ll
@@ -0,0 +1,193 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-- | FileCheck %s -check-prefix=X87
+; RUN: llc < %s -mtriple=i686-- -mattr=-x87 | FileCheck %s -check-prefixes=NOX87,NOSSE-NOX87
+; RUN: llc < %s -mtriple=i686-- -mattr=-x87,-sse2 | FileCheck %s -check-prefixes=NOX87,NOSSE-NOX87
+; RUN: llc < %s -mtriple=i686-- -mattr=-x87,+sse2 | FileCheck %s -check-prefixes=NOX87,SSE-NOX87
+
+define float @f1(float %a, float %b) nounwind {
+; X87-LABEL: f1:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:flds {{[0-9]+}}(%esp)
+; X87-NEXT:retl
+;
+; NOSSE-NOX87-LABEL: f1:
+; NOSSE-NOX87:   # %bb.0: # %entry
+; NOSSE-NOX87-NEXT:movl {{[0-9]+}}(%esp), %eax
+; NOSSE-NOX87-NEXT:retl
+;
+; SSE-NOX87-LABEL: f1:
+; SSE-NOX87:   # %bb.0: # %entry
+; SSE-NOX87-NEXT:movd {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; SSE-NOX87-NEXT:movd %xmm0, %eax
+; SSE-NOX87-NEXT:retl
+entry:
+  ret float %b
+}
+
+define double @f2(double %a, double %b) nounwind {
+; X87-LABEL: f2:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:fldl {{[0-9]+}}(%esp)
+; X87-NEXT:retl
+;
+; NOX87-LABEL: f2:
+; NOX87:   # %bb.0: # %entry
+; NOX87-NEXT:movl {{[0-9]+}}(%esp), %eax
+; NOX87-NEXT:movl {{[0-9]+}}(%esp), %edx
+; NOX87-NEXT:retl
+entry:
+  ret double %b
+}
+
+define x86_fp80 @f3(x86_fp80 %a, x86_fp80 %b) nounwind {
+; X87-LABEL: f3:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:fldt {{[0-9]+}}(%esp)
+; X87-NEXT:retl
+;
+; NOX87-LABEL: f3:
+; NOX87:   # %bb.0: # %entry
+; NOX87-NEXT:movl {{[0-9]+}}(%esp), %eax
+; NOX87-NEXT:movl {{[0-9]+}}(%esp), %edx
+; NOX87-NEXT:movl {{[0-9]+}}(%esp), %ecx
+; NOX87-NEXT:retl
+entry:
+  ret x86_fp80 %b
+}
+
+define float @f4(float %a, float %b) nounwind {
+; X87-LABEL: f4:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:flds {{[0-9]+}}(%esp)
+; X87-NEXT:fadds {{[0-9]+}}(%esp)
+; X87-NEXT:retl
+;
+; NOSSE-NOX87-LABEL: f4:
+; NOSSE-NOX87:   # %bb.0: # %entry
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:calll __addsf3
+; NOSSE-NOX87-NEXT:addl $8, %esp
+; NOSSE-NOX87-NEXT:retl
+;
+; SSE-NOX87-LABEL: f4:
+; SSE-NOX87:   # %bb.0: # %entry
+; SSE-NOX87-NEXT:movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; SSE-NOX87-NEXT:addss {{[0-9]+}}(%esp), %xmm0
+; SSE-NOX87-NEXT:movd %xmm0, %eax
+; SSE-NOX87-NEXT:retl
+entry:
+  %0 = fadd float %a, %b
+  ret float %0
+}
+
+define double @f5(double %a, double %b) nounwind {
+; X87-LABEL: f5:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:fldl {{[0-9]+}}(%esp)
+; X87-NEXT:faddl {{[0-9]+}}(%esp)
+; X87-NEXT:retl
+;
+; NOSSE-NOX87-LABEL: f5:
+; NOSSE-NOX87:   # %bb.0: # %entry
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:pushl {{[0-9]+}}(%esp)
+; NOSSE-NOX87-NEXT:calll __adddf3
+; NOSSE-NOX87-NEXT:addl $16, %esp
+; NOSSE-NOX87-NEXT:retl
+;
+; SSE-NOX87-LABEL: f5:
+; SSE-NOX87:   # %bb.0: # %entry
+; SSE-NOX87-NEXT:pushl %ebp
+; SSE-NOX87-NEXT:movl %esp, %ebp
+; SSE-NOX87-NEXT:andl $-8, %esp
+; SSE-NOX87-NEXT:subl $8, %esp
+; SSE-NOX87-NEXT:movsd {{.*#+}} xmm0 = mem[0],zero
+; SSE-NOX87-NEXT:addsd 16(%ebp), %xmm0
+; SSE-NOX87-NEXT:movsd %xmm0, (%esp)
+; SSE-NOX87-NEXT:movl (%esp), %eax
+; SSE-NOX87-NEXT:movl {{[0-9]+}}(%esp), %edx
+; SSE-NOX87-NEXT:movl %ebp, %esp
+; SSE-NOX87-NEXT:popl %ebp
+; SSE-NOX87-NEXT:retl
+entry:
+  %0 = fadd double %a, %b
+  ret double %0
+}
+
+; FIXME: We should not generate x87 instructions when x87 is disabled.
+define x86_fp80 @f6(x86_fp80 %a, x86_fp80 %b) nounwind {
+; X87-LABEL: f6:
+; X87:   # %bb.0: # %entry
+; X87-NEXT:fldt {{[0-9]+}}(%esp)
+; X87-NEXT:fldt {{[0-9]+}}(%esp)
+; X87-NEXT:faddp %st, %st(1)
+; X87-NEXT:retl
+;
+; NOX87-LABEL: f6:
+; NOX87:   # %bb.0: # %entry
+; NOX87-NEXT:pushl %ebp
+; NOX87-NEXT:movl %esp, %ebp
+; NOX87-NEXT:andl $-8, %esp
+; NOX87-NEXT:subl $48, %esp
+; NOX87-NEXT:movl 20(%ebp), %eax
+; NOX87-NEXT:movl 24(%ebp), %ecx
+; NOX87-NEXT:movl %ecx, {{[0-9]+}}(%esp)
+; NOX87-

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86CallingConv.td:279
+  CCIfNotSubtarget<"hasX87()",
+CCIfType<[f32], CCAssignToReg<[EAX, EDX, ECX]>>>,
   CCIfType<[f16], CCAssignToReg<[XMM0,XMM1,XMM2]>>,

nickdesaulniers wrote:
> Is there a test case that exercises the assignment to %ecx?
Add one test case for it, though I don't believe we can really generate it from 
frontend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112143

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

[PATCH] D114058: [clang] Add ObjC decls to Decl::isFunctionOrFunctionTemplate

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks for the patch.

Could you add a test exercising call hierarchy for obj-c methods to 
CallHierarchyTests.cpp 
 please?

In terms of the actual change, this function has some other callers 
 in 
Parser and Sema code, and I don't know how this change will affect them. It may 
be better to limit the change to the clangd call site 

 for now (that is, change that call site to use a local helper function, which 
checks `isFunctionOrFunctionTemplate() || `), and defer changing 
`Decl::isFunctionOrFunctionTemplate()` itself to a subsequent refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114058

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

[PATCH] D114034: [clang-tidy] fix debug-only test failure

[David Stenberg via Phabricator via cfe-commits]

dstenb added a comment.

In D114034#3136057 , @mattbeardsley 
wrote:

> @dstenb - wanted to check with you too to make sure that this change to 
> pr37091.cpp seems like it won't interfere with the original intent of the 
> test (https://reviews.llvm.org/D45686 )
>
> The lingering file issue you'd fixed seemed to be independent of any 
> particular clang-tidy check, but if not, hopefully it can at least get away 
> with not relying on the top-level .clang-tidy anymore!

Thanks for asking!

Yes, I think that the issue was seen independently of the checks used. We saw 
that issue originally with clang-tidy invocations with explicit -checks 
arguments (specifying clang-analyzer-* and some more checks). So, I guess that 
just specifying some check here should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114034

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

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 387874.
pengfei marked 3 inline comments as done.
pengfei added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112413

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/pr23258.c
  clang/test/Driver/x86_features.c
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/pr23258.ll

Index: llvm/test/CodeGen/X86/pr23258.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/pr23258.ll
@@ -0,0 +1,72 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=HAS-RAX
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+sse | FileCheck %s --check-prefix=HAS-RAX
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=-sse | FileCheck %s --check-prefix=NO-RAX
+
+define void @foo() {
+; HAS-RAX-LABEL: foo:
+; HAS-RAX:   # %bb.0:
+; HAS-RAX-NEXT:movl $1, %edi
+; HAS-RAX-NEXT:xorl %eax, %eax
+; HAS-RAX-NEXT:jmp bar@PLT # TAILCALL
+;
+; NO-RAX-LABEL: foo:
+; NO-RAX:   # %bb.0:
+; NO-RAX-NEXT:movl $1, %edi
+; NO-RAX-NEXT:jmp bar@PLT # TAILCALL
+  tail call void (i32, ...) @bar(i32 1)
+  ret void
+}
+
+define void @bar(i32, ...) nounwind {
+; HAS-RAX-LABEL: bar:
+; HAS-RAX:   # %bb.0:
+; HAS-RAX-NEXT:subq $56, %rsp
+; HAS-RAX-NEXT:movq %rsi, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movq %rdx, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movq %rcx, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movq %r8, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movq %r9, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:testb %al, %al
+; HAS-RAX-NEXT:je .LBB1_2
+; HAS-RAX-NEXT:  # %bb.1:
+; HAS-RAX-NEXT:movaps %xmm0, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm1, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm2, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm3, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm4, -{{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm5, (%rsp)
+; HAS-RAX-NEXT:movaps %xmm6, {{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:movaps %xmm7, {{[0-9]+}}(%rsp)
+; HAS-RAX-NEXT:  .LBB1_2:
+; HAS-RAX-NEXT:leaq {{[0-9]+}}(%rsp), %rax
+; HAS-RAX-NEXT:movq %rax, 8
+; HAS-RAX-NEXT:leaq -{{[0-9]+}}(%rsp), %rax
+; HAS-RAX-NEXT:movq %rax, 16
+; HAS-RAX-NEXT:movl $8, 0
+; HAS-RAX-NEXT:movl $48, 4
+; HAS-RAX-NEXT:addq $56, %rsp
+; HAS-RAX-NEXT:retq
+;
+; NO-RAX-LABEL: bar:
+; NO-RAX:   # %bb.0:
+; NO-RAX-NEXT:movq %rsi, -{{[0-9]+}}(%rsp)
+; NO-RAX-NEXT:movq %rdx, -{{[0-9]+}}(%rsp)
+; NO-RAX-NEXT:movq %rcx, -{{[0-9]+}}(%rsp)
+; NO-RAX-NEXT:movq %r8, -{{[0-9]+}}(%rsp)
+; NO-RAX-NEXT:movq %r9, -{{[0-9]+}}(%rsp)
+; NO-RAX-NEXT:leaq {{[0-9]+}}(%rsp), %rax
+; NO-RAX-NEXT:movq %rax, 8
+; NO-RAX-NEXT:leaq -{{[0-9]+}}(%rsp), %rax
+; NO-RAX-NEXT:movq %rax, 16
+; NO-RAX-NEXT:movl $8, 0
+; NO-RAX-NEXT:movl $48, 4
+; NO-RAX-NEXT:retq
+  call void @llvm.va_start(i8* null)
+  ret void
+}
+
+declare void @llvm.va_start(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"SkipRaxSetup", i32 1}
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -4414,7 +4414,8 @@
 }
   }
 
-  if (Is64Bit && isVarArg && !IsWin64 && !IsMustTail) {
+  if (Is64Bit && isVarArg && !IsWin64 && !IsMustTail &&
+  (Subtarget.hasSSE1() || !M->getModuleFlag("SkipRaxSetup"))) {
 // From AMD64 ABI document:
 // For calls that may call functions that use varargs or stdargs
 // (prototype-less calls or calls to functions containing ellipsis (...) in
Index: clang/test/Driver/x86_features.c
===
--- clang/test/Driver/x86_features.c
+++ clang/test/Driver/x86_features.c
@@ -5,3 +5,9 @@
 // Test that we don't produce an error with -mieee-fp.
 // RUN: %clang -### %s -mieee-fp -S 2>&1 | FileCheck --check-prefix=IEEE %s
 // IEEE-NOT: error: unknown argument
+
+// RUN: %clang -### %s -mskip-rax-setup -S 2>&1 | FileCheck --check-prefix=SRS %s
+// SRS: "-mskip-rax-setup"
+
+// RUN: %clang -### %s -mno-skip-rax-setup -S 2>&1 | FileCheck --check-prefix=NO-SRS %s
+// NO-SRS-NOT: "-mskip-rax-setup"
Index: clang/test/CodeGen/pr23258.c
===
--- /dev/null
+++ clang/test/CodeGen/pr23258.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=NO-SKIP
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -mskip-rax-setup -emit-llvm %s -o - | FileCheck %s -check-prefix=SKIP
+
+void 

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> It might be nice to add an llvm-link test that produces an error when this 
> module metadata doesn't match. Basically, it would be nice to error if during 
> LTO we link together two modules that differ in their use of -mskip-rax-setup 
> since that has ABI implications between the two modules. 
> https://llvm.org/docs/LangRef.html#module-flags-metadata

I don't think so. The flag looks like ABI breaker, but indeed it's just for 
performance. If some modules generated without the flag, there's only a bit 
performance lose. If two modules with and without the flag are linking 
together. The `Override` works correctly for them.




Comment at: clang/include/clang/Driver/Options.td:3193
+def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group, 
Flags<[NoXarchOption]>,
+  HelpText<"Skip setting up RAX register when passing variable arguments (x86 
only)">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

nickdesaulniers wrote:
> pengfei wrote:
> > MaskRay wrote:
> > > nickdesaulniers wrote:
> > > > I think GCC support `-mno-skip-rax-setup` as well. Can you please add 
> > > > that (and tests for it) as well?  We don't need to actually handle it, 
> > > > I think, but we shouldn't warn about the flag being unsupported, for 
> > > > example.
> > > Consider `SimpleMFlag`
> > Thanks for the suggestion. I think adding a `-mno-skip-rax-setup` is simply 
> > here.
> Via @MaskRay 's suggestion, can we do something like:
> 
> ```
> def skip_rax_setup : SimpleMFlag<"skip-rax-setup", "Skip", "Do not skip", 
> "setting up RAX register when passing variable arguments (x86 only)">;
> ```
> 
> Do we test that this flag is warned about being unused for non-x86 targets? 
> (Is it worth adding such a test to clang)?
Another reason I didn't use `SimpleMFlag` is GCC doesn't have help test for 
`-mno-skip-rax-setup`. I deliberately hide it in this way.

> Do we test that this flag is warned about being unused for non-x86 targets? 
> (Is it worth adding such a test to clang)?

I tested it locally. Driver will warn it for other target, Clang doesn't. I 
think it should be enough. I don't know if there's precedent, but adding an 
irrelevant option test to other target's test folder looks a bit weird to me.



Comment at: llvm/test/CodeGen/X86/pr23258.ll:4
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+sse | FileCheck %s 
--check-prefix=HAS-RAX
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=-sse | FileCheck %s 
--check-prefix=NO-RAX
+

nickdesaulniers wrote:
> Might be worth testing the fourth case: +sse, SkipRaxSetup == 0. Though I 
> think that exposes that `X86TargetLowering::X86TargetLowering` is only 
> checking whether the Metadata exists, not whether it has a particular value.
I don't think so. We use it like the way we use `#ifdef xxx` instead of `#if 
xxx`. The value can be simply ignored, it is just required by the metadata 
semantic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112413

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

[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This will allow the IncludeCleaner to suppress warnings on the lines with "IWYU
pragma: keep".

Clang APIs are not very convinient, so the code has to navigate around it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114072

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp

Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -80,8 +80,9 @@
 EXPECT_TRUE(
 Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
 IncludeStructure Includes;
-Clang->getPreprocessor().addPPCallbacks(
-Includes.collect(Clang->getSourceManager()));
+auto Collector = Includes.collect(Clang->getSourceManager());
+Clang->getPreprocessor().addCommentHandler(Collector.get());
+Clang->getPreprocessor().addPPCallbacks(move(Collector));
 EXPECT_FALSE(Action.Execute());
 Action.EndSourceFile();
 return Includes;
@@ -143,6 +144,7 @@
 MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; }
 MATCHER_P(IncludeLine, N, "") { return arg.HashLine == N; }
 MATCHER_P(Directive, D, "") { return arg.Directive == D; }
+MATCHER_P(HasPragmaKeep, H, "") { return arg.BehindPragmaKeep == H; }
 
 MATCHER_P2(Distance, File, D, "") {
   if (arg.getFirst() != File)
@@ -258,6 +260,18 @@
Directive(tok::pp_include_next)));
 }
 
+TEST_F(HeadersTest, IWYUPragmaKeep) {
+  FS.Files[MainFile] = R"cpp(
+#include "bar.h" // IWYU pragma: keep
+#include "foo.h"
+)cpp";
+
+  EXPECT_THAT(
+  collectIncludes().MainFileIncludes,
+  UnorderedElementsAre(AllOf(Written("\"foo.h\""), HasPragmaKeep(false)),
+   AllOf(Written("\"bar.h\""), HasPragmaKeep(true;
+}
+
 TEST_F(HeadersTest, InsertInclude) {
   std::string Path = testPath("sub/bar.h");
   FS.Files[Path] = "";
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -286,7 +286,9 @@
   const auto &SM = Clang->getSourceManager();
   Preprocessor &PP = Clang->getPreprocessor();
   IncludeStructure Includes;
-  PP.addPPCallbacks(Includes.collect(SM));
+  auto Collector = Includes.collect(SM);
+  PP.addCommentHandler(Collector.get());
+  PP.addPPCallbacks(move(Collector));
   ScannedPreamble SP;
   SP.Bounds = Bounds;
   PP.addPPCallbacks(
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -442,8 +442,9 @@
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...
   // (We can't *just* use the replayed includes, they don't have Resolved path).
-  Clang->getPreprocessor().addPPCallbacks(
-  Includes.collect(Clang->getSourceManager()));
+  auto Collector = Includes.collect(Clang->getSourceManager());
+  Clang->getPreprocessor().addCommentHandler(Collector.get());
+  Clang->getPreprocessor().addPPCallbacks(move(Collector));
   // Copy over the macros in the preamble region of the main file, and combine
   // with non-preamble macros below.
   MainFileMacros Macros;
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -19,6 +19,7 @@
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -62,6 +63,7 @@
   int HashLine = 0;// Line number containing the directive, 0-indexed.
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
   llvm::Optional HeaderID;
+  bool BehindPragmaKeep = false; // Has IWYU pragma: keep right after.
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);
@@ -121,9 +123,21 @@
 RealPathNames.emplace_back();
   }
 
+  class IncludeCollector : public PPCallbacks, public CommentHandler {};
   // Returns a PPCallback that visits all inclusions in the main file and
-  // populates the structure.
-  std::unique_ptr collect(c

[PATCH] D114058: [clang] Add ObjC decls to Decl::isFunctionOrFunctionTemplate

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I agree with Nathan on this one. It's unclear why there are two functions that 
does the same thing in different ways and some usages in sema looks suspicious 
enough to require caution (there are subtle checks for shadowing and whatnot 
and I don't know how these rules come into play in objc contexts).

So I would suggest updating clangd side to look like:

  (isa(Decl) && cast(Decl)->isFunctionOrMethod()) || 
Decl->getlKind() == FunctionTemplate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114058

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

[PATCH] D114073: [clang-format][NFC] Add a default value to parseBlock()

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks.
owenpan added a project: clang-format.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114073

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h

Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -84,7 +84,7 @@
   void reset();
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
-  void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
+  void parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
   bool MunchSemi = true,
   bool UnindentWhitesmithsBraces = false);
   void parseChildBlock();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -385,7 +385,7 @@
   // be in a non-declaration context.
   if (!FormatTok->is(TT_MacroBlockBegin) && tryToParseBracedList())
 continue;
-  parseBlock(/*MustBeDeclaration=*/false);
+  parseBlock();
   addUnwrappedLine();
   break;
 case tok::r_brace:
@@ -1313,7 +1313,7 @@
   if (Style.BraceWrapping.AfterControlStatement ==
   FormatStyle::BWACS_Always)
 addUnwrappedLine();
-  parseBlock(/*MustBeDeclaration=*/false);
+  parseBlock();
 }
 addUnwrappedLine();
 return;
@@ -1326,7 +1326,7 @@
   if (Style.BraceWrapping.AfterControlStatement ==
   FormatStyle::BWACS_Always)
 addUnwrappedLine();
-  parseBlock(/*MustBeDeclaration=*/false);
+  parseBlock();
 }
 addUnwrappedLine();
 return;
@@ -1434,7 +1434,7 @@
 if (Style.BraceWrapping.AfterFunction)
   addUnwrappedLine();
 FormatTok->setType(TT_FunctionLBrace);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 addUnwrappedLine();
 return;
   }
@@ -2078,7 +2078,7 @@
   bool NeedsUnwrappedLine = false;
   if (FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 if (Style.BraceWrapping.BeforeElse)
   addUnwrappedLine();
 else
@@ -2096,7 +2096,7 @@
   parseSquare();
 if (FormatTok->Tok.is(tok::l_brace)) {
   CompoundStatementIndenter Indenter(this, Style, Line->Level);
-  parseBlock(/*MustBeDeclaration=*/false);
+  parseBlock();
   addUnwrappedLine();
 } else if (FormatTok->Tok.is(tok::kw_if)) {
   FormatToken *Previous = AllTokens[Tokens->getPosition() - 1];
@@ -2158,7 +2158,7 @@
   }
   if (FormatTok->is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 if (Style.BraceWrapping.BeforeCatch) {
   addUnwrappedLine();
 } else {
@@ -2196,7 +2196,7 @@
 }
 NeedsUnwrappedLine = false;
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 if (Style.BraceWrapping.BeforeCatch)
   addUnwrappedLine();
 else
@@ -2309,7 +2309,7 @@
 parseParens();
   if (FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 addUnwrappedLine();
   } else {
 addUnwrappedLine();
@@ -2324,7 +2324,7 @@
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 if (Style.BraceWrapping.BeforeWhile)
   addUnwrappedLine();
   } else {
@@ -2363,7 +2363,7 @@
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Style.BraceWrapping.IndentBraces);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 if (FormatTok->Tok.is(tok::kw_break)) {
   if (Style.BraceWrapping.AfterControlStatement ==
   FormatStyle::BWACS_Always) {
@@ -2405,7 +2405,7 @@
 parseParens();
   if (FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Style, Line->Level);
-parseBlock(/*MustBeDeclaration=*/false);
+parseBlock();
 addUnwrappedLine();
   } else {
 addUnwrappedLine();
@@ -2457,7 +2457,7 @@
 if (Style.BraceWrapping.AfterFunction)
   addUnwrappedLine();
 FormatTok->setType(TT_FunctionLBrace);
-  

[PATCH] D113999: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113999

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

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Added a bit of analysis on https://github.com/clangd/clangd/issues/888.

The short version is we built a PCH. When consuming it, the parser decides to 
re-lex and this means sources must be loaded from disk.
These sources are inconsistent with the PCH, so the precondition that the input 
range describes a reasonable literal doesn't hold.

This patch fixes it by removing the precondition and defending against it 
instead.
This seems like an OK fallback unless there's hundreds of other places where we 
might re-lex and make similar assumptions.
I'm going to dig and try to find out but if anyone knows about this it'd be 
nice to chime in :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114003

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

[PATCH] D114073: [clang-format][NFC] Add a default value to parseBlock()

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM

I like it being cleaner


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114073

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

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I find the code more readable and robust when the `check*` function checks its 
applicability itself. After this refactoring, it is not so clear when each 
check functions applies, what are the correct conditions to call them. To 
ensure correct usage, probably we should be adding equivalent asserts to the 
beginning of each function; at which point I'm not sure if the new code is 
better.

For example, consider `checkContainerDecl`. It used to start with:

  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
  if (!Info->IsRecordLikeDetailCommand || isRecordLikeDecl())
return;

This code clearly shows the condition for the warning: the command is 
"RecordDetailLike" but the decl is not "RecordLike". In the new code this logic 
is split across two functions.

Is repeated `CommandInfo` fetching a performance issue? It shouldn't be because 
it is basically address arithmetic on the static array of commands using the 
command ID as an index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113793

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

[PATCH] D113795: Comment Sema: Eliminate or factor out DeclInfo inspection (NFC)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Sorry, here I also find the old code to be more readable.

- I don't see a problem with checks that are only used once. They are 
encapsulated in functions with meaningful names, making the code more readable. 
Compare `Sema::checkFunctionDeclVerbatimLine` before and after, for example.

- checkDecl looks like a too complex abstraction for the task at hand: it 
accepts a function pointer (and it is incorrect for the user to call those 
other functions directly), it hardcodes the result value of `false` when the 
decl is not available etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113795

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

[PATCH] D114077: [clangd] Basic IncludeCleaner support for c/c++ standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

There are some limitations here, so this is behind a flag for now (in addition
to the config setting for the overall feature).

- symbols without exactly one associated header aren't handled right
- no macro support
- referencing std::size_t usually doesn't leave any trace in the AST that the 
alias in std was used, so we associate with stddef.h instead of cstddef. (An 
AST issue not specific to stdlib, but much worse there)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114077

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,7 +8,9 @@
 
 #include "Annotations.h"
 #include "IncludeCleaner.h"
+#include "SourceCode.h"
 #include "TestTU.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -17,6 +19,8 @@
 namespace {
 
 using ::testing::ElementsAre;
+using ::testing::ElementsAreArray;
+using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
 std::string guard(llvm::StringRef Code) {
@@ -190,7 +194,7 @@
 auto AST = TU.build();
 
 std::vector Points;
-for (const auto &Loc : findReferencedLocations(AST)) {
+for (const auto &Loc : findReferencedLocations(AST).User) {
   if (AST.getSourceManager().getBufferName(Loc).endswith(
   TU.HeaderFilename)) {
 Points.push_back(offsetToPosition(
@@ -204,6 +208,81 @@
   }
 }
 
+TEST(IncludeCleaner, Stdlib) {
+  // Smoke tests only for finding used symbols/headers.
+  // Details of Decl -> stdlib::Symbol -> stdlib::Headers mapping tested there.
+  auto TU = TestTU::withHeaderCode(R"cpp(
+namespace std { class error_code {}; }
+class error_code {};
+namespace nonstd { class error_code {}; }
+  )cpp");
+  struct {
+llvm::StringRef Code;
+std::vector Symbols;
+std::vector Headers;
+  } Tests[] = {
+  {"std::error_code x;", {"std::error_code"}, {""}},
+  {"error_code x;", {}, {}},
+  {"nonstd::error_code x;", {}, {}},
+  };
+
+  for (const auto &Test : Tests) {
+TU.Code = Test.Code.str();
+ParsedAST AST = TU.build();
+std::vector WantSyms;
+for (const auto &SymName : Test.Symbols) {
+  auto QName = splitQualifiedName(SymName);
+  auto Sym = stdlib::Symbol::named(QName.first, QName.second);
+  EXPECT_TRUE(Sym) << SymName;
+  WantSyms.push_back(*Sym);
+}
+std::vector WantHeaders;
+for (const auto &HeaderName : Test.Headers) {
+  auto Header = stdlib::Header::named(HeaderName);
+  EXPECT_TRUE(Header) << HeaderName;
+  WantHeaders.push_back(*Header);
+}
+
+ReferencedLocations Locs = findReferencedLocations(AST);
+EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms));
+ReferencedFiles Files = findReferencedFiles(Locs, AST.getSourceManager());
+EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders));
+  }
+}
+
+MATCHER_P(WrittenInclusion, Written, "") {
+  if (arg.Written != Written)
+*result_listener << arg.Written;
+  return arg.Written == Written;
+}
+
+TEST(IncludeCleaner, StdlibUnused) {
+  setIncludeCleanerAnalyzesStdlib(true);
+  auto Cleanup =
+  llvm::make_scope_exit([] { setIncludeCleanerAnalyzesStdlib(false); });
+
+  auto TU = TestTU::withCode(R"cpp(
+#include 
+#include 
+std::list x;
+  )cpp");
+  // Layout of std library impl is not relevant.
+  TU.AdditionalFiles["bits"] = R"cpp(
+#pragma once
+namespace std {
+  template  class list {};
+  template  class queue {};
+}
+  )cpp";
+  TU.AdditionalFiles["list"] = "#include ";
+  TU.AdditionalFiles["queue"] = "#include ";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+
+  auto Unused = computeUnusedIncludes(AST);
+  EXPECT_THAT(Unused, ElementsAre(Pointee(WrittenInclusion("";
+}
+
 TEST(IncludeCleaner, GetUnusedHeaders) {
   llvm::StringLiteral MainFile = R"cpp(
 #include "a.h"
@@ -279,7 +358,7 @@
 
   auto ReferencedFiles = findReferencedFiles(findReferencedLocations(AST), SM);
   llvm::StringSet<> ReferencedFileNames;
-  for (FileID FID : ReferencedFiles)
+  for (FileID FID : ReferencedFiles.User)
 ReferencedFileNames.insert(
 SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
   // Note we

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:95
+  if (HS.CurrentSearchPathIdx != ~0U)
+HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx;
+}

These indices will be out of date if the search paths are changed via 
`HeaderSearch::SetSearchPaths` or `HeaderSearch::AddSearchPath` after the first 
module map has been loaded. One could probably adjust the indices in 
`AddSearchPath` but maybe it’s easier to not use the index into `SearchDirs` as 
the key. An alternative suggestion would be to use 
`std::shared_ptr` instead, changing some of the data 
structures as follows:
```
- std::vector SearchDirs
+ std::vector> SearchDirs

- std::vector SearchDirsUsage;
+ std::map, bool> SearchDirsUsage; 

- llvm::DenseMap ModuleToSearchDirIdx;
+ llvm::DenseMap> 
ModuleToSearchDirIdx;

- llvm::DenseMap SearchDirToHSEntry;
+ llvm::DenseMap, unsigned> 
SearchDirToHSEntry; 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Bradley Smith via Phabricator via cfe-commits]

bsmith updated this revision to Diff 387900.
bsmith added a comment.

- Use more brute force approach to ensure ordering is accounted for
  - This massively simplifies things and removes what was becoming very 
confusing logic
- Add tests for missing cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/aarch64-implied-sve-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -903,11 +903,11 @@
  AArch64::AEK_SIMD | AArch64::AEK_RAS |
  AArch64::AEK_LSE | AArch64::AEK_RDM |
  AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
- AArch64::AEK_SVE2 | AArch64::AEK_BF16 |
- AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
- AArch64::AEK_PAUTH | AArch64::AEK_MTE |
- AArch64::AEK_SSBS | AArch64::AEK_FP16FML |
- AArch64::AEK_SB,
+ AArch64::AEK_BF16 | AArch64::AEK_I8MM |
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_PAUTH |
+ AArch64::AEK_MTE | AArch64::AEK_SSBS |
+ AArch64::AEK_FP16FML | AArch64::AEK_SB,
  "9-A"),
 ARMCPUTestParams("cortex-a57", "armv8-a", "crypto-neon-fp-armv8",
  AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
@@ -1012,12 +1012,12 @@
  AArch64::AEK_CRC | AArch64::AEK_FP |
  AArch64::AEK_SIMD | AArch64::AEK_RAS |
  AArch64::AEK_LSE | AArch64::AEK_RDM |
- AArch64::AEK_RCPC | AArch64::AEK_SVE2 |
- AArch64::AEK_DOTPROD | AArch64::AEK_MTE |
- AArch64::AEK_PAUTH | AArch64::AEK_I8MM |
- AArch64::AEK_BF16 | AArch64::AEK_SVE2BITPERM |
- AArch64::AEK_SSBS | AArch64::AEK_SB |
- AArch64::AEK_FP16FML,
+ AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+ AArch64::AEK_MTE | AArch64::AEK_PAUTH |
+ AArch64::AEK_I8MM | AArch64::AEK_BF16 |
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_SSBS |
+ AArch64::AEK_SB | AArch64::AEK_FP16FML,
  "9-A"),
 ARMCPUTestParams("cyclone", "armv8-a", "crypto-neon-fp-armv8",
  AArch64::AEK_NONE | AArch64::AEK_CRYPTO |
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,9 +145,10 @@
 AARCH64_CPU_NAME("cortex-a55", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC))
 AARCH64_CPU_NAME("cortex-a510", ARMV9A, FK_NEON_FP_ARMV8, false,
- (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
+ (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SB |
   AArch64::AEK_PAUTH | AArch64::AEK_MTE | AArch64::AEK_SSBS |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("cortex-a57", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a65", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
@@ -184,8 +185,9 @@
   AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-x2", ARMV9A, FK_NEON_FP_ARMV8, false,
  (AArch64::AEK_MTE | AArch64::AEK_BF16 | AArch64::AEK_I8MM |
-  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SVE2BITPERM |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SB |
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("neoverse-e1", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_DOTPROD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
  

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

Based on Sam's comments and our offline discussion, I would propose:

1. abandon this patch
2. start a discussion on the mailing list to disable this check for all of the 
llvm monorepo.

Other thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This check was written specifically for the LLVM monorepo, I doubt there'll be 
consensus to disable it.
https://reviews.llvm.org/D72217 has some previous discussion - some people want 
the consts spelled out, but we don't do this in clangd.
And I don't particularly think that would be the right thing to do: it's 
throwing out the baby with the bathwater.

I think we should rather:

- fix the check to exempt strings (I don't think this would be terribly 
controversial, but I might be wrong)
- land a version of this patch that doesn't add `const` in the places we don't 
normally use it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[PATCH] D113902: [NFC][clangd] exclude test data from clang-tidy

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

In D113902#3131707 , @sammccall wrote:

> Hmm, clang-tidy should only be running on entries in compile_commands.json. 
> are these files listed somehow?

No, they are not.

The question is: 
Do the tools our contributors use care about this? This is not the default 
behavior of clang-tidy, it's rather implemented in a LLVM-specific wrapper 
script. So do we require our contributors to use that script to check their 
code?

I was playing with the VS Code extension 
 to see 
the findings right in the IDE and that calls clang-tidy directly. So it ignores 
compile_commands.json


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113902

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds diagnosing on attempt to use zero length arrays, pointers, refs, arrays
of them and structs/classes containing all of it.
A couple of assertions were removed from SemaSYCL because the new
diagnostics are deferred and emitted later than SemaSYCL routines.
In case a struct/class with zero length array is used this emits a set
of notes pointing out how zero length array got into used struct, like
this:

  struct ContainsArr {
int A[0]; // note: field of illegal type declared here
  };
  struct Wrapper {
ContainsArr F; // note: within field of type ContainsArr declared here
// ...
  }
  
  // Device code
  Wrapper W;
  W.use(); // error: zero-length arrays are not permitted

Total deep check of each used declaration may result in double
diagnosing at the same location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114080

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/test/SemaSYCL/zero-length-arrays.cpp

Index: clang/test/SemaSYCL/zero-length-arrays.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/zero-length-arrays.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -fsyntax-only -verify %s
+//
+// This test checks if compiler reports compilation error on an attempt to use
+// a zero-length array inside device code.
+
+template 
+__attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc(); // #KernelObjCall
+}
+
+typedef float ZEROARR[0];
+
+struct Wrapper {
+  int A;
+  int BadArray[0]; // expected-note 3{{field of illegal type 'int[0]' declared here}}
+};
+
+struct WrapperOfWrapper { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  Wrapper F;  // expected-note 2{{within field of type 'Wrapper' declared here}}
+  ZEROARR *Ptr;   //expected-note 5{{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+};
+
+template  struct InnerTemplated {
+  double Array[Size]; // expected-note 8{{field of illegal type 'double[0]' declared here}}
+};
+
+template  struct Templated {
+  unsigned A;
+  Ty Arr[Size];
+  InnerTemplated Array[Size + 1]; // expected-note 8{{within field of type 'InnerTemplated<0U>[1]' declared here}}
+};
+
+struct KernelSt {
+  int A;
+  int BadArray[0]; // expected-note {{field of illegal type 'int[0]' declared here}}
+  void operator()() const {}
+};
+
+WrapperOfWrapper offendingFoo() {
+  // expected-note@+1 {{called by 'offendingFoo'}}
+  return WrapperOfWrapper{};
+}
+
+template 
+void templatedContext() {
+  Templated Var;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@#KernelObjCall {{called by 'kernel([=] {
+// expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+(void)Var; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+  kernel([Var] {
+  });
+}
+
+void foo(const unsigned X) {
+  int Arr[0];  // expected-note 2{{declared here}}
+  ZEROARR TypeDef; // expected-note {{declared here}}
+  ZEROARR *Ptr;// expected-note {{declared here}}
+   // expected-error@#KernelObjCall 3{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=]() {
+(void)Arr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+(void)TypeDef; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+// expected-note@+1 {{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+(void)Ptr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{field of illegal type 'int[0]' declared here}}
+  kernel([Arr] { // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  WrapperOfWrapper St;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=] 

[PATCH] D112773: Allow __attribute__((swift_attr)) in attribute push pragmas

[Aaron Ballman via Phabricator via cfe-commits]

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

In D112773#3135975 , @beccadax wrote:

>> My reading of 
>> https://clang.llvm.org/docs/LanguageExtensions.html#specifying-an-attribute-for-multiple-declarations-pragma-clang-attribute
>>  suggests that any() /could/ apply to everything. (`The any rule applies 
>> attributes to all declarations that are matched by at least one of the rules 
>> in the any.`)
>
> My reading of that rule suggest that an `any()` with no subrules would not 
> match anything because, if you fed in a declaration, none of the 
> (nonexistent) subrules of the `any()` would match it.
>
>> My recollection of the discussions when we adopted #pragma clang attribute 
>> is that we tried our best to limit the damage from the pragma over-applying 
>> attributes, but I don't recall if we explicitly talked about any() with no 
>> subjects. What would make me comfortable with this patch is 1) doing some 
>> archeology to find the original reviews for pragma clang attribute to see if 
>> we made any sort of explicit decision here but forgot to document it. If we 
>> didn't make an explicit decision in this area, we should make one now. We 
>> can either decide "any() with no subjects applies to everything", "any() 
>> with no subjects is dangerous and will apply to nothing", or "any() with no 
>> subjects is dangerous and we will diagnose if the user tries this" (probably 
>> other ideas we could consider). Ultimately, I would like to see the pragma 
>> clang attribute documentation updated before/when we land this bit so that 
>> our documentation clearly sets user expectations.
>
> Looking at https://reviews.llvm.org/D30009, I don't see any relevant 
> discussion of how `any()` (without subrules) should behave. What I do see is 
> that they started with "match everything always" behavior, but concerns about 
> users not knowing which declarations it would be applied to (and how that 
> behavior might change as the compiler evolved) led them to the idea of 
> requiring users to list the declarations they expected the compiler to match 
> and having a way to allow a subset ; 
> then they realized that only the subset version was actually useful 
> . What I take from this is that there 
> was clearly concern about users not being able to tell what the pragma would 
> affect and they changed the design so that users would need to specify it 
> clearly, which suggests that a way to leave it unspecified was seen as an 
> anti-goal.
>
> Looking at the implementation of 
> `Parser::ParsePragmaAttributeSubjectMatchRuleSet()`, it looks like `any` or 
> `any()` would cause a parse error—we return `true` (as in the error branches) 
> if we do not parse an open parenthesis or do not parse an identifier after 
> the open parenthesis—which again makes me think that the design intentionally 
> left out any mechanism to match all subjects.
>
> We don't have a statement that there is and should be no way to apply 
> `attribute push` to everything, but I think we can conclude that having such 
> a capability wasn't a goal of the final design.
>
> What would you like me to do from here?

Thank you for diving into those details! I'm now sold on the idea that `any` 
with no subjects is dangerous and we will diagnose if the user tries this. So I 
think the only thing that needs to happen is a documentation update and some 
additional test coverage for this scenario, which I'm happy to take care of 
myself since it's orthogonal to these changes. Based on that, this LGTM as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112773

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

[PATCH] D114011: Add a clang-transformer tutorial

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 387906.
ymandel added a comment.

removed stray internal reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114011

Files:
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -64,6 +64,7 @@
RAVFrontendAction
LibASTMatchersTutorial
LibASTMatchers
+   ClangTransformerTutorial
LibASTImporter
HowToSetupToolingForLLVM
JSONCompilationDatabase
Index: clang/docs/ClangTransformerTutorial.rst
===
--- /dev/null
+++ clang/docs/ClangTransformerTutorial.rst
@@ -0,0 +1,402 @@
+==
+Clang Transformer Tutorial
+==
+
+A tutorial on how to write a source-to-source translation tool using Clang Transformer.
+
+.. contents::
+   :local:
+
+What is Clang Transformer?
+--
+
+Clang Transformer is a framework for writing C++ diagnostics and program
+transformations. It is built on the clang toolchain and the LibTooling library,
+but aims to hide much of the complexity of clang's native, low-level libraries.
+
+The core abstraction of Transformer is the *rewrite rule*, which specifies how
+to change a given program pattern into a new form. Here are some examples of
+tasks you can achieve with Transformer:
+
+*   warn against using the name ``MkX`` for a declared function,
+*   change ``MkX`` to ``MakeX``, where ``MkX`` is the name of a declared function,
+*   change ``s.size()`` to ``Size(s)``, where ``s`` is a ``string``,
+*   collapse ``e.child().m()`` to ``e.m()``, for any expression ``e`` and method named
+``m``.
+
+All of the examples have a common form: they identify a pattern that is the
+target of the transformation, they specify an *edit* to the code identified by
+the pattern, and their pattern and edit refer to common variables, like ``s``,
+``e``, and ``m``, that range over code fragments. Our first and second examples also
+specify constraints on the pattern that aren't apparent from the syntax alone,
+like "``s`` is a ``string``." Even the first example ("warn ...") shares this form,
+even though it doesn't change any of the code -- it's "edit" is simply a no-op.
+
+Transformer helps users succinctly specify rules of this sort and easily execute
+them locally over a collection of files, apply them to selected portions of
+a codebase, or even bundle them as a clang-tidy check for ongoing application.
+
+Who is Clang Transformer for?
+-
+
+Clang Transformer is for developers who want to write clang-tidy checks or write
+tools to modify a large number of C++ files in (roughly) the same way. What
+qualifies as "large" really depends on the nature of the change and your
+patience for repetitive editing. In our experience, automated solutions become
+worthwhile somewhere between 100 and 500 files.
+
+Getting Started
+---
+
+Patterns in Transformer are expressed with
+`clang's AST matchers `_.
+Matchers are a language of combinators for describing portions of a clang
+Abstract Syntax Tree (AST). Since clang's AST includes complete type information
+(within the limits of single `Translation Unit (TU)`_,
+these patterns can even encode rich constraints on the type properties of AST
+nodes.
+
+.. _`Translation Unit (TU)`: https://en.wikipedia.org/wiki/Translation_unit_\(programming\)
+
+We assume a familiarity with the clang AST and the corresponding AST matchers
+for the purpose of this tutorial. Users who are unfamiliar with either are
+encouraged to start with the recommended references in
+[Related Reading](#related-reading).
+
+Example: style-checking names
+^
+
+Assume you have an API which forbids functions from being named "MkX" and you
+want to write a check that catches any violations of this rule. We can express
+this a Transformer rewrite rule:
+
+.. code-block:: c++
+		
+   makeRule(functionDecl(hasName("MkX").bind("fun"),
+	noopEdit(node("fun")),
+	cat("The name ``MkX`` is not allowed for functions; please rename"));
+
+``makeRule`` is our go-to function for generating rewrite rules. It takes three
+arguments: the pattern, the edit, and (optionally) an explanatory note. In our
+example, the pattern (``functionDecl(...)``) identifies the declaration of the
+function ``MkX``. Since we're just diagnosing the problem, but not suggesting a
+fix, our edit is an no-op. But, it contains an *anchor* for the diagnostic
+message: ``node("fun")`` says to associate the message with the source range of
+the AST node bound to "fun"; in this case, the ill-named function declaration.
+Finally, we use ``cat`` to build a message that explains the change. Regardi

[PATCH] D114011: Add a clang-transformer tutorial

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/docs/ClangTransformerTutorial.rst:52
+Patterns in Transformer are expressed with
+`clang's AST matchers `_.
+Matchers are a language of combinators for describing portions of a clang

Please use relative links:

```
:doc:`LibASTMatchers`
```



Comment at: clang/docs/ClangTransformerTutorial.rst:64
+encouraged to start with the recommended references in
+[Related Reading](#related-reading).
+

Does this link work? Or does it need to use the RST syntax?



Comment at: clang/docs/ClangTransformerTutorial.rst:69
+
+Assume you have an API which forbids functions from being named "MkX" and you
+want to write a check that catches any violations of this rule. We can express

"an api" -> "a style guide rule"?



Comment at: clang/docs/ClangTransformerTutorial.rst:94
+
+Example: function rename
+

"renaming a function"



Comment at: clang/docs/ClangTransformerTutorial.rst:320
+
+That is, an ``EditGenerator`` is function that maps a ``MatchResult`` and to a 
set
+of edits, or fails. This signature supports a very general form of computation

Remove "and"?



Comment at: clang/docs/ClangTransformerTutorial.rst:355
+
+Sometimes, a modification to the code might require the inclusion a particular
+header file. To this end, users can modify rules to specify include directives

"of a particular"



Comment at: clang/docs/ClangTransformerTutorial.rst:396
+*   `Matching the Clang AST `_
+*   `AST Matcher Reference 
`_
+

Please use relative links with `:doc:`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114011

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@jdoerfert , @yaxunl , please let me know if similar thing would be useful for 
OMP/HIP/CUDA as well. I can generalize it by using `targetDiag`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> I am not sure how to reproduce this.

Attaching preprocessed source. With Clang built at 
508aa4fe0c82b3b409e2e194d591ee6d665c8623 it reproduces for me like this:

  $ clang++ -c /tmp/a.ii
  ../../base/containers/span_unittest.cc:11:13: error: no viable conversion 
from 'span' to 'span'
span s = make_span(v);
  ^   
  ../../base/containers/span.h:340:13: note: candidate constructor not viable: 
no known conversion from 'span' (aka 'span') to 'const base::span &' for 1st 
argument
constexpr span(const span& other) noexcept = default;
  ^
  ../../base/containers/span.h:303:13: note: candidate template ignored: could 
not match 'int[N]' against 'span' (aka 'span')
constexpr span(T (&array)[N]) noexcept : span(base::data(array), N) {}
  ^
  ../../base/containers/span.h:310:13: note: candidate template ignored: could 
not match 'array' against 'span'
constexpr span(std::array& array) noexcept
  ^
  ../../base/containers/span.h:317:13: note: candidate template ignored: could 
not match 'array' against 'span'
constexpr span(const std::array& array) noexcept
  ^
  ../../base/containers/span.h:328:13: note: candidate template ignored: 
requirement 
'conjunction>>, 
base::negation>>, base::negation>>, std::is_convertible, 
std::is_integral>::value' was not satisfied [with Container = 
base::span]
constexpr span(Container& container) noexcept{F20424748}
  ^
  ../../base/containers/span.h:337:13: note: candidate template ignored: 
requirement 
'conjunction>>, 
base::negation>>, base::negation>>, std::is_convertible, 
std::is_integral>::value' was not satisfied [with Container = 
base::span]
constexpr span(const Container& container) noexcept
  ^
  ../../base/containers/span.h:350:13: note: candidate template ignored: 
requirement 'is_convertible::value' was not 
satisfied [with U = const int, OtherExtent = 18446744073709551615]
constexpr span(const span& other)
  ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Sorry, the attached file is here: F20424768: a.ii 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D114082: [WIP] Normalize String Attributes

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

This is a WIP, to start the discussion. I've only turned on the X86 backend. 
Otherwise should be functional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114082

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

[clang] 3874277 - Improve docs & test for #pragma clang attribute's any clause; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-11-17T08:24:26-05:00
New Revision: 3874277f415dca0bb222956983f117a6211c0e39

URL: 
https://github.com/llvm/llvm-project/commit/3874277f415dca0bb222956983f117a6211c0e39
DIFF: 
https://github.com/llvm/llvm-project/commit/3874277f415dca0bb222956983f117a6211c0e39.diff

LOG: Improve docs & test for #pragma clang attribute's any clause; NFC

There was some confusion during the discussion of a patch as to whether
`any` can be used to blast an attribute with no subject list onto
basically everything in a program by not specifying a subrule. This
patch adds documentation and tests to make it clear that this situation
is not supported and will be diagnosed.

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/test/Parser/pragma-attribute.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 8f7727e27bf60..60b1ed383a1ff 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3798,7 +3798,8 @@ Multiple match rules can be specified using the ``any`` 
match rule, as shown
 in the example above. The ``any`` rule applies attributes to all declarations
 that are matched by at least one of the rules in the ``any``. It doesn't nest
 and can't be used inside the other match rules. Redundant match rules or rules
-that conflict with one another should not be used inside of ``any``.
+that conflict with one another should not be used inside of ``any``. Failing to
+specify a rule within the ``any`` rule results in an error.
 
 Clang supports the following match rules:
 

diff  --git a/clang/test/Parser/pragma-attribute.cpp 
b/clang/test/Parser/pragma-attribute.cpp
index d51f8159c263c..d82eaa3f01b0f 100644
--- a/clang/test/Parser/pragma-attribute.cpp
+++ b/clang/test/Parser/pragma-attribute.cpp
@@ -204,3 +204,9 @@ _Pragma("clang attribute pop");
 #pragma clang attribute pop
 #pragma clang attribute push([[clang::no_destroy]], apply_to = 
any(variable(is_parameter), variable(unless(is_parameter
 #pragma clang attribute pop
+
+// We explicitly do not wish to allow users to blast an attribute to everything
+// using an unconstrained "any", so "any" must have a valid argument list.
+#pragma clang attribute push([[clang::uninitialized]], apply_to=any) // 
expected-error {{expected '('}}
+#pragma clang attribute push([[clang::uninitialized]], apply_to = any()) // 
expected-error {{expected an identifier that corresponds to an attribute 
subject rule}}
+// NB: neither of these need to be popped; they were never successfully pushed.



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

[PATCH] D112773: Allow __attribute__((swift_attr)) in attribute push pragmas

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112773#3137338 , @aaron.ballman 
wrote:

> Thank you for diving into those details! I'm now sold on the idea that `any` 
> with no subjects is dangerous and we will diagnose if the user tries this. So 
> I think the only thing that needs to happen is a documentation update and 
> some additional test coverage for this scenario, which I'm happy to take care 
> of myself since it's orthogonal to these changes. Based on that, this LGTM 
> as-is.

FWIW, I updated the docs and added explicit tests in 
3874277f415dca0bb222956983f117a6211c0e39 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112773

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

[PATCH] D113902: [NFC][clangd] exclude test data from clang-tidy

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Details below, but TL;DR is running only on files in `compile_commands.json` is 
definitely the established way to analyze a project.
If you're proposing a *change* then that might be worth considering, but this 
probably isn't the place: mailing lists are more public and offline is 
higher-bandwidth :-)

In D113902#3137311 , @kuhnel wrote:

> In D113902#3131707 , @sammccall 
> wrote:
>
>> Hmm, clang-tidy should only be running on entries in compile_commands.json. 
>> are these files listed somehow?
>
> No, they are not.
>
> The question is: 
> Do the tools our contributors use care about this? This is not the default 
> behavior of clang-tidy, it's rather implemented in a LLVM-specific wrapper 
> script. So do we require our contributors to use that script to check their 
> code?



---

A few things to unpack here:

- contributor status quo: our contributors mostly don't currently run 
clang-tidy, and we don't require them to.
- clang tools in general: when running tools on a codebase, we use the CDB to 
define the scope of the sources as well as to ensure we're only running on 
sources with reliable flags.
- clang-tidy proper. Most clang tools support `--executor=all-TUs` which 
iterates over the CDB to run on a project. It looks like clang-tidy does not 
have this, only explicitly running on one file at a time.
- wrapper script: I assume you're talking about `run-clang-tidy.py`?  It 
provides the `--executor=all-TUs` behavior (and parallelism) as an external 
script. This is not LLVM-specific, it's part of the clang-tidy apt package etc 
along with the clang-tidy binary.

> I was playing with the VS Code extension 
>  to 
> see the findings right in the IDE and that calls clang-tidy directly. So it 
> ignores compile_commands.json

(In fact it doesn't quite, it'll use it to pick a best approximate set of flags 
via InterpolatingCompilationDatabase. clangd behaves the same way.)

But yes, there's a difference in behavior between what happens when you run a 
tool on a file (you get best effort, even if the file is unknown) vs when you 
run a tool on a project (you get only the files that are considered part of the 
project). This is pretty consistent: e.g. clangd will try to assist in editing 
any file, but will only index files in compile_commands.json.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113902

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

Why the need for this restriction ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114011: Add a clang-transformer tutorial

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 387914.
ymandel added a comment.

Responses to comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114011

Files:
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -64,6 +64,7 @@
RAVFrontendAction
LibASTMatchersTutorial
LibASTMatchers
+   ClangTransformerTutorial
LibASTImporter
HowToSetupToolingForLLVM
JSONCompilationDatabase
Index: clang/docs/ClangTransformerTutorial.rst
===
--- /dev/null
+++ clang/docs/ClangTransformerTutorial.rst
@@ -0,0 +1,400 @@
+==
+Clang Transformer Tutorial
+==
+
+A tutorial on how to write a source-to-source translation tool using Clang Transformer.
+
+.. contents::
+   :local:
+
+What is Clang Transformer?
+--
+
+Clang Transformer is a framework for writing C++ diagnostics and program
+transformations. It is built on the clang toolchain and the LibTooling library,
+but aims to hide much of the complexity of clang's native, low-level libraries.
+
+The core abstraction of Transformer is the *rewrite rule*, which specifies how
+to change a given program pattern into a new form. Here are some examples of
+tasks you can achieve with Transformer:
+
+*   warn against using the name ``MkX`` for a declared function,
+*   change ``MkX`` to ``MakeX``, where ``MkX`` is the name of a declared function,
+*   change ``s.size()`` to ``Size(s)``, where ``s`` is a ``string``,
+*   collapse ``e.child().m()`` to ``e.m()``, for any expression ``e`` and method named
+``m``.
+
+All of the examples have a common form: they identify a pattern that is the
+target of the transformation, they specify an *edit* to the code identified by
+the pattern, and their pattern and edit refer to common variables, like ``s``,
+``e``, and ``m``, that range over code fragments. Our first and second examples also
+specify constraints on the pattern that aren't apparent from the syntax alone,
+like "``s`` is a ``string``." Even the first example ("warn ...") shares this form,
+even though it doesn't change any of the code -- it's "edit" is simply a no-op.
+
+Transformer helps users succinctly specify rules of this sort and easily execute
+them locally over a collection of files, apply them to selected portions of
+a codebase, or even bundle them as a clang-tidy check for ongoing application.
+
+Who is Clang Transformer for?
+-
+
+Clang Transformer is for developers who want to write clang-tidy checks or write
+tools to modify a large number of C++ files in (roughly) the same way. What
+qualifies as "large" really depends on the nature of the change and your
+patience for repetitive editing. In our experience, automated solutions become
+worthwhile somewhere between 100 and 500 files.
+
+Getting Started
+---
+
+Patterns in Transformer are expressed with :doc:`clang's AST matchers `. 
+Matchers are a language of combinators for describing portions of a clang
+Abstract Syntax Tree (AST). Since clang's AST includes complete type information
+(within the limits of single `Translation Unit (TU)`_,
+these patterns can even encode rich constraints on the type properties of AST
+nodes.
+
+.. _`Translation Unit (TU)`: https://en.wikipedia.org/wiki/Translation_unit_\(programming\)
+
+We assume a familiarity with the clang AST and the corresponding AST matchers
+for the purpose of this tutorial. Users who are unfamiliar with either are
+encouraged to start with the recommended references in `Related Reading`_.
+
+Example: style-checking names
+^
+
+Assume you have a style-guide rule which forbids functions from being named
+"MkX" and you want to write a check that catches any violations of this rule. We
+can express this a Transformer rewrite rule:
+
+.. code-block:: c++
+		
+   makeRule(functionDecl(hasName("MkX").bind("fun"),
+	noopEdit(node("fun")),
+	cat("The name ``MkX`` is not allowed for functions; please rename"));
+
+``makeRule`` is our go-to function for generating rewrite rules. It takes three
+arguments: the pattern, the edit, and (optionally) an explanatory note. In our
+example, the pattern (``functionDecl(...)``) identifies the declaration of the
+function ``MkX``. Since we're just diagnosing the problem, but not suggesting a
+fix, our edit is an no-op. But, it contains an *anchor* for the diagnostic
+message: ``node("fun")`` says to associate the message with the source range of
+the AST node bound to "fun"; in this case, the ill-named function declaration.
+Finally, we use ``cat`` to build a message that explains the change. Regarding the
+name ``cat`` -- we'll discuss it in more detail below, 

[PATCH] D114011: Add a clang-transformer tutorial

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 7 inline comments as done.
ymandel added a comment.

Thanks for the review!




Comment at: clang/docs/ClangTransformerTutorial.rst:69
+
+Assume you have an API which forbids functions from being named "MkX" and you
+want to write a check that catches any violations of this rule. We can express

gribozavr2 wrote:
> "an api" -> "a style guide rule"?
yes. this example came from an API with its own style guide. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114011

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

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

[Adam Czachorowski via Phabricator via cfe-commits]

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

Looks good. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111971

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

pengfei wrote:
> zahiraam wrote:
> > zahiraam wrote:
> > > pengfei wrote:
> > > > rjmccall wrote:
> > > > > pengfei wrote:
> > > > > > rjmccall wrote:
> > > > > > > pengfei wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > Okay, this condition is pretty ridiculous to be repeating in 
> > > > > > > > > three different places across the compiler.  Especially since 
> > > > > > > > > you're going to change it when you implement the new option, 
> > > > > > > > > right?
> > > > > > > > > 
> > > > > > > > > Can we state this condition more generally?  I'm not sure why 
> > > > > > > > > this is so narrowly restricted, and the variable name isn't 
> > > > > > > > > telling me anything, since `_Float16` must by definition be 
> > > > > > > > > "allowed" if we have an expression of `_Float16` type.
> > > > > > > > > since _Float16 must by definition be "allowed" if we have an 
> > > > > > > > > expression of _Float16 type.
> > > > > > > > 
> > > > > > > > _Float16 is allowed only on a few targets. 
> > > > > > > > https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
> > > > > > > > By the way, we should update for X86 since it's not limited to 
> > > > > > > > avx512fp16 now.
> > > > > > > > _Float16 is allowed only on a few targets.
> > > > > > > 
> > > > > > > Yes, I know that.  But if `SrcType->isFloat16Type()` is true, we 
> > > > > > > must be on one of those targets, because the type doesn't 
> > > > > > > otherwise exist.
> > > > > > I see your point now. The problem here is we want to allow the 
> > > > > > `_Float16` to be used more broadly. But the target doesn't really 
> > > > > > support it sometime. Currently full arithmatic operations are 
> > > > > > supported only on target with AVX512FP16.
> > > > > > We should cast for those targets without AVX512FP16 while avoid to 
> > > > > > do on AVX512FP16.
> > > > > I agree that many targets don't natively support arithmetic on this 
> > > > > format, but x86 is not the first one that does.  Unless I'm 
> > > > > misunderstanding, we already track this property in Clang's 
> > > > > TargetInfo as `hasLegalHalfType()`.  `+avx512fp16` presumably ought 
> > > > > to set this.
> > > > > 
> > > > > I'm not sure what the interaction with the `NativeHalfType` LangOpt 
> > > > > is supposed to be here.  My understanding is that that option is just 
> > > > > supposed to affect `__fp16`, basically turning it into a proper 
> > > > > arithmetic type, i.e. essentially `_Float16`.  Whatever effect you 
> > > > > want to apply to `_Float16` should presumably happen even if that 
> > > > > option not set.
> > > > > 
> > > > > More broadly, I don't think your approach in this patch is correct.  
> > > > > The type of operations on `_Float16` should not change based on 
> > > > > whether the target natively supports `_Float16`.  If we need to 
> > > > > emulate those operations on targets that don't provide them natively, 
> > > > > we should do that at a lower level than the type system.
> > > > > 
> > > > > The most appropriate place to do that is going to depend on the exact 
> > > > > semantics we want.
> > > > > 
> > > > > If we want to preserve `half` semantics exactly regardless of target, 
> > > > > we should have Clang's IR generation actually emit `half` operations. 
> > > > >  Targets that don't support those operations natively will have to 
> > > > > lower at least some of those operations into compiler-rt calls, but 
> > > > > that's not at all unprecedented.
> > > > > 
> > > > > If we're okay with playing loose for performance reasons, we can 
> > > > > promote to `float` immediately around individual arithmetic 
> > > > > operations.  IR generation is probably the most appropriate place to 
> > > > > do that.  But I'm quite concerned about that making `_Float16` feel 
> > > > > like an unpredictable/unportable type; it seems to me that software 
> > > > > emulation is much better.
> > > > > 
> > > > > If you're proposing the latter, I think you need to raise that more 
> > > > > widely than a code review; please make a post on llvm-dev.
> > > > > we already track this property in Clang's TargetInfo as 
> > > > > `hasLegalHalfType()`
> > > > 
> > > > That sounds a good approch. Thank you.
> > > > 
> > > > > The type of operations on `_Float16` should not change based on 
> > > > > whether the target natively supports `_Float16`. If we need to 
> > > > > emulate those operations on targets that don't provide them natively, 
> > > > > we should do that at a lower level than the type system.
> > > > 
> > > > Unfortunately, we can't do it at low level. The reason is (I'm not 
> > > > expert in frontend, just recalled fr

[clang] 2b49484 - Add a clang-transformer tutorial

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-11-17T13:40:46Z
New Revision: 2b4948448f03104b4b957860dd8c019d0b9df2f0

URL: 
https://github.com/llvm/llvm-project/commit/2b4948448f03104b4b957860dd8c019d0b9df2f0
DIFF: 
https://github.com/llvm/llvm-project/commit/2b4948448f03104b4b957860dd8c019d0b9df2f0.diff

LOG: Add a clang-transformer tutorial

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

Added: 
clang/docs/ClangTransformerTutorial.rst

Modified: 
clang/docs/index.rst

Removed: 




diff  --git a/clang/docs/ClangTransformerTutorial.rst 
b/clang/docs/ClangTransformerTutorial.rst
new file mode 100644
index ..33931ad201a5
--- /dev/null
+++ b/clang/docs/ClangTransformerTutorial.rst
@@ -0,0 +1,400 @@
+==
+Clang Transformer Tutorial
+==
+
+A tutorial on how to write a source-to-source translation tool using Clang 
Transformer.
+
+.. contents::
+   :local:
+
+What is Clang Transformer?
+--
+
+Clang Transformer is a framework for writing C++ diagnostics and program
+transformations. It is built on the clang toolchain and the LibTooling library,
+but aims to hide much of the complexity of clang's native, low-level libraries.
+
+The core abstraction of Transformer is the *rewrite rule*, which specifies how
+to change a given program pattern into a new form. Here are some examples of
+tasks you can achieve with Transformer:
+
+*   warn against using the name ``MkX`` for a declared function,
+*   change ``MkX`` to ``MakeX``, where ``MkX`` is the name of a declared 
function,
+*   change ``s.size()`` to ``Size(s)``, where ``s`` is a ``string``,
+*   collapse ``e.child().m()`` to ``e.m()``, for any expression ``e`` and 
method named
+``m``.
+
+All of the examples have a common form: they identify a pattern that is the
+target of the transformation, they specify an *edit* to the code identified by
+the pattern, and their pattern and edit refer to common variables, like ``s``,
+``e``, and ``m``, that range over code fragments. Our first and second 
examples also
+specify constraints on the pattern that aren't apparent from the syntax alone,
+like "``s`` is a ``string``." Even the first example ("warn ...") shares this 
form,
+even though it doesn't change any of the code -- it's "edit" is simply a no-op.
+
+Transformer helps users succinctly specify rules of this sort and easily 
execute
+them locally over a collection of files, apply them to selected portions of
+a codebase, or even bundle them as a clang-tidy check for ongoing application.
+
+Who is Clang Transformer for?
+-
+
+Clang Transformer is for developers who want to write clang-tidy checks or 
write
+tools to modify a large number of C++ files in (roughly) the same way. What
+qualifies as "large" really depends on the nature of the change and your
+patience for repetitive editing. In our experience, automated solutions become
+worthwhile somewhere between 100 and 500 files.
+
+Getting Started
+---
+
+Patterns in Transformer are expressed with :doc:`clang's AST matchers 
`. 
+Matchers are a language of combinators for describing portions of a clang
+Abstract Syntax Tree (AST). Since clang's AST includes complete type 
information
+(within the limits of single `Translation Unit (TU)`_,
+these patterns can even encode rich constraints on the type properties of AST
+nodes.
+
+.. _`Translation Unit (TU)`: 
https://en.wikipedia.org/wiki/Translation_unit_\(programming\)
+
+We assume a familiarity with the clang AST and the corresponding AST matchers
+for the purpose of this tutorial. Users who are unfamiliar with either are
+encouraged to start with the recommended references in `Related Reading`_.
+
+Example: style-checking names
+^
+
+Assume you have a style-guide rule which forbids functions from being named
+"MkX" and you want to write a check that catches any violations of this rule. 
We
+can express this a Transformer rewrite rule:
+
+.. code-block:: c++
+   
+   makeRule(functionDecl(hasName("MkX").bind("fun"),
+   noopEdit(node("fun")),
+   cat("The name ``MkX`` is not allowed for functions; please 
rename"));
+
+``makeRule`` is our go-to function for generating rewrite rules. It takes three
+arguments: the pattern, the edit, and (optionally) an explanatory note. In our
+example, the pattern (``functionDecl(...)``) identifies the declaration of the
+function ``MkX``. Since we're just diagnosing the problem, but not suggesting a
+fix, our edit is an no-op. But, it contains an *anchor* for the diagnostic
+message: ``node("fun")`` says to associate the message with the source range of
+the AST node bound to "fun"; in this case, the ill-named function declaration.
+Finally, we use ``cat`` to build a message that explains the change. Regarding 
the
+name ``cat`` -- we'll discuss it in more 

[PATCH] D114011: Add a clang-transformer tutorial

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ymandel marked an inline comment as done.
Closed by commit rG2b4948448f03: Add a clang-transformer tutorial (authored by 
ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114011

Files:
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -64,6 +64,7 @@
RAVFrontendAction
LibASTMatchersTutorial
LibASTMatchers
+   ClangTransformerTutorial
LibASTImporter
HowToSetupToolingForLLVM
JSONCompilationDatabase
Index: clang/docs/ClangTransformerTutorial.rst
===
--- /dev/null
+++ clang/docs/ClangTransformerTutorial.rst
@@ -0,0 +1,400 @@
+==
+Clang Transformer Tutorial
+==
+
+A tutorial on how to write a source-to-source translation tool using Clang Transformer.
+
+.. contents::
+   :local:
+
+What is Clang Transformer?
+--
+
+Clang Transformer is a framework for writing C++ diagnostics and program
+transformations. It is built on the clang toolchain and the LibTooling library,
+but aims to hide much of the complexity of clang's native, low-level libraries.
+
+The core abstraction of Transformer is the *rewrite rule*, which specifies how
+to change a given program pattern into a new form. Here are some examples of
+tasks you can achieve with Transformer:
+
+*   warn against using the name ``MkX`` for a declared function,
+*   change ``MkX`` to ``MakeX``, where ``MkX`` is the name of a declared function,
+*   change ``s.size()`` to ``Size(s)``, where ``s`` is a ``string``,
+*   collapse ``e.child().m()`` to ``e.m()``, for any expression ``e`` and method named
+``m``.
+
+All of the examples have a common form: they identify a pattern that is the
+target of the transformation, they specify an *edit* to the code identified by
+the pattern, and their pattern and edit refer to common variables, like ``s``,
+``e``, and ``m``, that range over code fragments. Our first and second examples also
+specify constraints on the pattern that aren't apparent from the syntax alone,
+like "``s`` is a ``string``." Even the first example ("warn ...") shares this form,
+even though it doesn't change any of the code -- it's "edit" is simply a no-op.
+
+Transformer helps users succinctly specify rules of this sort and easily execute
+them locally over a collection of files, apply them to selected portions of
+a codebase, or even bundle them as a clang-tidy check for ongoing application.
+
+Who is Clang Transformer for?
+-
+
+Clang Transformer is for developers who want to write clang-tidy checks or write
+tools to modify a large number of C++ files in (roughly) the same way. What
+qualifies as "large" really depends on the nature of the change and your
+patience for repetitive editing. In our experience, automated solutions become
+worthwhile somewhere between 100 and 500 files.
+
+Getting Started
+---
+
+Patterns in Transformer are expressed with :doc:`clang's AST matchers `. 
+Matchers are a language of combinators for describing portions of a clang
+Abstract Syntax Tree (AST). Since clang's AST includes complete type information
+(within the limits of single `Translation Unit (TU)`_,
+these patterns can even encode rich constraints on the type properties of AST
+nodes.
+
+.. _`Translation Unit (TU)`: https://en.wikipedia.org/wiki/Translation_unit_\(programming\)
+
+We assume a familiarity with the clang AST and the corresponding AST matchers
+for the purpose of this tutorial. Users who are unfamiliar with either are
+encouraged to start with the recommended references in `Related Reading`_.
+
+Example: style-checking names
+^
+
+Assume you have a style-guide rule which forbids functions from being named
+"MkX" and you want to write a check that catches any violations of this rule. We
+can express this a Transformer rewrite rule:
+
+.. code-block:: c++
+		
+   makeRule(functionDecl(hasName("MkX").bind("fun"),
+	noopEdit(node("fun")),
+	cat("The name ``MkX`` is not allowed for functions; please rename"));
+
+``makeRule`` is our go-to function for generating rewrite rules. It takes three
+arguments: the pattern, the edit, and (optionally) an explanatory note. In our
+example, the pattern (``functionDecl(...)``) identifies the declaration of the
+function ``MkX``. Since we're just diagnosing the problem, but not suggesting a
+fix, our edit is an no-op. But, it contains an *anchor* for the diagnostic
+message: ``node("fun")`` says to associate the message with the source range of
+the AST node bound to "fun"; in this case, the ill-n

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

right, TIL `sizeof (ContainsArr)` is 0 according to clang. I can see trouble 
arising.

@aaron.ballman maybe you know better, is that intended for the extension ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel updated this revision to Diff 387918.
kuhnel marked 4 inline comments as done.
kuhnel added a comment.

addressed Sam's comments

Sorry for the continued mess with automatic code formatting. Somehow VSCode 
forgot my settings :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113892

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FuzzyMatch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/JSONTransport.cpp
  clang-tools-extra/clangd/PathMapping.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -223,8 +223,7 @@
   while (const auto *CS = llvm::dyn_cast(Last)) {
 if (CS->body_empty())
   return false;
-else
-  Last = CS->body_back();
+Last = CS->body_back();
   }
   return llvm::isa(Last);
 }
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -250,7 +250,8 @@
   for (; Node->Parent; Node = Node->Parent) {
 if (Node->ASTNode.get()) {
   continue;
-} else if (auto *T = Node->ASTNode.get()) {
+}
+if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
   } else if (Node->Parent->ASTNode.get() ||
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -687,7 +687,8 @@
 llvm::Expected readIndexFile(llvm::StringRef Data) {
   if (Data.startswith("RIFF")) {
 return readRIFF(Data);
-  } else if (auto YAMLContents = readYAML(Data)) {
+  }
+  if (auto YAMLContents = readYAML(Data)) {
 return std::move(*YAMLContents);
   } else {
 return error("Not a RIFF file and failed to parse as YAML: {0}",
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -826,10 +826,9 @@
 log("Found definition heuristically using nearby identifier {0}",
 NearbyIdent->text(SM));
 return ASTResults;
-  } else {
-vlog("No definition found using nearby identifier {0} at {1}",
- Word->Text, Word->Location.printToString(SM));
   }
+  vlog("No definition found using nearby identifier {0} at {1}", Word->Text,
+   Word->Location.printToString(SM));
 }
 // No nearby word, or it didn't refer to anything either. Try the index.
 auto TextualResults =
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1282,8 +1282,8 @@
 if (Done) {
   if (Requests.empty())
 return;
-  else // Even though Done is set, finish pending requests.
-break; // However, skip delays to shutdown fast.
+  // Even though Done is set, finish pending requests.
+  break; // However, skip delays to shutdown fast.
 }
 
 // Tracing: we have a next request, attribute this sleep to it.
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -346,7 +346,7 @@
 SM.getTopMacroCallerLoc(Batch.back().location());
 return testTokenRange(SM.getFileOffset(ArgStart),
   SM.getFileOffset(ArgEnd));
-  } else {
+  } else { // NOLINT(llvm-else-after-return)
 /* fall through and treat as part of the macro body */
   }
 }
@@ -357,8 +357,7 @@
 if (Expansion.first == SelFile)
   // FIXME: also check ( and ) for function-like macros?
   return testToken(Expansion.second);
-else
-  return NoTokens;
+return NoTokens;
   }
 
   // Is the closed token range [Begin, End] selected?
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -695,7 +695,8 @@
  

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114080#3137462 , @Naghasan wrote:

> right, TIL `sizeof (ContainsArr)` is 0 according to clang. I can see trouble 
> arising.
>
> @aaron.ballman maybe you know better, is that intended for the extension ?

I don't *know*, but I have two educated guesses that it is intended. GCC 
compatibility (GCC also has this behavior) and the relationship between 
zero-sized array members and flexible array members. Clang allows a flexible 
array member to be used in a structure with no members and it behaves the same 
as a zero-sized array in a structure with no members: 
https://godbolt.org/z/Gx7ozxYKz


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D114080#3137430 , @Naghasan wrote:

> Why the need for this restriction ?

Zero length arrays are not supported by pure C++. It is an extension supported 
by clang. They are also not allowed in OpenCL and SPIR-V, and I even though I 
cannot grep an explicit restriction for them in SYCL 2020 spec (I guess that is 
just because it is an extension and not pure C++) It doesn't make sense to 
allow them here since memory allocation in device code is prohibited in SYCL by 
the spec. This diagnostic prevents further not always user friendly errors 
coming from SPIR-V emitters or from backends like OpenCL that explicitly 
disallow them on early stage and in user-friendly form. I believe other GPU 
programming models also may not allow them for the same reason - restriction on 
memory allocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D113899: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:504
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.

kadircet wrote:
> while here `llvm::isa_and_nonnull` we try to qualify symbols from llvm 
> namespace.
Sorry, I'm not getting what you wanted to say. 

You want me to add a `llvm::`?
Or is it about a `operator bool()`(however I didn't find one for `Decl` )?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113899

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

[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113892

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

In D114080#3137480 , @aaron.ballman 
wrote:

> In D114080#3137462 , @Naghasan 
> wrote:
>
>> right, TIL `sizeof (ContainsArr)` is 0 according to clang. I can see trouble 
>> arising.
>>
>> @aaron.ballman maybe you know better, is that intended for the extension ?
>
> I don't *know*, but I have two educated guesses that it is intended. GCC 
> compatibility (GCC also has this behavior) and the relationship between 
> zero-sized array members and flexible array members. Clang allows a flexible 
> array member to be used in a structure with no members and it behaves the 
> same as a zero-sized array in a structure with no members: 
> https://godbolt.org/z/Gx7ozxYKz

I see. I was confused as it normally C++ requires at least 1 byte. But I guess 
you need that for compatibility with C.

In D114080#3137482 , @Fznamznon wrote:

> In D114080#3137430 , @Naghasan 
> wrote:
>
>> Why the need for this restriction ?
>
> Zero length arrays are not supported by pure C++. It is an extension 
> supported by clang. They are also not allowed in OpenCL and SPIR-V, and I 
> even though I cannot grep an explicit restriction for them in SYCL 2020 spec 
> (I guess that is just because it is an extension and not pure C++) It doesn't 
> make sense to allow them here since memory allocation in device code is 
> prohibited in SYCL by the spec. This diagnostic prevents further not always 
> user friendly errors coming from SPIR-V emitters or from backends like OpenCL 
> that explicitly disallow them on early stage and in user-friendly form. I 
> believe other GPU programming models also may not allow them for the same 
> reason - restriction on memory allocation.

SYCL won't mention this as it is an extension indeed. Same for VLA. SPIR-V is 
another question though, while 0 element array are not allowed, empty structs 
are (but IIRC size of an empty struct is not clear).

> It doesn't make sense to allow them here since memory allocation in device 
> code is prohibited in SYCL by the spec

I agree, but having a field defined is different from it being used. It is 
actually not uncommon to see SYCL functor passed with many unused fields (Eigen 
is good example).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:82-88
+if (Feature == "sve2")
+  Features.push_back("+sve");
+else if (Feature == "sve2-bitperm" || Feature == "sve2-sha3" ||
+ Feature == "sve2-aes" || Feature == "sve2-sm4") {
+  Features.push_back("+sve");
+  Features.push_back("+sve2");
+} else if (Feature == "nosve") {

^^^ Are the above changes necessary? i.e. if only `+sve2-sha3` is set as target 
feature, I thought LLVM automatically infers that it requires `+sve` and 
`+sve2`. Is that not the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

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

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 387926.
avogelsgesang marked an inline comment as not done.
avogelsgesang added a comment.

Fix test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,192 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+  void *find(const Key &K);
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map &MyMap) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map &MyMap) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+  auto C5 = MyMap.find(-5) == MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+  return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(std::map &M, std::unordered_set &US, std::set &S, std::multimap &MM) {
+  bool C1 = M.count(1001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = M.contains(1001);
+  bool C2 = US.count(1002);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'contains' to check for membership [readability-container-contains]
+  

[clang-tools-extra] 4ea066a - [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-11-17T15:31:38+01:00
New Revision: 4ea066acc928d772c2a610024b7f0656b36e6afd

URL: 
https://github.com/llvm/llvm-project/commit/4ea066acc928d772c2a610024b7f0656b36e6afd
DIFF: 
https://github.com/llvm/llvm-project/commit/4ea066acc928d772c2a610024b7f0656b36e6afd.diff

LOG: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

The overload shouldSuppressDiagnostic seems unnecessary, and it is only
used in clangd.

This patch removes it and use the real one (suppression diagnostics are
discarded in clangd at the moment).

Fixes https://github.com/clangd/clangd/issues/929

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index dd590f7c79fb0..5b410ba9d2ea4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -517,16 +517,6 @@ static bool lineIsMarkedWithNOLINTinMacro(
 namespace clang {
 namespace tidy {
 
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-  const Diagnostic &Info, ClangTidyContext 
&Context,
-  bool AllowIO) {
-  SmallVector Unused;
-  bool ShouldSuppress =
-  shouldSuppressDiagnostic(DiagLevel, Info, Context, Unused, AllowIO);
-  assert(Unused.empty());
-  return ShouldSuppress;
-}
-
 bool shouldSuppressDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
 ClangTidyContext &Context,

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 234455c5bea21..9f519fb605ad6 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -223,10 +223,6 @@ class ClangTidyContext {
 /// for example, the use of a "NOLINTBEGIN" comment that is not followed by a
 /// "NOLINTEND" comment - a diagnostic regarding the improper use is returned
 /// via the output argument `SuppressionErrors`.
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-  const Diagnostic &Info, ClangTidyContext 
&Context,
-  bool AllowIO = true);
-
 bool shouldSuppressDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
 ClangTidyContext &Context,

diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 8fce756b83c1d..719c374d8b859 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -391,9 +391,13 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs &Inputs,
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
+  SmallVector TidySuppressedErrors;
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ TidySuppressedErrors,
  /*AllowIO=*/false)) {
+// FIXME: should we expose the suppression error (invalid use of
+// NOLINT comments)?
 return DiagnosticsEngine::Ignored;
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 277b9b181d9b6..89a3cc060f925 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -476,6 +476,13 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
   double g = [[8]] / i;
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
+  // NOLINTBEGIN
+  double x = BAD2;
+  double y = BAD2;
+  // NOLINTEND
+
+  // verify no crashes on unmatched nolints.
+  // NOLINTBEIGN
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());



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

[PATCH] D113999: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ea066acc928: [clangd] Fix assertion crashes on unmatched 
NOLINTBEGIN comments. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113999

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -476,6 +476,13 @@
   double g = [[8]] / i;
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
+  // NOLINTBEGIN
+  double x = BAD2;
+  double y = BAD2;
+  // NOLINTEND
+
+  // verify no crashes on unmatched nolints.
+  // NOLINTBEIGN
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -391,9 +391,13 @@
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
+  SmallVector TidySuppressedErrors;
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ TidySuppressedErrors,
  /*AllowIO=*/false)) {
+// FIXME: should we expose the suppression error (invalid use of
+// NOLINT comments)?
 return DiagnosticsEngine::Ignored;
   }
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -223,10 +223,6 @@
 /// for example, the use of a "NOLINTBEGIN" comment that is not followed by a
 /// "NOLINTEND" comment - a diagnostic regarding the improper use is returned
 /// via the output argument `SuppressionErrors`.
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-  const Diagnostic &Info, ClangTidyContext 
&Context,
-  bool AllowIO = true);
-
 bool shouldSuppressDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
 ClangTidyContext &Context,
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -517,16 +517,6 @@
 namespace clang {
 namespace tidy {
 
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-  const Diagnostic &Info, ClangTidyContext 
&Context,
-  bool AllowIO) {
-  SmallVector Unused;
-  bool ShouldSuppress =
-  shouldSuppressDiagnostic(DiagLevel, Info, Context, Unused, AllowIO);
-  assert(Unused.empty());
-  return ShouldSuppress;
-}
-
 bool shouldSuppressDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
 ClangTidyContext &Context,


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -476,6 +476,13 @@
   double g = [[8]] / i;
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
+  // NOLINTBEGIN
+  double x = BAD2;
+  double y = BAD2;
+  // NOLINTEND
+
+  // verify no crashes on unmatched nolints.
+  // NOLINTBEIGN
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -391,9 +391,13 @@
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
+  SmallVector TidySuppressedErrors;
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ TidySuppressedErrors,
  /*AllowIO=*/false)) {
+// FIXME: should we expose the suppression error (invalid use of
+   

[PATCH] D113896: [NFC][clangd] cleanup of header guard names

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel updated this revision to Diff 387929.
kuhnel marked 3 inline comments as done.
kuhnel added a comment.

addresed Sam's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/ExpectedTypes.h
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/HeaderSourceSwitch.h
  clang-tools-extra/clangd/HeuristicResolver.h
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/InlayHints.h
  clang-tools-extra/clangd/PathMapping.h
  clang-tools-extra/clangd/URI.h
  clang-tools-extra/clangd/index/BackgroundIndexLoader.h
  clang-tools-extra/clangd/index/BackgroundRebuild.h
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/IndexAction.h
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/SymbolLocation.h
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/clangd/index/remote/Client.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/Annotations.h
  clang-tools-extra/clangd/unittests/LSPClient.h
  clang-tools-extra/clangd/unittests/Matchers.h
  clang-tools-extra/clangd/unittests/SyncAPI.h
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestIndex.h
  clang-tools-extra/clangd/unittests/TestScheme.h
  clang-tools-extra/clangd/unittests/TestTU.h
  clang-tools-extra/clangd/unittests/TestWorkspace.h
  clang-tools-extra/clangd/unittests/support/TestTracer.h
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 
 #include "TestTU.h"
 #include "index/Index.h"
Index: clang-tools-extra/clangd/unittests/support/TestTracer.h
===
--- clang-tools-extra/clangd/unittests/support/TestTracer.h
+++ clang-tools-extra/clangd/unittests/support/TestTracer.h
@@ -10,8 +10,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
 
 #include "support/Trace.h"
 #include "llvm/ADT/StringMap.h"
Index: clang-tools-extra/clangd/unittests/TestWorkspace.h
===
--- clang-tools-extra/clangd/unittests/TestWorkspace.h
+++ clang-tools-extra/clangd/unittests/TestWorkspace.h
@@ -13,8 +13,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
 
 #include "TestFS.h"
 #include "TestTU.h"
@@ -56,4 +56,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -14,8 +14,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
@@ -104,4 +104,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
Index: clang-tools-extra/clangd/unittests/TestIndex.h
===

[PATCH] D113896: [clangd] cleanup of header guard names

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added inline comments.



Comment at: clang-tools-extra/clangd/PathMapping.h:3
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHMAPPING_H
+
 //===--- PathMapping.h - apply path mappings to LSP messages -===//

Looks like the include guards were missing here. I suppose this is something we 
want to add.



Comment at: 
clang-tools-extra/clangd/test/Inputs/background-index/sub_dir/foo.h:1
-#ifndef FOO_H
-#define FOO_H
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H

sammccall wrote:
> Please don't modify tests, they don't need to follow LLVM style and should be 
> as simple as possible
I was iterating over all .cpp and .h files, so the test data ended up in the 
changes as well.

I suppose the only way to avoid this is to use the `run-clang-tidy.py` wraper 
script instead



Comment at: clang-tools-extra/clangd/unittests/LSPClient.h:2
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+

Same here. I suppose we want header guards in this one.



Comment at: clang-tools-extra/clangd/unittests/TestScheme.h:1
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H

sammccall wrote:
> What's up with this change?
> If this file is empty it should just be deleted instead
Deleting the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[clang-tools-extra] ec4a2c9 - [NFC][clangd] cleanup llvm-else-after-return findings

[Christian Kühnel via cfe-commits]

Author: Christian Kühnel
Date: 2021-11-17T14:37:03Z
New Revision: ec4a2c956591e97904eae5102022f343dadfb8f1

URL: 
https://github.com/llvm/llvm-project/commit/ec4a2c956591e97904eae5102022f343dadfb8f1
DIFF: 
https://github.com/llvm/llvm-project/commit/ec4a2c956591e97904eae5102022f343dadfb8f1.diff

LOG: [NFC][clangd] cleanup llvm-else-after-return findings

Cleanup of clang-tidy findings: removing "else" after a return statement
to improve readability of the code.

This patch was created by applying the clang-tidy fixes automatically.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/FuzzyMatch.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/JSONTransport.cpp
clang-tools-extra/clangd/PathMapping.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index c01a4286884b8..6c000d3290fda 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -929,8 +929,7 @@ void ClangdLSPServer::onDocumentSymbol(const 
DocumentSymbolParams &Params,
 adjustSymbolKinds(*Items, SupportedSymbolKinds);
 if (SupportsHierarchicalDocumentSymbol)
   return Reply(std::move(*Items));
-else
-  return Reply(flattenSymbolHierarchy(*Items, FileURI));
+return Reply(flattenSymbolHierarchy(*Items, FileURI));
   });
 }
 

diff  --git a/clang-tools-extra/clangd/FuzzyMatch.cpp 
b/clang-tools-extra/clangd/FuzzyMatch.cpp
index 57f9554a7d0cf..2a908fd684da3 100644
--- a/clang-tools-extra/clangd/FuzzyMatch.cpp
+++ b/clang-tools-extra/clangd/FuzzyMatch.cpp
@@ -320,8 +320,9 @@ llvm::SmallString<256> 
FuzzyMatcher::dumpLast(llvm::raw_ostream &OS) const {
   if (!WordContainsPattern) {
 OS << "Substring check failed.\n";
 return Result;
-  } else if (isAwful(std::max(Scores[PatN][WordN][Match].Score,
-  Scores[PatN][WordN][Miss].Score))) {
+  }
+  if (isAwful(std::max(Scores[PatN][WordN][Match].Score,
+   Scores[PatN][WordN][Miss].Score))) {
 OS << "Substring check passed, but all matches are forbidden\n";
   }
   if (!(PatTypeSet & 1 << Upper))

diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 4285b3595059f..faea812712ab2 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -74,7 +74,7 @@ std::string getLocalScope(const Decl *D) {
   // - Classes, categories, and protocols: "MyClass(Category)"
   if (const ObjCMethodDecl *MD = dyn_cast(DC))
 return printObjCMethod(*MD);
-  else if (const ObjCContainerDecl *CD = dyn_cast(DC))
+  if (const ObjCContainerDecl *CD = dyn_cast(DC))
 return printObjCContainer(*CD);
 
   auto GetName = [](const TypeDecl *D) {

diff  --git a/clang-tools-extra/clangd/JSONTransport.cpp 
b/clang-tools-extra/clangd/JSONTransport.cpp
index fd405c192591b..ecf6c67d8ae4a 100644
--- a/clang-tools-extra/clangd/JSONTransport.cpp
+++ b/clang-tools-extra/clangd/JSONTransport.cpp
@@ -194,8 +194,7 @@ bool JSONTransport::handleMessage(llvm::json::Value Message,
 
   if (ID)
 return Handler.onCall(*Method, std::move(Params), std::move(*ID));
-  else
-return Handler.onNotify(*Method, std::move(Params));
+  return Handler.onNotify(*Method, std::move(Params));
 }
 
 // Tries to read a line up to and including \n.
@@ -254,14 +253,14 @@ bool JSONTransport::readStandardMessage(std::string 
&JSON) {
   }
   llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
   continue;
-} else if (!LineRef.trim().empty()) {
-  // It's another header, ignore it.
-  continue;
-} else {
-  // An empty line indicates the end of headers.
-  // Go ahead and read the JSON.
-  break;
 }
+
+// An empty line indicates the end of headers.
+// Go ahead and read the JSON.
+if (LineRef.trim().empty())
+  break;
+
+// It's another header, ignore it.
   }
 
   // The fuzzer likes crashing us by sending "Content-Length: "

diff  --git a/clang-tools-extra/clangd/PathMapping.cpp 
b/clang-tools-extra/clangd/PathMapping.cpp
index cc98025393841..882e3fba50810 100644
--- a/clang-tools-extra/clangd/PathMapping.cpp
+++ b/clang-tools-extra/clangd/PathMapping.cpp
@@ -151,7 +151,8 @@ llvm::Expected parsePath(llvm::StringRef Path) 
{
   namespace path = llvm::sys::path;
   if (path::is_absolute(Path, path::Style::posix)) 

[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Christian Kühnel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGec4a2c956591: [NFC][clangd] cleanup llvm-else-after-return 
findings (authored by kuhnel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113892

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FuzzyMatch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/JSONTransport.cpp
  clang-tools-extra/clangd/PathMapping.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -223,8 +223,7 @@
   while (const auto *CS = llvm::dyn_cast(Last)) {
 if (CS->body_empty())
   return false;
-else
-  Last = CS->body_back();
+Last = CS->body_back();
   }
   return llvm::isa(Last);
 }
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -250,7 +250,8 @@
   for (; Node->Parent; Node = Node->Parent) {
 if (Node->ASTNode.get()) {
   continue;
-} else if (auto *T = Node->ASTNode.get()) {
+}
+if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
   } else if (Node->Parent->ASTNode.get() ||
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -687,7 +687,8 @@
 llvm::Expected readIndexFile(llvm::StringRef Data) {
   if (Data.startswith("RIFF")) {
 return readRIFF(Data);
-  } else if (auto YAMLContents = readYAML(Data)) {
+  }
+  if (auto YAMLContents = readYAML(Data)) {
 return std::move(*YAMLContents);
   } else {
 return error("Not a RIFF file and failed to parse as YAML: {0}",
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -826,10 +826,9 @@
 log("Found definition heuristically using nearby identifier {0}",
 NearbyIdent->text(SM));
 return ASTResults;
-  } else {
-vlog("No definition found using nearby identifier {0} at {1}",
- Word->Text, Word->Location.printToString(SM));
   }
+  vlog("No definition found using nearby identifier {0} at {1}", Word->Text,
+   Word->Location.printToString(SM));
 }
 // No nearby word, or it didn't refer to anything either. Try the index.
 auto TextualResults =
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1282,8 +1282,8 @@
 if (Done) {
   if (Requests.empty())
 return;
-  else // Even though Done is set, finish pending requests.
-break; // However, skip delays to shutdown fast.
+  // Even though Done is set, finish pending requests.
+  break; // However, skip delays to shutdown fast.
 }
 
 // Tracing: we have a next request, attribute this sleep to it.
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -346,7 +346,7 @@
 SM.getTopMacroCallerLoc(Batch.back().location());
 return testTokenRange(SM.getFileOffset(ArgStart),
   SM.getFileOffset(ArgEnd));
-  } else {
+  } else { // NOLINT(llvm-else-after-return)
 /* fall through and treat as part of the macro body */
   }
 }
@@ -357,8 +357,7 @@
 if (Expansion.first == SelFile)
   // FIXME: also check ( and ) for function-like macros?
   return testToken(Expansion.second);
-else
-  return NoTokens;
+return NoTokens;
   }
 
   // Is the closed token range [Begin, End] selected?
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -695,7 +695,8 @@
   if (ArgsArray->size() > 1) {
 P.field("arguments

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> I agree, but having a field defined is different from it being used. It is 
> actually not uncommon to see SYCL functor passed with many unused fields 
> (Eigen is good example).

Are you saying that it is better to diagnose just used fields?
I believe it is safer to diagnose everything, because some backends or SPIR-V 
emitters may not expect that and crash or throw non-user friendly errors, 
because usually it is up to front-end to emit errors when some unsupported 
feature appears in device code (even unused).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D113902: [NFC][clangd] exclude test data from clang-tidy

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel abandoned this revision.
kuhnel added a comment.

thx for the explanation, makes sense
--> abandoning this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113902

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

[clang] 8924ba3 - [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

[Zarko Todorovski via cfe-commits]

Author: Zarko Todorovski
Date: 2021-11-17T09:43:02-05:00
New Revision: 8924ba3bf8c6b0e8d14dff455e4e449a426a2700

URL: 
https://github.com/llvm/llvm-project/commit/8924ba3bf8c6b0e8d14dff455e4e449a426a2700
DIFF: 
https://github.com/llvm/llvm-project/commit/8924ba3bf8c6b0e8d14dff455e4e449a426a2700.diff

LOG: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

Replace filenames, variable names, check prefixes uses of blacklist with ignore 
list.

Reviewed By: jkorous

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

Added: 
clang/test/CodeGen/Inputs/sanitizer-ignorelist-vfsoverlay.yaml
clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
clang/test/CodeGen/catch-nullptr-and-nonzero-offset-ignorelist.c
clang/test/CodeGen/ubsan-ignorelist.c
clang/test/CodeGenCXX/cfi-ignorelist.cpp

Modified: 
clang/test/CodeGen/address-safety-attr.cpp
clang/test/CodeGen/asan-globals.cpp
clang/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
clang/test/CodeGen/catch-implicit-integer-truncations-basics-negatives.c
clang/test/CodeGen/catch-implicit-integer-truncations.c

clang/test/CodeGen/catch-implicit-signed-integer-truncations-basics-negatives.c

clang/test/CodeGen/catch-implicit-unsigned-integer-truncations-basics-negatives.c
clang/test/CodeGen/cfi-check-fail2.c
clang/test/CodeGen/sanitize-thread-attr.cpp
clang/test/CodeGen/ubsan-ignorelist-vfs.c
clang/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp

Removed: 
clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
clang/test/CodeGen/catch-alignment-assumption-blacklist.c
clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
clang/test/CodeGen/ubsan-blacklist.c
clang/test/CodeGenCXX/cfi-blacklist.cpp



diff  --git a/clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml 
b/clang/test/CodeGen/Inputs/sanitizer-ignorelist-vfsoverlay.yaml
similarity index 63%
rename from clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
rename to clang/test/CodeGen/Inputs/sanitizer-ignorelist-vfsoverlay.yaml
index df2b221897693..7fb4069034351 100644
--- a/clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
+++ b/clang/test/CodeGen/Inputs/sanitizer-ignorelist-vfsoverlay.yaml
@@ -3,10 +3,10 @@
   'roots': [
 { 'name': '@DIR@', 'type': 'directory',
   'contents': [
-{ 'name': 'only-virtual-file.blacklist', 'type': 'file',
+{ 'name': 'only-virtual-file.ignorelist', 'type': 'file',
   'external-contents': '@REAL_FILE@'
 },
-{ 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
+{ 'name': 'invalid-virtual-file.ignorelist', 'type': 'file',
   'external-contents': '@NONEXISTENT_FILE@'
 }
   ]

diff  --git a/clang/test/CodeGen/address-safety-attr.cpp 
b/clang/test/CodeGen/address-safety-attr.cpp
index 742263ae353dc..bd899ed2f9a74 100644
--- a/clang/test/CodeGen/address-safety-attr.cpp
+++ b/clang/test/CodeGen/address-safety-attr.cpp
@@ -6,13 +6,13 @@ int DefinedInDifferentFile(int *a);
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp | FileCheck 
-check-prefix=WITHOUT %s
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address | FileCheck 
-check-prefix=ASAN %s
 
-// RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.func.blacklist | FileCheck -check-prefix=BLFUNC %s
+// RUN: echo "fun:*IgnorelistedFunction*" > %t.func.ignorelist
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-ignorelist=%t.func.ignorelist | FileCheck -check-prefix=BLFUNC %s
 
-// The blacklist file uses regexps, so escape backslashes, which are common in
+// The ignorelist file uses regexps, so escape backslashes, which are common in
 // Windows paths.
-// RUN: echo "src:%s" | sed -e 's/\\//g' > %t.file.blacklist
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.file.blacklist | FileCheck -check-prefix=BLFILE %s
+// RUN: echo "src:%s" | sed -e 's/\\//g' > %t.file.ignorelist
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-ignorelist=%t.file.ignorelist | FileCheck -check-prefix=BLFILE %s
 
 // The sanitize_address attribute should be attached to functions
 // when AddressSanitizer is enabled, u

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

[Zarko Todorovski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8924ba3bf8c6: [NFC][clang] Inclusive terms: replace uses of 
blacklist in clang/test/ (authored by ZarkoCA).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113211

Files:
  clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
  clang/test/CodeGen/Inputs/sanitizer-ignorelist-vfsoverlay.yaml
  clang/test/CodeGen/address-safety-attr.cpp
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/catch-alignment-assumption-blacklist.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  clang/test/CodeGen/catch-implicit-integer-truncations-basics-negatives.c
  clang/test/CodeGen/catch-implicit-integer-truncations.c
  
clang/test/CodeGen/catch-implicit-signed-integer-truncations-basics-negatives.c
  
clang/test/CodeGen/catch-implicit-unsigned-integer-truncations-basics-negatives.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-ignorelist.c
  clang/test/CodeGen/cfi-check-fail2.c
  clang/test/CodeGen/sanitize-thread-attr.cpp
  clang/test/CodeGen/ubsan-blacklist.c
  clang/test/CodeGen/ubsan-ignorelist-vfs.c
  clang/test/CodeGen/ubsan-ignorelist.c
  clang/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
  clang/test/CodeGenCXX/cfi-blacklist.cpp
  clang/test/CodeGenCXX/cfi-ignorelist.cpp

Index: clang/test/CodeGenCXX/cfi-ignorelist.cpp
===
--- clang/test/CodeGenCXX/cfi-ignorelist.cpp
+++ clang/test/CodeGenCXX/cfi-ignorelist.cpp
@@ -1,18 +1,18 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s
 
-// Check that blacklisting cfi and cfi-vcall work correctly
+// Check that ignore listing cfi and cfi-vcall work correctly
 // RUN: echo "[cfi-vcall]" > %t.vcall.txt
 // RUN: echo "type:std::*" >> %t.vcall.txt
-// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.vcall.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-ignorelist=%t.vcall.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
 //
 // RUN: echo "[cfi]" > %t.cfi.txt
 // RUN: echo "type:std::*" >> %t.cfi.txt
-// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.cfi.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-ignorelist=%t.cfi.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
 
-// Check that blacklisting non-vcall modes does not affect vcalls
+// Check that ignore listing non-vcall modes does not affect vcalls
 // RUN: echo "[cfi-icall|cfi-nvcall|cfi-cast-strict|cfi-derived-cast|cfi-unrelated-cast]" > %t.other.txt
 // RUN: echo "type:std::*" >> %t.other.txt
-// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.other.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-ignorelist=%t.other.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s
 
 struct S1 {
   virtual void f();
Index: clang/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
===
--- clang/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
+++ clang/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
@@ -12,30 +12,30 @@
 // Sanitization is explicitly disabled.
 // == //
 
-// CHECK-LABEL: @blacklist_0
-__attribute__((no_sanitize("undefined"))) unsigned int blacklist_0(signed int src) {
+// CHECK-LABEL: @ignorelist_0
+__attribute__((no_sanitize("undefined"))) unsigned int ignorelist_0(signed int src) {
   // We are not in "undefined" group, so that doesn't work.
   // CHECK-SANITIZE: call
   // CHECK: }
   return src;
 }
 
-// CHECK-LABEL: @blacklist_1
-__attribute__((no_sanitize("integer"))) unsigned int blacklist_1(signed int src) {
+// CHECK-LABEL: @ignorelist_1
+__attribute__((no_sanitize("integer"))) unsigned int ignorelist_1(signed int src) {
   // CHECK-SANITIZE-NOT: call
   // CHECK: }
   return src;
 }
 
-// CHECK-LABE

[PATCH] D112359: [RISCV] Unify depedency check and extension implication parsing logics

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

OK, that reasoning makes sense. I think my only outstanding request would be to 
ensure there's some test coverage for the case of .attribute arch with an 
experimental extension without version info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112359

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

[PATCH] D113638: [xray] Add support for hexagon architecture

[Brian Cain via Phabricator via cfe-commits]

androm3da added a comment.

Ping - any thoughts/concerns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113638

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

[clang] 35ff3a0 - [analyzer][NFC] Make the API of CallDescription safer slightly

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-11-17T15:55:35+01:00
New Revision: 35ff3a0095d5b2dafa2fc8dd762377342aef9c50

URL: 
https://github.com/llvm/llvm-project/commit/35ff3a0095d5b2dafa2fc8dd762377342aef9c50
DIFF: 
https://github.com/llvm/llvm-project/commit/35ff3a0095d5b2dafa2fc8dd762377342aef9c50.diff

LOG: [analyzer][NFC] Make the API of CallDescription safer slightly

The new //deleted// constructor overload makes sure that no implicit
conversion from `0` would happen to `ArrayRef`.

Also adds nodiscard to the `CallDescriptionMap::lookup()`

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 9148dcaa87544..bbf58d753b1f6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -18,6 +18,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/Compiler.h"
 #include 
 
 namespace clang {
@@ -67,6 +68,8 @@ class CallDescription {
   Optional RequiredArgs = None,
   Optional RequiredParams = None);
 
+  CallDescription(std::nullptr_t) = delete;
+
   /// Get the name of the function that this object matches.
   StringRef getFunctionName() const { return QualifiedName.back(); }
 
@@ -104,7 +107,7 @@ template  class CallDescriptionMap {
   CallDescriptionMap(const CallDescriptionMap &) = delete;
   CallDescriptionMap &operator=(const CallDescription &) = delete;
 
-  const T *lookup(const CallEvent &Call) const {
+  LLVM_NODISCARD const T *lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.
 // TODO: Implement some sort of fast path.
 for (const std::pair &I : LinearMap)



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

[PATCH] D114088: [PowerPC] Add BCD add/sub/cmp builtins

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai created this revision.
nemanjai added reviewers: bmahjour, PowerPC.
Herald added subscribers: shchenz, kbarton, hiraditya.
nemanjai requested review of this revision.
Herald added projects: clang, LLVM.

Support for builtins that use `bcdadd./bcdsub.` to add/subtract Binary Coded 
Decimal values as well as to determine validity and compare BCD values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114088

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p8vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/P10InstrResources.td
  llvm/lib/Target/PowerPC/P9InstrResources.td
  llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
  llvm/lib/Target/PowerPC/PPCInstrAltivec.td
  llvm/test/CodeGen/PowerPC/bcd-intrinsics.ll

Index: llvm/test/CodeGen/PowerPC/bcd-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/bcd-intrinsics.ll
@@ -0,0 +1,212 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names \
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 -ppc-asm-full-reg-names \
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-P9
+
+define dso_local i64 @test_invalid(<16 x i8> %a) local_unnamed_addr #0 {
+; CHECK-LABEL: test_invalid:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdsub. v2, v2, v2, 0
+; CHECK-NEXT:setbc r3, 4*cr6+un
+; CHECK-NEXT:extsw r3, r3
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_invalid:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdsub. v2, v2, v2, 0
+; CHECK-P9-NEXT:mfocrf r3, 2
+; CHECK-P9-NEXT:rlwinm r3, r3, 28, 31, 31
+; CHECK-P9-NEXT:extsw r3, r3
+; CHECK-P9-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.bcdsub.p(i32 6, <16 x i8> %a, <16 x i8> %a) #2
+  %conv.i = sext i32 %0 to i64
+  ret i64 %conv.i
+}
+
+define dso_local <16 x i8> @test_add(<16 x i8> %a, <16 x i8> %b, i64 %ps) local_unnamed_addr #0 {
+; CHECK-LABEL: test_add:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdadd. v2, v2, v3, 1
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_add:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdadd. v2, v2, v3, 1
+; CHECK-P9-NEXT:blr
+entry:
+  %0 = tail call <16 x i8> @llvm.ppc.bcdadd(<16 x i8> %a, <16 x i8> %b, i32 1)
+  ret <16 x i8> %0
+}
+
+define dso_local i64 @test_add_ofl(<16 x i8> %a, <16 x i8> %b, i64 %ps) local_unnamed_addr #0 {
+; CHECK-LABEL: test_add_ofl:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdadd. v2, v2, v3, 0
+; CHECK-NEXT:setbc r3, 4*cr6+un
+; CHECK-NEXT:extsw r3, r3
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_add_ofl:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdadd. v2, v2, v3, 0
+; CHECK-P9-NEXT:mfocrf r3, 2
+; CHECK-P9-NEXT:rlwinm r3, r3, 28, 31, 31
+; CHECK-P9-NEXT:extsw r3, r3
+; CHECK-P9-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.bcdadd.p(i32 6, <16 x i8> %a, <16 x i8> %b) #2
+  %conv.i = sext i32 %0 to i64
+  ret i64 %conv.i
+}
+
+define dso_local <16 x i8> @test_sub(<16 x i8> %a, <16 x i8> %b, i64 %ps) local_unnamed_addr #0 {
+; CHECK-LABEL: test_sub:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_sub:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-P9-NEXT:blr
+entry:
+  %0 = tail call <16 x i8> @llvm.ppc.bcdsub(<16 x i8> %a, <16 x i8> %b, i32 0)
+  ret <16 x i8> %0
+}
+
+define dso_local i64 @test_sub_ofl(<16 x i8> %a, <16 x i8> %b, i64 %ps) local_unnamed_addr #0 {
+; CHECK-LABEL: test_sub_ofl:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-NEXT:setbc r3, 4*cr6+un
+; CHECK-NEXT:extsw r3, r3
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_sub_ofl:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-P9-NEXT:mfocrf r3, 2
+; CHECK-P9-NEXT:rlwinm r3, r3, 28, 31, 31
+; CHECK-P9-NEXT:extsw r3, r3
+; CHECK-P9-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.bcdsub.p(i32 6, <16 x i8> %a, <16 x i8> %b) #2
+  %conv.i = sext i32 %0 to i64
+  ret i64 %conv.i
+}
+
+define dso_local i64 @test_cmplt(<16 x i8> %a, <16 x i8> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_cmplt:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-NEXT:setbc r3, 4*cr6+lt
+; CHECK-NEXT:extsw r3, r3
+; CHECK-NEXT:blr
+;
+; CHECK-P9-LABEL: test_cmplt:
+; CHECK-P9:   # %bb.0: # %entry
+; CHECK-P9-NEXT:bcdsub. v2, v2, v3, 0
+; CHECK-P9-NEXT:mfocrf r3, 2
+; CHECK-P9-NEXT:rlwinm r3, r3, 25, 31, 31
+; CHECK-P9-NEXT:extsw r3, r3
+; CHECK-P9-NEXT:

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ

> If we ever prove that strict aliasing is violated on a given execution path 
> (while being enabled), the ideal thing to do is to terminate the analysis 
> immediately by generating a sink. We can then optionally develop a checker 
> that emits a warning in such cases.

thinking about warnings I assume that the Store will produce `UndefinedVals` 
and //Undef-related// checkers will catch them. But yes, you're right. They 
wouldn't know anything about the origin of such `UndefinedVals`.

> For the cases where you eliminate possibilities through recognizing strict 
> aliasing, I wonder if a note can be added to the bug report to notify the 
> user that the strict aliasing rule was invoked to add a certain assumption.

I didn't elaborate an idea with a checker yet but, I think, it can be the next 
step. What about a mechanism which would store the reason of UndefinedVal for 
existing checkers? It could make any checker more detailed and flexible.


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

https://reviews.llvm.org/D110927

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm




Comment at: clang/lib/Headers/x86gprintrin.h:29
 #define __SSC_MARK(Tag)
\
-  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
-   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
+  __asm__ __volatile__("mov{l} {%%ebx, %%eax|eax, ebx}; "  
\
+   "mov{l} {%0, %%ebx|ebx, %0}; "  
\

the 'l' suffix is probably not even needed for any of these movs?



Comment at: clang/test/CodeGen/inline-asm-intel.c:3
+
+/// Accept intel inline asm but write it out as att:
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig 
-target-feature +sgx -ffreestanding -triple i386-unknown-unknown -mllvm 
-x86-asm-syntax=att -inline-asm=intel -O0 -S %s -o - | FileCheck 
--check-prefix=ATT %s

Could the interesting flags (-triple x86_64-unknown-unknown -mllvm 
-x86-asm-syntax=att -inline-asm=intel) be put right at the beginning or end of 
the run lines? They're pretty long, so easy to get lost in the woods otherwise.

(I think the triple slash prefix is uncommon in clang tests.)



Comment at: clang/test/CodeGen/inline-asm-intel.c:11
+
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig 
-target-feature +sgx -ffreestanding -triple i386-pc-win32 -mllvm 
-x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - -fms-extensions 
-fms-compatibility -fms-compatibility-version=17.00 | FileCheck  
--check-prefix=INTEL %s
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig 
-target-feature +sgx -ffreestanding -triple x86_64-pc-win32 -mllvm 
-x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - -fms-extensions 
-fms-compatibility -fms-compatibility-version=17.00 | FileCheck  
--check-prefix=INTEL %s

Maybe give these a comment too?


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

https://reviews.llvm.org/D113707

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Bradley Smith via Phabricator via cfe-commits]

bsmith added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:82-88
+if (Feature == "sve2")
+  Features.push_back("+sve");
+else if (Feature == "sve2-bitperm" || Feature == "sve2-sha3" ||
+ Feature == "sve2-aes" || Feature == "sve2-sm4") {
+  Features.push_back("+sve");
+  Features.push_back("+sve2");
+} else if (Feature == "nosve") {

sdesmalen wrote:
> ^^^ Are the above changes necessary? i.e. if only `+sve2-sha3` is set as 
> target feature, I thought LLVM automatically infers that it requires `+sve` 
> and `+sve2`. Is that not the case?
That is the case yes, however this code is to catch combinations, for example 
`+sve2-bitperm+nosve2` needs to keep `sve` enabled, if +sve was never in the 
feature set this wouldn't be the case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Sander de Smalen via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:82-88
+if (Feature == "sve2")
+  Features.push_back("+sve");
+else if (Feature == "sve2-bitperm" || Feature == "sve2-sha3" ||
+ Feature == "sve2-aes" || Feature == "sve2-sm4") {
+  Features.push_back("+sve");
+  Features.push_back("+sve2");
+} else if (Feature == "nosve") {

bsmith wrote:
> sdesmalen wrote:
> > ^^^ Are the above changes necessary? i.e. if only `+sve2-sha3` is set as 
> > target feature, I thought LLVM automatically infers that it requires `+sve` 
> > and `+sve2`. Is that not the case?
> That is the case yes, however this code is to catch combinations, for example 
> `+sve2-bitperm+nosve2` needs to keep `sve` enabled, if +sve was never in the 
> feature set this wouldn't be the case.
That makes sense, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

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

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

[Becca Royal-Gordon via Phabricator via cfe-commits]

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

The diagnostics are now much better—thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114003

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for your patience with this review! I'm currently in WG14 meetings this 
week and out on vacation next week, so my review availability is a bit limited 
at the moment (sorry for that).

I think this is heading in the right direction, but there are some lifetime 
issues we need to figure out how to resolve. I pointed out a few such places, 
but I did not do an exhaustive search to catch them all.




Comment at: clang/include/clang/Basic/Sarif.h:80-82
+  static SarifArtifactLocation create(StringRef URI) {
+return SarifArtifactLocation{URI};
+  }

One thing that's worth calling out is that `StringRef` is non-owning which 
means that the argument passed to create the `SarifArtifactLocation` has to 
outlive the returned object to avoid dangling references. This makes the class 
a bit more dangerous to use because `Twine` or automatic `std::string` objects 
may cause lifetime concerns.

Should these classes be storing a `std::string` so that the memory is owned by 
SARIF?



Comment at: clang/include/clang/Basic/Sarif.h:116
+  static SarifArtifact create(const SarifArtifactLocation &Loc) {
+return SarifArtifactLocation{Loc};
+  }

`Loc` is already a `SarifArtifactLocation`, did you mean `SarifArtifact` by any 
chance?

(Note, this suggests to me we should mark the ctor's here as `explicit`.)



Comment at: clang/include/clang/Basic/Sarif.h:229-230
+/// used to create an empty shell onto which attributes can be added using the
+/// \c setX(...) methods. The
+///
+/// For example:





Comment at: clang/include/clang/Basic/Sarif.h:243-245
+  // While this cannot be negative, since this type needs to be serialized
+  // to JSON, it needs to be `int64_t`. The best we can do is assert that
+  // a negative value isn't used to create it

This comment looks incorrect to me.



Comment at: clang/include/clang/Basic/Sarif.h:367
+
+  /// Associate the given rule with the current run
+  ///

It'd be good to explain why `createRule()` returns a value here and above.



Comment at: clang/include/clang/Basic/Sarif.h:378
+  /// There must be a run associated with the document, failing to do so will
+  /// cause undefined behaviour
+  /// \pre





Comment at: clang/include/clang/Basic/SourceLocation.h:441-456
+bool operator()(const FullSourceLoc &lhs, const FullSourceLoc &rhs) const {
   return lhs.isBeforeInTranslationUnitThan(rhs);
 }
   };
 
   /// Prints information about this FullSourceLoc to stderr.
   ///

Looks like unrelated formatting changes; feel free to submit these as an NFC 
change if you'd like, but the changes should be reverted from this patch for 
clarity.



Comment at: clang/lib/Basic/Sarif.cpp:199
+const SarifArtifactLocation &Location =
+SarifArtifactLocation::create(FileURI).setIndex(Idx);
+const SarifArtifact &Artifact = SarifArtifact::create(Location)

This seems like it'll be a use-after-free because the local `std::string` will 
be destroyed before the lifetime of the `SarifArtifactLocation` ends.



Comment at: clang/lib/Basic/Sarif.cpp:207-209
+if (statusIter.second) {
+  I = statusIter.first;
+}

vaibhav.y wrote:
> aaron.ballman wrote:
> > Our usual coding style elides these too. Btw, you can find the coding style 
> > document at: https://llvm.org/docs/CodingStandards.html
> Thanks, sorry there's so many of these! I definitely need to not auto-pilot 
> with style.
No worries! Our style is not... all that typical... so it can be hard to 
remember.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D114090: [NFC] Inclusive language: rename master flag to main flag

[Quinn Pham via Phabricator via cfe-commits]

quinnp created this revision.
Herald added subscribers: pengfei, hiraditya.
quinnp requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

[NFC] As part of using inclusive language within the llvm project, this patch
renames master flag to main flag in these comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114090

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp


Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
===
--- llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -148,7 +148,7 @@
   AlignBranchType.addKind(X86::AlignBranchJcc);
   AlignBranchType.addKind(X86::AlignBranchJmp);
 }
-// Allow overriding defaults set by master flag
+// Allow overriding defaults set by main flag
 if (X86AlignBranchBoundary.getNumOccurrences())
   AlignBoundary = assumeAligned(X86AlignBranchBoundary);
 if (X86AlignBranch.getNumOccurrences())
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -403,7 +403,7 @@
 }
 
 /// Adds exception related arguments to the driver command arguments. There's a
-/// master flag, -fexceptions and also language specific flags to 
enable/disable
+/// main flag, -fexceptions and also language specific flags to enable/disable
 /// C++ and Objective-C exceptions. This makes it possible to for example
 /// disable C++ exceptions but enable Objective-C exceptions.
 static bool addExceptionArgs(const ArgList &Args, types::ID InputType,


Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
===
--- llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -148,7 +148,7 @@
   AlignBranchType.addKind(X86::AlignBranchJcc);
   AlignBranchType.addKind(X86::AlignBranchJmp);
 }
-// Allow overriding defaults set by master flag
+// Allow overriding defaults set by main flag
 if (X86AlignBranchBoundary.getNumOccurrences())
   AlignBoundary = assumeAligned(X86AlignBranchBoundary);
 if (X86AlignBranch.getNumOccurrences())
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -403,7 +403,7 @@
 }
 
 /// Adds exception related arguments to the driver command arguments. There's a
-/// master flag, -fexceptions and also language specific flags to enable/disable
+/// main flag, -fexceptions and also language specific flags to enable/disable
 /// C++ and Objective-C exceptions. This makes it possible to for example
 /// disable C++ exceptions but enable Objective-C exceptions.
 static bool addExceptionArgs(const ArgList &Args, types::ID InputType,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

The clang binary size increases about +1.4%. Compared to +2.3% previously, it 
is better in the implementation.

The test time of lit testing under `clang/test/CodeGen/RISCV/` is 25.59s. It 
spends 112.07s without this patch in my local environment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

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

[PATCH] D102449: [WIP][Clang][OpenMP] Add the support for compare clause in atomic directive

[Carlo Bertolli via Phabricator via cfe-commits]

carlo.bertolli added a comment.

This is already a lot of code with parse+sema. I wonder if we should split the 
patch into two, i.e. 1. parse+sema; 2. code gen? @ABataev ?
It should simplify maintenance of the patch and allow time to extend the OpenMP 
IR builder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102449

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

[PATCH] D114092: [clang][lex] NFC: Remove unused HeaderFileInfo member

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch removes `HeaderFileInfo::isNonDefault`, which is not being used 
anywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114092

Files:
  clang/include/clang/Lex/HeaderSearch.h


Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -130,13 +130,6 @@
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-  ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply


Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -130,13 +130,6 @@
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-  ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114093: [clang][lex] Refactor check for the first file include

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch refactors the code that checks whether a file has just been included 
for the first time.

The `HeaderSearch::FirstTimeLexingFile` function is removed and the information 
is threaded to the original call site from 
`HeaderSearch::ShouldEnterIncludeFile`. This will make it possible to avoid 
tracking the number of includes in a follow up patch.

Depends on D114092 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114093

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Lexer.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp

Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -67,7 +67,8 @@
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
 bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
-   SourceLocation Loc) {
+   SourceLocation Loc,
+   bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
   ++NumEnteredSourceFiles;
 
@@ -91,7 +92,8 @@
 CodeCompletionFileLoc.getLocWithOffset(CodeCompletionOffset);
   }
 
-  EnterSourceFileWithLexer(new Lexer(FID, *InputFile, *this), CurDir);
+  EnterSourceFileWithLexer(
+  new Lexer(FID, *InputFile, *this, IsFirstIncludeOfFile), CurDir);
   return false;
 }
 
@@ -377,7 +379,7 @@
   CurPPLexer->MIOpt.GetDefinedMacro()) {
   if (!isMacroDefined(ControllingMacro) &&
   DefinedMacro != ControllingMacro &&
-  HeaderInfo.FirstTimeLexingFile(FE)) {
+  CurLexer->isFirstTimeLexingFile()) {
 
 // If the edit distance between the two macros is more than 50%,
 // DefinedMacro may not be header guard, or can be header guard of
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2143,12 +2143,14 @@
   IsImportDecl ||
   IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import;
 
+  bool IsFirstIncludeOfFile = false;
+
   // Ask HeaderInfo if we should enter this #include file.  If not, #including
   // this file will have no effect.
   if (Action == Enter && File &&
-  !HeaderInfo.ShouldEnterIncludeFile(*this, &File->getFileEntry(),
- EnterOnce, getLangOpts().Modules,
- SuggestedModule.getModule())) {
+  !HeaderInfo.ShouldEnterIncludeFile(
+  IsFirstIncludeOfFile, *this, &File->getFileEntry(), EnterOnce,
+  getLangOpts().Modules, SuggestedModule.getModule())) {
 // Even if we've already preprocessed this header once and know that we
 // don't need to see its contents again, we still need to import it if it's
 // modular because we might not have imported it from this submodule before.
@@ -2340,7 +2342,8 @@
   }
 
   // If all is good, enter the new file!
-  if (EnterSourceFile(FID, CurDir, FilenameTok.getLocation()))
+  if (EnterSourceFile(FID, CurDir, FilenameTok.getLocation(),
+  IsFirstIncludeOfFile))
 return {ImportAction::None};
 
   // Determine if we're switching to building a new submodule, and which one.
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -133,10 +133,10 @@
 /// assumes that the associated file buffer and Preprocessor objects will
 /// outlive it, so it doesn't take ownership of either of them.
 Lexer::Lexer(FileID FID, const llvm::MemoryBufferRef &InputFile,
- Preprocessor &PP)
+ Preprocessor &PP, bool IsFirstIncludeOfFile)
 : PreprocessorLexer(&PP, FID),
   FileLoc(PP.getSourceManager().getLocForStartOfFile(FID)),
-  LangOpts(PP.getLangOpts()) {
+  LangOpts(PP.getLangOpts()), IsFirstTimeLexingFile(IsFirstIncludeOfFile) {
   InitLexer(InputFile.getBufferStart(), InputFile.getBufferStart(),
 InputFile.getBufferEnd());
 
@@ -147,8 +147,10 @@
 /// suitable for calls to 'LexFromRawLexer'.  This lexer assumes that the text
 /// range will outlive it, so it doesn't take ownership of it.
 Lexer::Lexer(SourceLocation fileloc, const LangOptions &langOpts,
- const char *BufStart, const char *BufPtr, const char *BufEnd)
-

[PATCH] D114095: [clang][lex][modules] Move number of includes from HeaderFileInfo to Preprocessor

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
Herald added a subscriber: mgrang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch moves the number of times a file was included from `HeaderFileInfo` 
to `Preprocessor`. It's not a property of the file itself, but rather a 
preprocessor state.

Depends on D114093 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114095

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1696,7 +1696,7 @@
 std::pair
 EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1728,7 +1728,6 @@
   | (Data.HFI.DirInfo << 1)
   | Data.HFI.IndexHeaderMapHeader;
   LE.write(Flags);
-  LE.write(Data.HFI.NumIncludes);
 
   if (!Data.HFI.ControllingMacro)
 LE.write(Data.HFI.ControllingMacroID);
@@ -2171,6 +2170,27 @@
   return false;
 }
 
+void ASTWriter::writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP) {
+  using namespace llvm::support;
+
+  std::vector> IncludedInputFiles;
+  for (const auto &File : PP.getIncludedFiles()) {
+auto InputFileIt = InputFileIDs.find(File.first);
+if (InputFileIt != InputFileIDs.end())
+  IncludedInputFiles.emplace_back(InputFileIt->second, File.second);
+  }
+
+  llvm::sort(IncludedInputFiles);
+
+  endian::Writer LE(Out, little);
+  LE.write(IncludedInputFiles.size());
+
+  for (const auto &IncludedInputFile : IncludedInputFiles) {
+LE.write(IncludedInputFile.first);
+LE.write(IncludedInputFile.second);
+  }
+}
+
 /// Writes the block containing the serialized form of the
 /// preprocessor.
 void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
@@ -2379,6 +2399,20 @@
MacroOffsetsBase - ASTBlockStartOffset};
 Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
+
+  {
+auto Abbrev = std::make_shared();
+Abbrev->Add(BitCodeAbbrevOp(PP_INCLUDED_FILES));
+Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+unsigned IncludedFilesAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
+
+SmallString<2048> Buffer;
+raw_svector_ostream Out(Buffer);
+writeIncludedFiles(Out, PP);
+RecordData::value_type Record[] = {PP_INCLUDED_FILES};
+Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(),
+  Buffer.size());
+  }
 }
 
 void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1888,10 +1888,6 @@
   HFI.isPragmaOnce |= (Flags >> 4) & 0x01;
   HFI.DirInfo = (Flags >> 1) & 0x07;
   HFI.IndexHeaderMapHeader = Flags & 0x01;
-  // FIXME: Find a better way to handle this. Maybe just store a
-  // "has been included" flag?
-  HFI.NumIncludes = std::max(endian::readNext(d),
- HFI.NumIncludes);
   HFI.ControllingMacroID = Reader.getGlobalIdentifierID(
   M, endian::readNext(d));
   if (unsigned FrameworkOffset =
@@ -2963,6 +2959,25 @@
   }
 }
 
+void ASTReader::readIncludedFiles(ModuleFile &F, StringRef Blob,
+  Preprocessor &PP) {
+  using namespace llvm::support;
+
+  const unsigned char *D = (const unsigned char *)Blob.data();
+  unsigned FileCount = endian::readNext(D);
+
+  for (unsigned I = 0; I < FileCount; ++I) {
+auto InputFileID = endian::readNext(D);
+auto NumIncludes = endian::readNext(D);
+
+auto InputFileInfo = readInputFileInfo(F, InputFileID);
+if (auto File = PP.getFileManager().getFile(InputFileInfo.Filename)) {
+  unsigned short &PPNumIncludes = PP.getIncludedFiles()[*File];
+  PPNumIncludes = std::max(PPNumIncludes, NumIncludes);
+}
+  }
+}
+
 llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
 unsigned ClientLoadCapabiliti

[PATCH] D114096: [clang][lex][modules] Stop tracking number of includes

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
Herald added a subscriber: mgrang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch stops tracking the number of includes of a file. The information is 
not used anywhere except for stat printing. Only tracking the fact whether a 
file was included or not simplifies the code and should be more efficient.

Depends on D114095 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114096

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2173,22 +2173,20 @@
 void ASTWriter::writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP) {
   using namespace llvm::support;
 
-  std::vector> IncludedInputFiles;
+  std::vector IncludedInputFiles;
   for (const auto &File : PP.getIncludedFiles()) {
-auto InputFileIt = InputFileIDs.find(File.first);
-if (InputFileIt != InputFileIDs.end())
-  IncludedInputFiles.emplace_back(InputFileIt->second, File.second);
+auto InputFileIt = InputFileIDs.find(File);
+if (InputFileIt == InputFileIDs.end())
+  continue;
+auto InputFileID = InputFileIt->second;
+if (IncludedInputFiles.size() <= InputFileID)
+  IncludedInputFiles.resize(InputFileID + 1);
+IncludedInputFiles[InputFileIt->second] = true;
   }
 
-  llvm::sort(IncludedInputFiles);
-
   endian::Writer LE(Out, little);
   LE.write(IncludedInputFiles.size());
-
-  for (const auto &IncludedInputFile : IncludedInputFiles) {
-LE.write(IncludedInputFile.first);
-LE.write(IncludedInputFile.second);
-  }
+  Out << bytes(IncludedInputFiles);
 }
 
 /// Writes the block containing the serialized form of the
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2966,16 +2966,13 @@
   const unsigned char *D = (const unsigned char *)Blob.data();
   unsigned FileCount = endian::readNext(D);
 
-  for (unsigned I = 0; I < FileCount; ++I) {
-auto InputFileID = endian::readNext(D);
-auto NumIncludes = endian::readNext(D);
-
-auto InputFileInfo = readInputFileInfo(F, InputFileID);
-if (auto File = PP.getFileManager().getFile(InputFileInfo.Filename)) {
-  unsigned short &PPNumIncludes = PP.getIncludedFiles()[*File];
-  PPNumIncludes = std::max(PPNumIncludes, NumIncludes);
-}
-  }
+  for (unsigned I = 0; I < FileCount; ++D)
+for (unsigned Bit = 0; Bit < 8 && I < FileCount; ++Bit, ++I)
+  if (*D & (1 << Bit)) {
+auto InputFileInfo = readInputFileInfo(F, I);
+if (auto File = PP.getFileManager().getFile(InputFileInfo.Filename))
+  PP.getIncludedFiles().insert(*File);
+  }
 }
 
 llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -286,15 +286,6 @@
  << " token paste (##) operations performed, "
  << NumFastTokenPaste << " on the fast path.\n";
 
-  unsigned MaxNumIncludes = 0, NumSingleIncludedFiles = 0;
-  for (const auto &IncludedFile : IncludedFiles) {
-if (MaxNumIncludes < IncludedFile.second)
-  MaxNumIncludes = IncludedFile.second;
-NumSingleIncludedFiles += IncludedFile.second == 1;
-  }
-  llvm::errs() << NumSingleIncludedFiles << " included exactly once.\n"
-   << MaxNumIncludes << " max times a file is included.\n";
-
   llvm::errs() << "\nPreprocessor Memory: " << getTotalMemory() << "B total";
 
   llvm::errs() << "\n  BumpPtr: " << BP.getTotalMemory();
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -765,8 +765,8 @@
   /// in a submodule.
   SubmoduleState *CurSubmoduleState;
 
-  /// The number of times each file was included.
-  llvm::DenseMap IncludedFiles;
+  /// The files that have been included.
+  llvm::DenseSet IncludedFiles;
 
   /// The set of known macros exported from modules.
   llvm::FoldingSet ModuleMacros;
@@ -1230,9 +1230,7 @@
   /// Increment the count for the number of times the specified FileEntry has
   /// been entered. Returns true if this is the first time it file was included.
   bool incrementIncludeCount(const FileEntry *File) {
-auto It = IncludedFiles.insert({File, 0});
-++It.first->second;
-return It.second;
+return IncludedFiles.ins

[PATCH] D112915: [clang][modules] Track number of includes per submodule

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 387952.
jansvoboda11 added a comment.

Rebase on top of extracted patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112915

Files:
  clang/include/clang/Lex/ExternalPreprocessorSource.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Basic/Module.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/import-submodule-visibility.c

Index: clang/test/Modules/import-submodule-visibility.c
===
--- /dev/null
+++ clang/test/Modules/import-submodule-visibility.c
@@ -0,0 +1,99 @@
+// This test checks that imports of headers that appeared in a different submodule than
+// what is imported by the current TU don't affect the compilation.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- A.framework/Headers/A.h
+#include "Textual.h"
+//--- A.framework/Modules/module.modulemap
+framework module A { header "A.h" }
+
+//--- B.framework/Headers/B1.h
+#include "Textual.h"
+//--- B.framework/Headers/B2.h
+//--- B.framework/Modules/module.modulemap
+framework module B {
+  module B1 { header "B1.h" }
+  module B2 { header "B2.h" }
+}
+
+//--- C/C.h
+#include "Textual.h"
+//--- C/module.modulemap
+module C { header "C.h" }
+
+//--- D/D1.h
+#include "Textual.h"
+//--- D/D2.h
+//--- D/module.modulemap
+module D {
+  module D1 { header "D1.h" }
+  module D2 { header "D2.h" }
+}
+
+//--- E/E1.h
+#include "E2.h"
+//--- E/E2.h
+#include "Textual.h"
+//--- E/module.modulemap
+module E {
+  module E1 { header "E1.h" }
+  module E2 { header "E2.h" }
+}
+
+//--- Textual.h
+#define MACRO_TEXTUAL 1
+
+//--- test.c
+
+#ifdef A
+//
+#endif
+
+#ifdef B
+#import 
+#endif
+
+#ifdef C
+//
+#endif
+
+#ifdef D
+#import "D/D2.h"
+#endif
+
+#ifdef E
+#import "E/E1.h"
+#endif
+
+#import "Textual.h"
+
+static int x = MACRO_TEXTUAL;
+
+// Specifying the PCM file on the command line (without actually importing "A") should not
+// prevent "Textual.h" to be included in the TU.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/A.framework/Modules/module.modulemap -fmodule-name=A -o %t/A.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DA -fmodule-file=%t/A.pcm
+
+// Specifying the PCM file on the command line and importing "B2" in the source does not
+// prevent "Textual.h" to be included in the TU.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/B.framework/Modules/module.modulemap -fmodule-name=B -o %t/B.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DB -iframework %t -fmodule-file=%t/B.pcm
+
+// Module-only version of the test with framework A.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/C/module.modulemap -fmodule-name=C -o %t/C.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DC -fmodule-file=%t/C.pcm
+
+// Module-only version of the test with framework B.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/D/module.modulemap -fmodule-name=D -o %t/D.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DD -fmodule-file=%t/D.pcm
+
+// Transitively imported, but not exported.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/E/module.modulemap -fmodule-name=E -o %t/E.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DE -fmodule-file=%t/E.pcm
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2170,11 +2170,12 @@
   return false;
 }
 
-void ASTWriter::writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP) {
+void ASTWriter::writeIncludedFiles(
+raw_ostream &Out, const llvm::DenseSet &Files) {
   using namespace llvm::support;
 
   std::vector IncludedInputFiles;
-  for (const auto &File : PP.getIncludedFiles()) {
+  for (const auto &File : Files) {
 auto InputFileIt = InputFileIDs.find(File);
 if (InputFileIt == InputFileIDs.end())
   continue;
@@ -2406,7 +2407,7 @@
 
 SmallString<2048> Buffer;
 raw_svector_ostream Out(Buffer);
-writeIncludedFiles(Out, PP);
+writeIncludedFiles(Out, PP.getNullSubmoduleIncludes());
 RecordData::value_type Record[] = {PP_INCLUDED_FILES};
 Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(),
   Buffer.size());
@@ -2666,6 +2667,11 @@
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));// Macro name
   unsigned ExportAsAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
 
+  Abbrev = std::make_shared();
+  Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_INCLUDED_FILES));
+  

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

[Kristina Bessonova via Phabricator via cfe-commits]

krisb updated this revision to Diff 387956.
krisb marked an inline comment as done.
krisb added a comment.

Applied the comment & silence clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113743

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/debug-info-lexcial-block.cpp

Index: clang/test/CodeGenCXX/debug-info-lexcial-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-lexcial-block.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+void foo() {
+  static int bar = 1;
+  {
+struct X {};
+typedef char Y;
+static int bar = 0;
+// The following basic block is intended, in order to check the case where
+// types "X", "Y" are defined in a different scope than where they are used.
+// They should have the scope they are defined at as their parent scope.
+{
+  X a;
+  Y b;
+}
+  }
+}
+
+// CHECK: !{{[0-9]+}} = distinct !DIGlobalVariable(name: "bar", scope: [[FSCOPE:![0-9]+]]
+// CHECK: [[FSCOPE]] = distinct !DISubprogram(name: "foo"
+// CHECK: !{{[0-9]+}} = distinct !DIGlobalVariable(name: "bar", scope: [[LBSCOPE:![0-9]+]]
+// CHECK: [[LBSCOPE]] = distinct !DILexicalBlock(scope: [[FSCOPE]]
+// CHECK: !{{[0-9]+}} = !DILocalVariable(name: "a", scope: [[LBSCOPE2:![0-9]+]], {{.*}} type: [[STRUCT:![0-9]+]]
+// CHECK: [[LBSCOPE2]] = distinct !DILexicalBlock(scope: [[LBSCOPE]]
+// CHECK: [[STRUCT]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X", scope: [[LBSCOPE]]
+// CHECK: !{{[0-9]+}} = !DILocalVariable(name: "b", scope: [[LBSCOPE2]], {{.*}} type: [[TYPEDEF:![0-9]+]]
+// CHECK: [[TYPEDEF]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Y", scope: [[LBSCOPE]]
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -103,17 +103,21 @@
 llvm_unreachable("Declaration should not be in declstmts!");
   case Decl::Record:// struct/union/class X;
   case Decl::CXXRecord: // struct/union/class X; [C++]
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
+  DI->recordDeclarationLexicalScope(D);
   if (cast(D).getDefinition())
 DI->EmitAndRetainType(getContext().getRecordType(cast(&D)));
+}
 return;
   case Decl::Enum:  // enum X;
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
+  DI->recordDeclarationLexicalScope(D);
   if (cast(D).getDefinition())
 DI->EmitAndRetainType(getContext().getEnumType(cast(&D)));
+}
 return;
-  case Decl::Function: // void X();
   case Decl::EnumConstant: // enum ? { X = ? }
+  case Decl::Function: // void X();
   case Decl::StaticAssert: // static_assert(X, ""); [C++0x]
   case Decl::Label:// __label__ x;
   case Decl::Import:
@@ -132,11 +136,11 @@
 
   case Decl::NamespaceAlias:
 if (CGDebugInfo *DI = getDebugInfo())
-DI->EmitNamespaceAlias(cast(D));
+  DI->EmitNamespaceAlias(cast(D));
 return;
   case Decl::Using:  // using X; [C++]
 if (CGDebugInfo *DI = getDebugInfo())
-DI->EmitUsingDecl(cast(D));
+  DI->EmitUsingDecl(cast(D));
 return;
   case Decl::UsingEnum: // using enum X; [C++]
 if (CGDebugInfo *DI = getDebugInfo())
@@ -172,8 +176,10 @@
   case Decl::Typedef:  // typedef int X;
   case Decl::TypeAlias: {  // using X = int; [C++0x]
 QualType Ty = cast(D).getUnderlyingType();
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
+  DI->recordDeclarationLexicalScope(D);
   DI->EmitAndRetainType(Ty);
+}
 if (Ty->isVariablyModifiedType())
   EmitVariablyModifiedType(Ty);
 return;
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -139,6 +139,11 @@
 
   /// Keep track of our current nested lexical block.
   std::vector> LexicalBlockStack;
+
+  /// Map of AST declaration to its lexical block scope.
+  llvm::DenseMap>
+  LexicalBlockMap;
+
   llvm::DenseMap RegionMap;
   /// Keep track of LexicalBlockStack counter at the beginning of a
   /// function. This is used to pop unbalanced regions at the end of a
@@ -543,6 +548,12 @@
   /// Emit an Objective-C interface type standalone debug info.
   llvm::DIType *getOrCreateInterfaceType(QualType Ty, SourceLocation Loc);
 
+  /// Map AST declaration to its lexical block scope if available.
+  void recordDeclarationLexicalScope(const Decl &D);
+
+  /// Get lexical scope of AST declaration.
+  llvm::DIScope *getDeclarationLexicalScope(const Decl *D);
+

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

@aprantl thank you for taking a look at this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113743

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

[PATCH] D113977: [Coroutine] Warn deprecated 'std::experimental::coro' uses

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

FWIW, the problem with

  SpacesInAngles: Leave
  ^

is that the debian builder has an outdated (or maybe just 13.x?) 
`clang-format`; this option is super new. I don't know what the appropriate 
course of action is, there.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11015-11017
+  "Found deprecated std::experimental::%0. Consider to update your libc++ "
+  "or move coroutine components into std namespace in case you are using "
+  "self-defined coroutine components">,

Quuxplusone wrote:
> ldionne wrote:
> > I would simply reword as "Please move from std::experimental::%0 to 
> > std::%0. Support for std::experimental::%0 will be removed in LLVM 15."
> > 
> > IMO the set of users defining their own coroutines header is small and it's 
> > not worth making the warning more obtuse for that corner case.
> +1; people who write their own "coroutine.h" header are also operating 
> outside of standard C++ //and// outside of the Clang/libc++ ecosystem, so 
> they should be quite used to figuring things out for themselves. If they want 
> friendly help from the toolchain, they should use the toolchain's headers.
Nit: `s/Warning  Quuxplusone wrote:
> > I'd like to see the next word in this diagnostic. (Ditto throughout. I'd 
> > also like to make sure that the diagnostic is not being printed with 
> > single-quotes in the wrong place, e.g. `Found deprecated 
> > std::experimental::'coroutine_handle'`; if it is, please fix that.)
> > 
> > This code is also misindented; please fix it up, as long as you're touching 
> > it. Should be
> > ```
> > for co_await (auto i : arr) {}
> > ```
> Oh, clang-format couldn't recognize the new grammar. I need to be careful to 
> use it to format codes.
FYI: I'd prefer a space before the `(`. Compare `for (~~~)`, `for co_await 
(~~~)`. (I've left a comment in the clang-format Discord channel, too.)



Comment at: 
libcxx/test/libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp:10
 // REQUIRES: fcoroutines-ts
-// ADDITIONAL_COMPILE_FLAGS: -fcoroutines-ts
+// ADDITIONAL_COMPILE_FLAGS: -fcoroutines-ts -Wno-coroutine
 

Doesn't the change in lit.local.cfg make this change redundant?


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

https://reviews.llvm.org/D113977

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

[clang] f1c159c - [Format, Sema] Use range-based for loops with llvm::reverse (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-11-17T08:52:35-08:00
New Revision: f1c159cc908d23a381500c68ccabcd84bb6355c8

URL: 
https://github.com/llvm/llvm-project/commit/f1c159cc908d23a381500c68ccabcd84bb6355c8
DIFF: 
https://github.com/llvm/llvm-project/commit/f1c159cc908d23a381500c68ccabcd84bb6355c8.diff

LOG: [Format, Sema] Use range-based for loops with llvm::reverse (NFC)

Added: 


Modified: 
clang/lib/Format/FormatTokenLexer.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatTokenLexer.cpp 
b/clang/lib/Format/FormatTokenLexer.cpp
index a9cfb4a247f0..8075756cca03 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -506,11 +506,11 @@ void FormatTokenLexer::tryParseJSRegexLiteral() {
 return;
 
   FormatToken *Prev = nullptr;
-  for (auto I = Tokens.rbegin() + 1, E = Tokens.rend(); I != E; ++I) {
+  for (FormatToken *FT : llvm::drop_begin(llvm::reverse(Tokens))) {
 // NB: Because previous pointers are not initialized yet, this cannot use
 // Token.getPreviousNonComment.
-if ((*I)->isNot(tok::comment)) {
-  Prev = *I;
+if (FT->isNot(tok::comment)) {
+  Prev = FT;
   break;
 }
   }

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index ace3d25ca460..3897241cb858 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2322,11 +2322,9 @@ class ExpressionParser {
 void TokenAnnotator::setCommentLineLevels(
 SmallVectorImpl &Lines) {
   const AnnotatedLine *NextNonCommentLine = nullptr;
-  for (SmallVectorImpl::reverse_iterator I = Lines.rbegin(),
-  E = Lines.rend();
-   I != E; ++I) {
+  for (AnnotatedLine *AL : llvm::reverse(Lines)) {
 bool CommentLine = true;
-for (const FormatToken *Tok = (*I)->First; Tok; Tok = Tok->Next) {
+for (const FormatToken *Tok = AL->First; Tok; Tok = Tok->Next) {
   if (!Tok->is(tok::comment)) {
 CommentLine = false;
 break;
@@ -2338,21 +2336,21 @@ void TokenAnnotator::setCommentLineLevels(
 if (NextNonCommentLine && CommentLine &&
 NextNonCommentLine->First->NewlinesBefore <= 1 &&
 NextNonCommentLine->First->OriginalColumn ==
-(*I)->First->OriginalColumn) {
+AL->First->OriginalColumn) {
   // Align comments for preprocessor lines with the # in column 0 if
   // preprocessor lines are not indented. Otherwise, align with the next
   // line.
-  (*I)->Level =
+  AL->Level =
   (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
(NextNonCommentLine->Type == LT_PreprocessorDirective ||
 NextNonCommentLine->Type == LT_ImportStatement))
   ? 0
   : NextNonCommentLine->Level;
 } else {
-  NextNonCommentLine = (*I)->First->isNot(tok::r_brace) ? (*I) : nullptr;
+  NextNonCommentLine = AL->First->isNot(tok::r_brace) ? AL : nullptr;
 }
 
-setCommentLineLevels((*I)->Children);
+setCommentLineLevels(AL->Children);
   }
 }
 

diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp 
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 796590c26d3f..8544a4fccf4c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1112,10 +1112,8 @@ namespace {
   continue; // Case label is preceded with a normal label, good.
 
 if (!ReachableBlocks.count(P)) {
-  for (CFGBlock::const_reverse_iterator ElemIt = P->rbegin(),
-ElemEnd = P->rend();
-   ElemIt != ElemEnd; ++ElemIt) {
-if (Optional CS = ElemIt->getAs()) {
+  for (const CFGElement &Elem : llvm::reverse(*P)) {
+if (Optional CS = Elem.getAs()) {
   if (const AttributedStmt *AS = asFallThroughAttr(CS->getStmt())) 
{
 // Don't issue a warning for an unreachable fallthrough
 // attribute in template instantiations as it may not be
@@ -1199,12 +1197,9 @@ namespace {
 static const Stmt *getLastStmt(const CFGBlock &B) {
   if (const Stmt *Term = B.getTerminatorStmt())
 return Term;
-  for (CFGBlock::const_reverse_iterator ElemIt = B.rbegin(),
-ElemEnd = B.rend();
-ElemIt != ElemEnd; ++ElemIt) {
-if (Optional CS = ElemIt->getAs())
+  for (const CFGElement &Elem : llvm::reverse(B))
+if (Optional CS = Elem.getAs())
   return CS->getStmt();
-  }
   // Workaround to detect a statement thrown out by CFGBuilder:
   //   case X: {} case Y:
   /

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
zahiraam added reviewers: rjmccall, pengfei, andrew.w.kaylor.
zahiraam requested review of this revision.
Herald added a project: clang.

The _Float16 type is supported on x86 systems with SSE2 enabled. Operations are 
emulated by software emulation and “float” instructions. This patch is allowing 
the support of _Float16 type without the use of -max512fp16 flag. The final 
goal being, perform _Float16 emulation for all arithmetic expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114099

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/avx512fp16-abi.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/CodeGen/X86/fp16-abi.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
-
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main() {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
 _Float16 f;
 
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex() {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
Index: clang/test/CodeGen/X86/fp16-abi.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/fp16-abi.c
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm < %s | FileCheck %s
+
+struct half1 {
+  _Float16 a;
+};
+
+struct half1 h1(_Float16 a) {
+  // CHECK: define{{.*}}half @h1
+  struct half1 x;
+  x.a = a;
+  return x;
+}
+
+struct half2 {
+  _Float16 a;
+  _Float16 b;
+};
+
+struct half2 h2(_Float16 a, _Float16 b) {
+  // CHECK: define{{.*}}<2 x half> @h2
+  struct half2 x;
+  x.a = a;
+  x.b = b;
+  return x;
+}
+
+struct half3 {
+  _Float16 a;
+  _Float16 b;
+  _Float16 c;
+};
+
+struct half3 h3(_Float16 a, _Float16 b, _Float16 c) {
+  // CHECK: define{{.*}}<4 x half> @h3
+  struct half3 x;
+  x.a = a;
+  x.b = b;
+  x.c = c;
+  return x;
+}
+
+struct half4 {
+  _Float16 a;
+  _Float16 b;
+  _Float16 c;
+  _Float16 d;
+};
+
+struct half4 h4(_Float16 a, _Float16 b, _Float16 c, _Float16 d) {
+  // CHECK: define{{.*}}<4 x half> @h4
+  struct half4 x;
+  x.a = a;
+  x.b = b;
+  x.c = c;
+  x.d = d;
+  return x;
+}
+
+struct floathalf {
+  float a;
+  _Float16 b;
+};
+
+struct floathalf fh(float a, _Float16 b) {
+  // CHECK: define {{.*}} <4 x half> @fh
+  struct floathalf x;
+  x.a = a;
+  x.b = b;
+  return x;
+}
+
+struct floathalf2 {
+  float a;
+  _Float16 b;
+  _Float16 c;
+};
+
+struct floathalf

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

There doesn't seem to be any objection to this approach, so I think you're free 
to land @housel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111863

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

[PATCH] D114100: [NFC][clang-tools-extra] Inclusive language: replace master with main

[Quinn Pham via Phabricator via cfe-commits]

quinnp created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
quinnp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

[NFC] As part of using inclusive language within the llvm project, this patch
replaces master with main in these comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114100

Files:
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/test/modularize/SubModule2.h


Index: clang-tools-extra/test/modularize/SubModule2.h
===
--- clang-tools-extra/test/modularize/SubModule2.h
+++ clang-tools-extra/test/modularize/SubModule2.h
@@ -1,3 +1,3 @@
-// SubModule2.h - Master header with same name as directory.
+// SubModule2.h - Main header with same name as directory.
 #include "SubModule2/Header3.h"
 #include "SubModule2/Header4.h"
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -138,7 +138,7 @@
   /// identifier is sent from the server to the client and the file is not open
   /// in the editor (the server has not received an open notification before)
   /// the server can send `null` to indicate that the version is known and the
-  /// content on disk is the master (as speced with document content 
ownership).
+  /// content on disk is the main (as speced with document content ownership).
   ///
   /// The version number of a document will increase after each change,
   /// including undo/redo. The number doesn't need to be consecutive.


Index: clang-tools-extra/test/modularize/SubModule2.h
===
--- clang-tools-extra/test/modularize/SubModule2.h
+++ clang-tools-extra/test/modularize/SubModule2.h
@@ -1,3 +1,3 @@
-// SubModule2.h - Master header with same name as directory.
+// SubModule2.h - Main header with same name as directory.
 #include "SubModule2/Header3.h"
 #include "SubModule2/Header4.h"
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -138,7 +138,7 @@
   /// identifier is sent from the server to the client and the file is not open
   /// in the editor (the server has not received an open notification before)
   /// the server can send `null` to indicate that the version is known and the
-  /// content on disk is the master (as speced with document content ownership).
+  /// content on disk is the main (as speced with document content ownership).
   ///
   /// The version number of a document will increase after each change,
   /// including undo/redo. The number doesn't need to be consecutive.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D113107#3136464 , @rjmccall wrote:

> Does GCC actually change the formal types of expressions to `float`, or is it 
> doing this "no intermediate casts thing" as some sort of fp_contract-style 
> custom emission of trees of expressions that involve `_Float16`?
>
> In any case, doing operation-by-operation emulation seems like the right 
> first approach rather than starting by doing the less conformant thing.

I have created another patch https://reviews.llvm.org/D114099 that does the 
first step.

Not sure what you mean by "no intermediate cases thing". 
This https://godbolt.org/z/cPfbKq3zx shows what gcc does for a simple 
expression. It looks like it is using half-precision to calculate the 
intermediate results since it is using library calls to __extendhfsf2 and  
__truncsfhf2?


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

https://reviews.llvm.org/D113107

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

[PATCH] D114088: [PowerPC] Add BCD add/sub/cmp builtins

[Ahsan Saghir via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114088

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

[PATCH] D113642: [PowerPC] Provide XL-compatible vec_round implementation

[Ahsan Saghir via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113642

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

[PATCH] D113899: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!




Comment at: clang-tools-extra/clangd/Selection.cpp:504
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.

kuhnel wrote:
> kadircet wrote:
> > while here `llvm::isa_and_nonnull` we try to qualify symbols from llvm 
> > namespace.
> Sorry, I'm not getting what you wanted to say. 
> 
> You want me to add a `llvm::`?
> Or is it about a `operator bool()`(however I didn't find one for `Decl` )?
> You want me to add a llvm::?

right, I was asking for qualifying the symbol with `llvm::` namespace specifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113899

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

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for the feature.  I'd still prefer to see `SimpleMFlag` used, but it's 
not a big deal to me.




Comment at: clang/include/clang/Driver/Options.td:3193
+def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group, 
Flags<[NoXarchOption]>,
+  HelpText<"Skip setting up RAX register when passing variable arguments (x86 
only)">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

pengfei wrote:
> nickdesaulniers wrote:
> > pengfei wrote:
> > > MaskRay wrote:
> > > > nickdesaulniers wrote:
> > > > > I think GCC support `-mno-skip-rax-setup` as well. Can you please add 
> > > > > that (and tests for it) as well?  We don't need to actually handle 
> > > > > it, I think, but we shouldn't warn about the flag being unsupported, 
> > > > > for example.
> > > > Consider `SimpleMFlag`
> > > Thanks for the suggestion. I think adding a `-mno-skip-rax-setup` is 
> > > simply here.
> > Via @MaskRay 's suggestion, can we do something like:
> > 
> > ```
> > def skip_rax_setup : SimpleMFlag<"skip-rax-setup", "Skip", "Do not skip", 
> > "setting up RAX register when passing variable arguments (x86 only)">;
> > ```
> > 
> > Do we test that this flag is warned about being unused for non-x86 targets? 
> > (Is it worth adding such a test to clang)?
> Another reason I didn't use `SimpleMFlag` is GCC doesn't have help test for 
> `-mno-skip-rax-setup`. I deliberately hide it in this way.
> 
> > Do we test that this flag is warned about being unused for non-x86 targets? 
> > (Is it worth adding such a test to clang)?
> 
> I tested it locally. Driver will warn it for other target, Clang doesn't. I 
> think it should be enough. I don't know if there's precedent, but adding an 
> irrelevant option test to other target's test folder looks a bit weird to me.
> Another reason I didn't use SimpleMFlag is GCC doesn't have help test for 
> -mno-skip-rax-setup. I deliberately hide it in this way.

While clang does try hard to emulate GCC, both in features and interface, 
that's trying too hard to emulate poor interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112413

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

[clang-tools-extra] e76e572 - [clangd] Dont include file version in task name

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-11-17T19:10:09+01:00
New Revision: e76e5729896c4a62f1f5ccbf784a59de96f74cbd

URL: 
https://github.com/llvm/llvm-project/commit/e76e5729896c4a62f1f5ccbf784a59de96f74cbd
DIFF: 
https://github.com/llvm/llvm-project/commit/e76e5729896c4a62f1f5ccbf784a59de96f74cbd.diff

LOG: [clangd] Dont include file version in task name

This will drop file version information from span names, reducing
overall cardinality and also effect logging when skipping actions in scheduler.

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

Added: 


Modified: 
clang-tools-extra/clangd/TUScheduler.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index e75867322cdc1..e87aa59ec6f5c 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -992,7 +992,7 @@ void 
ASTWorker::updatePreamble(std::unique_ptr CI,
std::shared_ptr Preamble,
std::vector CIDiags,
WantDiagnostics WantDiags) {
-  std::string TaskName = llvm::formatv("Build AST for ({0})", PI.Version);
+  llvm::StringLiteral TaskName = "Build AST";
   // Store preamble and build diagnostics with new preamble if requested.
   auto Task = [this, Preamble = std::move(Preamble), CI = std::move(CI),
PI = std::move(PI), CIDiags = std::move(CIDiags),
@@ -1032,7 +1032,7 @@ void 
ASTWorker::updatePreamble(std::unique_ptr CI,
   }
   {
 std::lock_guard Lock(Mutex);
-PreambleRequests.push_back({std::move(Task), std::move(TaskName),
+PreambleRequests.push_back({std::move(Task), std::string(TaskName),
 steady_clock::now(), 
Context::current().clone(),
 llvm::None, llvm::None,
 TUScheduler::NoInvalidation, nullptr});



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

[PATCH] D113390: [clangd] Dont include file version in task name

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe76e5729896c: [clangd] Dont include file version in task 
name (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113390

Files:
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -992,7 +992,7 @@
std::shared_ptr Preamble,
std::vector CIDiags,
WantDiagnostics WantDiags) {
-  std::string TaskName = llvm::formatv("Build AST for ({0})", PI.Version);
+  llvm::StringLiteral TaskName = "Build AST";
   // Store preamble and build diagnostics with new preamble if requested.
   auto Task = [this, Preamble = std::move(Preamble), CI = std::move(CI),
PI = std::move(PI), CIDiags = std::move(CIDiags),
@@ -1032,7 +1032,7 @@
   }
   {
 std::lock_guard Lock(Mutex);
-PreambleRequests.push_back({std::move(Task), std::move(TaskName),
+PreambleRequests.push_back({std::move(Task), std::string(TaskName),
 steady_clock::now(), 
Context::current().clone(),
 llvm::None, llvm::None,
 TUScheduler::NoInvalidation, nullptr});


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -992,7 +992,7 @@
std::shared_ptr Preamble,
std::vector CIDiags,
WantDiagnostics WantDiags) {
-  std::string TaskName = llvm::formatv("Build AST for ({0})", PI.Version);
+  llvm::StringLiteral TaskName = "Build AST";
   // Store preamble and build diagnostics with new preamble if requested.
   auto Task = [this, Preamble = std::move(Preamble), CI = std::move(CI),
PI = std::move(PI), CIDiags = std::move(CIDiags),
@@ -1032,7 +1032,7 @@
   }
   {
 std::lock_guard Lock(Mutex);
-PreambleRequests.push_back({std::move(Task), std::move(TaskName),
+PreambleRequests.push_back({std::move(Task), std::string(TaskName),
 steady_clock::now(), Context::current().clone(),
 llvm::None, llvm::None,
 TUScheduler::NoInvalidation, nullptr});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: aaron.ballman, whisperity, alexfh, hokein, JonasToth, 
courbet, ymandel.
Herald added subscribers: carlosgalvezp, martong, rnkovacs, kbarton, 
kristof.beyls, xazax.hun, nemanjai.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Consider this example:

  struct SmallBitfield { unsigned int id : 4; } x;
  x.id << 1;

The corresponding AST looks like this:

  BinaryOperator 'int' '<<'
  |-ImplicitCastExpr 'int' 
  | `-ImplicitCastExpr 'unsigned int' 
  |   `-MemberExpr 'unsigned int' lvalue bitfield .id
  | `-DeclRefExpr 'SmallBitfield' lvalue ParmVar 'x' 'SmallBitfield'
  `-IntegerLiteral 'int' 1

There are two implicit casts:

1. LValueToRValue, loading from the bitfield
2. IntegralCast, casting the 'unsigned int' to 'int' to process the bitshift 
operation

This patch aims to detect this case and ignore it.

The patch might be too restrictive in terms of matching for only this
exact pattern, but I did not find any other similar occurrences of this
kind.


https://reviews.llvm.org/D114105

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
+// RUN:   -std=c++17 -- -target x86_64-unknown-linux
+
+#define CHAR_BITS 8
+static_assert(sizeof(unsigned int) == 32 / CHAR_BITS);
+
+template 
+struct is_same {
+  static constexpr bool value = false;
+};
+template 
+struct is_same {
+  static constexpr bool value = true;
+};
+
+template 
+static constexpr bool is_same_v = is_same::value;
+
+struct NoBitfield {
+  unsigned int id;
+};
+struct SmallBitfield {
+  unsigned int id : 4;
+};
+struct BigBitfield {
+  unsigned int id : 32;
+};
+
+void test_binary_and(SmallBitfield x) {
+  // CHECK-MESSAGES: [[@LINE+1]]:36: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  static_assert(is_same_v);
+  static_assert(is_same_v); // no-warning
+
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  x.id & 1;
+  x.id & 1u; // no-warning
+
+  // CHECK-MESSAGES: [[@LINE+1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  1 & x.id;
+  1u & x.id; // no-warning
+}
+
+void test_binary_or(SmallBitfield x) {
+  // CHECK-MESSAGES: [[@LINE+1]]:36: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  static_assert(is_same_v);
+  static_assert(is_same_v); // no-warning
+
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  x.id | 1;
+  x.id | 1u; // no-warning
+
+  // CHECK-MESSAGES: [[@LINE+1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  1 | x.id;
+  1u | x.id; // no-warning
+}
+
+void test(NoBitfield x) {
+  // no-warning in this function
+  static_assert(is_same_v);
+  static_assert(is_same_v);
+
+  x.id << 1;
+  x.id << 1u;
+  x.id >> 1;
+  x.id >> 1u;
+
+  1 << x.id;
+  1u << x.id;
+  1 >> x.id;
+  1u >> x.id;
+}
+
+void test(SmallBitfield x) {
+  // no-warning in this function: bitfield reads should be ignored within shift operations
+
+  // 'int' is large enough to hold the value of 'x.id', since that could at
+  // most populate 4 bits and an 'int' has 32.
+  static_assert(is_same_v);
+  static_assert(is_same_v);
+
+  x.id << 1;
+  x.id << 1u;
+  x.id >> 1;
+  x.id >> 1u;
+
+  1 << x.id;
+  1u << x.id;
+  1 >> x.id;
+  1u >> x.id;
+}
+
+void test(BigBitfield x) {
+  // no-warning in this function: there is no int -> unsigned conversion here
+
+  static_assert(is_same_v);
+  static_assert(is_same_v);
+
+  x.id << 1;
+  x.id << 1u;
+  x.id >> 1;
+  x.id >> 1u;
+
+  1 << x.id;
+  1u << x.id;
+  1 >> x.id;
+  1u >> x.id;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -83,6 +83,28 @@
 binaryOperator

[PATCH] D114100: [NFC][clang-tools-extra] Inclusive language: replace master with main

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.h:141
   /// the server can send `null` to indicate that the version is known and the
-  /// content on disk is the master (as speced with document content 
ownership).
+  /// content on disk is the main (as speced with document content ownership).
   ///

This is quoting 
https://microsoft.github.io/language-server-protocol/specification.
I think they accept PRs though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114100

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

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added inline comments.



Comment at: llvm/test/CodeGen/X86/no-ret-in-x87-reg.ll:147-150
+; NOX87-NEXT:fldt {{[0-9]+}}(%esp)
+; NOX87-NEXT:fldt {{[0-9]+}}(%esp)
+; NOX87-NEXT:faddp %st, %st(1)
+; NOX87-NEXT:fstpt (%esp)

:(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112143

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

[PATCH] D102449: [WIP][Clang][OpenMP] Add the support for compare clause in atomic directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D102449#3137713 , @carlo.bertolli 
wrote:

> This is already a lot of code with parse+sema. I wonder if we should split 
> the patch into two, i.e. 1. parse+sema; 2. code gen? @ABataev ?
> It should simplify maintenance of the patch and allow time to extend the 
> OpenMP IR builder.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102449

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

[PATCH] D102449: [WIP][Clang][OpenMP] Add the support for compare clause in atomic directive

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

I'll separate them later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102449

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

[PATCH] D114108: [NFC][clang] Inclusive language: rename master variable to controller in debug-info-block-helper.m

[Quinn Pham via Phabricator via cfe-commits]

quinnp created this revision.
quinnp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[NFC] As part of using inclusive language within the llvm project, this patch
replaces master with controller in `debug-info-block-helper.m`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114108

Files:
  clang/test/CodeGenObjC/debug-info-block-helper.m


Index: clang/test/CodeGenObjC/debug-info-block-helper.m
===
--- clang/test/CodeGenObjC/debug-info-block-helper.m
+++ clang/test/CodeGenObjC/debug-info-block-helper.m
@@ -12,17 +12,17 @@
 @interface A:NSObject @end
 @implementation A
 - (void) helper {
- int master = 0;
+ int controller = 0;
  __block int m2 = 0;
  __block int dbTransaction = 0;
  int (^x)(void) = ^(void) { (void) self; 
-   (void) master; 
+   (void) controller; 
(void) dbTransaction; 
m2++;
return m2;
 
};
-  master = x();
+  controller = x();
 }
 @end
 


Index: clang/test/CodeGenObjC/debug-info-block-helper.m
===
--- clang/test/CodeGenObjC/debug-info-block-helper.m
+++ clang/test/CodeGenObjC/debug-info-block-helper.m
@@ -12,17 +12,17 @@
 @interface A:NSObject @end
 @implementation A
 - (void) helper {
- int master = 0;
+ int controller = 0;
  __block int m2 = 0;
  __block int dbTransaction = 0;
  int (^x)(void) = ^(void) { (void) self; 
-	(void) master; 
+	(void) controller; 
 	(void) dbTransaction; 
 	m2++;
 	return m2;
 
 	};
-  master = x();
+  controller = x();
 }
 @end
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114100: [NFC][clang-tools-extra] Inclusive language: replace master with main

[Quinn Pham via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Protocol.h:141
   /// the server can send `null` to indicate that the version is known and the
-  /// content on disk is the master (as speced with document content 
ownership).
+  /// content on disk is the main (as speced with document content ownership).
   ///

sammccall wrote:
> This is quoting 
> https://microsoft.github.io/language-server-protocol/specification.
> I think they accept PRs though.
Thanks @sammccall, I'll remove this change from the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114100

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

[PATCH] D114100: [NFC][clang-tools-extra] Inclusive language: replace master with main

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 387990.
quinnp marked an inline comment as done.
quinnp added a comment.

Addressing review comments. Reverting change to `Protocol.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114100

Files:
  clang-tools-extra/test/modularize/SubModule2.h


Index: clang-tools-extra/test/modularize/SubModule2.h
===
--- clang-tools-extra/test/modularize/SubModule2.h
+++ clang-tools-extra/test/modularize/SubModule2.h
@@ -1,3 +1,3 @@
-// SubModule2.h - Master header with same name as directory.
+// SubModule2.h - Main header with same name as directory.
 #include "SubModule2/Header3.h"
 #include "SubModule2/Header4.h"


Index: clang-tools-extra/test/modularize/SubModule2.h
===
--- clang-tools-extra/test/modularize/SubModule2.h
+++ clang-tools-extra/test/modularize/SubModule2.h
@@ -1,3 +1,3 @@
-// SubModule2.h - Master header with same name as directory.
+// SubModule2.h - Main header with same name as directory.
 #include "SubModule2/Header3.h"
 #include "SubModule2/Header4.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits