[clang] 4954196 - [NFC] Add test of sized deallocation for coroutines

2022-09-05 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-09-06T14:44:16+08:00
New Revision: 495419628bbab1a99cfc00d0c44b877bfc8e4a14

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

LOG: [NFC] Add test of sized deallocation for coroutines

[dcl.fct.def.coroutine]p12 says:

> If both a usual deallocation function with only a pointer parameter
> and a usual deallocation function with both a pointer parameter and a
> size parameter are found, then the selected deallocation function
> shall be the one with two parameters.

However, the sized deallocation function is disabled by default for ABI
reasons. This leads the sentence never get tested and covered. This
commit tries to add a test for it

Added: 
clang/test/CodeGenCoroutines/coro-dealloc.cpp

Modified: 


Removed: 




diff  --git a/clang/test/CodeGenCoroutines/coro-dealloc.cpp 
b/clang/test/CodeGenCoroutines/coro-dealloc.cpp
new file mode 100644
index 0..1f7d04b3689eb
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-dealloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 \
+// RUN:   -S -emit-llvm %s -o - -disable-llvm-passes \
+// RUN:   -fsized-deallocation \
+// RUN:   | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+enum class align_val_t : size_t {};
+}
+
+struct task {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+  };
+};
+
+// Test the compiler will chose sized deallocation correctly.
+// This is only enabled with `-fsized-deallocation` which is off by default.
+void operator delete(void *ptr, std::size_t size) noexcept;
+
+// CHECK: define{{.*}}@_Z1fv
+// CHECK: %[[coro_free:.+]] = call{{.*}}@llvm.coro.free
+// CHECK: coro.free:
+// CHECK: %[[coro_size:.+]] = call{{.*}}@llvm.coro.size
+// CHECK: call{{.*}}void @_ZdlPvm(ptr{{.*}}%[[coro_free]], 
i64{{.*}}%[[coro_size]])
+
+task f() {
+  co_return 43;
+}



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


[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

2022-09-05 Thread Ting Wang via Phabricator via cfe-commits
tingwang created this revision.
tingwang added reviewers: uweigand, wschmidt, PowerPC.
tingwang added a project: clang.
Herald added subscribers: shchenz, kbarton, nemanjai.
Herald added a project: All.
tingwang requested review of this revision.
Herald added a subscriber: cfe-commits.

This is an attempt to fix issue: 
https://github.com/llvm/llvm-project/issues/55900

PPC64_SVR4_ABI handles those by-value aggregate fits in one register using 
coerced integer type.
https://github.com/llvm/llvm-project/blob/51d33afcbe0a81bb8508d5685f38dc9fdb2b60c9/clang/lib/CodeGen/TargetInfo.cpp#L5351

Regarding the issue, the aggregate is passed using i8 as parameter. On 
big-endian, after register content stored to memory, the char locates at 7th 
byte. However current `PPC64_SVR4_ABIInfo::EmitVAArg()` generates argument 
access using the original type, so there is type mismatch between caller and 
callee.

This patch tries to teach `PPC64_SVR4_ABIInfo::EmitVAArg()` regarding the type 
coerce. I'm not sure if this should be fixed in clang or backend, but I guess 
in the clang more likely, since there is logic taking care of argument smaller 
than a slot:
https://github.com/llvm/llvm-project/blob/51d33afcbe0a81bb8508d5685f38dc9fdb2b60c9/clang/lib/CodeGen/TargetInfo.cpp#L356

Please help me review and let me know if any comments. Thank you!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D18

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/PowerPC/ppc64-align-struct.c


Index: clang/test/CodeGen/PowerPC/ppc64-align-struct.c
===
--- clang/test/CodeGen/PowerPC/ppc64-align-struct.c
+++ clang/test/CodeGen/PowerPC/ppc64-align-struct.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -target-feature +altivec -triple 
powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -target-feature +altivec -triple 
powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s 
--check-prefix=CHECK-LE
 
 #include 
 
@@ -9,6 +10,7 @@
 struct test5 { int x[17]; };
 struct test6 { int x[17]; } __attribute__((aligned (16)));
 struct test7 { int x[17]; } __attribute__((aligned (32)));
+struct test8 { char x; };
 
 // CHECK: define{{.*}} void @test1(i32 noundef signext %x, i64 %y.coerce)
 void test1 (int x, struct test1 y)
@@ -132,20 +134,17 @@
 // CHECK: %[[CUR:[^ ]+]] = load i8*, i8** %ap
 // CHECK: %[[NEXT:[^ ]+]] = getelementptr inbounds i8, i8* %[[CUR]], i64 8
 // CHECK: store i8* %[[NEXT]], i8** %ap
-// CHECK: [[T0:%.*]] = bitcast i8* %[[CUR]] to %struct.test8*
+// CHECK: [[SRC:%.*]] = getelementptr inbounds i8, i8* %[[CUR]], i64 7
 // CHECK: [[DEST:%.*]] = bitcast %struct.test8* %[[AGG_RESULT]] to i8*
-// CHECK: [[SRC:%.*]] = bitcast %struct.test8* [[T0]] to i8*
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* align 
8 [[SRC]], i64 1, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* align 
1 [[SRC]], i64 1, i1 false)
 
 // CHECK-LE: define{{.*}} i8 @test8va(i32 noundef signext %x, ...)
 // CHECK-LE: [[RETVAL:%.*]] = alloca %struct.test8
 // CHECK-LE: %[[CUR:[^ ]+]] = load i8*, i8** %ap
 // CHECK-LE: %[[NEXT:[^ ]+]] = getelementptr inbounds i8, i8* %[[CUR]], i64 8
 // CHECK-LE: store i8* %[[NEXT]], i8** %ap
-// CHECK-LE: [[T0:%.*]] = bitcast i8* %[[CUR]] to %struct.test8*
 // CHECK-LE: [[DEST:%.*]] = bitcast %struct.test8* [[RETVAL]] to i8*
-// CHECK-LE: [[SRC:%.*]] = bitcast %struct.test8* [[T0]] to i8*
-// CHECK-LE: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* 
align 8 [[SRC]], i64 1, i1 false)
+// CHECK-LE: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[DEST]], i8* 
align 8 %[[CUR]], i64 1, i1 false)
 // CHECK-LE: [[COERCE:%.*]] = getelementptr inbounds %struct.test8, 
%struct.test8* [[RETVAL]], i32 0, i32 0
 // CHECK-LE: [[RET:%.*]] = load i8, i8* [[COERCE]], align 1
 // CHECK-LE: ret i8 [[RET]]
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5451,6 +5451,22 @@
   return complexTempStructure(CGF, VAListAddr, Ty, SlotSize, EltSize, CTy);
   }
 
+  // An aggregate may end up coerced to integer type in single register. When
+  // DirectSize is less than SlotSize on big-endian, need to use coerced type 
so
+  // that the argument will be right-adjusted in its slot.
+  ABIArgInfo AI = classifyArgumentType(Ty);
+  if (AI.isDirect() && AI.getCoerceToType()) {
+llvm::Type *CoerceTy = AI.getCoerceToType();
+if (CoerceTy->isIntegerTy() &&
+llvm::alignTo(CoerceTy->getIntegerBitWidth(), 8) < GPRBits)
+  return emitVoidPtrDirectVAArg(
+  CGF, VAListAddr, CoerceTy,
+  CharUnits::fromQuantity(
+  llvm::alignTo(CoerceTy->getIntegerBitWidth(), 8) / 8),
+  CharUnits::fromQuantity(AI.getDirectAlign()), SlotSize,
+  /*

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

2022-09-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In D132843#3771029 , @jrtc27 wrote:

> In D132843#3770936 , @efriedma 
> wrote:
>
>>> There is also an issue with how to store and determine the final LTO target 
>>> features. For example, A object built with -march=rv64g and B object built 
>>> with -march=rv64gc, so which arch should we use in the LTO code generation 
>>> stage? see [5] for more discussion around this issue.
>>
>> On other targets, the instruction set used is encoded at a per-function 
>> level.  So on x86, for example, you can have an "AVX" codepath and an "SSE" 
>> codepath, and use runtime CPU detection to figure out which to use.
>
> The IR records that too, but the "default" set of extensions gets encoded in 
> the output file so loaders can know what instruction sets are _required_ (as 
> opposed to optional via IFUNCs). It's not a perfect match, but it's about as 
> good as you can get. With GNU ld these get merged together, though currently 
> LLD just picks the one from the first file and ignores the rest (I think?); 
> ideally the LLVM module linker would do similar merging.

Oh, I see.  Most other targets don't have ELF attributes like that.  32-bit ARM 
does, but I'm not sure how the interaction with LTO works there off the top of 
my head.

We probably want to encode the architecture into the IR in some way that allows 
us to produce the same attributes whether or not LTO is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132843

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


[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

2022-09-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma planned changes to this revision.
efriedma added a comment.

Just realized I forgot to add tests for va_arg.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:2457
+/*IsVectorCall=*/false, /*IsRegCall=*/false);
+  }
+

rjmccall wrote:
> efriedma wrote:
> > rjmccall wrote:
> > > Hmm.  Doesn't EC ABI lowering need to preserve this same state, or else 
> > > you'll get incompatibilities when you exhaust SSE registers?
> > > 
> > > Should you just be calling over to this at a higher-level point, like in 
> > > `computeInfo`?
> > If both IsVectorCall and IsRegCall are false, WinX86_64ABIInfo doesn't use 
> > FreeSSERegs, so we don't need to preserve its state.  There isn't any other 
> > relevant state to preserve.
> > 
> > Return values follow the Arm64 ABI rules, not the x64 rules, so using 
> > computeInfo() as an entry point doesn't really make sense.
> That's odd.  I assume this whole thing is about matching the CC you would get 
> from an instruction-by-instruction translation of x86_64 assembly so that 
> arm64-compiled code can call / be called from translated assembly as long as 
> it uses this special CC.  Are there just no functions with interesting return 
> types that anybody happens to care about calling across such a boundary?  I 
> would be amazed if the standard arm64 and x64 return rules just happily match 
> for all types.
Normally, arm64ec code uses the arm64 calling convention, and x64 code in the 
same process uses the x64 calling convention.  When there are calls between the 
two, a thunk translates the calling convention.  (See D126811 etc.)

For varargs, things work slightly differently.  It's impossible to translate an 
arbitrary arm64 varargs call to an x64 varargs call, or vice versa, because 
then thunk doesn't know how the varargs arguments are laid out.  (Also, va_list 
has to be ABI-compatible.)  So varargs calls use a special calling convention 
that ensures varargs arguments in arm64ec calls are laid out exactly the same 
way as x64 varargs arguments.

There are still thunks for varargs calls, though; among other things, they 
translates the return value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125419

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


[PATCH] D133336: [msan] Convert Msan to ModulePass

2022-09-05 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka updated this revision to Diff 458098.
vitalybuka added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D16

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/attributes.ll
  llvm/test/Instrumentation/MemorySanitizer/check-array.ll
  llvm/test/Instrumentation/MemorySanitizer/check-constant-shadow.ll
  llvm/test/Instrumentation/MemorySanitizer/check-struct.ll
  llvm/test/Instrumentation/MemorySanitizer/msan-disable-checks.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_debug_info.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/reduce.ll
  llvm/test/Other/new-pm-print-pipeline.ll

Index: llvm/test/Other/new-pm-print-pipeline.ll
===
--- llvm/test/Other/new-pm-print-pipeline.ll
+++ llvm/test/Other/new-pm-print-pipeline.ll
@@ -40,8 +40,8 @@
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(early-cse<>,early-cse)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-12
 ; CHECK-12: function(early-cse<>,early-cse)
 
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan-module,function(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
-; CHECK-13: msan-module,function(msan,msan,msan)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan,module(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
+; CHECK-13: msan,msan,msan,msan
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='module(hwasan<>,hwasan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-14
 ; CHECK-14: hwasan<>,hwasan
Index: llvm/test/Instrumentation/MemorySanitizer/reduce.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/reduce.ll
+++ llvm/test/Instrumentation/MemorySanitizer/reduce.ll
@@ -1,5 +1,5 @@
 
-; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan-module),function(msan)' 2>&1 | \
+; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan)' 2>&1 | \
 ; RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=CHECK %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=CHECK %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=CHECK %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
+++ llvm/t

[PATCH] D133336: [msan] Convert Msan to ModulePass

2022-09-05 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka updated this revision to Diff 458097.
vitalybuka added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D16

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/attributes.ll
  llvm/test/Instrumentation/MemorySanitizer/check-array.ll
  llvm/test/Instrumentation/MemorySanitizer/check-constant-shadow.ll
  llvm/test/Instrumentation/MemorySanitizer/check-struct.ll
  llvm/test/Instrumentation/MemorySanitizer/msan-disable-checks.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_debug_info.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/reduce.ll
  llvm/test/Other/new-pm-print-pipeline.ll

Index: llvm/test/Other/new-pm-print-pipeline.ll
===
--- llvm/test/Other/new-pm-print-pipeline.ll
+++ llvm/test/Other/new-pm-print-pipeline.ll
@@ -40,8 +40,8 @@
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(early-cse<>,early-cse)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-12
 ; CHECK-12: function(early-cse<>,early-cse)
 
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan-module,function(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
-; CHECK-13: msan-module,function(msan,msan,msan)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan,module(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
+; CHECK-13: msan,msan,msan,msan
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='module(hwasan<>,hwasan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-14
 ; CHECK-14: hwasan<>,hwasan
Index: llvm/test/Instrumentation/MemorySanitizer/reduce.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/reduce.ll
+++ llvm/test/Instrumentation/MemorySanitizer/reduce.ll
@@ -1,5 +1,5 @@
 
-; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan-module),function(msan)' 2>&1 | \
+; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan)' 2>&1 | \
 ; RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=CHECK %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=CHECK %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=CHECK %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
+++ llvm/test/In

[PATCH] D133336: [msan] Convert Msan to ModulePass

2022-09-05 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka created this revision.
Herald added subscribers: Enna1, ormris, jdoerfert, hiraditya.
Herald added a project: All.
vitalybuka requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

MemorySanitizerPass function pass violatied requirement 4 of function
pass to do not insert globals. Msan nees to insert globals for origin
tracking, and paramereters tracking.

https://llvm.org/docs/WritingAnLLVMPass.html#the-functionpass-class


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D16

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/attributes.ll
  llvm/test/Instrumentation/MemorySanitizer/check-array.ll
  llvm/test/Instrumentation/MemorySanitizer/check-constant-shadow.ll
  llvm/test/Instrumentation/MemorySanitizer/check-struct.ll
  llvm/test/Instrumentation/MemorySanitizer/msan-disable-checks.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_debug_info.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_eager.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
  llvm/test/Instrumentation/MemorySanitizer/reduce.ll
  llvm/test/Other/new-pm-print-pipeline.ll

Index: llvm/test/Other/new-pm-print-pipeline.ll
===
--- llvm/test/Other/new-pm-print-pipeline.ll
+++ llvm/test/Other/new-pm-print-pipeline.ll
@@ -40,8 +40,8 @@
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(early-cse<>,early-cse)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-12
 ; CHECK-12: function(early-cse<>,early-cse)
 
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan-module,function(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
-; CHECK-13: msan-module,function(msan,msan,msan)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='msan,module(msan,msan<>,msan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-13
+; CHECK-13: msan,msan,msan,msan
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='module(hwasan<>,hwasan)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-14
 ; CHECK-14: hwasan<>,hwasan
Index: llvm/test/Instrumentation/MemorySanitizer/reduce.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/reduce.ll
+++ llvm/test/Instrumentation/MemorySanitizer/reduce.ll
@@ -1,5 +1,5 @@
 
-; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan-module),function(msan)' 2>&1 | \
+; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan)' 2>&1 | \
 ; RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=CHECK %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_strip_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=CHECK %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
===
--- llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
+++ llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll
@@ -1,7 +1,7 @@
 ; Make sure MSan handles llvm.launder.invariant.group correctly.
 
-; RUN: opt < %s -passes='function(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
-; RUN: opt < %s -passes='function(msan),default' -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -msan-kernel=1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -passes='module(msan),default' -S | FileCheck -check-prefixes=

[PATCH] D133248: [clang] Fix crash upon stray coloncolon token in C2x mode

2022-09-05 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:863
   bool MightBeCXXScopeToken() {
-return Tok.is(tok::identifier) || Tok.is(tok::coloncolon) ||
-   (Tok.is(tok::annot_template_id) &&
-NextToken().is(tok::coloncolon)) ||
-   Tok.is(tok::kw_decltype) || Tok.is(tok::kw___super);
+return (getLangOpts().CPlusPlus) &&
+   (Tok.is(tok::identifier) || Tok.is(tok::coloncolon) ||




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133248

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


[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/include/llvm/Support/CommandLine.h:2132
+
+  bool getSearchAsConfig() const { return SearchAsConfig; }
+  ExpansionContext &setSearchAsConfig(bool X) {

Unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133325

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2022-09-05 Thread Guillot Tony via Phabricator via cfe-commits
to268 updated this revision to Diff 458092.
to268 added a comment.

Fixed `DeclSpec.cpp` formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/test/C/C2X/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1193,7 +1193,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007
-  No
+  Yes
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+#include 
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_structs(void) {
+  struct s_auto { auto a; };  // expected-error {{'auto' not allowed in struct member}}
+  auto s_int = (struct { int a; } *)0;
+  typedef auto auto_type; // expected-error {{'auto' not allowed in typedef}}
+}
+
+void test_sizeof_alignas(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  alignas(4) auto b[4];   // expected-error {{'b' declared as array of 'auto'}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 'auto'}}
+}
+
+void test_qualifiers(int x, const int y) {
+  const auto a = x;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa = &a; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = &b;
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+  auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+int a;
+int b;
+union {
+  char c;
+  auto smth; // c2x-error {{'auto' not allowed in union member}} \
+c17-error {{type name does not allow storage class to be specified}} \
+c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+} u;
+};
+
+enum E : auto { // c2x-error {{'auto' not allowed here}} \
+   c17-error {{expected a type}} \
+   c17-error {{type name does not allow storage class to be specified}}
+  One,
+  Two,
+  Tree,
+};
+
+auto auto_usage(auto auto) {  // c2x-error {{'auto' not allowed in function prototype}} \
+ c2x-error {{'auto' not allowed in function return type}} \
+ c2x-error {{cannot combine with previous 'auto' declaration specifier}} \
+ c17-error {{invalid storage class specifier in function declarator}} \
+ c17-error {{illegal storage class on function}} \
+ c17-warning {{duplicate 'auto' declaration specifier}} \
+ c17-warning {{omitting the parameter name in a function definition is a C2x extension}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+
+  auto = 4;   // expected-error {{expected identifier or '('}}
+
+  auto a = 4; // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+
+  auto b[4];  // c2x-error {{'b' declared as array of 'auto'}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not sup

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> Normally a file included into configuration file by directive @file is
> searched for relative the including configuration file. In some cases it
> is not convenient.

Sorry, I am puzzled by the description. A configuration file may specify 
`--option=file` and `@cfg`.
Can you clarify which "file" you mean?

> Configuration parameters may be logically partitioned
> into caterories (like 'platform variant' and 'processor variant') and it
> is convenient to have different configuration files for these
> categories maintained separately. Similar motivation was described in
> https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606.
> Simple file inclusion by @file does not help, because in this case the
> path to file is specified relative to the directory of the used
> configuration file, but it actually is not dependent on the latter. Using
> absolute paths is not convenient and it still do not allow user to
> easily override the settings.

If the intention is to have `@cfg` in a configuration file to be resolved
relative to `--config-system-dir` or `--config-user-dir=`. Please describe the 
new semantics,
then mention the rationale. An example in `clang/docs/UsersManual.rst` will 
help.




Comment at: clang/include/clang/Driver/Options.td:909
+def search_config_dirs : Flag<["--"], "search-config-dirs">, 
Flags<[NoXarchOption]>,
+  HelpText<"Search for files included by configuration file is search 
directories">;
 def coverage : Flag<["-", "--"], "coverage">, Group, 
Flags<[CoreOption]>;

I am confused by the help text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133325

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2022-09-05 Thread Guillot Tony via Phabricator via cfe-commits
to268 updated this revision to Diff 458089.
to268 added a comment.

I have fixed my CodeGen test with the windows platform.
(Linux uses an i64 for the long type instead windows is using an i32)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/test/C/C2X/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1193,7 +1193,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007
-  No
+  Yes
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+#include 
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_structs(void) {
+  struct s_auto { auto a; };  // expected-error {{'auto' not allowed in struct member}}
+  auto s_int = (struct { int a; } *)0;
+  typedef auto auto_type; // expected-error {{'auto' not allowed in typedef}}
+}
+
+void test_sizeof_alignas(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  alignas(4) auto b[4];   // expected-error {{'b' declared as array of 'auto'}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 'auto'}}
+}
+
+void test_qualifiers(int x, const int y) {
+  const auto a = x;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa = &a; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = &b;
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+  auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+int a;
+int b;
+union {
+  char c;
+  auto smth; // c2x-error {{'auto' not allowed in union member}} \
+c17-error {{type name does not allow storage class to be specified}} \
+c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+} u;
+};
+
+enum E : auto { // c2x-error {{'auto' not allowed here}} \
+   c17-error {{expected a type}} \
+   c17-error {{type name does not allow storage class to be specified}}
+  One,
+  Two,
+  Tree,
+};
+
+auto auto_usage(auto auto) {  // c2x-error {{'auto' not allowed in function prototype}} \
+ c2x-error {{'auto' not allowed in function return type}} \
+ c2x-error {{cannot combine with previous 'auto' declaration specifier}} \
+ c17-error {{invalid storage class specifier in function declarator}} \
+ c17-error {{illegal storage class on function}} \
+ c17-warning {{duplicate 'auto' declaration specifier}} \
+ c17-warning {{omitting the parameter name in a function definition is a C2x extension}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+
+  auto = 4;   // expected-error {{expected identifier or '('}}
+
+  auto a = 4; // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+
+  auto b[4];  // c2x-error {{'b' declared as array of 'auto'}} \
+  

[clang] 7628f19 - [NFC] Remove invisible character in Diagnostic message and tests

2022-09-05 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-09-06T11:08:42+08:00
New Revision: 7628f19262e441b438a69176ec5229d8ccb96124

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

LOG: [NFC] Remove invisible character in Diagnostic message and tests

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/SemaCXX/coroutine-alloc-3.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 66cf0d8107934..15a4e2aefdb79 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11273,7 +11273,7 @@ def err_coroutine_unusable_new : Error<
   "'operator new' provided by %0 is not usable with the function signature of 
%1"
 >;
 def err_coroutine_unfound_nothrow_new : Error <
-  "unable to find '::operator new(size_­t, nothrow_­t)' for %0"
+  "unable to find '::operator new(size_t, nothrow_t)' for %0"
 >;
 } // end of coroutines issue category
 

diff  --git a/clang/test/SemaCXX/coroutine-alloc-3.cpp 
b/clang/test/SemaCXX/coroutine-alloc-3.cpp
index 6e47d36712f3f..629ac88a3df8e 100644
--- a/clang/test/SemaCXX/coroutine-alloc-3.cpp
+++ b/clang/test/SemaCXX/coroutine-alloc-3.cpp
@@ -46,6 +46,6 @@ struct std::coroutine_traits {
   };
 };
 
-extern "C" int f(promise_on_alloc_failure_tag) { // expected-error 1+{{unable 
to find '::operator new(size_­t, nothrow_­t)' for 'f'}}
+extern "C" int f(promise_on_alloc_failure_tag) { // expected-error 1+{{unable 
to find '::operator new(size_t, nothrow_t)' for 'f'}}
 co_return;
 }



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


[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

2022-09-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:2457
+/*IsVectorCall=*/false, /*IsRegCall=*/false);
+  }
+

efriedma wrote:
> rjmccall wrote:
> > Hmm.  Doesn't EC ABI lowering need to preserve this same state, or else 
> > you'll get incompatibilities when you exhaust SSE registers?
> > 
> > Should you just be calling over to this at a higher-level point, like in 
> > `computeInfo`?
> If both IsVectorCall and IsRegCall are false, WinX86_64ABIInfo doesn't use 
> FreeSSERegs, so we don't need to preserve its state.  There isn't any other 
> relevant state to preserve.
> 
> Return values follow the Arm64 ABI rules, not the x64 rules, so using 
> computeInfo() as an entry point doesn't really make sense.
That's odd.  I assume this whole thing is about matching the CC you would get 
from an instruction-by-instruction translation of x86_64 assembly so that 
arm64-compiled code can call / be called from translated assembly as long as it 
uses this special CC.  Are there just no functions with interesting return 
types that anybody happens to care about calling across such a boundary?  I 
would be amazed if the standard arm64 and x64 return rules just happily match 
for all types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125419

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-05 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp:1
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" 
-- -fno-delayed-template-parsing
 

felix642 wrote:
> Eugene.Zelenko wrote:
> > I think test should be separated to handle situations with and without 
> > option.
> Hi @Eugene.Zelenko, 
> 
> I'm not familiar with clang-tidy's testing environment. What do you mean 
> precisely by "test should be separated"? Does it mean I should define this 
> test in a different .cpp with the appropriate tests?  
I meant that dedicated test for new check option should be created. But will be 
good idea to expand original one, so difference in behavior could be observed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133308: [cmake] do not set execution permission to regular files

2022-09-05 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133308

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


[PATCH] D133087: [clang-format] [doc] Fix example of wrapping class definitions

2022-09-05 Thread passw_passw via Phabricator via cfe-commits
Passw added a comment.

In D133087#3769628 , 
@HazardyKnusperkeks wrote:

> In D133087#3768928 , @Passw wrote:
>
>> I do not have commit access, please help to commit this change. 
>> Thanks.
>
> Please state a name and email for the commit.

Passw
passw_pa...@outlook.com


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

https://reviews.llvm.org/D133087

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-05 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 458073.
felix642 added a comment.

Improved readability of release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" 
-- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -155,3 +155,25 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for 
accessing the data pointer instead of taking the address of the 0-th element 
[readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return holder.v.data();{{$}}
 }
+
+template
+struct ArrayType {
+  using size_type = size_t;
+
+  ArrayType();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+
+private:
+  T _value[N];
+};
+
+// Don't issue a warning because of the config above.
+int *s() {
+  ArrayType s;
+  return &s[0];
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
   The check now skips unions since in this case a default constructor with 
empty body
   is not equivalent to the explicitly defaulted one.
 
+- Improved `readability-container-data-pointer 
` check.
+
+  The check now skips containers that are defined in the option 
`IgnoredContainers`.  The default value is `::std::array`.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" -- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-05 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp:1
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" 
-- -fno-delayed-template-parsing
 

Eugene.Zelenko wrote:
> I think test should be separated to handle situations with and without option.
Hi @Eugene.Zelenko, 

I'm not familiar with clang-tidy's testing environment. What do you mean 
precisely by "test should be separated"? Does it mean I should define this test 
in a different .cpp with the appropriate tests?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D127082: [clangd] Add Macro Expansion to Hover

2022-09-05 Thread Qingyuan Zheng via Phabricator via cfe-commits
daiyousei-qz added a comment.

In D127082#3770763 , @sammccall wrote:

> Sorry for the delay :-( This looks great!
>
> It looks like this is your first patch, so you don't have commit access. I 
> can land the patch for you.
> Can you provide an email address for attribution?

I had a patch for clang which Nathan had merged for me. Probably it's because 
of the lack of the metadata since I was using the web interface to submit 
differentials. Anyway, here's the info:
Qingyuan Zheng qyzhe...@outlook.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127082

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


[PATCH] D130308: [clang] extend getCommonSugaredType to merge sugar nodes

2022-09-05 Thread David Rector via Phabricator via cfe-commits
davrec accepted this revision.
davrec added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130308

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-05 Thread David Rector via Phabricator via cfe-commits
davrec accepted this revision.
davrec added a comment.

I hope I'm not stepping on any toes given the recent changes in code ownership, 
but I'm accepting this and D130308  because

1. They are a nice improvement to the AST that naturally follows from the 
earlier work adding sugar to type deductions,
2. I have given it a fairly close look and it LGTM,
3. @rsmith has taken a step back and so may not be available to give it a final 
look,
4. @mizvekov has verified that the performance impact is negligible, and
5. Other reviewers seem to have at least given it a once over and have not 
identified major issues.

Anyone object/need more time to review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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


[PATCH] D133329: [Driver] Add --gcc-install-dir=

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 458065.
MaskRay added a comment.

improve test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133329

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/gcc-install-dir.cpp

Index: clang/test/Driver/gcc-install-dir.cpp
===
--- /dev/null
+++ clang/test/Driver/gcc-install-dir.cpp
@@ -0,0 +1,43 @@
+// UNSUPPORTED: system-windows
+
+/// Test native GCC installation on Arch Linux i686.
+// RUN: %clang -### %s --target=i686-linux-gnu --sysroot=%S/Inputs/archlinux_i686_tree -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN:   --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/archlinux_i686_tree/usr/lib/gcc/i686-pc-linux-gnu/11.1.0 2>&1 | FileCheck %s --check-prefix=ARCH_I686
+// ARCH_I686:  "-internal-isystem"
+// ARCH_I686-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/i686-pc-linux-gnu"
+// ARCH_I686:  "-L
+// ARCH_I686-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+/// Test native GCC installation on Debian amd64. --gcc-install-dir= may end with /.
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// DEBIAN_X86_64:  "-internal-isystem"
+// DEBIAN_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/x86_64-linux-gnu/c++/10"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/backward"
+/// We set explicit -ccc-install-dir ensure that Clang does not pick up extra
+/// library directories which may be present in the runtimes build.
+// DEBIAN_X86_64:  "-L
+// DEBIAN_X86_64-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10"
+// DEBIAN_X86_64-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../lib64"
+
+/// Test -m32.
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu -m32 --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64_M32
+// DEBIAN_X86_64_M32:  "-internal-isystem"
+// DEBIAN_X86_64_M32-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// DEBIAN_X86_64_M32-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/x86_64-linux-gnu/c++/10/32"
+// DEBIAN_X86_64_M32:  "-L
+// DEBIAN_X86_64_M32-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/32"
+// DEBIAN_X86_64_M32-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../lib32"
+
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu 2>&1 | FileCheck %s --check-prefix=INVALID
+// INVALID: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu' does not contain a GCC installation
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1997,6 +1997,28 @@
CandidateTripleAliases, CandidateBiarchLibDirs,
CandidateBiarchTripleAliases);
 
+  // If --gcc-install-dir= is specified, skip filesystem detection.
+  if (const Arg *A =
+  Args.getLastArg(clang::driver::options::OPT_gcc_install_dir_EQ);
+  A && A->getValue()[0]) {
+StringRef InstallDir = A->getValue();
+if (!ScanGCCForMultilibs(TargetTriple, Args, InstallDir, false)) {
+  D.Diag(diag::err_drv_invalid_gcc_install_dir) << InstallDir;
+} else {
+  (void)InstallDir.consume_back("/");
+  StringRef VersionText = llvm::sys

[PATCH] D133329: [Driver] Add --gcc-install-dir=

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
Herald added subscribers: StephenFan, pengfei.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option specifies a GCC installation directory such as
/usr/lib/gcc/x86_64-linux-gnu/12, /usr/lib/gcc/x86_64-gentoo-linux-musl/11.2.0 .

It is intended to replace --gcc-toolchain=, which specifies a directory where
`lib/gcc{,-cross}` can be found. In most cases the user does not specify
--gcc-toolchain=. When --gcc-toolchain= is specified, the selected
`lib/gcc/$triple/$version` installation may not be desired.

D25661  added gcc-config detection for Gentoo: 
`ScanGentooConfigs`.
The implementation may be simplified by using --gcc-install-dir=.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133329

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/gcc-install-dir.cpp

Index: clang/test/Driver/gcc-install-dir.cpp
===
--- /dev/null
+++ clang/test/Driver/gcc-install-dir.cpp
@@ -0,0 +1,41 @@
+// UNSUPPORTED: system-windows
+
+/// Test native GCC installation on Arch Linux i686.
+// RUN: %clang -### %s --target=i686-linux-gnu --sysroot=%S/Inputs/archlinux_i686_tree -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN:   -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/archlinux_i686_tree/usr/lib/gcc/i686-pc-linux-gnu/11.1.0 2>&1 | FileCheck %s --check-prefix=ARCH_I686
+// ARCH_I686:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// ARCH_I686:  "-internal-isystem"
+// ARCH_I686-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/i686-pc-linux-gnu"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/backward"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+/// This resolves to /usr/i686-linux-gnu/include. Because it does not exist,
+/// having it does no harm albeit not ideal.
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../i686-pc-linux-gnu/include"
+// ARCH_I686:  "-internal-externc-isystem"
+// ARCH_I686-SAME: {{^}} "[[SYSROOT]]/include"
+// ARCH_I686-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// ARCH_I686:  "-L
+// ARCH_I686-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+/// Test native GCC installation on Debian amd64.
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN:   --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// DEBIAN_X86_64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// DEBIAN_X86_64:  "-internal-isystem"
+// DEBIAN_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/x86_64-linux-gnu/c++/10"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/backward"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../x86_64-linux-gnu/include"
+/// We set explicit -ccc-install-dir ensure that Clang does not pick up extra
+/// library directories which may be present in the runtimes build.
+// DEBIAN_X86_64:  "-L
+// DEBIAN_X86_64-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10"
+// DEBIAN_X86_64-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../lib64"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1997,6 +1997,29 @@
CandidateTripleAliases, CandidateBiarchLibDirs,
CandidateBiarchTripleAliases);
 
+  // If --gcc-install-dir= is specified, skip filesystem detection.
+  if (const Arg *A =
+  Args.getLastArg(clang::driver::options::OP

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

2022-09-05 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

In D132843#3770936 , @efriedma wrote:

>> So we need to keep ABI in somewhere and read that at LTO phase, the most 
>> ideal place is the module flags. We already did that[6], but that comes with 
>> a problem is it's too late to update datalayout when we start to read a 
>> module, because LLVM will use datalayout to infer some info like the 
>> alignment of loads[7], and that means we might re-read the whole LLVM IR 
>> again to get the IR with the right info, and that requires fixing multiple 
>> places in LLVM (see[2]. Still, I am not sure it's enough or not).
>
> I'm not sure how the issues with datalayout in particular end up being an 
> issue in practice.

Maybe for IR DataLayout upgrades, but those are rather rare.

> - clang shouldn't be writing out object files without a datalayout.
> - The code to infer alignment on load/store/etc. only exists for 
> compatibility with old bitcode/IR; current LLVM bitcode always marks up 
> alignment for all operations.
>
> Or do you mean something else when you say "datalayout"?
>
>> There is also an issue with how to store and determine the final LTO target 
>> features. For example, A object built with -march=rv64g and B object built 
>> with -march=rv64gc, so which arch should we use in the LTO code generation 
>> stage? see [5] for more discussion around this issue.
>
> On other targets, the instruction set used is encoded at a per-function 
> level.  So on x86, for example, you can have an "AVX" codepath and an "SSE" 
> codepath, and use runtime CPU detection to figure out which to use.

The IR records that too, but the "default" set of extensions gets encoded in 
the output file so loaders can know what instruction sets are _required_ (as 
opposed to optional via IFUNCs). It's not a perfect match, but it's about as 
good as you can get. With GNU ld these get merged together, though currently 
LLD just picks the one from the first file and ignores the rest (I think?); 
ideally the LLVM module linker would do similar merging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132843

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D132131#3771005 , 
@HazardyKnusperkeks wrote:

> In D132131#3770170 , 
> @MyDeveloperDay wrote:
>
>> My personal preference is a new struct without Enabled
>
> We go for a new struct. But why without enabled?
> Currently we have a boolean on/off switch `AlignTrailingComments`. This 
> change wanted to add a second boolean to enable aligning ignoring empty 
> lines, naturally its value only matters if `AlignTrailingComments` is `true`.
> My request was to use the existing struct (and I'm still in favor of that, 
> but not strong enough to fight anything or anybody over it), but a struct 
> seems to be better than 2 booleans. But an on/off switch still needs to be 
> there, and at least using the same name as other options `Enabled` seems to 
> be the most consistent solution.

We don’t normally use Enabled for other structs, maybe we don’t use bool but 
enums (Always, Never, Leave)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D132131#3770170 , @MyDeveloperDay 
wrote:

> My personal preference is a new struct without Enabled

We go for a new struct. But why without enabled?
Currently we have a boolean on/off switch `AlignTrailingComments`. This change 
wanted to add a second boolean to enable aligning ignoring empty lines, 
naturally its value only matters if `AlignTrailingComments` is `true`.
My request was to use the existing struct (and I'm still in favor of that, but 
not strong enough to fight anything or anybody over it), but a struct seems to 
be better than 2 booleans. But an on/off switch still needs to be there, and at 
least using the same name as other options `Enabled` seems to be the most 
consistent solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132867: [Clang] Use virtual FS in processing config files

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D132867#3770898 , @sepavloff wrote:

> It seems that compatibility issue can appear if someone use VFS for all files 
> but configuration one, which reside in real FS. It is inconvenient and hard 
> to maintain

I agree, the situation is overall better after this change.

> probably nobody uses such combination, otherwise someone filed a bug or 
> prepared a patch.

I can tell you from experience this doesn't always hold. It's unreasonable to 
expect users to know that the observed behavior is incorrect.

> Strictly speaking this is incorrect behavior because file system object must 
> be used for all operations on files, according to documentation.

I don't think this is a strong argument against documenting the change (is that 
what we're disagreeing about?)

- what documentation? (E.g. Driver's constructor is undocumented)
- actual behavior trumps documentation: a breaking change that brings behavior 
into line with docs is still breaking
- it's not true in any case that all clang IO goes through the VFS, as a silly 
example consider plugin loading with `-Xclang -load`.
- in practice many users who rely on these details work it out by trial and 
error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132867

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


[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

2022-09-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added reviewers: rjmccall, mstorsjo, dpaoliello, rnk.
efriedma added a comment.

I think I'd like to continue moving forward with approximately this approach, 
at least for the moment.  As far as I know, D132926 
 solves the remaining issues with translating 
the calling conventions.  (I'll try to review D132926 
 soon.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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


[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

2022-09-05 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments.



Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+

Mordante wrote:
> mstorsjo wrote:
> > I don't understand this paragraph - each CI run is run through the 
> > configured set of supported Clang versions - not only `main`? Or does this 
> > talk about specifics about manually running tests with the Docker image?
> This is the version of Clang in the main branch. How about:
> `Unless specified otherwise the nightly build of Clang from the ``main`` 
> branch is used.`
I still don't quite understand what this tries to say. "Unless specified 
otherwise" - where would I specify a different preference, when I upload a 
patch and it gets run through CI?

There's three different concepts involved here:
- The CI build matrix (buildkite-pipeline.yml) which specifies a bunch of 
different versions of tools and standards to run the tests on. Here you can't 
specify a different preference - all of them are run (unless you edit the file 
in your patch).
- The Docker images, where the default `clang` executable is a recent nightly 
build, but older releases are available as `clang-`
- The `run-buildbot` script, which just uses whatever compiler is set as 
default (via e.g. the `CXX` env var).

So, what about these three concepts is this paragraph trying to say - the 
second or the third point? This really needs to specify what it talks about - 
build matrix, docker images or run-buildbot script.

The same thing goes for the following paragraph.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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


[PATCH] D133262: [clang] Represent __make_integer_seq as alias template in the AST

2022-09-05 Thread David Rector via Phabricator via cfe-commits
davrec accepted this revision.
davrec added a comment.

LGTM aside from a nit




Comment at: clang/include/clang/AST/DeclTemplate.h:455
 
+  bool isTypeAlias() const;
+

Add doc, e.g. "Whether this is a written or built-in type alias template."

And nit: maybe move this above the `classof` functions, since those and other 
boilerplate functions are usually the last public members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133262

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


[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

2022-09-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> So we need to keep ABI in somewhere and read that at LTO phase, the most 
> ideal place is the module flags. We already did that[6], but that comes with 
> a problem is it's too late to update datalayout when we start to read a 
> module, because LLVM will use datalayout to infer some info like the 
> alignment of loads[7], and that means we might re-read the whole LLVM IR 
> again to get the IR with the right info, and that requires fixing multiple 
> places in LLVM (see[2]. Still, I am not sure it's enough or not).

I'm not sure how the issues with datalayout in particular end up being an issue 
in practice.

- clang shouldn't be writing out object files without a datalayout.
- The code to infer alignment on load/store/etc. only exists for compatibility 
with old bitcode/IR; current LLVM bitcode always marks up alignment for all 
operations.

Or do you mean something else when you say "datalayout"?

> There is also an issue with how to store and determine the final LTO target 
> features. For example, A object built with -march=rv64g and B object built 
> with -march=rv64gc, so which arch should we use in the LTO code generation 
> stage? see [5] for more discussion around this issue.

On other targets, the instruction set used is encoded at a per-function level.  
So on x86, for example, you can have an "AVX" codepath and an "SSE" codepath, 
and use runtime CPU detection to figure out which to use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132843

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


[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-05 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

This change also broken emscripten in some odd ways: 
https://app.circleci.com/pipelines/github/emscripten-core/emscripten/23359/workflows/4080be5f-bd82-45b9-a355-3a5d4f4ef977/jobs/564543.
   The revert seems to have fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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


[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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


[PATCH] D132867: [Clang] Use virtual FS in processing config files

2022-09-05 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment.

In D132867#3769791 , @sammccall wrote:

> This seems like the right thing in principle, but it's still pretty scary:
>
> - this is going to break anyone who's using VFS != RealFS together with 
> config files, as we no longer search the old locations
> - the breakage is likely to be quiet/subtle, especially if it's e.g. an 
> obscure flag being set in an arch-specific config file
> - the fixes for "part of the toolchain is missing from VFS" tend to need code 
> changes in tools and tend to be hard to test
>
> The thing the change has going for it is that I think config files and 
> nontrivial VFS are both pretty rarely used, so maybe there are few people 
> using them together.
> (Google uses both, but having looked internally I don't believe we ever 
> actually combine them).

It seems that compatibility issue can appear if someone use VFS for all files 
but configuration one, which reside in real FS. It is inconvenient and hard to 
maintain, so probably nobody uses such combination, otherwise someone filed a 
bug or prepared a patch. Strictly speaking this is incorrect behavior because 
file system object must be used for all operations on files, according to 
documentation.




Comment at: llvm/lib/Support/CommandLine.cpp:1268
+  if (!CurrentDir) {
+if (auto CWD = FS.getCurrentWorkingDirectory()) {
+  CurrDir = *CWD;

sammccall wrote:
> the old behavior was explicitly documented ("process' cwd is used instead", 
> not FS's) so that documentation should be updated.
> 
> (I just talked to @kadircet about the patch that added that documentation, 
> and we can't think of any reason that the old behavior is actually desirable)
You are right, I fixed that documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132867

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2930
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"

Why not verifyFormat here too and below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.
Closed by commit rG91d8324366f4: [MinGW] Reject explicit non-default visibility 
applied to dllexport/dllimport… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133266

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/dllstorage-visibility.cpp


Index: clang/test/CodeGenCXX/dllstorage-visibility.cpp
===
--- clang/test/CodeGenCXX/dllstorage-visibility.cpp
+++ clang/test/CodeGenCXX/dllstorage-visibility.cpp
@@ -4,10 +4,18 @@
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec 
-fvisibility-inlines-hidden -o - %s | FileCheck %s --check-prefix=GNU
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec 
-fvisibility=hidden -o - %s | FileCheck %s --check-prefix=GNU
 
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -DERR1 -o - %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -fvisibility=hidden -DERR1 -o - %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -DERR2 -o - %s
+
 #define CONCAT2(x, y) x##y
 #define CONCAT(x, y) CONCAT2(x, y)
 #define USE(func) void CONCAT(use, __LINE__)() { func(); }
 
+#define HIDDEN __attribute__((visibility("hidden")))
+#define PROTECTED __attribute__((visibility("protected")))
+#define DEFAULT __attribute__((visibility("default")))
+
 // MSVC-DAG: declare dllimport void @"?bar@foo@@QEAAXXZ"(
 // GNU-DAG: define linkonce_odr hidden void @_ZN3foo3barEv(
 
@@ -24,3 +32,17 @@
 // GNU-DAG: define weak_odr dso_local dllexport void @_Z15exported_inlinev(
 __declspec(dllexport) inline void exported_inline() {}
 USE(exported_inline)
+
+#if defined(ERR1)
+// expected-error@+1 {{non-default visibility cannot be applied to 'dllimport' 
declaration}}
+__attribute__((dllimport)) HIDDEN void imported_hidden();
+
+__attribute__((dllexport)) DEFAULT void exported_default() {}
+// expected-error@+1 {{non-default visibility cannot be applied to 'dllexport' 
declaration}}
+__attribute__((dllexport)) HIDDEN void exported_hidden() { imported_hidden(); }
+#elif defined(ERR2)
+struct PROTECTED C {
+  // expected-error@+1 {{non-default visibility cannot be applied to 
'dllexport' declaration}}
+  __attribute__((dllexport)) void exported_protected() {}
+};
+#endif
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1099,8 +1099,6 @@
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
 const NamedDecl *D) const {
-  if (GV->hasDLLExportStorageClass() || GV->hasDLLImportStorageClass())
-return;
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
 GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
@@ -,6 +1109,14 @@
   // Set visibility for definitions, and for declarations if requested globally
   // or set explicitly.
   LinkageInfo LV = D->getLinkageAndVisibility();
+  if (GV->hasDLLExportStorageClass() || GV->hasDLLImportStorageClass()) {
+// Reject explicit non-default visibility on dllexport/dllimport.
+if (LV.isVisibilityExplicit() && LV.getVisibility() != DefaultVisibility)
+  getDiags().Report(D->getLocation(),
+diag::err_non_default_visibility_dllstorage)
+  << (GV->hasDLLExportStorageClass() ? "dllexport" : "dllimport");
+return;
+  }
   if (LV.isVisibilityExplicit() || getLangOpts().SetVisibilityForExternDecls ||
   !GV->isDeclarationForLinker())
 GV->setVisibility(GetLLVMVisibility(LV.getVisibility()));
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -282,6 +282,8 @@
   "definition with same mangled name '%0' as another definition">;
 def err_cyclic_alias : Error<
   "%select{alias|ifunc}0 definition is part of a cycle">;
+def err_non_default_visibility_dllstorage : Error<
+  "non-default visibility cannot be applied to '%0' declaration">;
 def err_ifunc_resolver_return : Error<
   "ifunc resolver function must return a pointer">;
 


Index: clang/test/CodeGenCXX/dllstorage-visibility.cpp
===
--- clang/test/CodeGenCXX/dllstorage-visibility.cpp
+++ clang/test/CodeGenCXX/dllstorage-visibility.cpp
@@ -4,10 +4,18 @@
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec -fvisibility-inlines-hidden -o - %s | FileCheck %s --check-prefix=G

[clang] 91d8324 - [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-05 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2022-09-05T10:17:19-07:00
New Revision: 91d8324366f405e871aa8174ab61fc66912964dd

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

LOG: [MinGW] Reject explicit non-default visibility applied to 
dllexport/dllimport declaration

dllimport/dllexport is incompatible with protected/hidden visibilities.
(Arguably dllexport semantics is compatible with protected but let's reject the
combo for simplicity.)

When an explicit visibility attribute applies on a dllexport/dllimport
declaration, report a Frontend error (Sema does not compute visibility).

Reviewed By: mstorsjo

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/dllstorage-visibility.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 50f7e2b9221d0..224c290219ef6 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -282,6 +282,8 @@ def err_duplicate_mangled_name : Error<
   "definition with same mangled name '%0' as another definition">;
 def err_cyclic_alias : Error<
   "%select{alias|ifunc}0 definition is part of a cycle">;
+def err_non_default_visibility_dllstorage : Error<
+  "non-default visibility cannot be applied to '%0' declaration">;
 def err_ifunc_resolver_return : Error<
   "ifunc resolver function must return a pointer">;
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f9087cdd5d4dc..64a9e9d6f4c42 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1099,8 +1099,6 @@ llvm::ConstantInt *CodeGenModule::getSize(CharUnits size) 
{
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
 const NamedDecl *D) const {
-  if (GV->hasDLLExportStorageClass() || GV->hasDLLImportStorageClass())
-return;
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
 GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
@@ -,6 +1109,14 @@ void 
CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
   // Set visibility for definitions, and for declarations if requested globally
   // or set explicitly.
   LinkageInfo LV = D->getLinkageAndVisibility();
+  if (GV->hasDLLExportStorageClass() || GV->hasDLLImportStorageClass()) {
+// Reject explicit non-default visibility on dllexport/dllimport.
+if (LV.isVisibilityExplicit() && LV.getVisibility() != DefaultVisibility)
+  getDiags().Report(D->getLocation(),
+diag::err_non_default_visibility_dllstorage)
+  << (GV->hasDLLExportStorageClass() ? "dllexport" : "dllimport");
+return;
+  }
   if (LV.isVisibilityExplicit() || getLangOpts().SetVisibilityForExternDecls ||
   !GV->isDeclarationForLinker())
 GV->setVisibility(GetLLVMVisibility(LV.getVisibility()));

diff  --git a/clang/test/CodeGenCXX/dllstorage-visibility.cpp 
b/clang/test/CodeGenCXX/dllstorage-visibility.cpp
index 8f34afbb801ff..0e3c32ec9aecd 100644
--- a/clang/test/CodeGenCXX/dllstorage-visibility.cpp
+++ b/clang/test/CodeGenCXX/dllstorage-visibility.cpp
@@ -4,10 +4,18 @@
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec 
-fvisibility-inlines-hidden -o - %s | FileCheck %s --check-prefix=GNU
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec 
-fvisibility=hidden -o - %s | FileCheck %s --check-prefix=GNU
 
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -DERR1 -o - %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -fvisibility=hidden -DERR1 -o - %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -triple x86_64-windows-gnu 
-fdeclspec -DERR2 -o - %s
+
 #define CONCAT2(x, y) x##y
 #define CONCAT(x, y) CONCAT2(x, y)
 #define USE(func) void CONCAT(use, __LINE__)() { func(); }
 
+#define HIDDEN __attribute__((visibility("hidden")))
+#define PROTECTED __attribute__((visibility("protected")))
+#define DEFAULT __attribute__((visibility("default")))
+
 // MSVC-DAG: declare dllimport void @"?bar@foo@@QEAAXXZ"(
 // GNU-DAG: define linkonce_odr hidden void @_ZN3foo3barEv(
 
@@ -24,3 +32,17 @@ __attribute__((dllexport)) void exported() {}
 // GNU-DAG: define weak_odr dso_local dllexport void @_Z15exported_inlinev(
 __declspec(dllexport) inline void exported_inline() {}
 USE(exported_inline)
+
+#if defined(ERR1)
+// expected-error@+1 {{non-default visibility cannot be applied to 'dllimport' 
declaration}}
+__attribute__((dllimport)) HIDDEN void imported_hidden

[PATCH] D127695: WIP: clang: Implement Template Specialization Resugaring

2022-09-05 Thread David Rector via Phabricator via cfe-commits
davrec added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:534-540
+  QualType TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
+  SubstTemplateTypeParmTypeLoc TL) 
{
+QualType QT = TL.getType();
+const SubstTemplateTypeParmType *T = TL.getTypePtr();
+Decl *ReplacedDecl = T->getReplacedDecl();
+
+Optional PackIndex = T->getPackIndex();

Haven't thought this through fully, but: would the following make D128113 
(storing the pack index in the STTPT or introducing a new sugar type) 
unnecessary?
```
map, Optional> CurPackIndices;
QualType TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
  SubstTemplateTypeParmTypeLoc TL) {
   QualType QT = TL.getType();
   const SubstTemplateTypeParmType *T = TL.getTypePtr();
   Decl *ReplacedDecl = T->getReplacedDecl();
   
   Optional &PackIndex = CurPackIndices[{ReplacedDecl, 
T->getIndex()}];
   if (!PackIndex && T->getReplacedParameter()->isParameterPack())
 PackIndex = 0;

   ...

   if (PackIndex)
 ++PackIndex;
 // ^ maybe reset to zero if > pack size, if we might be resugaring 
multiple expansions
   return QT;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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


[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-09-05 Thread David Rector via Phabricator via cfe-commits
davrec added a comment.

This looks good, except for the confusion around the name `getReplacedDecl` - I 
would really like the public AST methods to be clearly named and documented, so 
please address that.

Thanks for doing the performance tests.  It may create some additional unique 
STTPTs, but not to do with pack expansion, i.e. not to the extent that created 
problems for D128113 , so I think performance 
concerns have been addressed.

And, it seems possible that the additional info added to STTPT here might make 
D128113  (storing pack index or introducing a 
new sugar type to help infer it) unnecessary for resugaring purposes - I will 
add an inline comment to https://reviews.llvm.org/D127695 to show you what I 
mean/ask about this.

(I'm tempted to hit accept but a. I think there is at least one other issue 
raised by another reviewer which hasn't been addressed and b. I'm unsure of the 
etiquette and don't want to step on toes particularly given the recent changes 
to code ownership.)




Comment at: clang/include/clang/AST/ASTContext.h:1618
+  QualType getSubstTemplateTypeParmType(QualType Replacement,
+Decl *ReplacedDecl,
+unsigned Index) const;

ReplacedDecl -> ReplacedParamParent (see below)



Comment at: clang/include/clang/AST/Type.h:4998
+  /// Gets the templated entity that was substituted.
+  Decl *getReplacedDecl() const { return ReplacedDecl; }
+

To reiterate an earlier comment which may have been overlooked: I think this 
needs a clearer name and documentation (so long as I do not misunderstand what 
kinds of decls it may be - see other inline comment).  

Given that we have `getReplacedParameter()`, I think something like 
`getReplacedParamParent()` makes the most sense.

And the documentation should suggest the relationship between 
`getReplacedParameter` and this (i.e. `getReplacedParamParent()` + `getIndex()` 
-> `getReplacedParameter()`).  

Plus, add back in the original documentation for `getReplacedParameter()`, and 
a brief line of documentation for `getIndex()` (and do same for the Pack 
version below too).




Comment at: clang/lib/AST/Type.cpp:3709
+unsigned Index) {
+  if (const auto *TTP = dyn_cast(D))
+return TTP;

Will this cast ever succeed?  I.e. are there still places 
`Context.getSubstTemplateTypeParmType(...)` is called with a TTPD as the 
`ReplacedDecl`?  That would make `getReplacedDecl()` much more complicated/less 
understandable - can we change any such places to always pass the parent 
template/templated entity as the ReplacedDecl, for consistency (and so that we 
can rename ReplacedDecl to ReplacedParamParent)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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


[PATCH] D132867: [Clang] Use virtual FS in processing config files

2022-09-05 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 458035.
sepavloff added a comment.

Fix documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132867

Files:
  clang/lib/Driver/Driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1264,10 +1264,17 @@
 // always have an absolute path deduced from the containing file.
 SmallString<128> CurrDir;
 if (llvm::sys::path::is_relative(FName)) {
-  if (!CurrentDir)
-llvm::sys::fs::current_path(CurrDir);
-  else
+  if (!CurrentDir) {
+if (auto CWD = FS.getCurrentWorkingDirectory()) {
+  CurrDir = *CWD;
+} else {
+  // TODO: The error should be propagated up the stack.
+  llvm::consumeError(llvm::errorCodeToError(CWD.getError()));
+  return false;
+}
+  } else {
 CurrDir = *CurrentDir;
+  }
   llvm::sys::path::append(CurrDir, FName);
   FName = CurrDir.c_str();
 }
@@ -1357,24 +1364,26 @@
 }
 
 bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
-SmallVectorImpl &Argv) {
+SmallVectorImpl &Argv,
+llvm::vfs::FileSystem &FS) {
   SmallString<128> AbsPath;
   if (sys::path::is_relative(CfgFile)) {
-llvm::sys::fs::current_path(AbsPath);
-llvm::sys::path::append(AbsPath, CfgFile);
+AbsPath.assign(CfgFile);
+if (std::error_code EC = FS.makeAbsolute(AbsPath))
+  return false;
 CfgFile = AbsPath.str();
   }
-  if (llvm::Error Err = ExpandResponseFile(
-  CfgFile, Saver, cl::tokenizeConfigFile, Argv,
-  /*MarkEOLs=*/false, /*RelativeNames=*/true, /*ExpandBasePath=*/true,
-  *llvm::vfs::getRealFileSystem())) {
+  if (llvm::Error Err =
+  ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
+ /*MarkEOLs=*/false, /*RelativeNames=*/true,
+ /*ExpandBasePath=*/true, FS)) {
 // TODO: The error should be propagated up the stack.
 llvm::consumeError(std::move(Err));
 return false;
   }
   return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv,
  /*MarkEOLs=*/false, /*RelativeNames=*/true,
- /*ExpandBasePath=*/true, llvm::None);
+ /*ExpandBasePath=*/true, llvm::None, FS);
 }
 
 static void initCommonOptions();
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2078,7 +2078,8 @@
 /// current config file.
 ///
 bool readConfigFile(StringRef CfgFileName, StringSaver &Saver,
-SmallVectorImpl &Argv);
+SmallVectorImpl &Argv,
+llvm::vfs::FileSystem &FS);
 
 /// Expand response files on a command line recursively using the given
 /// StringSaver and tokenization strategy.  Argv should contain the command line
@@ -2099,7 +2100,7 @@
 /// the current response file.
 /// \param [in] FS File system used for all file access when running the tool.
 /// \param [in] CurrentDir Path used to resolve relative rsp files. If set to
-/// None, process' cwd is used instead.
+/// None, the file system current directory is used instead.
 /// \return true if all @files were expanded successfully or there were none.
 bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
  SmallVectorImpl &Argv, bool MarkEOLs,
Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -484,4 +484,61 @@
   }
 }
 
+TEST(ToolChainTest, ConfigFileSearch) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr FS(
+  new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+  const char *TestRoot = "C:\\";
+#else
+  const char *TestRoot = "/";
+#endif
+  FS->setCurrentWorkingDirectory(TestRoot);
+
+  FS->addFile(
+  "/opt/sdk/root.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/opt/sdk/platform0\n"));
+  FS->addFile(
+  "/home/test/sdk/root.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/opt/sdk/platform1\n"));
+  FS->addFile(
+  "/home/test/bin/root.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/opt/sdk/platfo

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-05 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision.
sepavloff added reviewers: rjmccall, aaron.ballman, kadircet, sammccall, 
jackoalan.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added projects: clang, LLVM.

Normally a file included into configuration file by directive `@file` is
searched for relative the including configuration file. In some cases it
is not convenient. Configuration parameters may be logically partitioned
into caterories (like 'platform variant' and 'processor variant') and it
is convenient to have different configuration files for these
categories maintained separately. Similar motivation was described in
https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606.
Simple file inclusion by `@file` does not help, because in this case the
path to `file` is specified relative to the directory of the used
configuration file, but it actually is not dependent on the latter. Using
absolute paths is not convenient and it still do not allow user to
easily override the settings.

This change implements option `--search-config-dirs`, which modifies the
search algorithm, - files included by `@file` are searched for as
configuration files, in the same directories, and can be overridden by a
user. With this facility a configuration file can include other files
like in:

  @platform.cfg
  @processor.cfg
  ...other options...

The effect is as if two configuration files are loaded, both are
searched for in well-known places and can be overridden if user
provides a file with the same name in user configuration directory.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133325

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/config/config-3.cfg
  clang/test/Driver/config-file.c
  clang/unittests/Driver/ToolChainTest.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1184,6 +1184,15 @@
 
   if (!RelativeNames)
 return Error::success();
+
+  if (InConfigFile) {
+if (std::find_if(NewArgv.begin(), NewArgv.end(),
+ [](const char *Arg) -> bool {
+   return StringRef("--search-config-dirs") == Arg;
+ }) != NewArgv.end())
+  setSearchAsConfig(true);
+  }
+
   llvm::StringRef BasePath = llvm::sys::path::parent_path(FName);
   // If names of nested response files should be resolved relative to including
   // file, replace the included response file names with their full paths
@@ -1207,8 +1216,17 @@
 
 SmallString<128> ResponseFile;
 ResponseFile.push_back('@');
-ResponseFile.append(BasePath);
-llvm::sys::path::append(ResponseFile, FileName);
+if (SearchAsConfig) {
+  std::string FilePath;
+  if (!findConfigFile(FileName, FilePath))
+return llvm::createStringError(
+std::make_error_code(std::errc::no_such_file_or_directory),
+"Could not find config file: " + FileName);
+  ResponseFile.append(FilePath);
+} else {
+  ResponseFile.append(BasePath);
+  llvm::sys::path::append(ResponseFile, FileName);
+}
 Arg = Saver.save(ResponseFile.str()).data();
   }
   return Error::success();
@@ -1350,15 +1368,48 @@
llvm::vfs::FileSystem *FS)
 : Saver(S), Tokenizer(T), FS(FS ? FS : vfs::getRealFileSystem().get()) {}
 
-bool ExpansionContext::readConfigFile(StringRef CfgFile,
-  SmallVectorImpl &Argv) {
-  SmallString<128> AbsPath;
-  if (sys::path::is_relative(CfgFile)) {
-AbsPath.assign(CfgFile);
-if (std::error_code EC = FS->makeAbsolute(AbsPath))
+bool ExpansionContext::findConfigFile(StringRef FileName,
+  std::string &FilePath) {
+  SmallString<128> CfgFilePath;
+  const auto FileExists = [this](SmallString<128> Path) -> bool {
+auto Status = FS->status(Path);
+return Status &&
+   Status->getType() == llvm::sys::fs::file_type::regular_file;
+  };
+
+  // If file name contains directory separator, treat it as a path to
+  // configuration file.
+  if (llvm::sys::path::has_parent_path(FileName)) {
+CfgFilePath = FileName;
+if (llvm::sys::path::is_relative(FileName)) {
+  if (FS->makeAbsolute(CfgFilePath))
+return false;
+}
+if (!FileExists(CfgFilePath))
   return false;
-CfgFile = AbsPath.str();
+FilePath = CfgFilePath.str();
+return true;
+  }
+
+  // Look for the file in search directories.
+  for (const StringRef &Dir : SearchDirs) {
+if (Dir.empty())
+  continue;
+CfgFilePath.assign(Dir);
+  

[clang-tools-extra] 05737fa - [clangd] Trace preamble throttle time

2022-09-05 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-09-05T18:34:41+02:00
New Revision: 05737fa209ba97b330739af3b00834c21b0547b7

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

LOG: [clangd] Trace preamble throttle time

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 4c6c8c679be3..1e64fac80d94 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -390,13 +390,16 @@ class PreambleThrottlerRequest {
   PreambleThrottlerRequest(llvm::StringRef Filename,
PreambleThrottler *Throttler,
std::condition_variable &CV)
-  : Throttler(Throttler), Satisfied(Throttler == nullptr) {
+  : Throttler(Throttler),
+Satisfied(Throttler == nullptr) {
 // If there is no throttler, this dummy request is always satisfied.
 if (!Throttler)
   return;
+Tracer.emplace("PreambleThrottled");
 ID = Throttler->acquire(Filename, [&] {
   Satisfied.store(true, std::memory_order_release);
   CV.notify_all();
+  Tracer.reset();
 });
   }
 
@@ -411,6 +414,7 @@ class PreambleThrottlerRequest {
   }
 
 private:
+  llvm::Optional Tracer;
   PreambleThrottler::RequestID ID;
   PreambleThrottler *Throttler;
   std::atomic Satisfied = {false};
@@ -473,13 +477,18 @@ class PreambleThread {
 if (Done)
   break;
 
-Throttle.emplace(FileName, Throttler, ReqCV);
-// If acquire succeeded synchronously, avoid status jitter.
-if (!Throttle->satisfied())
-  Status.update([&](TUStatus &Status) {
-Status.PreambleActivity = PreambleAction::Queued;
-  });
-ReqCV.wait(Lock, [&] { return Throttle->satisfied() || Done; });
+{
+  Throttle.emplace(FileName, Throttler, ReqCV);
+  llvm::Optional Tracer;
+  // If acquire succeeded synchronously, avoid status jitter.
+  if (!Throttle->satisfied()) {
+Tracer.emplace("PreambleThrottle");
+Status.update([&](TUStatus &Status) {
+  Status.PreambleActivity = PreambleAction::Queued;
+});
+  }
+  ReqCV.wait(Lock, [&] { return Throttle->satisfied() || Done; });
+}
 if (Done)
   break;
 // While waiting for the throttler, the request may have been updated!



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


[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

2022-09-05 Thread Nikolas Klauser via Phabricator via cfe-commits
philnik accepted this revision as: philnik.
philnik added a comment.

Mostly LGTM. Just a few nits.




Comment at: clang/www/hacking.html:276
+  
+  Testing changes affecting libcxx
+  

To make it consistent throughout



Comment at: clang/www/hacking.html:284
+  Changing compiler builtins, especially the builtins used for type 
traits
+  or replacements of library functions like std::move,
+  std::forward.





Comment at: clang/www/hacking.html:296
+  file will also add the libc++ group to the list of reviewers. The status of
+  the build will be available in Phabricator.
+





Comment at: clang/www/hacking.html:300
+  CI lacks the resources to build every Clang diff. A single run takes about
+  one hour. Secondly most changes of Clang won't affect libc++.
+

Having a secondly without a firstly seems weird.



Comment at: clang/www/hacking.html:310
+
+  Unlike Clang, libc++ supports multiple versions of Clang. Therefore when a
+  patch changes the diagnostics it might be required to use a regex in the

This reads like you want to say that clang doesn't support multiple versions of 
clang, which seems self-evident. Maybe just drop the `Unlike Clang, `?



Comment at: libcxx/docs/Contributing.rst:87
 
-* C++20 for the Linux platform.
-* MacOS C++20 for the Apple platform.
+* C++XX for the Linux platform (where XX is the latest C++ version).
+* MacOS X86_64 and MacOS arm64 for the Apple platform.

Complete nit, but `XX` reads to me like the digits have to be the same. I'd 
suggest `XY` to make it obvious that they can be different.



Comment at: libcxx/docs/Contributing.rst:88
+* C++XX for the Linux platform (where XX is the latest C++ version).
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+





Comment at: libcxx/docs/Contributing.rst:107
+
+The CI tests libc++ for all :ref:`supported platforms `.
+The build is started for every diff uploaded. A complete CI run takes

Not really currently. We still claim FreeBSD support without a CI runner.



Comment at: libcxx/docs/Contributing.rst:231-233
+  export CC=clang-14
+  export CXX=clang++-14
+  run-buildbot generic-cxx17

Complete nit, but I think `CC=clang-14 CXX=clang++-14 run-builtbot 
generic-cxx17` as an example would be better to avoid polluting people's 
environment if they're unfamiliar with a terminal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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


[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

2022-09-05 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

Thanks for the review comments. I'll address them in a little while. I want to 
wait for more review comments.




Comment at: clang/www/hacking.html:302
+
+  For most builds, the pre-commit CI uses a recent
+  https://apt.llvm.org/";>nightly build of Clang from LLVM's main

mstorsjo wrote:
> I'm wondering if there's a better synonym for "builds" here which would be 
> clearer? Initially I read this as if "most times a patch is run through the 
> CI", not "most of the configurations in the CI run". So maybe 
> "configurations"?
Agreed configurations seems more appropriate in this context.



Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+

mstorsjo wrote:
> I don't understand this paragraph - each CI run is run through the configured 
> set of supported Clang versions - not only `main`? Or does this talk about 
> specifics about manually running tests with the Docker image?
This is the version of Clang in the main branch. How about:
`Unless specified otherwise the nightly build of Clang from the ``main`` branch 
is used.`



Comment at: libcxx/docs/Contributing.rst:158
+* ``Clang XX`` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and

mstorsjo wrote:
> Side note - these tests only test one specific version of C++ with these 
> versions of Clang - we don't have fully coverage of all standard versions 
> with all supported versions of Clang. (Not really sure if that's worth 
> mentioning though. Also I do see this kinda hinted at at the end of the file.)
We might spell it our more explicitly. But maybe not here but where the hints 
are. There are a lot of possible permutations not tested. For example the 
modular build is only tested with C++2b.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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


[PATCH] D127082: [clangd] Add Macro Expansion to Hover

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry for the delay :-( This looks great!

It looks like this is your first patch, so you don't have commit access. I can 
land the patch for you.
Can you provide an email address for attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127082

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


[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

2022-09-05 Thread Lin Yurong via Phabricator via cfe-commits
yronglin updated this revision to Diff 458019.
yronglin added a project: Sanitizers.
yronglin added a comment.

Fix windows build bolt broken 
https://lab.llvm.org/buildbot/#/builders/127/builds/35304


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #endif
 
+int test13(int *a) {
+  a = (int *)__builtin_assume_aligned(a, 2 * 2.0); // expected-error {{argument to '__builtin_assume_aligned' must be a constant integer}}
+  return a[0];
+}
+
 void test_void_assume_aligned(void) __attribute__((assume_aligned(32))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 int test_int_assume_aligned(void) __attribute__((assume_aligned(16))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 void *test_ptr_assume_aligned(void) __attribute__((assume_aligned(64))); // no-warning
Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -26,3 +26,9 @@
 void *ignore_volatiles(volatile void * x) {
   return __builtin_assume_aligned(x, 1);
 }
+
+// CHECK-LABEL: ignore_array_volatiles
+void *ignore_array_volatiles() {
+  volatile int arr[] = {1};
+  return __builtin_assume_aligned(arr, 4);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 31, i32 35 }, {{.*}}* @[[CHAR]] }
+
+void *caller(void) {
+  char str[] = "";
+  // CHECK:   define{{.*}}
+  // CHECK-NEXT:  entry:
+  // CHECK-NEXT:%[[STR:.*]] = alloca [1 x i8], align 1
+  // CHECK-NEXT:%[[BITCAST:.*]] = bitcast [1 x i8]* %[[STR]] to i8*
+  // CHECK-NEXT:call void @llvm.memset.p0i8.i64(i8* align 1 %[[BITCAST]], i8 0, i64 1, i1 false)
+  // CHECK-NEXT:%[[ARRAYDECAY:.*]] = getelementptr inbounds [1 x i8], [1 x i8]* %[[STR]], i64 0, i64 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64
+  // CHECK-SANITIZE-NEXT:   %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 0
+  // CHECK-SANITIZE-NEXT:   %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:   br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:   call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i6

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-09-05 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 458012.
rymiel added a comment.

Rebase on main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132295

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -809,18 +809,85 @@
 
   Tokens = annotate("[]() -> auto {}");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto & {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto * {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] {}");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] noexcept {}");
+  ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] -> auto {}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21730,6 +21730,18 @@
"g();\n"
"  }\n"
"};\n");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
 
   // Multiple lambdas in the same parentheses change indentation rules. These
   // lambdas are forced to start on new lines.
Index: clang/lib/Format/UnwrappedLineParser.

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

2022-09-05 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng added a subscriber: khchen.
kito-cheng added a comment.

This is dump from my mailbox, few month ago I written a offlist mail to 
describe about RISC-V LTO status:

---

LTO for RISC-V is really kind of a long long story. @khchen has been fighting 
for that for a long time, but he is no longer focusing on RISC-V.

A short summary is we still have issues with dealing with ABI, target features, 
and datalayout,

Datalayout is determined by target triple in most other targets, but RISC-V 
can't only use target triple to determine the datalayout.

So we need to keep ABI in somewhere and read that at LTO phase, the most ideal 
place is the module flags. We already did that[6], but that comes with a 
problem is it's too late to update datalayout when we start to read a module, 
because LLVM will use datalayout to infer some info like the alignment of 
loads[7], and that means we might re-read the whole LLVM IR again to get the IR 
with the right info, and that requires fixing multiple places in LLVM (see[2]. 
Still, I am not sure it's enough or not).

There is also an issue with how to store and determine the final LTO target 
features. For example, A object built with -march=rv64g and B object built with 
-march=rv64gc, so which arch should we use in the LTO code generation stage? 
see [5] for more discussion around this issue.

Currently, our downstream work-around is passing `-march` and `-mabi` into LTO 
by `-Wl,-plugin-opt=`, and ABI and ISA are determined by the linking stage to 
avoid (or work-around) the above issues.

Related open revision:
[1] https://reviews.llvm.org/D71387
[2] https://reviews.llvm.org/D72624 (This one is abandoned, but it
still necessary if we need store ABI info in module flags)
[3] https://reviews.llvm.org/D72245
[4] https://reviews.llvm.org/D102582
[5] https://reviews.llvm.org/D106347
[6] https://reviews.llvm.org/D72755

Other revision for ref.
---

[7] https://reviews.llvm.org/D78403


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132843

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


[PATCH] D133298: [clang-rename] RecursiveSymbolVisitor - visit namespace aliases and using directives

2022-09-05 Thread imctrading via Phabricator via cfe-commits
imctrading updated this revision to Diff 458010.
imctrading added a comment.

Fix formatting in `RecursiveSymbolVisitor.h`


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

https://reviews.llvm.org/D133298

Files:
  clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
  clang/test/clang-rename/Namespace.cpp


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc;// CHECK: namespace gcc_alias = clang;
+using namespace gcc;  // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | 
FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,16 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(),
+ Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(),
+ Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc;// CHECK: namespace gcc_alias = clang;
+using namespace gcc;  // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,16 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(),
+ Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(),
+ Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133308: [cmake] do not set execution permission to regular files

2022-09-05 Thread Sinan Lin via Phabricator via cfe-commits
sinan created this revision.
sinan added reviewers: MaskRay, phosek, DavidSpickett.
Herald added subscribers: StephenFan, whisperity, mgorny.
Herald added a reviewer: NoQ.
Herald added a project: All.
sinan requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

some regular files(e.g. files that have no shebang and no execute bit in source 
dir) are wrongly assigned execution permission through cmake install, such as 
scanview.css and ear.c from libscanbuild, which is unnecessary and introduces 
warnings in some platforms:

install(PROGRAMS ...) is identical to the install(FILES ...) except that the 
default permissions for the installed file also include OWNER_EXECUTE, 
GROUP_EXECUTE, and WORLD_EXECUTE. 
(https://cmake.org/cmake/help/latest/command/install.html#installing-files)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133308

Files:
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt

Index: clang/tools/scan-build-py/CMakeLists.txt
===
--- clang/tools/scan-build-py/CMakeLists.txt
+++ clang/tools/scan-build-py/CMakeLists.txt
@@ -87,7 +87,7 @@
${CMAKE_BINARY_DIR}/lib/libscanbuild/
  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/lib/libscanbuild/${lib})
   list(APPEND Depends ${CMAKE_BINARY_DIR}/lib/libscanbuild/${lib})
-  install(PROGRAMS lib/libscanbuild/${lib}
+  install(FILES lib/libscanbuild/${lib}
   DESTINATION lib${CLANG_LIBDIR_SUFFIX}/libscanbuild
   COMPONENT scan-build-py)
 endforeach()
@@ -105,7 +105,7 @@
${CMAKE_BINARY_DIR}/lib/libscanbuild/resources
  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/lib/libscanbuild/resources/${resource})
   list(APPEND Depends ${CMAKE_BINARY_DIR}/lib/libscanbuild/resources/${resource})
-  install(PROGRAMS lib/libscanbuild/resources/${resource}
+  install(FILES lib/libscanbuild/resources/${resource}
   DESTINATION lib${CLANG_LIBDIR_SUFFIX}/libscanbuild/resources
   COMPONENT scan-build-py)
 endforeach()
@@ -121,7 +121,7 @@
${CMAKE_BINARY_DIR}/lib/libear/
  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/lib/libear/${lib})
   list(APPEND Depends ${CMAKE_BINARY_DIR}/lib/libear/${lib})
-  install(PROGRAMS lib/libear/${lib}
+  install(FILES lib/libear/${lib}
   DESTINATION lib${CLANG_LIBDIR_SUFFIX}/libear
   COMPONENT scan-build-py)
 endforeach()
Index: clang/tools/clang-rename/CMakeLists.txt
===
--- clang/tools/clang-rename/CMakeLists.txt
+++ clang/tools/clang-rename/CMakeLists.txt
@@ -18,9 +18,9 @@
   clangToolingRefactoring
   )
 
-install(PROGRAMS clang-rename.py
+install(FILES clang-rename.py
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-rename)
-install(PROGRAMS clang-rename.el
+install(FILES clang-rename.el
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-rename)
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -20,19 +20,19 @@
   add_subdirectory(fuzzer)
 endif()
 
-install(PROGRAMS clang-format-bbedit.applescript
+install(FILES clang-format-bbedit.applescript
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-format)
 install(PROGRAMS clang-format-diff.py
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-format)
-install(PROGRAMS clang-format-sublime.py
+install(FILES clang-format-sublime.py
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-format)
-install(PROGRAMS clang-format.el
+install(FILES clang-format.el
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-format)
-install(PROGRAMS clang-format.py
+install(FILES clang-format.py
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-format)
 install(PROGRAMS git-clang-format
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -418,7 +418,7 @@
   endif()
 
   add_custom_target(bash-autocomplete DEPENDS utils/bash-autocomplete.sh)
-  install(PROGRAMS utils/bash-autocomplete.sh
+  install(FILES utils/bash-autocomplete.sh
   DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT bash-autocomplete)
   if(NOT LLVM_ENABLE_IDE)
Index: clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
+++ clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
@@ -20,9 +20,9 @@
   findAllS

[PATCH] D131262: [analyzer] Track trivial copy/move constructors and initializer lists in the BugReporter

2022-09-05 Thread Domján Dániel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa11e51e91f73: [analyzer] Track trivial copy/move 
constructors and initializer lists in the… (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131262

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/ctor-bug-path.cpp

Index: clang/test/Analysis/ctor-bug-path.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctor-bug-path.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-output=text -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-output=text -std=c++17 -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace copyMoveTrackCtor {
+struct S {
+  int *p1, *p2;
+  S(int *a, int *b) : p1(a), p2(b) {} // expected-note{{Null pointer value stored to 's.p1'}}
+};
+
+void CtorDirect() {
+  int *x = nullptr, *y = nullptr; 
+  // expected-note@-1{{'x' initialized to a null pointer value}}
+
+  S s(x, y); 
+  // expected-note@-1{{Passing null pointer value via 1st parameter 'a'}}
+  // expected-note@-2{{Calling constructor for 'S'}}
+  // expected-note@-3{{Returning from constructor for 'S'}}
+  // expected-note@-4{{'s' initialized here}}
+  S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+  // expected-note@-1{{'s2' initialized here}}
+  S s3 = s2;  // expected-note{{Null pointer value stored to 's3.p1'}}
+  // expected-note@-1{{'s3' initialized here}}
+  S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+  // expected-note@-1{{'s4' initialized here}}
+  S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+  int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer (loaded from field 'p1')}}
+
+  (void) i;
+}
+} // namespace copyMoveTrackCtor
+
+namespace copyMoveTrackInitList {
+struct S {
+  int *p1, *p2;
+};
+
+void InitListDirect() {
+  int *x = nullptr, *y = nullptr; //expected-note{{'x' initialized to a null pointer value}}
+
+  S s{x, y}; //expected-note{{'s.p1' initialized to a null pointer value}}
+  //expected-note@-1{{'s' initialized here}}
+  S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+  // expected-note@-1{{'s2' initialized here}}
+  S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+  // expected-note@-1{{'s3' initialized here}}
+  S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+  // expected-note@-1{{'s4' initialized here}}
+  S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+  int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer (loaded from field 'p1')}}
+
+  (void) i;
+}
+
+void InitListAssign() {
+  int *x = nullptr, *y = nullptr; //expected-note{{'x' initialized to a null pointer value}}
+
+  S s = {x, y}; //expected-note{{'s.p1' initialized to a null pointer value}}
+  //expected-note@-1{{'s' initialized here}}
+  S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+  // expected-note@-1{{'s2' initialized here}}
+  S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+  // expected-note@-1{{'s3' initialized here}}
+  S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+  // expected-note@-1{{'s4' initialized here}}
+  S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+  int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer (loaded from field 'p1')}}
+
+  (void) i;
+}
+
+} // namespace copyMoveTrackInitList
+
+namespace copyMoveTrackCtorMemberInitList {
+struct S {
+  int *p1, *p2;
+  S(int *a, int *b) : p1{a}, p2{b} {} // expected-note{{Null pointer value stored to 's.p1'}}
+};
+
+void CtorDirect() {
+  int *x = nullptr, *y = nullptr; 
+  // expected-note@-1{{'x' initialized to a null pointer value}}
+
+  S s{x, y}; 
+  // expected-note@-1{{Passing null pointer value via 1st parameter 'a'}}
+  // expected-note@-2{{Calling constructor for 'S'}}
+  // expected-note@-3{{Returning from constructor for 'S'}}
+  // expected-note@-4{{'s' initialized here}}
+  S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+  // expected-note@-1{{'s2' initialized here}}
+  S s3 = s2;  // expected-note{{Null pointer value stored to 's3.p1'}}
+  // expected-note@-1{{'s3' initialized here}}
+  S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+  // expected-note@-1{{'s4' initialized here}}
+  S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+  int i

[clang] a11e51e - [analyzer] Track trivial copy/move constructors and initializer lists in the BugReporter

2022-09-05 Thread via cfe-commits

Author: isuckatcs
Date: 2022-09-05T17:06:27+02:00
New Revision: a11e51e91f73e413eb044bc0b2f2e205735618d7

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

LOG: [analyzer] Track trivial copy/move constructors and initializer lists in 
the BugReporter

If an object has a trivial copy/move constructor, it's not inlined
on invocation but a trivial copy is performed instead. This patch
handles trivial copies in the bug reporter by matching the field
regions of the 2 objects involved in the copy/move construction,
and tracking the appropriate region further. This patch also
introduces some support for tracking values in initializer lists.

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

Added: 
clang/test/Analysis/ctor-bug-path.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 3a90c37a36dab..336376b78ce80 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1410,6 +1410,83 @@ static void 
showBRDefaultDiagnostics(llvm::raw_svector_ostream &OS,
   }
 }
 
+static bool isTrivialCopyOrMoveCtor(const CXXConstructExpr *CE) {
+  if (!CE)
+return false;
+
+  const auto *CtorDecl = CE->getConstructor();
+
+  return CtorDecl->isCopyOrMoveConstructor() && CtorDecl->isTrivial();
+}
+
+static const Expr *tryExtractInitializerFromList(const InitListExpr *ILE,
+ const MemRegion *R) {
+
+  const auto *TVR = dyn_cast_or_null(R);
+
+  if (!TVR)
+return nullptr;
+
+  const auto ITy = ILE->getType().getCanonicalType();
+
+  // Push each sub-region onto the stack.
+  std::stack TVRStack;
+  while (isa(TVR) || isa(TVR)) {
+// We found a region that matches the type of the init list,
+// so we assume this is the outer-most region. This can happen
+// if the initializer list is inside a class. If our assumption
+// is wrong, we return a nullptr in the end.
+if (ITy == TVR->getValueType().getCanonicalType())
+  break;
+
+TVRStack.push(TVR);
+TVR = cast(TVR->getSuperRegion());
+  }
+
+  // If the type of the outer most region doesn't match the type
+  // of the ILE, we can't match the ILE and the region.
+  if (ITy != TVR->getValueType().getCanonicalType())
+return nullptr;
+
+  const Expr *Init = ILE;
+  while (!TVRStack.empty()) {
+TVR = TVRStack.top();
+TVRStack.pop();
+
+// We hit something that's not an init list before
+// running out of regions, so we most likely failed.
+if (!isa(Init))
+  return nullptr;
+
+ILE = cast(Init);
+auto NumInits = ILE->getNumInits();
+
+if (const auto *FR = dyn_cast(TVR)) {
+  const auto *FD = FR->getDecl();
+
+  if (FD->getFieldIndex() >= NumInits)
+return nullptr;
+
+  Init = ILE->getInit(FD->getFieldIndex());
+} else if (const auto *ER = dyn_cast(TVR)) {
+  const auto Ind = ER->getIndex();
+
+  // If index is symbolic, we can't figure out which expression
+  // belongs to the region.
+  if (!Ind.isConstant())
+return nullptr;
+
+  const auto IndVal = Ind.getAsInteger()->getLimitedValue();
+  if (IndVal >= NumInits)
+return nullptr;
+
+  Init = ILE->getInit(IndVal);
+}
+  }
+
+  return Init;
+}
+
 PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ,
   BugReporterContext &BRC,
   PathSensitiveBugReport &BR) {
@@ -1456,12 +1533,88 @@ PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const 
ExplodedNode *Succ,
 
 StoreSite = Succ;
 
-// If this is an assignment expression, we can track the value
-// being assigned.
-if (Optional P = Succ->getLocationAs())
-  if (const BinaryOperator *BO = P->getStmtAs())
+if (Optional P = Succ->getLocationAs()) {
+  // If this is an assignment expression, we can track the value
+  // being assigned.
+  if (const BinaryOperator *BO = P->getStmtAs()) {
 if (BO->isAssignmentOp())
   InitE = BO->getRHS();
+  }
+  // If we have a declaration like 'S s{1,2}' that needs special
+  // handling, we handle it here.
+  else if (const auto *DS = P->getStmtAs()) {
+const auto *Decl = DS->getSingleDecl();
+if (isa(Decl)) {
+  const auto *VD = cast(Decl);
+
+  // FIXME: Here we only track the inner most region, so we lose
+  // information, but it's still better than a crash or no information
+  // at all.
+  //
+  // E.g.: The region we have is 's.s2.s3.s4.y' and

[PATCH] D131066: [clangd] Add option to disable inlay hints for init lists when building array.

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

We **have** been thinking about problems with inlay hints being verbose, and 
array initializers that are visible by default are clearly seen as spammy.
Thanks for sending a patch. I filed 
https://github.com/clangd/clangd/issues/1277 about this just now and added some 
thoughts.
FWIW when *not* shown by default (VSCode's offUnlessPressed) these don't seem 
to be spammy.

Adding a new option is always easy, but actually helps relatively few people 
unless we change the default.
So I think we should change the default behavior, and **consider** making it 
configurable if we get requests to add it back.

(I have reservations about the details of the config schema in this patch, but 
we can improve it if needed).




Comment at: clang-tools-extra/clangd/InlayHints.cpp:77
+  }
+  return false;
 }

This means that both array-nested-in-struct and struct-nested-in-array will 
produce no hints at all.
This is probably a rare case, but IMO one that will likely benefit from 
designators.

The smallest behavior change to address the common case would be to drop 
designators that only contain a single array index.

I suppose the neatest way to achieve this is to bail out at (old) line 153:

```
if (Fields.isArray() && !BraceElidedSubobject && Prefix.empty())
  // Here we'd emit only a trivial array designator.
  // This isn't useful: https://github.com/clangd/clangd/issues/1277
  continue;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131066

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


[PATCH] D133236: [clang-format] Use utf-8 for JSON object load

2022-09-05 Thread Luke Drummond via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f4e6cfe73d8: [clang-format] Use utf-8 for JSON object load 
(authored by kaitingwang, committed by ldrumm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133236

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -10,9 +10,9 @@
 # imap  :py3f /clang-format.py
 #   endif
 #
-# The if-elseif-endif conditional should pick either the python3 or python2 
+# The if-elseif-endif conditional should pick either the python3 or python2
 # integration depending on your vim setup.
-# 
+#
 # The first mapping enables clang-format for NORMAL and VISUAL mode, the second
 # mapping adds support for INSERT mode. Change "C-I" to another binding if you
 # need clang-format on a different key (C-I stands for Ctrl+i).
@@ -134,7 +134,7 @@
 )
   else:
 header, content = stdout.split(b'\n', 1)
-header = json.loads(header)
+header = json.loads(header.decode('utf-8'))
 # Strip off the trailing newline (added above).
 # This maintains trailing empty lines present in the buffer if
 # the -lines specification requests them to remain unchanged.


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -10,9 +10,9 @@
 # imap  :py3f /clang-format.py
 #   endif
 #
-# The if-elseif-endif conditional should pick either the python3 or python2 
+# The if-elseif-endif conditional should pick either the python3 or python2
 # integration depending on your vim setup.
-# 
+#
 # The first mapping enables clang-format for NORMAL and VISUAL mode, the second
 # mapping adds support for INSERT mode. Change "C-I" to another binding if you
 # need clang-format on a different key (C-I stands for Ctrl+i).
@@ -134,7 +134,7 @@
 )
   else:
 header, content = stdout.split(b'\n', 1)
-header = json.loads(header)
+header = json.loads(header.decode('utf-8'))
 # Strip off the trailing newline (added above).
 # This maintains trailing empty lines present in the buffer if
 # the -lines specification requests them to remain unchanged.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 8f4e6cf - [clang-format] Use utf-8 for JSON object load

2022-09-05 Thread Luke Drummond via cfe-commits

Author: Amy Wang
Date: 2022-09-05T15:40:47+01:00
New Revision: 8f4e6cfe73d803697470e817845d9b94c4530ca3

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

LOG: [clang-format] Use utf-8 for JSON object load

>From Python 3.6 and above, it should be able to automatically select a
decoding for json.loads. However, with a vim encoding that defaults
to utf-8, clang-format.py runs into the following error

TypeError: the JSON object must be str, not 'bytes'

This patch explicitly specifies utf-8 decoding for the header.

Reviewed by: ldrumm, sammcall

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

Added: 


Modified: 
clang/tools/clang-format/clang-format.py

Removed: 




diff  --git a/clang/tools/clang-format/clang-format.py 
b/clang/tools/clang-format/clang-format.py
index 76fedb6481474..5bc108bfc713b 100644
--- a/clang/tools/clang-format/clang-format.py
+++ b/clang/tools/clang-format/clang-format.py
@@ -10,9 +10,9 @@
 # imap  :py3f /clang-format.py
 #   endif
 #
-# The if-elseif-endif conditional should pick either the python3 or python2 
+# The if-elseif-endif conditional should pick either the python3 or python2
 # integration depending on your vim setup.
-# 
+#
 # The first mapping enables clang-format for NORMAL and VISUAL mode, the second
 # mapping adds support for INSERT mode. Change "C-I" to another binding if you
 # need clang-format on a 
diff erent key (C-I stands for Ctrl+i).
@@ -134,7 +134,7 @@ def main():
 )
   else:
 header, content = stdout.split(b'\n', 1)
-header = json.loads(header)
+header = json.loads(header.decode('utf-8'))
 # Strip off the trailing newline (added above).
 # This maintains trailing empty lines present in the buffer if
 # the -lines specification requests them to remain unchanged.



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


[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

2022-09-05 Thread Lin Yurong via Phabricator via cfe-commits
yronglin added a comment.

In D133202#3768770 , @vitalybuka 
wrote:

> Broken by the patch https://lab.llvm.org/buildbot/#/builders/127/builds/35304

Thanks @vitalybuka , seems `intrin0.inl.h` have a forward declaration 
`__MACHINE(void * __cdecl __builtin_assume_aligned(const void *, size_t, ...) 
noexcept)`, should we use `nc` in `BUILTIN(__builtin_assume_aligned, "v*vC*z.", 
"nct")` but not `nct` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits
yusuke-kadowaki added inline comments.



Comment at: clang/lib/Format/Format.cpp:1196
   LLVMStyle.AlignConsecutiveMacros = {};
+  LLVMStyle.AlignConsecutiveTrailingComments = {};
+  LLVMStyle.AlignConsecutiveTrailingComments.Enabled = true;

MyDeveloperDay wrote:
> Won’t you need to initialise the members here as above
Members are `false` when initialized so I don't know why those above are 
initialized explicitly as `false`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits
yusuke-kadowaki updated this revision to Diff 457987.
yusuke-kadowaki marked 6 inline comments as done.
yusuke-kadowaki added a comment.

- Change to use verifyFormat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,182 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 1;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 2;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very long\n"
+"  // comment which is\n"
+"  // wrapped arround.\n"
+"\n"
+"int x =\n"
+"2; // Is this still aligned?\

[PATCH] D133298: [clang-rename] RecursiveSymbolVisitor - visit namespace aliases and using directives

2022-09-05 Thread imctrading via Phabricator via cfe-commits
imctrading updated this revision to Diff 457978.
imctrading added a comment.

Fix formatting in the test.


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

https://reviews.llvm.org/D133298

Files:
  clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
  clang/test/clang-rename/Namespace.cpp


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc;// CHECK: namespace gcc_alias = clang;
+using namespace gcc;  // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | 
FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,14 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(), 
Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(), 
Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc;// CHECK: namespace gcc_alias = clang;
+using namespace gcc;  // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,14 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(), Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(), Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 1698799 - [clangd] Fix LineFoldingOnly flag is not propagated correctly to ClangdServer.

2022-09-05 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-09-05T15:55:29+02:00
New Revision: 16987998e6a0ad62efd4e387c18b0b9b5ef51947

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

LOG: [clangd] Fix LineFoldingOnly flag is not propagated correctly to 
ClangdServer.

The Opts.LineFoldingOnly must be set before the clangdServer
construction, otherwise this flag is always false when using clangd in VSCode.

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

Added: 
clang-tools-extra/clangd/test/folding-range.test

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

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 449ac9e3a85bd..c5238e426d692 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -466,32 +466,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   if (Server)
 return Reply(llvm::make_error("server already initialized",
 ErrorCode::InvalidRequest));
-  if (Opts.UseDirBasedCDB) {
-DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
-if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
-  CDBOpts.CompileCommandsDir = Dir;
-CDBOpts.ContextProvider = Opts.ContextProvider;
-BaseCDB =
-std::make_unique(CDBOpts);
-BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
-  }
-  auto Mangler = CommandMangler::detect();
-  if (Opts.ResourceDir)
-Mangler.ResourceDir = *Opts.ResourceDir;
-  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
-  tooling::ArgumentsAdjuster(std::move(Mangler)));
-  {
-// Switch caller's context with LSPServer's background context. Since we
-// rather want to propagate information from LSPServer's context into the
-// Server, CDB, etc.
-WithContext MainContext(BackgroundContext.clone());
-llvm::Optional WithOffsetEncoding;
-if (Opts.Encoding)
-  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
-Server.emplace(*CDB, TFS, Opts,
-   static_cast(this));
-  }
 
   Opts.CodeComplete.EnableSnippets = Params.capabilities.CompletionSnippets;
   Opts.CodeComplete.IncludeFixIts = Params.capabilities.CompletionFixes;
@@ -521,6 +495,33 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
   Opts.ImplicitCancellation = !Params.capabilities.CancelsStaleRequests;
 
+  if (Opts.UseDirBasedCDB) {
+DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
+if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
+  CDBOpts.CompileCommandsDir = Dir;
+CDBOpts.ContextProvider = Opts.ContextProvider;
+BaseCDB =
+std::make_unique(CDBOpts);
+BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
+ std::move(BaseCDB));
+  }
+  auto Mangler = CommandMangler::detect();
+  if (Opts.ResourceDir)
+Mangler.ResourceDir = *Opts.ResourceDir;
+  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
+  tooling::ArgumentsAdjuster(std::move(Mangler)));
+  {
+// Switch caller's context with LSPServer's background context. Since we
+// rather want to propagate information from LSPServer's context into the
+// Server, CDB, etc.
+WithContext MainContext(BackgroundContext.clone());
+llvm::Optional WithOffsetEncoding;
+if (Opts.Encoding)
+  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
+Server.emplace(*CDB, TFS, Opts,
+   static_cast(this));
+  }
+
   llvm::json::Object ServerCaps{
   {"textDocumentSync",
llvm::json::Object{

diff  --git a/clang-tools-extra/clangd/test/folding-range.test 
b/clang-tools-extra/clangd/test/folding-range.test
new file mode 100644
index 0..758980d2061d2
--- /dev/null
+++ b/clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+void f() {
+
+}
+---
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":
 {"foldingRange": {"lineFoldingOnly": true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void
 f() {\n\n}\n","uri":"test:///foo.cpp","version":1}}}
+---
+{"id":1,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:  "id": 

[PATCH] D133299: [clangd] Fix LineFoldingOnly flag is not propagated correctly to ClangdServer.

2022-09-05 Thread Haojian Wu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG16987998e6a0: [clangd] Fix LineFoldingOnly flag is not 
propagated correctly to ClangdServer. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133299

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/test/folding-range.test


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+void f() {
+
+}
+---
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":
 {"foldingRange": {"lineFoldingOnly": true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void
 f() {\n\n}\n","uri":"test:///foo.cpp","version":1}}}
+---
+{"id":1,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:  "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "endLine": 1,
+# CHECK-NEXT: "kind": "region", 
+# CHECK-NEXT: "startCharacter": 10,
+# CHECK-NEXT: "startLine": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -466,32 +466,6 @@
   if (Server)
 return Reply(llvm::make_error("server already initialized",
 ErrorCode::InvalidRequest));
-  if (Opts.UseDirBasedCDB) {
-DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
-if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
-  CDBOpts.CompileCommandsDir = Dir;
-CDBOpts.ContextProvider = Opts.ContextProvider;
-BaseCDB =
-std::make_unique(CDBOpts);
-BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
-  }
-  auto Mangler = CommandMangler::detect();
-  if (Opts.ResourceDir)
-Mangler.ResourceDir = *Opts.ResourceDir;
-  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
-  tooling::ArgumentsAdjuster(std::move(Mangler)));
-  {
-// Switch caller's context with LSPServer's background context. Since we
-// rather want to propagate information from LSPServer's context into the
-// Server, CDB, etc.
-WithContext MainContext(BackgroundContext.clone());
-llvm::Optional WithOffsetEncoding;
-if (Opts.Encoding)
-  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
-Server.emplace(*CDB, TFS, Opts,
-   static_cast(this));
-  }
 
   Opts.CodeComplete.EnableSnippets = Params.capabilities.CompletionSnippets;
   Opts.CodeComplete.IncludeFixIts = Params.capabilities.CompletionFixes;
@@ -521,6 +495,33 @@
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
   Opts.ImplicitCancellation = !Params.capabilities.CancelsStaleRequests;
 
+  if (Opts.UseDirBasedCDB) {
+DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
+if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
+  CDBOpts.CompileCommandsDir = Dir;
+CDBOpts.ContextProvider = Opts.ContextProvider;
+BaseCDB =
+std::make_unique(CDBOpts);
+BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
+ std::move(BaseCDB));
+  }
+  auto Mangler = CommandMangler::detect();
+  if (Opts.ResourceDir)
+Mangler.ResourceDir = *Opts.ResourceDir;
+  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
+  tooling::ArgumentsAdjuster(std::move(Mangler)));
+  {
+// Switch caller's context with LSPServer's background context. Since we
+// rather want to propagate information from LSPServer's context into the
+// Server, CDB, etc.
+WithContext MainContext(BackgroundContext.clone());
+llvm::Optional WithOffsetEncoding;
+if (Opts.Encoding)
+  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
+Server.emplace(*CDB, TFS, Opts,
+   static_cast(this));
+  }
+
   llvm::json::Object ServerCaps{
   {"textDocumentSync",
llvm::json::Object{


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-too

[PATCH] D133299: [clangd] Fix LineFoldingOnly flag is not propagated correctly to ClangdServer.

2022-09-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 457972.
hokein marked an inline comment as done.
hokein added a comment.

address comment: create the ClangdServer when Opts is fully initialized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133299

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/test/folding-range.test


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+void f() {
+
+}
+---
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":
 {"foldingRange": {"lineFoldingOnly": true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void
 f() {\n\n}\n","uri":"test:///foo.cpp","version":1}}}
+---
+{"id":1,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:  "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "endLine": 1,
+# CHECK-NEXT: "kind": "region", 
+# CHECK-NEXT: "startCharacter": 10,
+# CHECK-NEXT: "startLine": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -466,32 +466,6 @@
   if (Server)
 return Reply(llvm::make_error("server already initialized",
 ErrorCode::InvalidRequest));
-  if (Opts.UseDirBasedCDB) {
-DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
-if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
-  CDBOpts.CompileCommandsDir = Dir;
-CDBOpts.ContextProvider = Opts.ContextProvider;
-BaseCDB =
-std::make_unique(CDBOpts);
-BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
-  }
-  auto Mangler = CommandMangler::detect();
-  if (Opts.ResourceDir)
-Mangler.ResourceDir = *Opts.ResourceDir;
-  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
-  tooling::ArgumentsAdjuster(std::move(Mangler)));
-  {
-// Switch caller's context with LSPServer's background context. Since we
-// rather want to propagate information from LSPServer's context into the
-// Server, CDB, etc.
-WithContext MainContext(BackgroundContext.clone());
-llvm::Optional WithOffsetEncoding;
-if (Opts.Encoding)
-  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
-Server.emplace(*CDB, TFS, Opts,
-   static_cast(this));
-  }
 
   Opts.CodeComplete.EnableSnippets = Params.capabilities.CompletionSnippets;
   Opts.CodeComplete.IncludeFixIts = Params.capabilities.CompletionFixes;
@@ -521,6 +495,33 @@
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
   Opts.ImplicitCancellation = !Params.capabilities.CancelsStaleRequests;
 
+  if (Opts.UseDirBasedCDB) {
+DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
+if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
+  CDBOpts.CompileCommandsDir = Dir;
+CDBOpts.ContextProvider = Opts.ContextProvider;
+BaseCDB =
+std::make_unique(CDBOpts);
+BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
+ std::move(BaseCDB));
+  }
+  auto Mangler = CommandMangler::detect();
+  if (Opts.ResourceDir)
+Mangler.ResourceDir = *Opts.ResourceDir;
+  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
+  tooling::ArgumentsAdjuster(std::move(Mangler)));
+  {
+// Switch caller's context with LSPServer's background context. Since we
+// rather want to propagate information from LSPServer's context into the
+// Server, CDB, etc.
+WithContext MainContext(BackgroundContext.clone());
+llvm::Optional WithOffsetEncoding;
+if (Opts.Encoding)
+  WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding);
+Server.emplace(*CDB, TFS, Opts,
+   static_cast(this));
+  }
+
   llvm::json::Object ServerCaps{
   {"textDocumentSync",
llvm::json::Object{


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-tes

[PATCH] D133236: [clang-format] Use utf-8 for JSON object load

2022-09-05 Thread Amy Wang via Phabricator via cfe-commits
kaitingwang added a comment.

In D133236#3769784 , @ldrumm wrote:

> Do you have commit access?

@ldrumm I don't have commit access.  Would you commit for me?  Really 
appreciate it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133236

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


[PATCH] D133299: [clangd] Fix LineFoldingOnly flag is not propagated correctly to ClangdServer.

2022-09-05 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 accepted this revision.
usaxena95 added a comment.
This revision is now accepted and ready to land.

Thanks. Sorry I missed this.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:487-497
   {
 // Switch caller's context with LSPServer's background context. Since we
 // rather want to propagate information from LSPServer's context into the
 // Server, CDB, etc.
 WithContext MainContext(BackgroundContext.clone());
 llvm::Optional WithOffsetEncoding;
 if (Opts.Encoding)

I think we should do this after the Opts has been fully set to avoid the 
confusion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133299

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


[clang-tools-extra] b06372a - [clangd] NFC, correct template argument type for two RetiredFlags.

2022-09-05 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-09-05T15:20:31+02:00
New Revision: b06372ae58f0c8ae61f0ba5cf6ad2e4a7615a2a4

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

LOG: [clangd] NFC, correct template argument type for two RetiredFlags.

Added: 


Modified: 
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 11c5acb71d978..4280e810a412d 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -307,8 +307,8 @@ RetiredFlag AsyncPreamble("async-preamble");
 RetiredFlag CollectMainFileRefs("collect-main-file-refs");
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
-RetiredFlag InlayHints("inlay-hints");
-RetiredFlag FoldingRanges("folding-ranges");
+RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
 
 
 opt LimitResults{



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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:298
+  /// Style of aligning consecutive trailing comments.
+  /// This option existed as ``AlignTrailingComments`` since version 3.7.
+  ///

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > Isn’t this completely wrong?
> Could you please be more specific about why you think so when pointing out 
> something? I don't see why you think it's wrong. I also would like to mention 
> that I followed @HazardyKnusperkeks's request on this.
Ignore my comment



Comment at: clang/include/clang/Format/Format.h:384
 
-  /// If ``true``, aligns trailing comments.
-  /// \code
-  ///   true:   false:
-  ///   int a; // My comment a  vs. int a; // My comment a
-  ///   int b = 2; // comment  bint b = 2; // comment about b
-  /// \endcode
+  /// This option is **deprecated**. See ``AlignConsecutiveTrailingComments``.
   /// \version 3.7

MyDeveloperDay wrote:
> And this
Ignore this comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132797: [clangd] Support renaming virtual methods

2022-09-05 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554
+  Req.Subjects = {Base};
+  Index.relations(Req, [&](const SymbolID &, const Symbol &Override) {
+IDs.insert(Override.ID);

sammccall wrote:
> ilya-biryukov wrote:
> > Should we put a limit on the number of requests we send during recursion 
> > here?
> > 
> > I see a few obvious failure modes:
> > - infinite recursion in the relations due to parts of index being stale, 
> > corrupted input data or other reasons,
> > - exponential blow up in hierarchies with multiple inheritance,
> > - sending a lot of network requests in case of deep inheritance hierarchies 
> > for remote index implementations. Since all requests are sequential, the 
> > network latency might add up to substantial numbers.
> > 
> > We could address these in some other manner, this just seems to be the 
> > simplest option to protect against catastrophic outcomes (running the 
> > request indefinitely, crashing due to infinite recursion, etc).
> > exponential blow up in hierarchies with multiple inheritance,
> 
> It seems with little loss of readability we could provide some useful bounds:
> 
> ```
> DenseSet Pending = {Base};
> while (!Pending.empty()) {
>   Req = {.Subjects = Pending};
>   Pending.clear();
>   Index.relations(Req, { IDs.insert(ID); Pending.insert(ID) });
> }
> ```
> in this case the #requests is clearly bounded by the length of the shortest 
> chain to the most distant SymbolID, and is IMO safe with no numeric cap.
> 
> whereas the current version could potentially get the same ID in multiple 
> branches and so the bound on #requests is harder to determine.
This looks good! Also avoids infinite recursion.
Having a potentially large number of sequential network requests still looks 
unfortunate, but I doubt this will bite us in practice (at least not for remote 
indicies for LLVM and Chrome).

To solve it, we could allow recursive requests and implement the recursion 
inside the index, but this could be done with a follow-up when we actually hit 
this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132797

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

My personal preference is a new struct without Enabled




Comment at: clang/lib/Format/Format.cpp:1196
   LLVMStyle.AlignConsecutiveMacros = {};
+  LLVMStyle.AlignConsecutiveTrailingComments = {};
+  LLVMStyle.AlignConsecutiveTrailingComments.Enabled = true;

Won’t you need to initialise the members here as above



Comment at: clang/unittests/Format/FormatTestComments.cpp:2908
+  Style.MaxEmptyLinesToKeep = 0;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"

Any reason why these are not verifyformats.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits
yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added a comment.

Can we all agree with the decision whether we use the `AlignConsecutiveStyle` 
or introduce a new struct before I start implementing or updating the document?
IMO, both are reasonable in some respects so I'd like you owners to decide.

> Please also mark the comments as done once addressed so we know you’ve read 
> and fixed our requests

I'm reading all the comments but  leaving some comments not done on purpose 
that I think the discussion is not done. Isn't it a right thing to do here?




Comment at: clang/include/clang/Format/Format.h:298
+  /// Style of aligning consecutive trailing comments.
+  /// This option existed as ``AlignTrailingComments`` since version 3.7.
+  ///

MyDeveloperDay wrote:
> Isn’t this completely wrong?
Could you please be more specific about why you think so when pointing out 
something? I don't see why you think it's wrong. I also would like to mention 
that I followed @HazardyKnusperkeks's request on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D133299: [clangd] Fix LineFoldingOnly flag is not propagated correctly to ClangdServer.

2022-09-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added reviewers: usaxena95, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

The Opts.LineFoldingOnly must be set before the clangdServer
construction, otherwise this flag is always false when using clangd in VSCode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133299

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/test/folding-range.test


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+void f() {
+
+}
+---
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":
 {"foldingRange": {"lineFoldingOnly": true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void
 f() {\n\n}\n","uri":"test:///foo.cpp","version":1}}}
+---
+{"id":1,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:  "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "endLine": 1,
+# CHECK-NEXT: "kind": "region", 
+# CHECK-NEXT: "startCharacter": 10,
+# CHECK-NEXT: "startLine": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -459,6 +459,9 @@
  "no longer supported. Migrate to standard semanticTokens request");
   }
 
+  // Must be initialized before creating a ClangdServer object!
+  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
+
   if (Params.rootUri && *Params.rootUri)
 Opts.WorkspaceRoot = std::string(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
@@ -514,7 +517,6 @@
   Params.capabilities.HierarchicalDocumentSymbol;
   SupportFileStatus = Params.initializationOptions.FileStatus;
   HoverContentFormat = Params.capabilities.HoverContentFormat;
-  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
   SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
   if (Params.capabilities.WorkDoneProgress)
 BackgroundIndexProgressState = BackgroundIndexProgress::Empty;


Index: clang-tools-extra/clangd/test/folding-range.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/folding-range.test
@@ -0,0 +1,24 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+void f() {
+
+}
+---
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"foldingRange": {"lineFoldingOnly": true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void f() {\n\n}\n","uri":"test:///foo.cpp","version":1}}}
+---
+{"id":1,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:  "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "endLine": 1,
+# CHECK-NEXT: "kind": "region", 
+# CHECK-NEXT: "startCharacter": 10,
+# CHECK-NEXT: "startLine": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -459,6 +459,9 @@
  "no longer supported. Migrate to standard semanticTokens request");
   }
 
+  // Must be initialized before creating a ClangdServer object!
+  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
+
   if (Params.rootUri && *Params.rootUri)
 Opts.WorkspaceRoot = std::string(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
@@ -514,7 +517,6 @@
   Params.capabilities.HierarchicalDocumentSymbol;
   SupportFileStatus = Params.initializationOptions.FileStatus;
   HoverContentFormat = Params.capabilities.HoverContentFormat;
-  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
   SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
   if (Params.capabilities.WorkDoneProgress)
 BackgroundIndexProgressSt

[PATCH] D132797: [clangd] Support renaming virtual methods

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:548
+namespace {
+void recursivelyInsertOverrides(const SymbolID &Base,
+llvm::DenseSet &IDs,

sammccall wrote:
> The fact that we only need to walk down wasn't obvious to me at first, maybe 
> add a comment on this function? e.g.
> 
> ```
> Walk down from a virtual method to overriding methods, we rename them as a 
> group.
> Note that canonicalRenameDecl() ensures we're starting from the base method.
> ```
nit: can pass SymbolID by value, it's 8 bytes



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554
+  Req.Subjects = {Base};
+  Index.relations(Req, [&](const SymbolID &, const Symbol &Override) {
+IDs.insert(Override.ID);

ilya-biryukov wrote:
> Should we put a limit on the number of requests we send during recursion here?
> 
> I see a few obvious failure modes:
> - infinite recursion in the relations due to parts of index being stale, 
> corrupted input data or other reasons,
> - exponential blow up in hierarchies with multiple inheritance,
> - sending a lot of network requests in case of deep inheritance hierarchies 
> for remote index implementations. Since all requests are sequential, the 
> network latency might add up to substantial numbers.
> 
> We could address these in some other manner, this just seems to be the 
> simplest option to protect against catastrophic outcomes (running the request 
> indefinitely, crashing due to infinite recursion, etc).
> exponential blow up in hierarchies with multiple inheritance,

It seems with little loss of readability we could provide some useful bounds:

```
DenseSet Pending = {Base};
while (!Pending.empty()) {
  Req = {.Subjects = Pending};
  Pending.clear();
  Index.relations(Req, { IDs.insert(ID); Pending.insert(ID) });
}
```
in this case the #requests is clearly bounded by the length of the shortest 
chain to the most distant SymbolID, and is IMO safe with no numeric cap.

whereas the current version could potentially get the same ID in multiple 
branches and so the bound on #requests is harder to determine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132797

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


[PATCH] D132797: [clangd] Support renaming virtual methods

2022-09-05 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Thanks for the patch! A drive-by comment from me, hopefully useful.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554
+  Req.Subjects = {Base};
+  Index.relations(Req, [&](const SymbolID &, const Symbol &Override) {
+IDs.insert(Override.ID);

Should we put a limit on the number of requests we send during recursion here?

I see a few obvious failure modes:
- infinite recursion in the relations due to parts of index being stale, 
corrupted input data or other reasons,
- exponential blow up in hierarchies with multiple inheritance,
- sending a lot of network requests in case of deep inheritance hierarchies for 
remote index implementations. Since all requests are sequential, the network 
latency might add up to substantial numbers.

We could address these in some other manner, this just seems to be the simplest 
option to protect against catastrophic outcomes (running the request 
indefinitely, crashing due to infinite recursion, etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132797

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


[PATCH] D133298: [clang-rename] RecursiveSymbolVisitor - visit namespace aliases and using directives

2022-09-05 Thread imctrading via Phabricator via cfe-commits
imctrading created this revision.
imctrading added reviewers: klimek, dcastagna, arphaman.
Herald added a subscriber: jeroen.dobbelaere.
Herald added a project: All.
imctrading requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`RecursiveSymbolVisitor` - used by clang-rename does not visit namespace 
aliases and using directives. This patch fixes that.
This allows the tool to be used to rename namespaces.

Fixes https://github.com/llvm/llvm-project/issues/57194


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133298

Files:
  clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
  clang/test/clang-rename/Namespace.cpp


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc; // CHECK: namespace gcc_alias = clang;
+using namespace gcc; // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | 
FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,14 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(), 
Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(), 
Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;


Index: clang/test/clang-rename/Namespace.cpp
===
--- clang/test/clang-rename/Namespace.cpp
+++ clang/test/clang-rename/Namespace.cpp
@@ -6,6 +6,9 @@
   gcc::x = 42;// CHECK: clang::x = 42;
 }
 
+namespace gcc_alias = gcc; // CHECK: namespace gcc_alias = clang;
+using namespace gcc; // CHECK: using namespace clang;
+
 // Test 1.
 // RUN: clang-rename -offset=10 -new-name=clang %s -- | sed 's,//.*,,' | FileCheck %s
 
Index: clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
===
--- clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
+++ clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
@@ -133,6 +133,14 @@
 return true;
   }
 
+  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *Declaration) {
+return visit(Declaration->getAliasedNamespace(), Declaration->getTargetNameLoc(), Declaration->getEndLoc());
+  }
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *Declaration) {
+return visit(Declaration->getNominatedNamespace(), Declaration->getIdentLocation(), Declaration->getEndLoc());
+  }
+
 private:
   const SourceManager &SM;
   const LangOptions &LangOpts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132797: [clangd] Support renaming virtual methods

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry for the delay, this patch is really neat!

The only serious thing is it uncovers an existing bug which asserts. As a 
potential new crash I think we should fix that - LMK whether you feel 
comfortable adding that fix in here or you'd like me to do it as a separate 
review.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:245
   trace::Span Tracer("FindOccurrencesWithinFile");
   assert(canonicalRenameDecl(&ND) == &ND &&
  "ND should be already canonicalized.");

Unfortunately we've uncovered a bug: this assertion fires (after this patch) on 
the following code:

```
template  class Foo { virtual void m(); };
class Bar : Foo { void ^m() override; };
```

`canonicalRenameDecl` is supposed to be idempotent. 
However:`canonicalRenameDecl(Bar::m)` is `Foo::m`, but 
`canonicalRenameDecl(Foo::m)` is `Foo::m`.

I think this is because the `getInstantiatedFromMemberFunction` check comes 
before the `overridden_methods` check, so if the override points to a member of 
an instantiation then it's too late to map it back to the member of the 
template.

Probably the best fix is to change line 103 to a recursive `return 
canonicalRenameDecl(...)`, as is done elsewhere in the function.

(Can you please add this case to the tests?)



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:547
 
+namespace {
+void recursivelyInsertOverrides(const SymbolID &Base,

we're already in an anon namespace here



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:548
+namespace {
+void recursivelyInsertOverrides(const SymbolID &Base,
+llvm::DenseSet &IDs,

The fact that we only need to walk down wasn't obvious to me at first, maybe 
add a comment on this function? e.g.

```
Walk down from a virtual method to overriding methods, we rename them as a 
group.
Note that canonicalRenameDecl() ensures we're starting from the base method.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132797

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


[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

2022-09-05 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

Generally LGTM. Please revisit the documentation and let's fix the style, and 
then we can land.

In D91000#3197851 , @aaron.ballman 
wrote:

> In terms of whether we should expose the check in C++: I think we shouldn't. 
> [...]
>
> I think we should probably also not enable the check when the user compiles 
> in C99 or earlier mode, because there is no Annex K available to provide 
> replacement functions.

@aaron.ballman I think the current version of the check satisfies these 
conditions. It seems the check **will** run for every TU, but in case there is 
no alternative the check could suggest, it will do nothing:

  if (!ReplacementFunctionName)
return;

Is this good enough? This seems more future-proof than juggling the `LangOpts` 
instance in yet another member function.




Comment at: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp:1
+//===--- UnsafeFunctionsCheck.cpp - clang-tidy ---===//
+//

Ditto.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp:150
+   Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) 
{
+  this->PP = PP;

(Just so that we do not get "unused variable" warnings when compiling.)



Comment at: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h:1
+//===--- UnsafeFunctionsCheck.h - clang-tidy ---*- C++ -*-===//
+//

Style nit: length of line is not padded to 80-col.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst:31-40
+The following functions are also checked (regardless of Annex K availability):
+ - `rewind`, suggested replacement: `fseek`
+ - `setbuf`, suggested replacement: `setvbuf`
+
+Based on the ReportMoreUnsafeFunctions option, the following functions are 
also checked:
+ - `bcmp`, suggested replacement: `memcmp`
+ - `bcopy`, suggested replacement: `memcpy_s` if Annex K is available, or 
`memcopy` if Annex K is not available

In general, the **rendered version** of the docs should be **visually** 
observed, as backtick in RST only turns on emphasis mode and not monospacing, 
i.e. it will be //memcpy_s// and not `memcpy_s`. Please look at the other 
checks and if there is a consistent style (e.g. symbol names (function) are 
monospaced, but options of the check is emphasised) align it to that so we have 
the documentation "fall into place" visually.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst:52
+Both macros have to be defined to suggest replacement functions from Annex K. 
`__STDC_LIB_EXT1__` is
+defined by the library implementation, and `__STDC_WANT_LIB_EXT1__` must be 
defined to "1" by the user
+before including any system headers.

(Style nit: yeah this will not look like a symbol at all here)


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

https://reviews.llvm.org/D91000

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2022-09-05 Thread Guillot Tony via Phabricator via cfe-commits
to268 created this revision.
to268 added reviewers: aaron.ballman, clang-language-wg, jyknight.
Herald added a project: All.
to268 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patches implements the `auto` keyword from the N3007 
 standard 
specification.
This allows deducing the type of the variable like in C++

  auto nb = 1;
  auto chr = 'A';
  auto str = "String";


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133289

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/test/C/C2X/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1193,7 +1193,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007
-  No
+  Yes
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+#include 
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_structs(void) {
+  struct s_auto { auto a; };  // expected-error {{'auto' not allowed in struct member}}
+  auto s_int = (struct { int a; } *)0;
+  typedef auto auto_type; // expected-error {{'auto' not allowed in typedef}}
+}
+
+void test_sizeof_alignas(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  alignas(4) auto b[4];   // expected-error {{'b' declared as array of 'auto'}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 'auto'}}
+}
+
+void test_qualifiers(int x, const int y) {
+  const auto a = x;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa = &a; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = &b;
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+  auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+int a;
+int b;
+union {
+  char c;
+  auto smth; // c2x-error {{'auto' not allowed in union member}} \
+c17-error {{type name does not allow storage class to be specified}} \
+c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+} u;
+};
+
+enum E : auto { // c2x-error {{'auto' not allowed here}} \
+   c17-error {{expected a type}} \
+   c17-error {{type name does not allow storage class to be specified}}
+  One,
+  Two,
+  Tree,
+};
+
+auto auto_usage(auto auto) {  // c2x-error {{'auto' not allowed in function prototype}} \
+ c2x-error {{'auto' not allowed in function return type}} \
+ c2x-error {{cannot combine with previous 'auto' declaration specifier}} \
+ c17-error {{invalid storage class specifier in function declarator}} \
+ c17-error {{illegal storage class on function}} \
+ c17-warning {{duplicate 'auto' declaration specifier}} \
+ c17-warning {{omitting the parameter name in a function definition is a C2x extension}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+ c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+
+  auto = 4;   // expected-error {{expected identifier or '('}}
+
+  auto

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

2022-09-05 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: clang/docs/UsersManual.rst:3602
 
+Linking is done using ``spirv-link`` from `the SPIRV-Tools project
+`_. Similar to other 
external

Anastasia wrote:
> bader wrote:
> > @Anastasia, sorry for late feedback.
> > I think being able to link SPIR-V modules is a great feature, but I have a 
> > concerns regarding `spirv-link` tool.
> > The documentation says that the linker tool is still under development and 
> > from our experience this tool had issues blocking us from using it for SYCL 
> > mode. The last time new features were added to this tool is almost 4 year 
> > ago.
> > Do you know if there are any plans for to finish the development and if ? 
> > Are you aware of any "real-world usages" of this tool? Have you tried to 
> > use it for SPIR-V module produced from C++ (e.g. C++ for OpenCL)?
> > I think supporting SPIR-V extensions like [[ 
> > https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_linkonce_odr.asciidoc
> >  | SPV_KHR_linkonce_odr ]] is quite important for code size and JIT 
> > compilation time reduction. As this extension was ratified recently, I 
> > suppose `spirv-link` doesn't support it yet.
> Hi Alexey,
> 
> Sorry for the late reply. Do you have any other suggestions about the tools 
> that can be used for linking SPIR-V binaries? 
> 
> I am not in contact with the maintainers but it is an open-source project so 
> I imagine contributions to enhance or improve functionality should be 
> welcome... unless you have other experiences?
> 
> Do you have any other suggestions about the tools that can be used for 
> linking SPIR-V binaries?

I'm unaware of other tools for SPIR-V binaries linking. To link SPIR-V binaries 
in our toolchain, we translate them to/from LLVM IR to link LLVM IR.

> I am not in contact with the maintainers but it is an open-source project so 
> I imagine contributions to enhance or improve functionality should be 
> welcome... unless you have other experiences?

I talked to the maintainers (but it was quite long time ago) and they told me 
that there are no active contributors to this tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116266

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:794
+
+AlignConsecutiveMacros: AcrossEmptyLines
+

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > Should this say `AlignedConsecutuveCommets`
> > No. This part is a documentation about AlignConsecutiveStyle type, which is 
> > appended automatically after all the options that take 
> > AlignConsecutiveStyle type.
> Which we could change to reflect that it's used for multiple options.
Or you could use your own struct! And it would be correct and not full of 
options that don’t apply. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

I feel this patch leave the documentation in a right state, I won’t be giving 
it an accept in this form. Please also mark the comments as done once addressed 
so we know you’ve read and fixed our requests




Comment at: clang/docs/ClangFormatStyleOptions.rst:823
+
+  * ``bool AcrossEmptyLines`` Whether to align across empty lines.
+

I’m sorry I just don’t think this documentation is now correct, all because you 
are trying I believe to reuse a structure that has options that’s not needed,



Comment at: clang/include/clang/Format/Format.h:298
+  /// Style of aligning consecutive trailing comments.
+  /// This option existed as ``AlignTrailingComments`` since version 3.7.
+  ///

Isn’t this completely wrong?



Comment at: clang/include/clang/Format/Format.h:384
 
-  /// If ``true``, aligns trailing comments.
-  /// \code
-  ///   true:   false:
-  ///   int a; // My comment a  vs. int a; // My comment a
-  ///   int b = 2; // comment  bint b = 2; // comment about b
-  /// \endcode
+  /// This option is **deprecated**. See ``AlignConsecutiveTrailingComments``.
   /// \version 3.7

And this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd3dd10a8b48: [clang-format] Concepts: allow identifiers 
after negation (authored by rymiel, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131978

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,13 @@
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("template \n"
+"concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
   Tokens = annotate("template \n"
 "concept C = requires(T t) {\n"
 "  { t.foo() };\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24217,6 +24217,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3544,7 +3544,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_requires: // Initial identifier of a requires clause.
   case tok::equal:   // Initial identifier of a concept declaration.
 break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,13 @@
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("template \n"
+"concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
   Tokens = annotate("template \n"
 "concept C = requires(T t) {\n"
 "  { t.foo() };\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24217,6 +24217,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3544,7 +3544,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_require

[PATCH] D132762: [clang-format] Allow `throw` to be a keyword in front of casts

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6e7752f8e14: [clang-format] Allow `throw` to be a keyword 
in front of casts (authored by rymiel, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132762

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -319,6 +319,34 @@
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
+  auto Tokens = annotate("(void)p;");
+  EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("auto x = (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("(std::vector)p;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("return (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("throw (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsDynamicExceptionSpecifier) {
+  auto Tokens = annotate("void f() throw(int);");
+  EXPECT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_throw, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsFunctionRefQualifiers) {
   auto Tokens = annotate("void f() &;");
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11038,6 +11038,7 @@
   verifyFormat("my_int a = (my_int)2.0f;");
   verifyFormat("my_int a = (my_int)sizeof(int);");
   verifyFormat("return (my_int)aaa;");
+  verifyFormat("throw (my_int)aaa;");
   verifyFormat("#define x ((int)-1)");
   verifyFormat("#define LENGTH(x, y) (x) - (y) + 1");
   verifyFormat("#define p(q) ((int *)&q)");
@@ -15730,6 +15731,7 @@
   verifyFormat("Type *A = ( Type * )P;", Spaces);
   verifyFormat("Type *A = ( vector )P;", Spaces);
   verifyFormat("x = ( int32 )y;", Spaces);
+  verifyFormat("throw ( int32 )x;", Spaces);
   verifyFormat("int a = ( int )(2.0f);", Spaces);
   verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
   verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
@@ -15793,6 +15795,7 @@
   verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
   verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
   verifyFormat("bool *y = ( bool * ) (x);", Spaces);
+  verifyFormat("throw ( int32 ) x;", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -15817,6 +15820,8 @@
   verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
+  verifyFormat("throw (int32) x;", Spaces);
+
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2153,7 +2153,7 @@
   // before the parentheses, this is unlikely to be a cast.
   if (LeftOfParens->Tok.getIdentifierInfo() &&
   !LeftOfParens->isOneOf(Keywords.kw_in, tok::kw_return, tok::kw_case,
- tok::kw_delete)) {
+ tok::kw_delete, tok::kw_throw)) {
 return false;
   }
 
@@ -3315,6 +3315,10 @@
   !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash)) {
 return true;
   }
+  if (Left.is(tok::kw_throw) && Right.is(tok::l_paren) && Right.MatchingParen 
&&
+  Right.MatchingParen->is(TT_CastRParen)) {
+return true;
+  }
   if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
 return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -319,6 +319,34 @@
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest,

[PATCH] D132189: [clang-format] Don't put `noexcept` on empty line following constructor

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf54d42abcf82: [clang-format] Don't put `noexcept` on 
empty line following constructor (authored by rymiel, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132189

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -788,6 +788,19 @@
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
+  auto Tokens = annotate("template \n"
+ "DEPRECATED(\"Use NewClass::NewFunction instead.\")\n"
+ "string OldFunction(const string ¶meter) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_FunctionAnnotationRParen);
+
+  Tokens = annotate("template \n"
+"A(T) noexcept;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9607,6 +9607,15 @@
   verifyFormat("template  // T can be A, B or C.\n"
"struct C {};",
AlwaysBreak);
+  verifyFormat("template \n"
+   "C(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "ClassName(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "POOR_NAME(T) noexcept;",
+   AlwaysBreak);
   verifyFormat("template  class A {\n"
"public:\n"
"  E *f();\n"
@@ -9617,6 +9626,9 @@
   verifyFormat("template  class C {};", NeverBreak);
   verifyFormat("template  void f();", NeverBreak);
   verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template  C(T) noexcept;", NeverBreak);
+  verifyFormat("template  ClassName(T) noexcept;", NeverBreak);
+  verifyFormat("template  POOR_NAME(T) noexcept;", NeverBreak);
   verifyFormat("template \nvoid foo(aa "
") {}",
NeverBreak);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1924,7 +1924,7 @@
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
  tok::comma, tok::period, tok::arrow,
- tok::coloncolon)) {
+ tok::coloncolon, tok::kw_noexcept)) {
 if (FormatToken *AfterParen = Current.MatchingParen->Next) {
   // Make sure this isn't the return type of an Obj-C block declaration
   if (AfterParen->isNot(tok::caret)) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -788,6 +788,19 @@
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
+  auto Tokens = annotate("template \n"
+ "DEPRECATED(\"Use NewClass::NewFunction instead.\")\n"
+ "string OldFunction(const string ¶meter) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_FunctionAnnotationRParen);
+
+  Tokens = annotate("template \n"
+"A(T) noexcept;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9607,6 +9607,15 @@
   verifyFormat("template  // T can be A, B or C.\n"
"struct C {};",
AlwaysBreak);
+  verifyFormat("template \n"
+   "C(T) noexcept;

[clang] bd3dd10 - [clang-format] Concepts: allow identifiers after negation

2022-09-05 Thread Björn Schäpers via cfe-commits

Author: Emilia Dreamer
Date: 2022-09-05T12:35:40+02:00
New Revision: bd3dd10a8b489ce50823b4cc0049f16610adeee2

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

LOG: [clang-format] Concepts: allow identifiers after negation

Previously, the formatter would refuse to treat identifiers within a
compound concept definition as actually part of the definition, if
they were after the negation operator !. It is now made consistent
with the likes of && and ||.

Fixes https://github.com/llvm/llvm-project/issues/55898

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 1e775f70a24a..02b13be0d92e 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3544,7 +3544,8 @@ void UnwrappedLineParser::parseConstraintExpression() {
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_requires: // Initial identifier of a requires clause.
   case tok::equal:   // Initial identifier of a concept declaration.
 break;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 4383a903b2f6..88c16884d0d8 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24217,6 +24217,15 @@ TEST_F(FormatTest, Concepts) {
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 60684d72605e..a839fb29115f 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,13 @@ TEST_F(TokenAnnotatorTest, 
UnderstandsRequiresClausesAndConcepts) {
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("template \n"
+"concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
   Tokens = annotate("template \n"
 "concept C = requires(T t) {\n"
 "  { t.foo() };\n"



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


[clang] c6e7752 - [clang-format] Allow `throw` to be a keyword in front of casts

2022-09-05 Thread Björn Schäpers via cfe-commits

Author: Emilia Dreamer
Date: 2022-09-05T12:35:39+02:00
New Revision: c6e7752f8e144ad78aee2e56a7c901c56be360de

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

LOG: [clang-format] Allow `throw` to be a keyword in front of casts

This makes throw more similar to return. However, unlike return,
it has to more strict as to not remove spaces after usages of throw as
a (deprecated) exception specifier.

Fixes https://github.com/llvm/llvm-project/issues/57391

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 663e8ae6a898..d7c2e66ca488 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2153,7 +2153,7 @@ class AnnotatingParser {
   // before the parentheses, this is unlikely to be a cast.
   if (LeftOfParens->Tok.getIdentifierInfo() &&
   !LeftOfParens->isOneOf(Keywords.kw_in, tok::kw_return, tok::kw_case,
- tok::kw_delete)) {
+ tok::kw_delete, tok::kw_throw)) {
 return false;
   }
 
@@ -3315,6 +3315,10 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
   !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash)) {
 return true;
   }
+  if (Left.is(tok::kw_throw) && Right.is(tok::l_paren) && Right.MatchingParen 
&&
+  Right.MatchingParen->is(TT_CastRParen)) {
+return true;
+  }
   if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
 return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index ce56570697cd..4383a903b2f6 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -11038,6 +11038,7 @@ TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("my_int a = (my_int)2.0f;");
   verifyFormat("my_int a = (my_int)sizeof(int);");
   verifyFormat("return (my_int)aaa;");
+  verifyFormat("throw (my_int)aaa;");
   verifyFormat("#define x ((int)-1)");
   verifyFormat("#define LENGTH(x, y) (x) - (y) + 1");
   verifyFormat("#define p(q) ((int *)&q)");
@@ -15730,6 +15731,7 @@ TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   verifyFormat("Type *A = ( Type * )P;", Spaces);
   verifyFormat("Type *A = ( vector )P;", Spaces);
   verifyFormat("x = ( int32 )y;", Spaces);
+  verifyFormat("throw ( int32 )x;", Spaces);
   verifyFormat("int a = ( int )(2.0f);", Spaces);
   verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
   verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
@@ -15793,6 +15795,7 @@ TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
   verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
   verifyFormat("bool *y = ( bool * ) (x);", Spaces);
+  verifyFormat("throw ( int32 ) x;", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -15817,6 +15820,8 @@ TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
+  verifyFormat("throw (int32) x;", Spaces);
+
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 21ef5c382a90..60684d72605e 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -319,6 +319,34 @@ TEST_F(TokenAnnotatorTest, UnderstandsDelete) {
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
+  auto Tokens = annotate("(void)p;");
+  EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("auto x = (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("(std::vector)p;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("return (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("throw (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << 

[clang] f54d42a - [clang-format] Don't put `noexcept` on empty line following constructor

2022-09-05 Thread Björn Schäpers via cfe-commits

Author: Emilia Dreamer
Date: 2022-09-05T12:35:39+02:00
New Revision: f54d42abcf82e122ed0ac4688f01c1f4da5599f2

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

LOG: [clang-format] Don't put `noexcept` on empty line following constructor

With the AlwaysBreakTemplateDeclarations option, having a constructor
template for a type consisting of all-uppercase letters with a
noexcept specifier would put said noexcept specifier on its own blank
line.

This is because the all-uppercase type is understood as a macro-like
attribute (such as DEPRECATED()), and noexcept is seen as the
declaration. However, noexcept is a keyword and cannot be an
identifier on its own.

Fixes https://github.com/llvm/llvm-project/issues/56216

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 2a64a3bb9c413..663e8ae6a8986 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1924,7 +1924,7 @@ class AnnotatingParser {
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
  tok::comma, tok::period, tok::arrow,
- tok::coloncolon)) {
+ tok::coloncolon, tok::kw_noexcept)) {
 if (FormatToken *AfterParen = Current.MatchingParen->Next) {
   // Make sure this isn't the return type of an Obj-C block declaration
   if (AfterParen->isNot(tok::caret)) {

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d330c65470f0d..ce56570697cd1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9607,6 +9607,15 @@ TEST_F(FormatTest, WrapsTemplateDeclarations) {
   verifyFormat("template  // T can be A, B or C.\n"
"struct C {};",
AlwaysBreak);
+  verifyFormat("template \n"
+   "C(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "ClassName(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "POOR_NAME(T) noexcept;",
+   AlwaysBreak);
   verifyFormat("template  class A {\n"
"public:\n"
"  E *f();\n"
@@ -9617,6 +9626,9 @@ TEST_F(FormatTest, WrapsTemplateDeclarations) {
   verifyFormat("template  class C {};", NeverBreak);
   verifyFormat("template  void f();", NeverBreak);
   verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template  C(T) noexcept;", NeverBreak);
+  verifyFormat("template  ClassName(T) noexcept;", NeverBreak);
+  verifyFormat("template  POOR_NAME(T) noexcept;", NeverBreak);
   verifyFormat("template \nvoid foo(aa "
") {}",
NeverBreak);

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 6a10d679fbc72..21ef5c382a900 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -788,6 +788,19 @@ TEST_F(TokenAnnotatorTest, UnderstandsLambdas) {
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
+  auto Tokens = annotate("template \n"
+ "DEPRECATED(\"Use NewClass::NewFunction instead.\")\n"
+ "string OldFunction(const string ¶meter) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_FunctionAnnotationRParen);
+
+  Tokens = annotate("template \n"
+"A(T) noexcept;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));



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


[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

Can you please rebase the patch, it doesn't apply anymore.
Sorry for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132295

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


[PATCH] D132867: [Clang] Use virtual FS in processing config files

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

This seems like the right thing in principle, but it's still pretty scary:

- this is going to break anyone who's using VFS != RealFS together with config 
files, as we no longer search the old locations
- the breakage is likely to be quiet/subtle, especially if it's e.g. an obscure 
flag being set in an arch-specific config file
- the fixes for "part of the toolchain is missing from VFS" tend to need code 
changes in tools and tend to be hard to test

The thing the change has going for it is that I think config files and 
nontrivial VFS are both pretty rarely used, so maybe there are few people using 
them together.
(Google uses both, but having looked internally I don't believe we ever 
actually combine them).

So I think this should land, but it needs a release note as it's a non-obvious 
breaking change.
@MaskRay in case he has any thoughts about VFS use in driver in general.




Comment at: llvm/lib/Support/CommandLine.cpp:1268
+  if (!CurrentDir) {
+if (auto CWD = FS.getCurrentWorkingDirectory()) {
+  CurrDir = *CWD;

the old behavior was explicitly documented ("process' cwd is used instead", not 
FS's) so that documentation should be updated.

(I just talked to @kadircet about the patch that added that documentation, and 
we can't think of any reason that the old behavior is actually desirable)



Comment at: llvm/lib/Support/CommandLine.cpp:1271
+} else {
+  // TODO: The error should be propagated up the stack.
+  llvm::consumeError(llvm::errorCodeToError(CWD.getError()));

(This seems unidiomatic: consumeError(errorCodeToError(...)) is a no-op, we 
usually spell TODO as FIXME... but it matches the surrounding code, so best 
as-is I guess)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132867

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


[PATCH] D133236: [clang-format] Use utf-8 for JSON object load

2022-09-05 Thread Luke Drummond via Phabricator via cfe-commits
ldrumm added a comment.

Do you have commit access?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133236

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


[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

2022-09-05 Thread Saiyedul Islam via Phabricator via cfe-commits
saiislam accepted this revision.
saiislam added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131763

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


[PATCH] D133236: [clang-format] Use utf-8 for JSON object load

2022-09-05 Thread Luke Drummond via Phabricator via cfe-commits
ldrumm accepted this revision.
ldrumm added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133236

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


[PATCH] D126498: [clangd] Fix hover crashing on integral or enumeral casts

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry about this! Landing now


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

https://reviews.llvm.org/D126498

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


[PATCH] D133236: [Clang][Tools] Use utf-8 for JSON object load

2022-09-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133236

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


[clang] 5de4d97 - Driver tests: remove `REQUIRES: x86-registered-target` and set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`.

2022-09-05 Thread Ying Yi via cfe-commits

Author: Ying Yi
Date: 2022-09-05T09:59:47+01:00
New Revision: 5de4d97a00b2a5d710892e96d77810784fd2cd5c

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

LOG: Driver tests: remove `REQUIRES: x86-registered-target` and set 
`--sysroot=""` to support clang with `DEFAULT_SYSROOT`.

When testing clang that has been compiled with -DDEFAULT_SYSROOT set to some 
path, ps4-ps5-header-search.c would fail.

The test needs to be updated.

1. Remove unnecessary REQUIRES: x86-registered-target.
2. Override sysroot to be empty string for the test to succeed when clang 
is configured with DEFAULT_SYSROOT.

Added: 


Modified: 
clang/test/Driver/ps4-ps5-header-search.c

Removed: 




diff  --git a/clang/test/Driver/ps4-ps5-header-search.c 
b/clang/test/Driver/ps4-ps5-header-search.c
index 6848901df5590..59d3f2ab2d1df 100644
--- a/clang/test/Driver/ps4-ps5-header-search.c
+++ b/clang/test/Driver/ps4-ps5-header-search.c
@@ -1,8 +1,6 @@
-// REQUIRES: x86-registered-target
-
 /// PS4 and PS5 use the same SDK layout, so use the same tree for both.
-// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target 
x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
-// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target 
x86_64-sie-ps5 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target 
x86_64-scei-ps4 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
+// RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target 
x86_64-sie-ps5 --sysroot="" -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
 // ENVPS4: Inputs/scei-ps4_tree/target/include{{$}}
 // ENVPS4: Inputs/scei-ps4_tree/target/include_common{{$}}
 // ENVPS4-NOT: /usr/include



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


[PATCH] D133197: [clang] Fix crash when parsing scanf format string with missing arguments

2022-09-05 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGab09043a1985: [clang] Fix crash when parsing scanf format 
string with missing arguments (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133197

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/format-strings-scanf.c


Index: clang/test/Sema/format-strings-scanf.c
===
--- clang/test/Sema/format-strings-scanf.c
+++ clang/test/Sema/format-strings-scanf.c
@@ -69,6 +69,11 @@
   scanf("%#.2Lf", ld); // expected-warning{{invalid conversion specifier '#'}}
 }
 
+void missing_argument_with_length_modifier() {
+  char buf[30];
+  scanf("%s:%900s", buf); // expected-warning{{more '%' conversions than data 
arguments}}
+}
+
 // Test that the scanf call site is where the warning is attached.  If the
 // format string is somewhere else, point to it in a note.
 void pr9751(void) {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1066,6 +1066,9 @@
   return llvm::None;
 unsigned NewIndex = *IndexOptional;
 
+if (NewIndex >= TheCall->getNumArgs())
+  return llvm::None;
+
 const Expr *ObjArg = TheCall->getArg(NewIndex);
 uint64_t Result;
 if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType))


Index: clang/test/Sema/format-strings-scanf.c
===
--- clang/test/Sema/format-strings-scanf.c
+++ clang/test/Sema/format-strings-scanf.c
@@ -69,6 +69,11 @@
   scanf("%#.2Lf", ld); // expected-warning{{invalid conversion specifier '#'}}
 }
 
+void missing_argument_with_length_modifier() {
+  char buf[30];
+  scanf("%s:%900s", buf); // expected-warning{{more '%' conversions than data arguments}}
+}
+
 // Test that the scanf call site is where the warning is attached.  If the
 // format string is somewhere else, point to it in a note.
 void pr9751(void) {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1066,6 +1066,9 @@
   return llvm::None;
 unsigned NewIndex = *IndexOptional;
 
+if (NewIndex >= TheCall->getNumArgs())
+  return llvm::None;
+
 const Expr *ObjArg = TheCall->getArg(NewIndex);
 uint64_t Result;
 if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ab09043 - [clang] Fix crash when parsing scanf format string with missing arguments

2022-09-05 Thread via cfe-commits

Author: serge-sans-paille
Date: 2022-09-05T10:54:18+02:00
New Revision: ab09043a1985bfb9f1e4393a29a9d83326d306fe

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

LOG: [clang] Fix crash when parsing scanf format string with missing arguments

When parsing a format string with less argument than specified, one should check
argument access because there may be no such argument.

This fixes #57517

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/format-strings-scanf.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 897ab701493c2..afe99ad24fb9d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1066,6 +1066,9 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
   return llvm::None;
 unsigned NewIndex = *IndexOptional;
 
+if (NewIndex >= TheCall->getNumArgs())
+  return llvm::None;
+
 const Expr *ObjArg = TheCall->getArg(NewIndex);
 uint64_t Result;
 if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType))

diff  --git a/clang/test/Sema/format-strings-scanf.c 
b/clang/test/Sema/format-strings-scanf.c
index aebb68c37fb98..eb5b8ec36bf7a 100644
--- a/clang/test/Sema/format-strings-scanf.c
+++ b/clang/test/Sema/format-strings-scanf.c
@@ -69,6 +69,11 @@ void bad_length_modifiers(char *s, void *p, wchar_t *ws, 
long double *ld) {
   scanf("%#.2Lf", ld); // expected-warning{{invalid conversion specifier '#'}}
 }
 
+void missing_argument_with_length_modifier() {
+  char buf[30];
+  scanf("%s:%900s", buf); // expected-warning{{more '%' conversions than data 
arguments}}
+}
+
 // Test that the scanf call site is where the warning is attached.  If the
 // format string is somewhere else, point to it in a note.
 void pr9751(void) {



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


[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

Please state a name and email for the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131978

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


[PATCH] D133087: [clang-format][NFC][Docs] fix wrong example of warping class definitions

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D133087#3768928 , @Passw wrote:

> I do not have commit access, please help to commit this change. 
> Thanks.

Please state a name and email for the commit.


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

https://reviews.llvm.org/D133087

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


[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

2022-09-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D126365#3736864 , @Eitot wrote:

> In D126365#3565891 , @curdeius 
> wrote:
>
>> @Eitot, do you need help landing this?
>
> I do need help. Could someone land this for me?

Please state a name and email for the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126365

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


[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D130181#3769083 , @JonasToth wrote:

> ...

Your concerns aren't actually that important. Because the transformations only 
work on for binary operators, and not CXXOperatorCallExpr, it would always 
never do any special logic, instead just wrap the whole thing in parens and 
negate it

  if (!(A && B))
continue;
  
  if (!(!B && C))
continue;
  
  padLines();

The only potential issue would be cases when the binary operator is type 
dependent, as binary operators where they type is unresolved are handled as 
BinaryOperators, even if every instantiation would be resolved to an operator 
call

  template 
  void fancyMatrix(Matrix A, Matrix B) {
  
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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


  1   2   >