[PATCH] D150089: [LoongArch] Support fcc* (condition flag) registers in inlineasm clobbers

2023-05-07 Thread Lu Weining via Phabricator via cfe-commits
SixWeining created this revision.
Herald added a project: All.
SixWeining requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150089

Files:
  clang/lib/Basic/Targets/LoongArch.cpp
  clang/test/CodeGen/LoongArch/inline-asm-gcc-regs-error.c
  clang/test/CodeGen/LoongArch/inline-asm-gcc-regs.c
  llvm/test/CodeGen/LoongArch/inline-asm-clobbers-fcc.mir


Index: llvm/test/CodeGen/LoongArch/inline-asm-clobbers-fcc.mir
===
--- /dev/null
+++ llvm/test/CodeGen/LoongArch/inline-asm-clobbers-fcc.mir
@@ -0,0 +1,33 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc --mtriple=loongarch64 --mattr=+d --run-pass=greedy %s -o - | 
FileCheck %s
+
+## Check that fcc register clobbered by inlineasm is correctly saved by examing
+## a pair of pseudos (PseudoST_CFR and PseudoLD_CFR) are generated before and
+## after the INLINEASM.
+...
+---
+name: test
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+liveins: $f0_64, $f1_64
+
+; CHECK-LABEL: name: test
+; CHECK: liveins: $f0_64, $f1_64
+; CHECK-NEXT: {{  $}}
+; CHECK-NEXT: [[COPY:%[0-9]+]]:fpr64 = COPY $f1_64
+; CHECK-NEXT: [[COPY1:%[0-9]+]]:fpr64 = COPY $f0_64
+; CHECK-NEXT: [[FCMP_CLT_D:%[0-9]+]]:cfr = FCMP_CLT_D [[COPY]], [[COPY1]]
+; CHECK-NEXT: PseudoST_CFR [[FCMP_CLT_D]], %stack.0, 0 :: (store (s64) 
into %stack.0)
+; CHECK-NEXT: INLINEASM , 1 /* sideeffect attdialect */, 12 /* clobber 
*/, implicit-def dead early-clobber $fcc0
+; CHECK-NEXT: [[PseudoLD_CFR:%[0-9]+]]:cfr = PseudoLD_CFR %stack.0, 0 :: 
(load (s64) from %stack.0)
+; CHECK-NEXT: $r4 = COPY [[PseudoLD_CFR]]
+; CHECK-NEXT: PseudoRET implicit killed $r4
+%1:fpr64 = COPY $f1_64
+%0:fpr64 = COPY $f0_64
+%2:cfr = FCMP_CLT_D %1, %0
+INLINEASM &"nop", 1 /* sideeffect attdialect */, 12 /* clobber */, 
implicit-def dead early-clobber $fcc0
+$r4 = COPY %2
+PseudoRET implicit killed $r4
+
+...
Index: clang/test/CodeGen/LoongArch/inline-asm-gcc-regs.c
===
--- clang/test/CodeGen/LoongArch/inline-asm-gcc-regs.c
+++ clang/test/CodeGen/LoongArch/inline-asm-gcc-regs.c
@@ -100,3 +100,11 @@
 register float a asm ("$fs2");
 asm ("" :: "f" (a));
 }
+
+// CHECK-LABEL: @test_fcc
+// CHECK: call void asm sideeffect "", "~{$fcc0}"()
+// CHECK: call void asm sideeffect "", "~{$fcc7}"()
+void test_fcc() {
+asm ("" ::: "$fcc0");
+asm ("" ::: "$fcc7");
+}
Index: clang/test/CodeGen/LoongArch/inline-asm-gcc-regs-error.c
===
--- clang/test/CodeGen/LoongArch/inline-asm-gcc-regs-error.c
+++ clang/test/CodeGen/LoongArch/inline-asm-gcc-regs-error.c
@@ -19,4 +19,6 @@
   register float a5 asm ("f0");
 // CHECK: :[[#@LINE+1]]:26: error: unknown register name 'fa0' in asm
   register float a6 asm ("fa0");
+// CHECK: :[[#@LINE+1]]:15: error: unknown register name 'fcc0' in asm
+  asm ("" ::: "fcc0");
 }
Index: clang/lib/Basic/Targets/LoongArch.cpp
===
--- clang/lib/Basic/Targets/LoongArch.cpp
+++ clang/lib/Basic/Targets/LoongArch.cpp
@@ -31,7 +31,9 @@
   "$f0", "$f1", "$f2", "$f3", "$f4", "$f5", "$f6", "$f7", "$f8", "$f9",
   "$f10", "$f11", "$f12", "$f13", "$f14", "$f15", "$f16", "$f17", "$f18",
   "$f19", "$f20", "$f21", "$f22", "$f23", "$f24", "$f25", "$f26", "$f27",
-  "$f28", "$f29", "$f30", "$f31"};
+  "$f28", "$f29", "$f30", "$f31",
+  // Condition flag registers.
+  "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7"};
   return llvm::ArrayRef(GCCRegNames);
 }
 


Index: llvm/test/CodeGen/LoongArch/inline-asm-clobbers-fcc.mir
===
--- /dev/null
+++ llvm/test/CodeGen/LoongArch/inline-asm-clobbers-fcc.mir
@@ -0,0 +1,33 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc --mtriple=loongarch64 --mattr=+d --run-pass=greedy %s -o - | FileCheck %s
+
+## Check that fcc register clobbered by inlineasm is correctly saved by examing
+## a pair of pseudos (PseudoST_CFR and PseudoLD_CFR) are generated before and
+## after the INLINEASM.
+...
+---
+name: test
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+liveins: $f0_64, $f1_64
+
+; CHECK-LABEL: name: test
+; CHECK: liveins: $f0_64, $f1_64
+; CHECK-NEXT: {{  $}}
+; CHECK-NEXT: [[COPY:%[0-9]+]]:fpr64 = COPY $f1_64
+; CHECK-NEXT: [[COPY1:%[0-9]+]]:fpr64 = COPY $f0_64
+; CHECK-NEXT: [[FCMP_CLT_D:%[0-9]+]]:cfr = FCMP_CLT_D [[COPY]], [[COPY1]]
+; CHECK-NEXT: PseudoST_CFR [[FCMP_CLT_D]], %stack.0, 0 :: (store (s64) into %stack.0)
+; CHECK-NEXT: INLINEASM , 1 /* sideeffect attdialect 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

2023-05-07 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:5473
+  if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro))
+return true;
+

HazardyKnusperkeks wrote:
> Does changing this return value make no difference? In other words is there 
> no combination of `Left.is(TT_AttributeSquare)` and 
> `Right.is(tok::kw___attribute)`?
Yes, that combo can happen; example below.

The patch that changed the left-diff line from `true` to 
`!Left.is(TT_AttributeSquare)`
 was done _only_ contemplating the `[[` case 
(5a4ddbd69db2b0e09398214510501d0e59a0c30b); tagging @MyDeveloperDay who wrote 
that patch and can perhaps offer more insight on your question.

My reasoning is to revert that part of the patch just a bit and limit it to 
that case only.

I couldn't come up with a use-case where you'd want to avoid splitting between 
`TT_AttributeSquare` and `kw___attribute`, but the example below shows that 
allowing it to break in that combination is preferable to the alternative of 
breaking in the parens of the attribute:
```
// Style: "{BasedOnStyle: LLVM, ColumnLimit: 40}"
// Existing Behavior
int ff(
double)
__attribute__((overloadable))
[[unused]] __attribute__((
overloadable));

// With Patch
int ff(
double)
__attribute__((overloadable))
[[unused]]
__attribute__((overloadable));
```



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

https://reviews.llvm.org/D145262

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


[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

2023-05-07 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

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


[PATCH] D146030: [clang][Interp] Handle LambdaExprs

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



Comment at: clang/test/AST/Interp/lambda.cpp:92
+  static_assert(foo() == 1); // expected-error {{not an integral constant 
expression}}
+}
+

tbaeder wrote:
> aaron.ballman wrote:
> > How about some tests like:
> > ```
> > constexpr int call_thru_func_ptr(int i) {
> >   auto l = [](int i) { return i; };
> >   int (*fp)(int) = l;
> >   return fp(i);  
> > }
> > static_assert(call_thru_func_ptr(12) == 12);
> > 
> > constexpr int call_through_copied_lambda(auto lam, int i) {
> >   auto copy = lam;
> >   return copy(i);
> > }
> > 
> > constexpr int call_through_copied_lambda(auto lam) {
> >   auto copy = lam;
> >   return copy();
> > }
> > 
> > void func() {
> >   constexpr int i = 12;
> >   static_assert(call_through_copied_lambda([i]() { return i; }) == 12);
> > }
> > ```
> Heh:
> ```
> array.cpp:1245:15: error: static assertion expression is not an integral 
> constant expression
>  1245 | static_assert(call_thru_func_ptr(12) == 12);
>   |   ^~~~
> array.cpp:1243:10: note: non-constexpr function '__invoke' cannot be used in 
> a constant expression
>  1243 |   return fp(i);
>   |  ^
> array.cpp:1245:15: note: in call to 'call_thru_func_ptr(12)'
>  1245 | static_assert(call_thru_func_ptr(12) == 12);
>   |   ^
> array.cpp:1239:12: note: declared here
>  1239 |   auto l = [](int i) { return i; };
>   |^
> 
> ```
Ah, I didn't know there is something like a "lambda static invoker". I see the 
current interpreter basically checks for that and then calls the lambda call 
operator instead. Doing that is hard for me though, because the call operator 
requires different arguments and I can't just itnogre the static invoker either 
because it has an empty body.


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

https://reviews.llvm.org/D146030

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


[PATCH] D150087: [Clang] Support more stdio builtins

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



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:102
   // std libcall builtins are implemented.
   static SmallDenseMap F128Builtins{
+  {Builtin::BI__builtin___fprintf_chk, "__fprintfieee128"},

The size should be increased I guess?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150087

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


[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

2023-05-07 Thread Xinlong Wu via Phabricator via cfe-commits
VincentWu updated this revision to Diff 520253.
VincentWu marked 3 inline comments as done.
VincentWu added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmp-invalid.s
  llvm/test/MC/RISCV/rv32zcmp-valid.s
  llvm/test/MC/RISCV/rv64zcmp-invalid.s
  llvm/test/MC/RISCV/rv64zcmp-valid.s

Index: llvm/test/MC/RISCV/rv64zcmp-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv64zcmp-valid.s
@@ -0,0 +1,149 @@
+# RUN: llvm-mc %s -triple=riscv64 -mattr=experimental-zcmp -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=experimental-zcmp < %s \
+# RUN: | llvm-objdump --mattr=-c,experimental-zcmp -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: cm.mvsa01 s1, s0
+# CHECK-ASM: encoding: [0xa2,0xac]
+cm.mvsa01 s1, s0
+
+# CHECK-ASM-AND-OBJ: cm.mva01s s1, s0
+# CHECK-ASM: encoding: [0xe2,0xac]
+cm.mva01s s1, s0
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xbe]
+cm.popret {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xbe]
+cm.popret {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0}, 64
+# CHECK-ASM: encoding: [0x5e,0xbe]
+cm.popret {ra, s0}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xbe]
+cm.popret {ra,s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xbe]
+cm.popret {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s3}, 64
+# CHECK-ASM: encoding: [0x86,0xbe]
+cm.popret {ra, s0-s3}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xbe]
+cm.popret {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xbe]
+cm.popret {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xbe]
+cm.popret {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xbc]
+cm.popretz {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xbc]
+cm.popretz {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0}, 64
+# CHECK-ASM: encoding: [0x5e,0xbc]
+cm.popretz {ra, s0}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xbc]
+cm.popretz {ra, s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xbc]
+cm.popretz {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s3}, 64
+# CHECK-ASM: encoding: [0x86,0xbc]
+cm.popretz {ra, s0-s3}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xbc]
+cm.popretz {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xbc]
+cm.popretz {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xbc]
+cm.popretz {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xba]
+cm.pop {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xba]
+cm.pop {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0}, 16
+# CHECK-ASM: encoding: [0x52,0xba]
+cm.pop {ra, s0}, 16
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xba]
+cm.pop {ra, s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xba]
+cm.pop {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xba]
+cm.pop {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xba]
+cm.pop {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xba]
+cm.pop {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.push {ra}, -16
+# CHECK-ASM: encoding: [0x42,0xb8]
+cm.push {ra}, -16
+
+# CHECK-ASM-AND-OBJ: cm.push {ra, s0}, -32
+# CHECK-ASM: encoding: [0x56,0xb8]
+cm.push {ra, s0}, -32
+
+# CHECK-ASM-AND-OBJ: cm.push {ra, s0-s1}, -32
+# 

[PATCH] D150087: [Clang] Support more stdio builtins

2023-05-07 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf created this revision.
qiucf added reviewers: nemanjai, craig.topper, PowerPC, tuliom, tstellar.
Herald added a subscriber: kbarton.
Herald added a project: All.
qiucf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add more builtins for stdio functions as in GCC ( 
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html ), along with their 
mutations under IEEE float128 ABI.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150087

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/PowerPC/ppc64-f128-builtins.c

Index: clang/test/CodeGen/PowerPC/ppc64-f128-builtins.c
===
--- clang/test/CodeGen/PowerPC/ppc64-f128-builtins.c
+++ clang/test/CodeGen/PowerPC/ppc64-f128-builtins.c
@@ -61,3 +61,52 @@
 long double test_nexttoward(long double a, long double b) {
   return __builtin_nexttowardl(a, b);
 }
+
+// IEEE128-LABEL: define dso_local void @test_scanf
+// IEEE128: call signext i32 (ptr, ...) @__scanfieee128
+// PPC128-LABEL: define dso_local void @test_scanf
+// PPC128: call signext i32 (ptr, ...) @scanf
+void test_scanf(int *x) {
+  __builtin_scanf("%d", x);
+}
+
+// IEEE128-LABEL: define dso_local void @test_sscanf
+// IEEE128: call signext i32 (ptr, ptr, ...) @__sscanfieee128
+// PPC128-LABEL: define dso_local void @test_sscanf
+// PPC128: call signext i32 (ptr, ptr, ...) @sscanf
+void test_sscanf(int *x) {
+  __builtin_sscanf(buf, "%d", x);
+}
+
+// IEEE128-LABEL: define dso_local void @test_vprintf
+// IEEE128: call signext i32 @__vprintfieee128
+// PPC128-LABEL: define dso_local void @test_vprintf
+// PPC128: call signext i32 @vprintf
+void test_vprintf(const char *fmt, ...) {
+  __builtin_va_list args;
+  __builtin_va_start(args, fmt);
+  __builtin_vprintf(fmt, args);
+  __builtin_va_end(args);
+}
+
+// IEEE128-LABEL: define dso_local void @test_vscanf
+// IEEE128: call signext i32 @__vscanfieee128
+// PPC128-LABEL: define dso_local void @test_vscanf
+// PPC128: call signext i32 @vscanf
+void test_vscanf(const char *fmt, ...) {
+  __builtin_va_list args;
+  __builtin_va_start(args, fmt);
+  __builtin_vscanf(fmt, args);
+  __builtin_va_end(args);
+}
+
+// IEEE128-LABEL: define dso_local void @test_vsscanf
+// IEEE128: call signext i32 @__vsscanfieee128
+// PPC128-LABEL: define dso_local void @test_vsscanf
+// PPC128: call signext i32 @vsscanf
+void test_vsscanf(const char *fmt, ...) {
+  __builtin_va_list args;
+  __builtin_va_start(args, fmt);
+  __builtin_vsscanf(buf, fmt, args);
+  __builtin_va_end(args);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -100,12 +100,27 @@
   // TODO: This list should be expanded or refactored after all GCC-compatible
   // std libcall builtins are implemented.
   static SmallDenseMap F128Builtins{
+  {Builtin::BI__builtin___fprintf_chk, "__fprintfieee128"},
+  {Builtin::BI__builtin___printf_chk, "__printfieee128"},
+  {Builtin::BI__builtin___snprintf_chk, "__snprintfieee128"},
+  {Builtin::BI__builtin___sprintf_chk, "__sprintfieee128"},
+  {Builtin::BI__builtin___vfprintf_chk, "__vfprintfieee128"},
+  {Builtin::BI__builtin___vsnprintf_chk, "__vsnprintfieee128"},
+  {Builtin::BI__builtin___vsprintf_chk, "__vsprintfieee128"},
+  {Builtin::BI__builtin_fprintf, "__fprintfieee128"},
+  {Builtin::BI__builtin_fscanf, "__fscanfieee128"},
   {Builtin::BI__builtin_printf, "__printfieee128"},
+  {Builtin::BI__builtin_scanf, "__scanfieee128"},
+  {Builtin::BI__builtin_snprintf, "__snprintfieee128"},
+  {Builtin::BI__builtin_sprintf, "__sprintfieee128"},
+  {Builtin::BI__builtin_sscanf, "__sscanfieee128"},
+  {Builtin::BI__builtin_vfprintf, "__vfprintfieee128"},
+  {Builtin::BI__builtin_vfscanf, "__vfscanfieee128"},
+  {Builtin::BI__builtin_vprintf, "__vprintfieee128"},
+  {Builtin::BI__builtin_vscanf, "__vscanfieee128"},
   {Builtin::BI__builtin_vsnprintf, "__vsnprintfieee128"},
   {Builtin::BI__builtin_vsprintf, "__vsprintfieee128"},
-  {Builtin::BI__builtin_sprintf, "__sprintfieee128"},
-  {Builtin::BI__builtin_snprintf, "__snprintfieee128"},
-  {Builtin::BI__builtin_fprintf, "__fprintfieee128"},
+  {Builtin::BI__builtin_vsscanf, "__vsscanfieee128"},
   {Builtin::BI__builtin_nexttowardf128, "__nexttowardieee128"},
   };
 
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -559,6 +559,7 @@
 BUILTIN(__builtin_bzero, "vv*z", "nF")
 BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
 BUILTIN(__builtin_free, "vv*", "nF")
+BUILTIN(__builtin_fscanf, "iP*RcC*R.", "Fs:1:")
 BUILTIN(__builtin_malloc, "v*z", "nF")
 

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

2023-05-07 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb updated this revision to Diff 520247.
jaredgrubb added a comment.

Update `ClangFormatStyleOptions.rst` as requested by the auto-comment (thanks 
auto-bot!).


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

https://reviews.llvm.org/D150083

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.h
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
  llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
  llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -1301,6 +1301,7 @@
 "lib/Format/FormatTokenLexer.h",
 "lib/Format/FormatTokenSource.h",
 "lib/Format/Macros.h",
+"lib/Format/ObjCPropertyAttributeOrderFixer.h",
 "lib/Format/QualifierAlignmentFixer.h",
 "lib/Format/UnwrappedLineParser.h",
 ] + glob([
Index: llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
+++ llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
@@ -36,6 +36,7 @@
 "MacroCallReconstructorTest.cpp",
 "MacroExpanderTest.cpp",
 "NamespaceEndCommentsFixerTest.cpp",
+"ObjCPropertyAttributeOrderFixerTest.cpp",
 "QualifierFixerTest.cpp",
 "SortImportsTestJS.cpp",
 "SortImportsTestJava.cpp",
Index: llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
@@ -20,6 +20,7 @@
 "MacroCallReconstructor.cpp",
 "MacroExpander.cpp",
 "NamespaceEndCommentsFixer.cpp",
+"ObjCPropertyAttributeOrderFixer.cpp",
 "QualifierAlignmentFixer.cpp",
 "SortJavaScriptImports.cpp",
 "TokenAnalyzer.cpp",
Index: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
@@ -0,0 +1,192 @@
+//===- unittest/Format/ObjCPropertyAttributeOrderFixerTest.cpp - unit tests
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../lib/Format/ObjCPropertyAttributeOrderFixer.h"
+#include "FormatTestBase.h"
+#include "TestLexer.h"
+
+#define DEBUG_TYPE "format-objc-property-attribute-order-fixer-test"
+
+namespace clang {
+namespace format {
+namespace test {
+namespace {
+
+#define CHECK_PARSE(TEXT, FIELD, VALUE)\
+  EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!";  \
+  EXPECT_EQ(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+#define FAIL_PARSE(TEXT, FIELD, VALUE) \
+  EXPECT_NE(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+class ObjCPropertyAttributeOrderFixerTest : public FormatTestBase {
+protected:
+  TokenList annotate(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle()) {
+return TestLexer(Allocator, Buffers, Style).annotate(Code);
+  }
+  llvm::SpecificBumpPtrAllocator Allocator;
+  std::vector> Buffers;
+};
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, ParsesStyleOption) {
+  FormatStyle Style = {};
+  Style.Language = FormatStyle::LK_ObjC;
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: [class]", ObjCPropertyAttributeOrder,
+  std::vector({"class"}));
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: [direct, strong]",
+  ObjCPropertyAttributeOrder,
+  std::vector({"direct", "strong"}));
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsSpecifiedAttributes) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
+
+  verifyFormat("@property() int p;", Style);
+
+  // One: shouldn't move.
+  verifyFormat("@property(a) int p;", Style);
+  

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

2023-05-07 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb added a comment.

Some implementation notes:

- The implementation was modeled after `QualifierAlignmentFixer` and 
`sortCppIncludes`.
- the additions to the `.bazel`/`.gn` build files was done naively based on 
searching for where "QualifierFixerTest" appeared in the repo; I can't really 
test these. It looks right, but it's a guess.
- I considered creating a top-level style similar to 
`QualifierAlignment/QualifierOrder`; that one has a few pre-canned values 
(`Leave, Left, Right, Custom`), where `Custom` opens up the style 
`QualifierOrder`, an array of qualifier names.
  - Pro: we could create a `[Leave, LLVM]` that would set them in the same 
order the `DeclPrinter.cpp` would dump them
  - Pro: users probably would like not having to think about 18 different 
attributes (I did try to document them in groups to make them easier to find)
  - Con: I couldn't find any style guides that provide a "standard order", so I 
don't know any other pre-canned settings that wouldn't just be an opinion of my 
own personal style. If there are, let me know and that might add weight to 
going this route.

I don't like that there are 18 attributes that users _probably_ want to specify 
for the style, so having some shortcut seems helpful. Perhaps I could just 
document it that way so it's easy to copy-paste? I also considered making it 
possible to have an alias so the style would only need to have a handful of 
them to get full coverage (eg, if you specify `assign` in the style, then the 
tool would infer you also mean to imply that the similar attributes `retain, 
strong, copy, weak, unsafe_unretained` would occupy the same "slot" unless you 
also specify them explicitly).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150083

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


[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

2023-05-07 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb created this revision.
jaredgrubb added reviewers: benhamilton, egorzhdan, owenpan, HazardyKnusperkeks.
jaredgrubb added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, MyDeveloperDay.
Herald added a comment.
jaredgrubb requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does 
not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Add a style option to specify the order that property attributes should appear 
in ObjC property declarations (property attributes are things like`nonatomic, 
strong, nullable`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150083

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
  clang/lib/Format/ObjCPropertyAttributeOrderFixer.h
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
  llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
  llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -1301,6 +1301,7 @@
 "lib/Format/FormatTokenLexer.h",
 "lib/Format/FormatTokenSource.h",
 "lib/Format/Macros.h",
+"lib/Format/ObjCPropertyAttributeOrderFixer.h",
 "lib/Format/QualifierAlignmentFixer.h",
 "lib/Format/UnwrappedLineParser.h",
 ] + glob([
Index: llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
+++ llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn
@@ -36,6 +36,7 @@
 "MacroCallReconstructorTest.cpp",
 "MacroExpanderTest.cpp",
 "NamespaceEndCommentsFixerTest.cpp",
+"ObjCPropertyAttributeOrderFixerTest.cpp",
 "QualifierFixerTest.cpp",
 "SortImportsTestJS.cpp",
 "SortImportsTestJava.cpp",
Index: llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn
@@ -20,6 +20,7 @@
 "MacroCallReconstructor.cpp",
 "MacroExpander.cpp",
 "NamespaceEndCommentsFixer.cpp",
+"ObjCPropertyAttributeOrderFixer.cpp",
 "QualifierAlignmentFixer.cpp",
 "SortJavaScriptImports.cpp",
 "TokenAnalyzer.cpp",
Index: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
@@ -0,0 +1,192 @@
+//===- unittest/Format/ObjCPropertyAttributeOrderFixerTest.cpp - unit tests
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../lib/Format/ObjCPropertyAttributeOrderFixer.h"
+#include "FormatTestBase.h"
+#include "TestLexer.h"
+
+#define DEBUG_TYPE "format-objc-property-attribute-order-fixer-test"
+
+namespace clang {
+namespace format {
+namespace test {
+namespace {
+
+#define CHECK_PARSE(TEXT, FIELD, VALUE)\
+  EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!";  \
+  EXPECT_EQ(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+#define FAIL_PARSE(TEXT, FIELD, VALUE) \
+  EXPECT_NE(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+class ObjCPropertyAttributeOrderFixerTest : public FormatTestBase {
+protected:
+  TokenList annotate(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle()) {
+return TestLexer(Allocator, Buffers, 

[PATCH] D149562: [clang-format] Stop comment disrupting indentation of Verilog ports

2023-05-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D149562#4312573 , @sstwcw wrote:

> The port list thing and the comment thing work fine for now, except
> when there is a comment for the first port.  The comment on the first
> line would cause the port list to be indented, like this:
>
>   module x
>   ( // first port
>   input x1,
>   // second port
>   input x2,
>   // third port
>   input x3,
>   // forth port
>   input x4);
>   endmodule
>
> After this patch, a comment on the first line would not cause the
> problem.  Now the code gets formatted like this.  The ports are
> indented the same whether or not there is a comment on the first line.
>
>   module x
>   (// first port
>input x1,
>// second port
>input x2,
>// third port
>input x3,
>// forth port
>input x4);
>   endmodule

See https://github.com/llvm/llvm-project/issues/55487#issuecomment-1321355199, 
which may be relevant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149562

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


[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520237.
ccotter marked an inline comment as done.
ccotter added a comment.

- Fix compile for non-libc++


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
@@ -5,6 +5,22 @@
 // CHECK-FIXES-NOT: for ({{.*[^:]:[^:].*}})
 // CHECK-MESSAGES-NOT: modernize-loop-convert
 
+namespace somenamespace {
+  template  auto begin(T& t) -> decltype(t.begin());
+  template  auto begin(const T& t) -> decltype(t.begin());
+  template  auto end(T& t) -> decltype(t.end());
+  template  auto end(const T& t) -> decltype(t.end());
+  template  auto size(const T& t) -> decltype(t.size());
+} // namespace somenamespace
+
+struct SomeClass {
+  template  static auto begin(T& t) -> decltype(t.begin());
+  template  static auto begin(const T& t) -> decltype(t.begin());
+  template  static auto end(T& t) -> decltype(t.end());
+  template  static auto end(const T& t) -> decltype(t.end());
+  template  static auto size(const T& t) -> decltype(t.size());
+};
+
 namespace Negative {
 
 const int N = 6;
@@ -92,7 +108,7 @@
   }
 }
 
-}
+} // namespace Negative
 
 namespace NegativeIterator {
 
@@ -103,6 +119,10 @@
 struct BadBeginEnd : T {
   iterator notBegin();
   iterator notEnd();
+  iterator begin(int);
+  iterator end(int);
+  iterator begin();
+  iterator end();
 };
 
 void notBeginOrEnd() {
@@ -112,6 +132,9 @@
 
   for (T::iterator I = Bad.begin(), E = Bad.notEnd();  I != E; ++I)
 int K = *I;
+
+  for (T::iterator I = Bad.begin(0), E = Bad.end(0);  I != E; ++I)
+int K = *I;
 }
 
 void badLoopShapes() {
@@ -202,6 +225,8 @@
   T Other;
   for (T::iterator I = Tt.begin(), E = Other.end();  I != E; ++I)
 int K = *I;
+  for (T::iterator I = begin(Tt), E = end(Other);  I != E; ++I)
+int K = *I;
 
   for (T::iterator I = Other.begin(), E = Tt.end();  I != E; ++I)
 int K = *I;
@@ -214,6 +239,24 @@
 MutableVal K = *I;
 }
 
+void mixedMemberAndADL() {
+  for (T::iterator I = Tt.begin(), E = end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = end(Tt);  I != E; ++I)
+int K = *I;
+}
+
+void nonADLOrStdCalls() {
+  for (T::iterator I = SomeClass::begin(Tt), E = SomeClass::end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = somenamespace::begin(Tt), E = somenamespace::end(Tt);  I != E; ++I)
+int K = *I;
+}
+
 void wrongIterators() {
   T::iterator Other;
   for (T::iterator I = Tt.begin(), E = Tt.end(); I != Other; ++I)
@@ -379,6 +422,13 @@
 Sum += V[I];
 }
 
+void nonADLOrStdCalls() {
+  for (int I = 0, E = somenamespace::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+  for (int I = 0, E = SomeClass::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+}
+
 // Checks to see that non-const member functions are not called on the container
 // object.
 // These could be conceivably allowed with a lower required confidence level.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,41 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s0 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s0 has value %d\n", It.X);
+
+  for (S::iterator It = std::begin(Ss), E = std::end(Ss); It != E; ++It) {
+printf("s1 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s1 has value %d\n", It.X);
+

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:481
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s4 has value %d\n", It.X);

PiotrZSL wrote:
> shouldnt it be `const& It` ?
Looks like when the object is trivial to copy (`IsCheapToCopy` logic in the 
check), `auto It` is used instead of `const auto& It`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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


[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520232.
ccotter added a comment.

Feedback and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
@@ -5,6 +5,22 @@
 // CHECK-FIXES-NOT: for ({{.*[^:]:[^:].*}})
 // CHECK-MESSAGES-NOT: modernize-loop-convert
 
+namespace somenamespace {
+  template  auto begin(T& t) -> decltype(t.begin());
+  template  auto begin(const T& t) -> decltype(t.begin());
+  template  auto end(T& t) -> decltype(t.end());
+  template  auto end(const T& t) -> decltype(t.end());
+  template  auto size(const T& t) -> decltype(t.size());
+} // namespace somenamespace
+
+struct SomeClass {
+  template  static auto begin(T& t) -> decltype(t.begin());
+  template  static auto begin(const T& t) -> decltype(t.begin());
+  template  static auto end(T& t) -> decltype(t.end());
+  template  static auto end(const T& t) -> decltype(t.end());
+  template  static auto size(const T& t) -> decltype(t.size());
+};
+
 namespace Negative {
 
 const int N = 6;
@@ -92,7 +108,7 @@
   }
 }
 
-}
+} // namespace Negative
 
 namespace NegativeIterator {
 
@@ -103,6 +119,10 @@
 struct BadBeginEnd : T {
   iterator notBegin();
   iterator notEnd();
+  iterator begin(int);
+  iterator end(int);
+  iterator begin();
+  iterator end();
 };
 
 void notBeginOrEnd() {
@@ -112,6 +132,9 @@
 
   for (T::iterator I = Bad.begin(), E = Bad.notEnd();  I != E; ++I)
 int K = *I;
+
+  for (T::iterator I = Bad.begin(0), E = Bad.end(0);  I != E; ++I)
+int K = *I;
 }
 
 void badLoopShapes() {
@@ -202,6 +225,8 @@
   T Other;
   for (T::iterator I = Tt.begin(), E = Other.end();  I != E; ++I)
 int K = *I;
+  for (T::iterator I = begin(Tt), E = end(Other);  I != E; ++I)
+int K = *I;
 
   for (T::iterator I = Other.begin(), E = Tt.end();  I != E; ++I)
 int K = *I;
@@ -214,6 +239,24 @@
 MutableVal K = *I;
 }
 
+void mixedMemberAndADL() {
+  for (T::iterator I = Tt.begin(), E = end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = end(Tt);  I != E; ++I)
+int K = *I;
+}
+
+void nonADLOrStdCalls() {
+  for (T::iterator I = SomeClass::begin(Tt), E = SomeClass::end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = somenamespace::begin(Tt), E = somenamespace::end(Tt);  I != E; ++I)
+int K = *I;
+}
+
 void wrongIterators() {
   T::iterator Other;
   for (T::iterator I = Tt.begin(), E = Tt.end(); I != Other; ++I)
@@ -379,6 +422,13 @@
 Sum += V[I];
 }
 
+void nonADLOrStdCalls() {
+  for (int I = 0, E = somenamespace::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+  for (int I = 0, E = SomeClass::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+}
+
 // Checks to see that non-const member functions are not called on the container
 // object.
 // These could be conceivably allowed with a lower required confidence level.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,41 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s0 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s0 has value %d\n", It.X);
+
+  for (S::iterator It = std::begin(Ss), E = std::end(Ss); It != E; ++It) {
+printf("s1 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s1 has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps), E = end(*Ps); 

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 10 inline comments as done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:477
+
+  for (S::const_iterator It = cbegin(Ss), E = cend(Ss); It != E; ++It) {
+printf("s4 has value %d\n", (*It).X);

PiotrZSL wrote:
> what if begin,end is missing, and there is only cbegin, cend then it will 
> fail with:
> "error: invalid range expression of type 'const A'; no viable 'begin' 
> function available|
> 
This looks to be an already existing bug in the check - mind if I fix that in a 
separate phab?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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


[clang] ad5bed5 - Revert "[clang] Make predefined expressions string literals under -fms-extensions"

2023-05-07 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2023-05-07T16:51:02-07:00
New Revision: ad5bed5372f3f73a07f0b98a05444e7acda2b9d9

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

LOG: Revert "[clang] Make predefined expressions string literals under 
-fms-extensions"

This reverts commit 856f384bf94513c89e754906b7d80fbe5377ab53.

Breaks bots, e.g. https://lab.llvm.org/buildbot/#/builders/123/builds/18775

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Expr.h
clang/include/clang/AST/IgnoreExpr.h
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp

Removed: 
clang/test/Modules/predefined.cpp
clang/test/Sema/ms_predefined_expr.cpp



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cd41106640f87..910a853eafc81 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,9 +416,6 @@ Bug Fixes to C++ Support
   initialization.
   (`#61567 `_)
 - Fix a crash when expanding a pack as the index of a subscript expression.
-- Some predefined expressions are now treated as string literals in MSVC
-  compatibility mode.
-  (`#114 `_)
 
 Bug Fixes to AST Handling
 ^

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index db4316d8faf1f..0ab778e5d8cd3 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -1992,7 +1992,7 @@ class PredefinedExpr final
 
 private:
   PredefinedExpr(SourceLocation L, QualType FNTy, IdentKind IK,
- bool IsTransparent, StringLiteral *SL);
+ StringLiteral *SL);
 
   explicit PredefinedExpr(EmptyShell Empty, bool HasFunctionName);
 
@@ -2007,12 +2007,8 @@ class PredefinedExpr final
 
 public:
   /// Create a PredefinedExpr.
-  ///
-  /// If IsTransparent, the PredefinedExpr is transparently handled as a
-  /// StringLiteral.
   static PredefinedExpr *Create(const ASTContext , SourceLocation L,
-QualType FNTy, IdentKind IK, bool 
IsTransparent,
-StringLiteral *SL);
+QualType FNTy, IdentKind IK, StringLiteral 
*SL);
 
   /// Create an empty PredefinedExpr.
   static PredefinedExpr *CreateEmpty(const ASTContext ,
@@ -2022,8 +2018,6 @@ class PredefinedExpr final
 return static_cast(PredefinedExprBits.Kind);
   }
 
-  bool isTransparent() const { return PredefinedExprBits.IsTransparent; }
-
   SourceLocation getLocation() const { return PredefinedExprBits.Loc; }
   void setLocation(SourceLocation L) { PredefinedExprBits.Loc = L; }
 

diff  --git a/clang/include/clang/AST/IgnoreExpr.h 
b/clang/include/clang/AST/IgnoreExpr.h
index ce30a7fc00558..f8d2d6c7d00c0 100644
--- a/clang/include/clang/AST/IgnoreExpr.h
+++ b/clang/include/clang/AST/IgnoreExpr.h
@@ -166,11 +166,6 @@ inline Expr *IgnoreParensSingleStep(Expr *E) {
   return CE->getChosenSubExpr();
   }
 
-  else if (auto *PE = dyn_cast(E)) {
-if (PE->isTransparent())
-  return PE->getFunctionName();
-  }
-
   return E;
 }
 

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index e466aa1755daf..ea979d791ce7b 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -364,10 +364,6 @@ class alignas(void *) Stmt {
 /// for the predefined identifier.
 unsigned HasFunctionName : 1;
 
-/// True if this PredefinedExpr should be treated as a StringLiteral (for
-/// MSVC compatibility).
-unsigned IsTransparent : 1;
-
 /// The location of this PredefinedExpr.
 SourceLocation Loc;
   };

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 2ba42f9b73763..87e72f000d494 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1190,7 +1190,6 @@ def MicrosoftCommentPaste : 
DiagGroup<"microsoft-comment-paste">;
 def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
 def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">;
 def MicrosoftStaticAssert : DiagGroup<"microsoft-static-assert">;
-def MicrosoftInitFromPredefined : DiagGroup<"microsoft-init-from-predefined">;
 
 // Aliases.
 def : DiagGroup<"msvc-include", [MicrosoftInclude]>;
@@ -1208,7 +1207,7 @@ def Microsoft : DiagGroup<"microsoft",
  MicrosoftFlexibleArray, 

[PATCH] D149976: adding bf16 support to NVPTX

2023-05-07 Thread Kushan Ahmadian via Phabricator via cfe-commits
kushanam updated this revision to Diff 520215.
kushanam added a comment.

removing bf16 specific registers classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149976

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
  llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXInstrInfo.td
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/lib/Target/NVPTX/NVPTXMCExpr.cpp
  llvm/lib/Target/NVPTX/NVPTXMCExpr.h
  llvm/lib/Target/NVPTX/NVPTXRegisterInfo.td
  llvm/lib/Target/NVPTX/NVPTXSubtarget.cpp
  llvm/lib/Target/NVPTX/NVPTXSubtarget.h
  llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
  llvm/test/CodeGen/NVPTX/bf16-instructions.ll

Index: llvm/test/CodeGen/NVPTX/bf16-instructions.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/bf16-instructions.ll
@@ -0,0 +1,88 @@
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | FileCheck %s
+; RUN: %if ptxas-11.0 %{ llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | %ptxas-verify -arch=sm_80 %}
+
+
+; CHECK-LABEL: test_fadd(
+; CHECK-DAG:  ld.param.b16[[A:%h[0-9]+]], [test_fadd_param_0];
+; CHECK-DAG:  ld.param.b16[[B:%h[0-9]+]], [test_fadd_param_1];
+; CHECK-NEXT: add.rn.bf16 [[R:%f[0-9]+]], [[A]], [[B]];
+; CHECK-NEXT: st.param.b16[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define bfloat @test_fadd(bfloat %0, bfloat %1) {
+  %3 = fadd bfloat %0, %1 
+  ret bfloat %3
+}
+
+; CHECK-LABEL: test_fsub(
+; CHECK-DAG:  ld.param.b16[[A:%h[0-9]+]], [test_fsub_param_0];
+; CHECK-DAG:  ld.param.b16[[B:%h[0-9]+]], [test_fsub_param_1];
+; CHECK-NEXT: sub.rn.bf16 [[R:%f[0-9]+]], [[A]], [[B]];
+; CHECK-NEXT: st.param.b16[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define bfloat @test_fsub(bfloat %0, bfloat %1) {
+  %3 = fsub bfloat %0, %1 
+  ret bfloat %3
+}
+
+; CHECK-LABEL: test_faddx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_faddx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_faddx2_param_1];
+; CHECK-NEXT: add.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_faddx2(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fadd <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fsubx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fsubx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fsubx2_param_1];
+; CHECK-NEXT: sub.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_fsubx2(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fsub <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fmulx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fmulx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fmulx2_param_1];
+; CHECK-NEXT: mul.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_fmul(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fmul <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fdiv(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fdiv_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fdiv_param_1];
+; CHECK-DAG:  mov.b32 {[[A0:%h[0-9]+]], [[A1:%h[0-9]+]]}, [[A]]
+; CHECK-DAG:  mov.b32 {[[B0:%h[0-9]+]], [[B1:%h[0-9]+]]}, [[B]]
+; CHECK-DAG:  cvt.f32.bf16 [[FA0:%f[0-9]+]], [[A0]];
+; CHECK-DAG:  cvt.f32.bf16 [[FA1:%f[0-9]+]], [[A1]];
+; CHECK-DAG:  cvt.f32.bf16 [[FB0:%f[0-9]+]], [[B0]];
+; CHECK-DAG:  cvt.f32.bf16 [[FB1:%f[0-9]+]], [[B1]];
+; CHECK-DAG:  div.rn.f32  [[FR0:%f[0-9]+]], [[FA0]], [[FB0]];
+; CHECK-DAG:  div.rn.f32  [[FR1:%f[0-9]+]], [[FA1]], [[FB1]];
+; CHECK-DAG:  cvt.rn.bf16.f32  [[R0:%h[0-9]+]], [[FR0]];
+; CHECK-DAG:  cvt.rn.bf16.f32  [[R1:%h[0-9]+]], [[FR1]];
+; CHECK-NEXT: mov.b32 [[R:%hh[0-9]+]], {[[R0]], [[R1]]}
+; CHECK-NEXT: st.param.b32[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define <2 x bfloat> @test_fdiv(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fdiv <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
Index: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
+++ llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
@@ -204,6 +204,14 @@
   return {Intrinsic::fma, FTZ_MustBeOff, true};
 case 

[PATCH] D149976: adding bf16 support to NVPTX

2023-05-07 Thread Kushan Ahmadian via Phabricator via cfe-commits
kushanam updated this revision to Diff 520210.
kushanam added a comment.

Addressing the review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149976

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
  llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXInstrInfo.td
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/lib/Target/NVPTX/NVPTXMCExpr.cpp
  llvm/lib/Target/NVPTX/NVPTXMCExpr.h
  llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp
  llvm/lib/Target/NVPTX/NVPTXRegisterInfo.td
  llvm/lib/Target/NVPTX/NVPTXSubtarget.cpp
  llvm/lib/Target/NVPTX/NVPTXSubtarget.h
  llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
  llvm/test/CodeGen/NVPTX/bf16-instructions.ll

Index: llvm/test/CodeGen/NVPTX/bf16-instructions.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/bf16-instructions.ll
@@ -0,0 +1,88 @@
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | FileCheck %s
+; RUN: %if ptxas-11.0 %{ llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | %ptxas-verify -arch=sm_80 %}
+
+
+; CHECK-LABEL: test_fadd(
+; CHECK-DAG:  ld.param.b16[[A:%h[0-9]+]], [test_fadd_param_0];
+; CHECK-DAG:  ld.param.b16[[B:%h[0-9]+]], [test_fadd_param_1];
+; CHECK-NEXT: add.rn.bf16 [[R:%f[0-9]+]], [[A]], [[B]];
+; CHECK-NEXT: st.param.b16[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define bfloat @test_fadd(bfloat %0, bfloat %1) {
+  %3 = fadd bfloat %0, %1 
+  ret bfloat %3
+}
+
+; CHECK-LABEL: test_fsub(
+; CHECK-DAG:  ld.param.b16[[A:%h[0-9]+]], [test_fsub_param_0];
+; CHECK-DAG:  ld.param.b16[[B:%h[0-9]+]], [test_fsub_param_1];
+; CHECK-NEXT: sub.rn.bf16 [[R:%f[0-9]+]], [[A]], [[B]];
+; CHECK-NEXT: st.param.b16[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define bfloat @test_fsub(bfloat %0, bfloat %1) {
+  %3 = fsub bfloat %0, %1 
+  ret bfloat %3
+}
+
+; CHECK-LABEL: test_faddx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_faddx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_faddx2_param_1];
+; CHECK-NEXT: add.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_faddx2(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fadd <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fsubx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fsubx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fsubx2_param_1];
+; CHECK-NEXT: sub.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_fsubx2(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fsub <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fmulx2(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fmulx2_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fmulx2_param_1];
+; CHECK-NEXT: mul.rn.bf16x2   [[R:%f[0-9]+]], [[A]], [[B]];
+
+; CHECK:  st.param.b32[func_retval0+0], [[R]];
+; CHECK:  ret;
+
+define <2 x bfloat> @test_fmul(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fmul <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
+
+; CHECK-LABEL: test_fdiv(
+; CHECK-DAG:  ld.param.b32[[A:%hh[0-9]+]], [test_fdiv_param_0];
+; CHECK-DAG:  ld.param.b32[[B:%hh[0-9]+]], [test_fdiv_param_1];
+; CHECK-DAG:  mov.b32 {[[A0:%h[0-9]+]], [[A1:%h[0-9]+]]}, [[A]]
+; CHECK-DAG:  mov.b32 {[[B0:%h[0-9]+]], [[B1:%h[0-9]+]]}, [[B]]
+; CHECK-DAG:  cvt.f32.bf16 [[FA0:%f[0-9]+]], [[A0]];
+; CHECK-DAG:  cvt.f32.bf16 [[FA1:%f[0-9]+]], [[A1]];
+; CHECK-DAG:  cvt.f32.bf16 [[FB0:%f[0-9]+]], [[B0]];
+; CHECK-DAG:  cvt.f32.bf16 [[FB1:%f[0-9]+]], [[B1]];
+; CHECK-DAG:  div.rn.f32  [[FR0:%f[0-9]+]], [[FA0]], [[FB0]];
+; CHECK-DAG:  div.rn.f32  [[FR1:%f[0-9]+]], [[FA1]], [[FB1]];
+; CHECK-DAG:  cvt.rn.bf16.f32  [[R0:%h[0-9]+]], [[FR0]];
+; CHECK-DAG:  cvt.rn.bf16.f32  [[R1:%h[0-9]+]], [[FR1]];
+; CHECK-NEXT: mov.b32 [[R:%hh[0-9]+]], {[[R0]], [[R1]]}
+; CHECK-NEXT: st.param.b32[func_retval0+0], [[R]];
+; CHECK-NEXT: ret;
+
+define <2 x bfloat> @test_fdiv(<2 x bfloat> %a, <2 x bfloat> %b) #0 {
+  %r = fdiv <2 x bfloat> %a, %b
+  ret <2 x bfloat> %r
+}
Index: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
+++ llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
@@ -204,6 +204,14 @@
   return {Intrinsic::fma, FTZ_MustBeOff, true};
 

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D150057#4325350 , 
@HazardyKnusperkeks wrote:

> I have to say, I don't understand it, but I believe you. Why is continuing, 
> when the token is finalized the right thing?

See D150057#inline-1449546 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:6371-6380
+  verifyFormat("#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "int ab = 2;\n"
+   "#endif\n"
+   "#ifdef BAR\n"
+   "int abc = 3;\n"

clang-format breaks the above into two separate sets:
```
#if FOO
int a = 1;
#else
#endif
#if BAR
int abc = 3;
#else
#endif
```
and:
```
#if FOO
#else
int ab = 2;
#endif
#if BAR
#else
int abcd = 4;
#endif
```
After it finishes with the first set, the preprocessor directives are marked as 
`Finalized`. Then, while the second set is being processed, the directives 
should //not// be skipped when tokens are added to the `Change` set. Otherwise, 
we would end up with the `Change` set below without any "scope" context:
```
int ab = 2;
int abcd = 4;
```



Comment at: clang/unittests/Format/FormatTestComments.cpp:4319
 /\
-/ 
+/
   )",

HazardyKnusperkeks wrote:
> Unrelated?
My editor strips trailing whitespaces on save. I'll leave the fix in because 
it's not worth doing it in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

2023-05-07 Thread Faisal Vali via Phabricator via cfe-commits
faisalv created this revision.
faisalv added reviewers: aaron.ballman, shafik, royjacobson.
faisalv added a project: clang.
Herald added a subscriber: pengfei.
Herald added a project: All.
faisalv requested review of this revision.
Herald added a subscriber: cfe-commits.

See issue https://github.com/llvm/llvm-project/issues/62594

This code does not work on win32:

  cpp
auto lstatic = []()  static  { return 0;  };
int (*f2)(void) = lstatic; 

Since a calling convention such as CC_X86ThisCall can rightly interfere with 
the implicit pointer to function conversion if erroneously marked on a static 
function, the fix entails checking the 'static' specifier on the lambda 
declarator prior to assigning it a calling convention of an non-static member 
(which pre-c++23 made sense).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150075

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp


Index: clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
+
+
+namespace ns1 {
+  auto lstatic = []() static { return 3; }; 
+  int (*f2)(void) = lstatic;   
+
+}
+
+namespace ns1_1 {
+  
+  auto lstatic = []() static consteval  //expected-note {{declared here}} 
expected-error{{cannot take address of consteval call}}
+  { return 3; };   
+  
+  // FIXME: the above error should indicate that it was triggered below.
+  int (*f2)(void) = lstatic;   
+
+}
+
+
+namespace ns2 {
+  auto lstatic = []() static { return 3; }; 
+  constexpr int (*f2)(void) = lstatic;  
+  static_assert(lstatic() == f2());
+}
+
+namespace ns3 {
+  void main() {
+static int x = 10;
+auto L = []() static { return x; };
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4057,8 +4057,9 @@
   D.getTypeObject(I).Kind == DeclaratorChunk::MemberPointer;
 } else if (D.getContext() == DeclaratorContext::LambdaExpr) {
   // This can only be a call operator for a lambda, which is an instance
-  // method.
-  IsCXXInstanceMethod = true;
+  // method, unless explicitly specified as 'static'.
+  IsCXXInstanceMethod =
+  D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static;
 } else {
   // We're the innermost decl chunk, so must be a function declarator.
   assert(D.isFunctionDeclarator());


Index: clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
+
+
+namespace ns1 {
+  auto lstatic = []() static { return 3; }; 
+  int (*f2)(void) = lstatic;   
+
+}
+
+namespace ns1_1 {
+  
+  auto lstatic = []() static consteval  //expected-note {{declared here}} expected-error{{cannot take address of consteval call}}
+  { return 3; };   
+  
+  // FIXME: the above error should indicate that it was triggered below.
+  int (*f2)(void) = lstatic;   
+
+}
+
+
+namespace ns2 {
+  auto lstatic = []() static { return 3; }; 
+  constexpr int (*f2)(void) = lstatic;  
+  static_assert(lstatic() == f2());
+}
+
+namespace ns3 {
+  void main() {
+static int x = 10;
+auto L = []() static { return x; };
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4057,8 +4057,9 @@
   D.getTypeObject(I).Kind == DeclaratorChunk::MemberPointer;
 } else if (D.getContext() == DeclaratorContext::LambdaExpr) {
   // This can only be a call operator for a lambda, which is an instance
-  // method.
-  IsCXXInstanceMethod = true;
+  // method, unless explicitly specified as 'static'.
+  IsCXXInstanceMethod =
+  D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static;
 } else {
   // We're the innermost decl chunk, so must be a function declarator.
   assert(D.isFunctionDeclarator());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-05-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:4371
+incorrect code, leading to an ABI mismatch. This case is prevented by emitting 
a
+diagnostic.
+

Suggestion:

```
``math.h`` defines the typedefs ``float_t`` and ``double_t`` based on the active
evaluation method at the point where the header is included, not where the
typedefs are used.  Because of this, it is unwise to combine these typedefs with
``#pragma clang fp eval_method``.  To catch obvious bugs, Clang will emit an
error for any references to these typedefs within the scope of this pragma;
however, this is not a fool-proof protection, and programmers must take care.
```



Comment at: clang/include/clang/Basic/IdentifierTable.h:96
 
+  unsigned InterestingIdentifierID : 5;
+

The suggestion was to use a range of values within `ObjCOrBuiltinID` rather 
than adding
storage to all identifiers.


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

https://reviews.llvm.org/D146148

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


[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-05-07 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added inline comments.



Comment at: clang/include/clang/Interpreter/Value.h:160-162
+  // Interpreter, QualType are stored as void* to reduce dependencies.
+  void *Interp = nullptr;
+  void *OpaqueType = nullptr;

aaron.ballman wrote:
> junaire wrote:
> > aaron.ballman wrote:
> > > v.g.vassilev wrote:
> > > > aaron.ballman wrote:
> > > > > v.g.vassilev wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Why don't forward declares suffice if we're storing the 
> > > > > > > information by pointer?
> > > > > > This is a performance-critical class. We literally measure the 
> > > > > > instruction count for it. We practically cannot include anything in 
> > > > > > this header file because the class needs to included as part of the 
> > > > > > interpreter runtime. For example:
> > > > > > 
> > > > > > ```
> > > > > > #include 
> > > > > > Value ResultV;
> > > > > > gInterpreter->evaluate("float i = 12.3; i++", );
> > > > > > printf("Value is %d\n", ResultV.castAs());
> > > > > > ```
> > > > > > 
> > > > > > This is how you can do things in Cling. This not yet there but 
> > > > > > that's our next step.
> > > > > > 
> > > > > > For performance reasons we have the `ValueKind` optimization which 
> > > > > > allows us to perform most of the operations we need very fast. 
> > > > > > There are some operations such as printing the concrete type which 
> > > > > > need the actual `QualType` and so on but they are outside of the 
> > > > > > performance critical paths and it is okay to resort back to the 
> > > > > > real types providing the level of accuracy we need.
> > > > > > 
> > > > > That sounds like it's going to lead to maintenance problems in the 
> > > > > long term, right? I can't think of another header file we say "don't 
> > > > > touch this because it may impact runtime performance", and so I can 
> > > > > easily imagine someone breaking your expectation that this file can't 
> > > > > include anything else.
> > > > > 
> > > > > Is there a long-term plan for addressing this?
> > > > We have a few components like the Lexer that are extremely prone to 
> > > > performance regressions.
> > > > 
> > > > In terms for a longer-term plan in addressing this there are some steps 
> > > > could help IMO. First, this component is relatively standalone and very 
> > > > few changes will be required over time, for these I am hoping to be 
> > > > listed as a reviewer. Second, we can add a comment in the include area, 
> > > > making a note that including anything here will degrade the performance 
> > > > of almost all interpreted code. Third, we will find out about this in 
> > > > our downstream use-cases as the things get significantly slower.
> > > > 
> > > > Neither is silver bullet but that's probably the best we could do at 
> > > > that time. Btw, we might be able to add a test that's part of LLVM's 
> > > > performance analysis infrastructure.
> > > > Neither is silver bullet but that's probably the best we could do at 
> > > > that time. Btw, we might be able to add a test that's part of LLVM's 
> > > > performance analysis infrastructure.
> > > 
> > > Yeah, we should probably consider doing that. But to make sure I 
> > > understand the performance concerns... when we change functionality in 
> > > the lexer, we (potentially) slow down the lexing phase of compilation. 
> > > That's straightforward and unsurprising. But in this case, it sounds like 
> > > the act of including another header file in this header file will cause a 
> > > runtime performance concern, even if no other changes are made. If I'm 
> > > correct, I can't think of anything else in the compiler that works like 
> > > that.
> > I believe what @v.g.vassilev means is that the repl itself might include 
> > `Value.h` as a part of *runtime*, so if the header is heavy, you can notice 
> > the repl is slowed down. (That's obvious) So keep in mind we're breaking 
> > the boundary between the compiled code and interpreted code (It's kinda 
> > confusing) here it actually impacts interpreted code.
> > I believe what @v.g.vassilev means is that the repl itself might include 
> > Value.h as a part of *runtime*, so if the header is heavy, you can notice 
> > the repl is slowed down. (That's obvious) So keep in mind we're breaking 
> > the boundary between the compiled code and interpreted code (It's kinda 
> > confusing) here it actually impacts interpreted code.
> 
> I'm not certain that's a reasonable design choice to make. Or, stated 
> somewhat differently, I'm really uncomfortable with having header files we 
> can't maintain because changing them impacts runtime performance in 
> surprising ways. That's not a sustainable design even if we think this header 
> will be reasonably stable. We need *some* amount of abstraction here so that 
> we can have a clean design for the REPL interpreter without NFC code changes 
> impacting performance. Otherwise, people will be discouraged from 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

2023-05-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4514-4515
+// Apply this logic for parens that are not function attribute macros.
+if ((Left.is(tok::r_paren) && Left.isNot(TT_AttributeParen)) &&
+canBeObjCSelectorComponent(Right)) {
   // Don't space between ')' and  or ')' and 'new'. 'new' is not a





Comment at: clang/lib/Format/TokenAnnotator.cpp:5473
+  if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro))
+return true;
+

Does changing this return value make no difference? In other words is there no 
combination of `Left.is(TT_AttributeSquare)` and 
`Right.is(tok::kw___attribute)`?


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

https://reviews.llvm.org/D145262

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


[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

I have to say, I don't understand it, but I believe you. Why is continuing, 
when the token is finalized the right thing?




Comment at: clang/unittests/Format/FormatTestComments.cpp:4319
 /\
-/ 
+/
   )",

Unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D148995: [clang-tidy] Extract areStatementsIdentical

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

> Separating the changes into two patches would only prolong this process and 
> potentially delay the completion of this task.

I understand the concern, but in practice it's actually the other way around. 2 
small and easy to review patches are more likely to get reviewed quickly than 
the same patches combined into 1 big patch. The easier you make life for 
reviewers, the more likely they are to be willing / have time to review a 
patch. When I post patches, I often try and reflect about what else I can 
remove or make more obvious/readable to make reviewer's life easier and improve 
my chances of getting a quick review. I understand it's more work on the author 
side, but from experience I've found there's a net gain to doing it.

Anyway, the patch is indeed fairly easy to review so I'll just approve it, 
thank you for the contribution!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148995

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


[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

2023-05-07 Thread Arthur Eubanks 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 rG856f384bf945: [clang] Make predefined expressions string 
literals under -fms-extensions (authored by aeubanks).

Changed prior to commit:
  https://reviews.llvm.org/D146764?vs=519613=520204#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/Modules/predefined.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- /dev/null
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
+
+void f() {
+ const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
+ const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+ const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+}
Index: clang/test/Modules/predefined.cpp
===
--- /dev/null
+++ clang/test/Modules/predefined.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -x c++ -std=c++20 -emit-module-interface a.h -o a.pcm -fms-extensions -verify
+// RUN: %clang_cc1 -std=c++20 a.cpp -fmodule-file=A=a.pcm -fms-extensions -fsyntax-only -verify
+
+//--- a.h
+
+// expected-no-diagnostics
+
+export module A;
+
+export template 
+void f() {
+char a[] = __func__;
+}
+
+//--- a.cpp
+
+// expected-warning@a.h:8 {{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+
+import A;
+
+void g() {
+f(); // expected-note {{in instantiation of function template specialization 'f' requested here}}
+}
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -593,6 +593,7 @@
   bool HasFunctionName = E->getFunctionName() != nullptr;
   Record.push_back(HasFunctionName);
   Record.push_back(E->getIdentKind()); // FIXME: stable encoding
+  Record.push_back(E->isTransparent());
   Record.AddSourceLocation(E->getLocation());
   if (HasFunctionName)
 Record.AddStmt(E->getFunctionName());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -582,6 +582,7 @@
   bool HasFunctionName = Record.readInt();
   E->PredefinedExprBits.HasFunctionName = HasFunctionName;
   E->PredefinedExprBits.Kind = Record.readInt();
+  E->PredefinedExprBits.IsTransparent = Record.readInt();
   E->setLocation(readSourceLocation());
   if (HasFunctionName)
 E->setFunctionName(cast(Record.readSubExpr()));
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -174,6 +174,8 @@
   E = GSE->getResultExpr();
 } else if (ChooseExpr *CE = dyn_cast(E)) {
   E = CE->getChosenSubExpr();
+} else if (PredefinedExpr *PE = dyn_cast(E)) {
+  E = PE->getFunctionName();
 } else {
   llvm_unreachable("unexpected expr in string literal init");
 }
@@ -8501,6 +8503,15 @@
 << Init->getSourceRange();
   }
 
+  if (S.getLangOpts().MicrosoftExt && Args.size() == 1 &&
+  isa(Args[0]) && Entity.getType()->isArrayType()) {
+// Produce a Microsoft compatibility warning when initializing from a
+// predefined expression since MSVC treats predefined expressions as string
+// literals.
+Expr *Init = Args[0];
+S.Diag(Init->getBeginLoc(), diag::ext_init_from_predefined) << Init;
+  }
+
   // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global scope
   QualType 

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

2023-05-07 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

added a release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

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


[clang] 856f384 - [clang] Make predefined expressions string literals under -fms-extensions

2023-05-07 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2023-05-07T11:27:02-07:00
New Revision: 856f384bf94513c89e754906b7d80fbe5377ab53

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

LOG: [clang] Make predefined expressions string literals under -fms-extensions

MSVC makes these string literals [1][2].

[1] https://godbolt.org/z/6vnTzbExx
[2] 
https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170

Fixes #114

Reviewed By: aaron.ballman

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

Added: 
clang/test/Modules/predefined.cpp
clang/test/Sema/ms_predefined_expr.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Expr.h
clang/include/clang/AST/IgnoreExpr.h
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 910a853eafc81..cd41106640f87 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -416,6 +416,9 @@ Bug Fixes to C++ Support
   initialization.
   (`#61567 `_)
 - Fix a crash when expanding a pack as the index of a subscript expression.
+- Some predefined expressions are now treated as string literals in MSVC
+  compatibility mode.
+  (`#114 `_)
 
 Bug Fixes to AST Handling
 ^

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 0ab778e5d8cd3..db4316d8faf1f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -1992,7 +1992,7 @@ class PredefinedExpr final
 
 private:
   PredefinedExpr(SourceLocation L, QualType FNTy, IdentKind IK,
- StringLiteral *SL);
+ bool IsTransparent, StringLiteral *SL);
 
   explicit PredefinedExpr(EmptyShell Empty, bool HasFunctionName);
 
@@ -2007,8 +2007,12 @@ class PredefinedExpr final
 
 public:
   /// Create a PredefinedExpr.
+  ///
+  /// If IsTransparent, the PredefinedExpr is transparently handled as a
+  /// StringLiteral.
   static PredefinedExpr *Create(const ASTContext , SourceLocation L,
-QualType FNTy, IdentKind IK, StringLiteral 
*SL);
+QualType FNTy, IdentKind IK, bool 
IsTransparent,
+StringLiteral *SL);
 
   /// Create an empty PredefinedExpr.
   static PredefinedExpr *CreateEmpty(const ASTContext ,
@@ -2018,6 +2022,8 @@ class PredefinedExpr final
 return static_cast(PredefinedExprBits.Kind);
   }
 
+  bool isTransparent() const { return PredefinedExprBits.IsTransparent; }
+
   SourceLocation getLocation() const { return PredefinedExprBits.Loc; }
   void setLocation(SourceLocation L) { PredefinedExprBits.Loc = L; }
 

diff  --git a/clang/include/clang/AST/IgnoreExpr.h 
b/clang/include/clang/AST/IgnoreExpr.h
index f8d2d6c7d00c0..ce30a7fc00558 100644
--- a/clang/include/clang/AST/IgnoreExpr.h
+++ b/clang/include/clang/AST/IgnoreExpr.h
@@ -166,6 +166,11 @@ inline Expr *IgnoreParensSingleStep(Expr *E) {
   return CE->getChosenSubExpr();
   }
 
+  else if (auto *PE = dyn_cast(E)) {
+if (PE->isTransparent())
+  return PE->getFunctionName();
+  }
+
   return E;
 }
 

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index ea979d791ce7b..e466aa1755daf 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -364,6 +364,10 @@ class alignas(void *) Stmt {
 /// for the predefined identifier.
 unsigned HasFunctionName : 1;
 
+/// True if this PredefinedExpr should be treated as a StringLiteral (for
+/// MSVC compatibility).
+unsigned IsTransparent : 1;
+
 /// The location of this PredefinedExpr.
 SourceLocation Loc;
   };

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 87e72f000d494..2ba42f9b73763 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1190,6 +1190,7 @@ def MicrosoftCommentPaste : 
DiagGroup<"microsoft-comment-paste">;
 def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
 def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">;
 def MicrosoftStaticAssert : DiagGroup<"microsoft-static-assert">;
+def MicrosoftInitFromPredefined : DiagGroup<"microsoft-init-from-predefined">;
 
 // Aliases.
 def : DiagGroup<"msvc-include", 

[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

2023-05-07 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:3164
 }
-
 return false;

Drop this change



Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:650
+LLVM_DEBUG(
+dbgs() << "Trying Zcmp table (16-bit Table Jump Instructions):\n");
+Result =

This description is from Zcmt.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:235
+}
+} // DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp]...
+

This brace only closes `Predicates = [HasStdExtZcmp]`. The DecoderNamespace 
ended on line 189. It's only covering CM_MVA01S and CM_MVSA01.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

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


[PATCH] D150072: [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable types

2023-05-07 Thread Nikolas Klauser via Phabricator via cfe-commits
philnik created this revision.
philnik added a reviewer: aaron.ballman.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150072

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) 
const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext ) const {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148458: [clang-tidy][NFC] Split bugprone-exception-escape tests

2023-05-07 Thread Piotr Zegar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5545f1bbd4e1: [clang-tidy][NFC] Split 
bugprone-exception-escape tests (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148458

Files:
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -32,11 +32,6 @@
   throw 1;
 }
 
-void throwing_throw_nothing() throw() {
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'throwing_throw_nothing' which should not throw exceptions
-  throw 1;
-}
-
 void throw_and_catch() noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown 
in function 'throw_and_catch' which should not throw exceptions
   try {
@@ -557,7 +552,9 @@
   throw 1;
 }
 
-void explicit_int_thrower() throw(int);
+void explicit_int_thrower() noexcept(false) {
+  throw 1;
+}
 
 void indirect_implicit() noexcept {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
@@ -676,15 +673,6 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws' which should not throw exceptions
 };
 
-struct super_throws_again {
-  super_throws_again() throw(int);
-};
-
-struct sub_throws_again : super_throws_again {
-  sub_throws_again() noexcept : super_throws_again() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws_again' which should not throw exceptions
-};
-
 struct init_member_throws {
   super_throws s;
 
Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy -std=c++11,c++14 %s bugprone-exception-escape %t -- 
-- -fexceptions
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void implicit_int_thrower() {
+  throw 5;
+}
+
+void indirect_implicit() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();
+}
+
+void indirect_explicit() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_explicit' which should not throw exceptions
+  explicit_int_thrower();
+}
+
+struct super_throws {
+  super_throws() throw(int) { throw 42; }
+};
+
+struct sub_throws : super_throws {
+  sub_throws() throw() : super_throws() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws' which should not throw exceptions
+};


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -32,11 +32,6 @@
   throw 1;
 }
 
-void throwing_throw_nothing() throw() {
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
-  throw 1;
-}
-
 void throw_and_catch() noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
   try {
@@ -557,7 +552,9 @@
   throw 1;
 }
 
-void explicit_int_thrower() throw(int);
+void explicit_int_thrower() noexcept(false) {
+  throw 1;
+}
 
 void indirect_implicit() noexcept {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_implicit' which should not throw exceptions
@@ -676,15 +673,6 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'sub_throws' which should not throw exceptions
 };
 
-struct super_throws_again {
-  super_throws_again() throw(int);
-};
-
-struct sub_throws_again : super_throws_again {
-  sub_throws_again() noexcept : super_throws_again() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'sub_throws_again' which should not throw exceptions
-};
-
 struct 

[clang-tools-extra] 5545f1b - [clang-tidy][NFC] Split bugprone-exception-escape tests

2023-05-07 Thread Piotr Zegar via cfe-commits

Author: Piotr Zegar
Date: 2023-05-07T17:56:47Z
New Revision: 5545f1bbd4e18b9ffda993ee13460d417194941a

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

LOG: [clang-tidy][NFC] Split bugprone-exception-escape tests

Split tests files into noexcept and throw().
This is preparation for a C++20 support in this check.

Reviewed By: carlosgalvezp

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp

Modified: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
new file mode 100644
index 0..4a0113b8be3b3
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy -std=c++11,c++14 %s bugprone-exception-escape %t -- 
-- -fexceptions
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void implicit_int_thrower() {
+  throw 5;
+}
+
+void indirect_implicit() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();
+}
+
+void indirect_explicit() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_explicit' which should not throw exceptions
+  explicit_int_thrower();
+}
+
+struct super_throws {
+  super_throws() throw(int) { throw 42; }
+};
+
+struct sub_throws : super_throws {
+  sub_throws() throw() : super_throws() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws' which should not throw exceptions
+};

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index 78d434854b095..f575a185897c2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -32,11 +32,6 @@ void throwing_noexcept() noexcept {
   throw 1;
 }
 
-void throwing_throw_nothing() throw() {
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'throwing_throw_nothing' which should not throw exceptions
-  throw 1;
-}
-
 void throw_and_catch() noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown 
in function 'throw_and_catch' which should not throw exceptions
   try {
@@ -557,7 +552,9 @@ void implicit_int_thrower() {
   throw 1;
 }
 
-void explicit_int_thrower() throw(int);
+void explicit_int_thrower() noexcept(false) {
+  throw 1;
+}
 
 void indirect_implicit() noexcept {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
@@ -676,15 +673,6 @@ struct sub_throws : super_throws {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws' which should not throw exceptions
 };
 
-struct super_throws_again {
-  super_throws_again() throw(int);
-};
-
-struct sub_throws_again : super_throws_again {
-  sub_throws_again() noexcept : super_throws_again() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in 
function 'sub_throws_again' which should not throw exceptions
-};
-
 struct init_member_throws {
   super_throws s;
 



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


[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

2023-05-07 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb updated this revision to Diff 520195.
jaredgrubb added a comment.

Address review feedback about `code/endcode`. Otherwise the patch is the same 
and I hope ready for merge? I'd love to get a green check :)


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

https://reviews.llvm.org/D145262

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestObjC.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1543,6 +1543,116 @@
   EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacros) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("__attribute__(X) void Foo(void);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("A(X) void Foo(void);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("A(X) void Foo(void);", Style);
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCDecl) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("__attribute__(X) @interface Foo");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("A(X) @interface Foo");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  // Note: Don't check token-type as a random token in this position is hard to
+  // reason about.
+  EXPECT_TOKEN_KIND(Tokens[0], tok::identifier);
+  EXPECT_TOKEN_KIND(Tokens[1], tok::l_paren);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("A(X) @interface Foo", Style);
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCMethodDecl) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("- (id)init __attribute__(X);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("- (id)init A(X);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  // Note: Don't check token-type as a random token in this position is hard to
+  // reason about.
+  EXPECT_TOKEN_KIND(Tokens[5], tok::identifier);
+  EXPECT_TOKEN_KIND(Tokens[6], tok::l_paren);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("- (id)init A(X);", Style);
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCProperty) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("@property(weak) id delegate __attribute__(X);");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_AttributeParen);
+  EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_AttributeParen);
+
+  // Generic macro has no special handling in this 

[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision.
Herald added subscribers: PiotrZSL, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Some time ago a patch was merged to disable all clang-tidy warnings
from system macros. This led to bugprone-assert-side-effect
silently no longer working, since the warnings came from a system
macro. The problem was not detected because the fake assert functions
were implemented in the same file as the test, instead of being a
system include like it's done in the real world.

Move the assert to a proper system header, and fix the code to
warn at the correct location.

This patch is breakdown from https://reviews.llvm.org/D147081
by PiotrZSL.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150071

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
@@ -1,47 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
-
-//===--- assert definition block --===//
-int abort() { return 0; }
-
-#ifdef NDEBUG
-#define assert(x) 1
-#else
-#define assert(x)  \
-  if (!(x))\
-  (void)abort()
-#endif
-
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?\
-   print (#e, __FILE__, __LINE__) : (void)0)
-
-#ifdef NDEBUG
-#define my_assert(x) 1
-#else
-#define my_assert(x)   \
-  ((void)((x) ? 1 : abort()))
-#endif
-
-#ifdef NDEBUG
-#define not_my_assert(x) 1
-#else
-#define not_my_assert(x)   \
-  if (!(x))\
-  (void)abort()
-#endif
-
-#define real_assert(x) ((void)((x) ? 1 : abort()))
-#define wrap1(x) real_assert(x)
-#define wrap2(x) wrap1(x)
-#define convoluted_assert(x) wrap2(x)
-
-#define msvc_assert(expression) (void)(\
-(!!(expression)) ||\
-(abort(), 0)   \
-)
-
-
-//===--===//
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions -isystem %S/Inputs/assert-side-effect
+#include 
 
 bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
 
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h
@@ -0,0 +1,38 @@
+int abort() { return 0; }
+
+#ifdef NDEBUG
+#define assert(x) 1
+#else
+#define assert(x)  \
+  if (!(x))\
+  (void)abort()
+#endif
+
+void print(...);
+#define assert2(e) (__builtin_expect(!(e), 0) ?\
+   print (#e, __FILE__, __LINE__) : (void)0)
+
+#ifdef NDEBUG
+#define my_assert(x) 1
+#else
+#define my_assert(x)   \
+  ((void)((x) ? 1 : abort()))
+#endif
+
+#ifdef NDEBUG
+#define not_my_assert(x) 1
+#else
+#define not_my_assert(x)   \
+  if (!(x))\
+  (void)abort()
+#endif
+
+#define real_assert(x) ((void)((x) ? 1 : abort()))

[PATCH] D148458: [clang-tidy][NFC] Split bugprone-exception-escape tests

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

LGTM!

> A lot of our test files uses macros to differentiate between specific C++ 
> standards, why not do that here too?

It's not that easy, because the #ifdef lines do not remove the `// CHECK` 
comments, so to work around that it gets messy very easily. Splitting into 2 
files seems to be common practice in other tests for easier maintenance and 
readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148458

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


[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG26f476286fbc: [clang-tidy] Support SystemHeaders in 
.clang-tidy (authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149899

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -120,6 +120,7 @@
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
   UseColor: false
+  SystemHeaders: false
   )",
"Options1"));
   ASSERT_TRUE(!!Options1);
@@ -134,6 +135,7 @@
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
   UseColor: true
+  SystemHeaders: true
   )",
"Options2"));
   ASSERT_TRUE(!!Options2);
@@ -154,6 +156,9 @@
Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.has_value());
   EXPECT_TRUE(*Options.UseColor);
+
+  ASSERT_TRUE(Options.SystemHeaders.has_value());
+  EXPECT_TRUE(*Options.SystemHeaders);
 }
 
 namespace {
@@ -249,6 +254,17 @@
  DiagKind(llvm::SourceMgr::DK_Error),
  DiagPos(Options.range().Begin),
  DiagRange(Options.range();
+
+  Options = llvm::Annotations(R"(
+SystemHeaders: [[NotABool]]
+  )");
+  ParsedOpt = ParseWithDiags(Options.code());
+  EXPECT_TRUE(!ParsedOpt);
+  EXPECT_THAT(Collector.getDiags(),
+  testing::ElementsAre(AllOf(DiagMessage("invalid boolean"),
+ DiagKind(llvm::SourceMgr::DK_Error),
+ DiagPos(Options.range().Begin),
+ DiagRange(Options.range();
 }
 
 namespace {
Index: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+#include 
+// CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM-HEADERS-NOT: system_header.h:1:13: warning: single-argument constructors must be marked explicit

[clang-tools-extra] 26f4762 - [clang-tidy] Support SystemHeaders in .clang-tidy

2023-05-07 Thread Carlos Galvez via cfe-commits

Author: Carlos Galvez
Date: 2023-05-07T16:36:30Z
New Revision: 26f476286fbcb5cde51176abb2d3c6c0986bc410

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

LOG: [clang-tidy] Support SystemHeaders in .clang-tidy

A previous patch update the clang-tidy documentation
incorrectly claiming that SystemHeaders can be provided
in the .clang-tidy configuration file.

This patch adds support for it, together with tests.

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

Added: 

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp

Modified: 
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index afa88cb4f80d1..bc2ecc6b54553 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -172,6 +172,7 @@ template <> struct MappingTraits {
 IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore);
 IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
 IO.mapOptional("UseColor", Options.UseColor);
+IO.mapOptional("SystemHeaders", Options.SystemHeaders);
   }
 };
 

diff  --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 2bed6dfda3a0a..74340e1b06cb0 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -134,10 +134,13 @@ option in .clang-tidy file, if any.
  cl::init(""),
  cl::cat(ClangTidyCategory));
 
-static cl::opt
-SystemHeaders("system-headers",
-  desc("Display the errors from system headers."),
-  cl::init(false), cl::cat(ClangTidyCategory));
+static cl::opt SystemHeaders("system-headers", desc(R"(
+Display the errors from system headers.
+This option overrides the 'SystemHeaders' option
+in .clang-tidy file, if any.
+)"),
+   cl::init(false), 
cl::cat(ClangTidyCategory));
+
 static cl::opt LineFilter("line-filter", desc(R"(
 List of files with line ranges to filter the
 warnings. Can be used together with

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6b2c36975b80c..f2393a4156548 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,9 @@ Improvements to clang-tidy
 
 - Fix a potential crash when using the `--dump-config` option.
 
+- Support specifying `SystemHeaders` in the `.clang-tidy` configuration file,
+  with the same functionality as the command-line option `--system-headers`.
+
 New checks
 ^^
 

diff  --git a/clang-tools-extra/docs/clang-tidy/index.rst 
b/clang-tools-extra/docs/clang-tidy/index.rst
index 444d03d6cb08a..41fde5064b8ee 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -211,6 +211,8 @@ An overview of all the command-line options:
  format to stderr. When this option is 
passed,
  these per-TU profiles are instead stored 
as JSON.
 --system-headers   - Display the errors from system headers.
+ This option overrides the 'SystemHeaders' 
option
+ in .clang-tidy file, if any.
 --use-color- Use colors in diagnostics. If not set, 
colors
  will be used if the terminal connected to
  standard output supports colors.

diff  --git 
a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
 
b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
new file mode 100644
index 0..1a3014e83745d
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
@@ -0,0 +1 @@
+class Foo { Foo(int); };

diff  --git 
a/clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
new file mode 100644
index 0..9fa990b6aac8c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ 

[PATCH] D150063: [clang] Restores some -std=c++2b tests.

2023-05-07 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added a reviewer: aaron.ballman.
Herald added a project: All.
Mordante updated this revision to Diff 520185.
Mordante added a comment.
Mordante edited the summary of this revision.
Mordante published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Retrigger CI.




Comment at: clang/test/Preprocessor/init.c:13
 // RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++23 -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix CXX2B %s
+// RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++2b -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix CXX2B %s
 //

FYI the revision had a parent revision to test some changes for the Clang 
pre-commit CI not related to this patch at all.


These tests should have added -std=c++23 instead of replacing -std=c++2b
in D149553 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150063

Files:
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -10,6 +10,7 @@
 //
 //
 // RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++23 -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix CXX2B %s
+// RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++2b -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix CXX2B %s
 //
 // CXX2B:#define __GNUG__ 4
 // CXX2B:#define __GXX_EXPERIMENTAL_CXX0X__ 1
@@ -134,6 +135,7 @@
 // FREESTANDING:#define __STDC_HOSTED__ 0
 //
 // RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=gnu++23 -E -dM < /dev/null 
| FileCheck -match-full-lines -check-prefix GXX2B %s
+// RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=gnu++2b -E -dM < /dev/null 
| FileCheck -match-full-lines -check-prefix GXX2B %s
 //
 // GXX2B:#define __GNUG__ 4
 // GXX2B:#define __GXX_WEAK__ 1


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -10,6 +10,7 @@
 //
 //
 // RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++23 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX2B %s
+// RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=c++2b -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX2B %s
 //
 // CXX2B:#define __GNUG__ 4
 // CXX2B:#define __GXX_EXPERIMENTAL_CXX0X__ 1
@@ -134,6 +135,7 @@
 // FREESTANDING:#define __STDC_HOSTED__ 0
 //
 // RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=gnu++23 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX2B %s
+// RUN: %clang_cc1 -x c++ -fgnuc-version=4.2.1 -std=gnu++2b -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX2B %s
 //
 // GXX2B:#define __GNUG__ 4
 // GXX2B:#define __GXX_WEAK__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-07 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM. Well done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

2023-05-07 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

So GCC gives me:

  warning: ‘stdcall’ attribute directive ignored [-Wattributes]

when trying to use `__attribute__((stdcall))` on m68k, which matches the fact 
it's only mentioned in the manage for the x86 option.

Interestingly it seems to ICE during expand when I use -mrtd though...




Comment at: clang/lib/Basic/Targets/M68k.cpp:258
+  // The M68k_RTD calling convention.
+  case CC_X86StdCall:
+return CCCR_OK;

Uhh



Comment at: clang/lib/CodeGen/CGCall.cpp:51
   default: return llvm::CallingConv::C;
-  case CC_X86StdCall: return llvm::CallingConv::X86_StdCall;
+  case CC_X86StdCall:
+return CGM.getTriple().getArch() == llvm::Triple::m68k

Ditto...



Comment at: clang/test/CodeGen/mrtd.c:4
 
-// CHECK: mrtd.c:10:3: warning: function with no prototype cannot use the 
stdcall calling convention
+// CHECK: mrtd.c:13:3: warning: function with no prototype cannot use the 
stdcall calling convention
 

Ew... this should be using -verify and `// expected-warning {{...}}`, then the 
line number is relative to the comment's location. Or, really, it should be in 
the Sema test...

Plus is it correct to call this stdcall on m68k? The GCC manpage only mentions 
it in the x86 option description, not the m68k one.


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

https://reviews.llvm.org/D149867

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


[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

2023-05-07 Thread Xinlong Wu via Phabricator via cfe-commits
VincentWu updated this revision to Diff 520170.
VincentWu marked 6 inline comments as done.
VincentWu added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmp-invalid.s
  llvm/test/MC/RISCV/rv32zcmp-valid.s
  llvm/test/MC/RISCV/rv64zcmp-invalid.s
  llvm/test/MC/RISCV/rv64zcmp-valid.s

Index: llvm/test/MC/RISCV/rv64zcmp-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv64zcmp-valid.s
@@ -0,0 +1,149 @@
+# RUN: llvm-mc %s -triple=riscv64 -mattr=experimental-zcmp -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=experimental-zcmp < %s \
+# RUN: | llvm-objdump --mattr=-c,experimental-zcmp -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: cm.mvsa01 s1, s0
+# CHECK-ASM: encoding: [0xa2,0xac]
+cm.mvsa01 s1, s0
+
+# CHECK-ASM-AND-OBJ: cm.mva01s s1, s0
+# CHECK-ASM: encoding: [0xe2,0xac]
+cm.mva01s s1, s0
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xbe]
+cm.popret {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xbe]
+cm.popret {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0}, 64
+# CHECK-ASM: encoding: [0x5e,0xbe]
+cm.popret {ra, s0}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xbe]
+cm.popret {ra,s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xbe]
+cm.popret {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s3}, 64
+# CHECK-ASM: encoding: [0x86,0xbe]
+cm.popret {ra, s0-s3}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xbe]
+cm.popret {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xbe]
+cm.popret {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.popret   {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xbe]
+cm.popret {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xbc]
+cm.popretz {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xbc]
+cm.popretz {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0}, 64
+# CHECK-ASM: encoding: [0x5e,0xbc]
+cm.popretz {ra, s0}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xbc]
+cm.popretz {ra, s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xbc]
+cm.popretz {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s3}, 64
+# CHECK-ASM: encoding: [0x86,0xbc]
+cm.popretz {ra, s0-s3}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xbc]
+cm.popretz {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xbc]
+cm.popretz {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.popretz   {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xbc]
+cm.popretz {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra}, 16
+# CHECK-ASM: encoding: [0x42,0xba]
+cm.pop {ra}, 16
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra}, 32
+# CHECK-ASM: encoding: [0x46,0xba]
+cm.pop {ra}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0}, 16
+# CHECK-ASM: encoding: [0x52,0xba]
+cm.pop {ra, s0}, 16
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s1}, 32
+# CHECK-ASM: encoding: [0x62,0xba]
+cm.pop {ra, s0-s1}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s2}, 32
+# CHECK-ASM: encoding: [0x72,0xba]
+cm.pop {ra, s0-s2}, 32
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s5}, 64
+# CHECK-ASM: encoding: [0xa2,0xba]
+cm.pop {ra, s0-s5}, 64
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s7}, 80
+# CHECK-ASM: encoding: [0xc2,0xba]
+cm.pop {ra, s0-s7}, 80
+
+# CHECK-ASM-AND-OBJ: cm.pop  {ra, s0-s11}, 112
+# CHECK-ASM: encoding: [0xf2,0xba]
+cm.pop {ra, s0-s11}, 112
+
+# CHECK-ASM-AND-OBJ: cm.push {ra}, -16
+# CHECK-ASM: encoding: [0x42,0xb8]
+cm.push {ra}, -16
+
+# CHECK-ASM-AND-OBJ: cm.push {ra, s0}, -32
+# CHECK-ASM: encoding: [0x56,0xb8]
+cm.push {ra, s0}, -32
+
+# CHECK-ASM-AND-OBJ: cm.push {ra, s0-s1}, -32
+# 

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-07 Thread Alvin Wong via Phabricator via cfe-commits
alvinhochun updated this revision to Diff 520158.
alvinhochun added a comment.

Apply suggestions: remove TODO, put negative tests in the same file as positive 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/test/MC/X86/intel-syntax-branch.s

Index: llvm/test/MC/X86/intel-syntax-branch.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,71 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+// RUN: not llvm-mc -triple i686-unknown-unknown --defsym ERR=1 -x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefixes=ERR-32
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // CHECK-64: jmpq *fn_ref(%rip)
+
+.else
+
+  t4:
+  call dword ptr [fn_ref]
+  jmp dword ptr [fn_ref]
+  // CHECK-32-LABEL: t4:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t5:
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t6:
+  call dword ptr [offset fn_ref]
+  jmp dword ptr [offset fn_ref]
+  // CHECK-32-LABEL: t6:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+.ifdef ERR
+
+  call offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+  jmp offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+
+.endif
+
+.endif
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -432,6 +432,7 @@
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -454,6 +455,7 @@
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -954,6 +956,7 @@
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2630,9 +2633,9 @@
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2654,6 +2657,9 @@
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+if (PtrInOperand || SM.isBracketUsed())
+  MaybeDirectBranchDest = false;
   }
 
   if ((BaseReg || IndexReg || RegNo || DefaultBaseReg != X86::NoRegister))
Index: clang/test/CodeGen/ms-inline-asm-functions.c
===
--- clang/test/CodeGen/ms-inline-asm-functions.c
+++ clang/test/CodeGen/ms-inline-asm-functions.c
@@ -24,10 +24,9 @@
   __asm call kimport;
   // CHECK: calll   *({{.*}})
 
-  // Broken case: Call through a global function pointer.
+  // Call through a global function pointer.
   __asm call kptr;
-  // CHECK: calll   _kptr
-  // CHECK-FIXME: calll   *_kptr
+  // CHECK: calll   *_kptr
 }
 
 int bar(void) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149946: [LoongArch] Define `ual` feature and override `allowsMisalignedMemoryAccesses`

2023-05-07 Thread Xi Ruoyao via Phabricator via cfe-commits
xry111 added a comment.

In D149946#4324877 , @SixWeining 
wrote:

> In D149946#4324803 , @xen0n wrote:
>
>> From a LoongArch developer's perspective, it may be better to only enable 
>> UAL for LA464 and other supporting models, instead of for the generic 
>> `loongarch64` model too. This is because although all server- and 
>> desktop-class LoongArch models have UAL, the embedded-class (Loongson-1 and 
>> Loongson-2 series' older models) doesn't, and some of them e.g. Loongson 
>> 2K1000LA are readily available on the market so they're arguably relevant. 
>> We don't want to generate misaligned memory accesses for those systems only 
>> to fall back to much slower emulation later.
>
> If so, CPUs that support UAL will not benefit from this feature in default 
> build (i.e. without -march or -mcpu or -mtune being specified).
>
> Does `generic` model mean the `lowest` model or the `most popular` model?

Technically `generic` should mean the `lowest`.  But as a desktop user, frankly 
I don't want to pay the extra cost for 2K models.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149946

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


[PATCH] D149946: [LoongArch] Define `ual` feature and override `allowsMisalignedMemoryAccesses`

2023-05-07 Thread Lu Weining via Phabricator via cfe-commits
SixWeining added a comment.

In D149946#4324803 , @xen0n wrote:

> From a LoongArch developer's perspective, it may be better to only enable UAL 
> for LA464 and other supporting models, instead of for the generic 
> `loongarch64` model too. This is because although all server- and 
> desktop-class LoongArch models have UAL, the embedded-class (Loongson-1 and 
> Loongson-2 series' older models) doesn't, and some of them e.g. Loongson 
> 2K1000LA are readily available on the market so they're arguably relevant. We 
> don't want to generate misaligned memory accesses for those systems only to 
> fall back to much slower emulation later.

If so, CPUs that support UAL will not benefit from this feature in default 
build (i.e. without -march or -mcpu or -mtune being specified).

Does `generic` model mean the `lowest` model or the `most popular` model?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149946

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


[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

2023-05-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@rZhBoYao are you still working on this? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139586

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


[PATCH] D146030: [clang][Interp] Handle LambdaExprs

2023-05-07 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 520155.
tbaeder marked 3 inline comments as done.

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

https://reviews.llvm.org/D146030

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeEmitter.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/EvalEmitter.h
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/lambda.cpp

Index: clang/test/AST/Interp/lambda.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/lambda.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify -std=c++20 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
+
+constexpr int a = 12;
+constexpr int f = [c = a]() { return c; }();
+static_assert(f == a);
+
+
+constexpr int inc() {
+  int a = 10;
+  auto f = []() {
+++a;
+  };
+
+  f();f();
+
+  return a;
+}
+static_assert(inc() == 12);
+
+constexpr int add(int a, int b) {
+  auto doIt = [a, b](int c) {
+return a + b + c;
+  };
+
+  return doIt(2);
+}
+static_assert(add(4, 5) == 11);
+
+
+constexpr int add2(int a, int b) {
+  auto doIt = [a, b](int c) {
+auto bar = [a]() { return a; };
+auto bar2 = [b]() { return b; };
+
+return bar() + bar2() + c;
+  };
+
+  return doIt(2);
+}
+static_assert(add2(4, 5) == 11);
+
+
+constexpr int div(int a, int b) {
+  auto f = [=]() {
+return a / b; // expected-note {{division by zero}} \
+  // ref-note {{division by zero}}
+  };
+
+  return f(); // expected-note {{in call to '>operator()()'}} \
+  // ref-note {{in call to '>operator()()'}}
+}
+static_assert(div(8, 2) == 4);
+static_assert(div(8, 0) == 4); // expected-error {{not an integral constant expression}} \
+   // expected-note {{in call to 'div(8, 0)'}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'div(8, 0)'}}
+
+
+struct F {
+  float f;
+};
+
+constexpr float captureStruct() {
+  F someF = {1.0};
+
+  auto p = [someF]() {
+return someF.f;
+  };
+
+  return p();
+}
+
+static_assert(captureStruct() == 1.0);
+
+namespace LambdaParams {
+  template
+  constexpr void callThis(T t) {
+return t();
+  }
+
+  constexpr int foo() {
+int a = 0;
+auto f = []() { ++a; };
+
+callThis(f);
+
+return a;
+  }
+  /// FIXME: This should work in the new interpreter.
+  static_assert(foo() == 1); // expected-error {{not an integral constant expression}}
+}
+
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -850,6 +850,7 @@
   const Pointer  = Obj.atField(I);
   if (!CheckStore(S, OpPC, Field))
 return false;
+  Field.initialize();
   Field.deref() = Value;
   return true;
 }
Index: clang/lib/AST/Interp/EvalEmitter.h
===
--- clang/lib/AST/Interp/EvalEmitter.h
+++ clang/lib/AST/Interp/EvalEmitter.h
@@ -76,6 +76,10 @@
 
   /// Parameter indices.
   llvm::DenseMap Params;
+  /// Lambda captures.
+  /// Map from Decl* to [Offset, IsReference] pair.
+  llvm::DenseMap> LambdaCaptures;
+  unsigned LambdaThisCapture;
   /// Local descriptors.
   llvm::SmallVector, 2> Descriptors;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -93,6 +93,7 @@
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
+  bool VisitLambdaExpr(const LambdaExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -949,6 +949,43 @@
   return this->emitConstBool(E->getValue(), E);
 }
 
+template 
+bool ByteCodeExprGen::VisitLambdaExpr(const LambdaExpr *E) {
+  // XXX We assume here that a pointer-to-initialize is on the stack.
+
+  const Record *R = P.getOrCreateRecord(E->getLambdaClass());
+
+  auto *CaptureInitIt = E->capture_init_begin();
+  // Initialize all fields (which represent lambda captures) of the
+  // record with their initializers.
+  for (const Record::Field  : R->fields()) {
+const Expr *Init = *CaptureInitIt;
+++CaptureInitIt;
+
+if (std::optional T = classify(Init)) {
+  if (!this->visit(Init))
+return false;
+
+  if (!this->emitSetField(*T, F.Offset, E))
+return false;
+} else {
+  if (!this->emitDupPtr(E))
+return false;
+
+  if 

[PATCH] D149834: [clang][Interp] Fix ignoring TypeTraitExprs

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



Comment at: clang/test/AST/Interp/literals.cpp:875
 1 ? 0 : 1;
+sizeof(A);
+alignof(A);

aaron.ballman wrote:
> Let's make sure we still reject this:
> ```
> constexpr int oh_my() {
>   int x = 0;
>   sizeof(int[x++]); // This would usually be evaluated because VLAs are 
> terrible
>   return x;
> }
> ```
> https://godbolt.org/z/E3jx6co46
Hm, no, doesn't get rejected:
```
array.cpp:1240:3: warning: expression result unused [-Wunused-value]
 1240 |   sizeof(int[x++]); // This would usually be evaluated because VLAs are 
terrible
  |   ^~~~
array.cpp:1243:15: error: static assertion failed due to requirement 'oh_my() 
== 1'
 1243 | static_assert(oh_my() == 1);
  |   ^~~~
array.cpp:1243:23: note: expression evaluates to '0 == 1'
 1243 | static_assert(oh_my() == 1);
  |   ^~~~
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149834

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


[PATCH] D146030: [clang][Interp] Handle LambdaExprs

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



Comment at: clang/test/AST/Interp/lambda.cpp:92
+  static_assert(foo() == 1); // expected-error {{not an integral constant 
expression}}
+}
+

aaron.ballman wrote:
> How about some tests like:
> ```
> constexpr int call_thru_func_ptr(int i) {
>   auto l = [](int i) { return i; };
>   int (*fp)(int) = l;
>   return fp(i);  
> }
> static_assert(call_thru_func_ptr(12) == 12);
> 
> constexpr int call_through_copied_lambda(auto lam, int i) {
>   auto copy = lam;
>   return copy(i);
> }
> 
> constexpr int call_through_copied_lambda(auto lam) {
>   auto copy = lam;
>   return copy();
> }
> 
> void func() {
>   constexpr int i = 12;
>   static_assert(call_through_copied_lambda([i]() { return i; }) == 12);
> }
> ```
Heh:
```
array.cpp:1245:15: error: static assertion expression is not an integral 
constant expression
 1245 | static_assert(call_thru_func_ptr(12) == 12);
  |   ^~~~
array.cpp:1243:10: note: non-constexpr function '__invoke' cannot be used in a 
constant expression
 1243 |   return fp(i);
  |  ^
array.cpp:1245:15: note: in call to 'call_thru_func_ptr(12)'
 1245 | static_assert(call_thru_func_ptr(12) == 12);
  |   ^
array.cpp:1239:12: note: declared here
 1239 |   auto l = [](int i) { return i; };
  |^

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146030

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


[PATCH] D150036: [Clang] Correctly handle allocation in template arguments

2023-05-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 520154.
cor3ntin added a comment.

Update commit message and add comment referencing the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150036

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp


Index: clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
===
--- clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -215,3 +215,27 @@
   }
   static_assert(h());
 }
+
+namespace GH62462 {
+
+class string {
+public:
+  char *mem;
+  constexpr string() {
+this->mem = new char(1);
+  }
+  constexpr ~string() {
+delete this->mem;
+  }
+  constexpr unsigned size() const { return 4; }
+};
+
+
+template 
+void test() {};
+
+void f() {
+test();
+}
+
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -15354,8 +15354,15 @@
   LValue LVal;
   LVal.set(Base);
 
-  if (!::EvaluateInPlace(Result.Val, Info, LVal, this) || 
Result.HasSideEffects)
-return false;
+  {
+// C++23 [intro.execution]/p5
+// A full-expression is [...] a constant-expression
+// So we need to make sure temporary objects are destroyed after having 
evaluating
+// the expression (per C++23 [class.temporary]/p4).
+FullExpressionRAII Scope(Info);
+if (!::EvaluateInPlace(Result.Val, Info, LVal, this) || 
Result.HasSideEffects  || !Scope.destroy())
+  return false;
+  }
 
   if (!Info.discardCleanups())
 llvm_unreachable("Unhandled cleanup; missing full expression marker?");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -381,6 +381,8 @@
 - Fix overly aggressive lifetime checks for parenthesized aggregate
   initialization.
   (`#61567 `_)
+- Fix handling of constexpr dynamic memory allocations in template
+  arguments. (`#62462 `)
 
 Bug Fixes to AST Handling
 ^


Index: clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
===
--- clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -215,3 +215,27 @@
   }
   static_assert(h());
 }
+
+namespace GH62462 {
+
+class string {
+public:
+  char *mem;
+  constexpr string() {
+this->mem = new char(1);
+  }
+  constexpr ~string() {
+delete this->mem;
+  }
+  constexpr unsigned size() const { return 4; }
+};
+
+
+template 
+void test() {};
+
+void f() {
+test();
+}
+
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -15354,8 +15354,15 @@
   LValue LVal;
   LVal.set(Base);
 
-  if (!::EvaluateInPlace(Result.Val, Info, LVal, this) || Result.HasSideEffects)
-return false;
+  {
+// C++23 [intro.execution]/p5
+// A full-expression is [...] a constant-expression
+// So we need to make sure temporary objects are destroyed after having evaluating
+// the expression (per C++23 [class.temporary]/p4).
+FullExpressionRAII Scope(Info);
+if (!::EvaluateInPlace(Result.Val, Info, LVal, this) || Result.HasSideEffects  || !Scope.destroy())
+  return false;
+  }
 
   if (!Info.discardCleanups())
 llvm_unreachable("Unhandled cleanup; missing full expression marker?");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -381,6 +381,8 @@
 - Fix overly aggressive lifetime checks for parenthesized aggregate
   initialization.
   (`#61567 `_)
+- Fix handling of constexpr dynamic memory allocations in template
+  arguments. (`#62462 `)
 
 Bug Fixes to AST Handling
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149837: [clang][Interp] Fix ignoring CompoundLiteralExprs

2023-05-07 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 520153.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D149837

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -875,9 +875,26 @@
 sizeof(A);
 alignof(A);
 
+(int){1};
+(int[]){1,2,3};
+
 return 0;
   }
 
+  constexpr int oh_my(int x) {
+(int){ x++ };
+return x;
+  }
+  static_assert(oh_my(0) == 1, "");
+
+  constexpr int oh_my2(int x) {
+int y{x++};
+return x;
+  }
+
+  static_assert(oh_my2(0) == 1, "");
+
+
   /// Ignored comma expressions still have their
   /// expressions evaluated.
   constexpr int Comma(int start) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -527,8 +527,13 @@
 template 
 bool ByteCodeExprGen::VisitInitListExpr(const InitListExpr *E) {
   for (const Expr *Init : E->inits()) {
-if (!this->visit(Init))
-  return false;
+if (DiscardResult) {
+  if (!this->discard(Init))
+return false;
+} else {
+  if (!this->visit(Init))
+return false;
+}
   }
   return true;
 }
@@ -1024,12 +1029,16 @@
   // Otherwise, use a local variable.
   if (T) {
 // For primitive types, we just visit the initializer.
-return this->visit(Init);
+return DiscardResult ? this->discard(Init) : this->visit(Init);
   } else {
 if (std::optional LocalIndex = allocateLocal(Init)) {
   if (!this->emitGetPtrLocal(*LocalIndex, E))
 return false;
-  return this->visitInitializer(Init);
+  if (!this->visitInitializer(Init))
+return false;
+  if (DiscardResult)
+return this->emitPopPtr(E);
+  return true;
 }
   }
 


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -875,9 +875,26 @@
 sizeof(A);
 alignof(A);
 
+(int){1};
+(int[]){1,2,3};
+
 return 0;
   }
 
+  constexpr int oh_my(int x) {
+(int){ x++ };
+return x;
+  }
+  static_assert(oh_my(0) == 1, "");
+
+  constexpr int oh_my2(int x) {
+int y{x++};
+return x;
+  }
+
+  static_assert(oh_my2(0) == 1, "");
+
+
   /// Ignored comma expressions still have their
   /// expressions evaluated.
   constexpr int Comma(int start) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -527,8 +527,13 @@
 template 
 bool ByteCodeExprGen::VisitInitListExpr(const InitListExpr *E) {
   for (const Expr *Init : E->inits()) {
-if (!this->visit(Init))
-  return false;
+if (DiscardResult) {
+  if (!this->discard(Init))
+return false;
+} else {
+  if (!this->visit(Init))
+return false;
+}
   }
   return true;
 }
@@ -1024,12 +1029,16 @@
   // Otherwise, use a local variable.
   if (T) {
 // For primitive types, we just visit the initializer.
-return this->visit(Init);
+return DiscardResult ? this->discard(Init) : this->visit(Init);
   } else {
 if (std::optional LocalIndex = allocateLocal(Init)) {
   if (!this->emitGetPtrLocal(*LocalIndex, E))
 return false;
-  return this->visitInitializer(Init);
+  if (!this->visitInitializer(Init))
+return false;
+  if (DiscardResult)
+return this->emitPopPtr(E);
+  return true;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 520152.
carlosgalvezp added a comment.

Revert unwanted change to UseColor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149899

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -120,6 +120,7 @@
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
   UseColor: false
+  SystemHeaders: false
   )",
"Options1"));
   ASSERT_TRUE(!!Options1);
@@ -134,6 +135,7 @@
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
   UseColor: true
+  SystemHeaders: true
   )",
"Options2"));
   ASSERT_TRUE(!!Options2);
@@ -154,6 +156,9 @@
Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.has_value());
   EXPECT_TRUE(*Options.UseColor);
+
+  ASSERT_TRUE(Options.SystemHeaders.has_value());
+  EXPECT_TRUE(*Options.SystemHeaders);
 }
 
 namespace {
@@ -249,6 +254,17 @@
  DiagKind(llvm::SourceMgr::DK_Error),
  DiagPos(Options.range().Begin),
  DiagRange(Options.range();
+
+  Options = llvm::Annotations(R"(
+SystemHeaders: [[NotABool]]
+  )");
+  ParsedOpt = ParseWithDiags(Options.code());
+  EXPECT_TRUE(!ParsedOpt);
+  EXPECT_THAT(Collector.getDiags(),
+  testing::ElementsAre(AllOf(DiagMessage("invalid boolean"),
+ DiagKind(llvm::SourceMgr::DK_Error),
+ DiagPos(Options.range().Begin),
+ DiagRange(Options.range();
 }
 
 namespace {
Index: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+#include 
+// CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM-HEADERS-NOT: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+
+// CHECK-CONFIG-NO-SYSTEM-HEADERS: SystemHeaders: false
+// 

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

2023-05-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 520151.
carlosgalvezp added a comment.
Herald added a subscriber: arphaman.

- Add test where both command line and config settings are used.
- Document that the command-line option overrides the config option, for 
consistency with other command-line options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149899

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -120,6 +120,7 @@
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
   UseColor: false
+  SystemHeaders: false
   )",
"Options1"));
   ASSERT_TRUE(!!Options1);
@@ -134,6 +135,7 @@
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
   UseColor: true
+  SystemHeaders: true
   )",
"Options2"));
   ASSERT_TRUE(!!Options2);
@@ -154,6 +156,9 @@
Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.has_value());
   EXPECT_TRUE(*Options.UseColor);
+
+  ASSERT_TRUE(Options.SystemHeaders.has_value());
+  EXPECT_TRUE(*Options.SystemHeaders);
 }
 
 namespace {
@@ -249,6 +254,17 @@
  DiagKind(llvm::SourceMgr::DK_Error),
  DiagPos(Options.range().Begin),
  DiagRange(Options.range();
+
+  Options = llvm::Annotations(R"(
+SystemHeaders: [[NotABool]]
+  )");
+  ParsedOpt = ParseWithDiags(Options.code());
+  EXPECT_TRUE(!ParsedOpt);
+  EXPECT_THAT(Collector.getDiags(),
+  testing::ElementsAre(AllOf(DiagMessage("invalid boolean"),
+ DiagKind(llvm::SourceMgr::DK_Error),
+ DiagPos(Options.range().Begin),
+ DiagRange(Options.range();
 }
 
 namespace {
Index: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=true -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -system-headers=false -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+#include 
+// CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked 

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-05-07 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

A user on Discord just ran into this 

 again; I'd like to try and find a way forward with this mitigation.

Since we haven't heard from @dgoldman, I'd like to explore an alternative 
strategy: plumb in information about whether the `-xobjective-c++-header` came 
from the fallback flags, and alter it to `-xc++-header` only in that case.

@kadircet, would you be open to accepting this with the above change?




Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340
+// is not installed.
+if (Lang == "objective-c++-header") {
+  Lang = "c++-header";

nridge wrote:
> kadircet wrote:
> > nridge wrote:
> > > kadircet wrote:
> > > > this feels like too much of a layering violation and might (will?) go 
> > > > wrong in cases where language was explicitly set to 
> > > > `objective-c++-header`.
> > > > 
> > > > if the user is relying on fallback commands with an overwrite of 
> > > > `Compiler:` in the config && --query-driver globs, would it be too much 
> > > > of a hassle to expect them to have a `CompileFlags: Add: ...` block too?
> > > > this feels like too much of a layering violation and might (will?) go 
> > > > wrong in cases where language was explicitly set to 
> > > > `objective-c++-header`.
> > > 
> > > This has occurred to me, and my first idea for a fix was to limit this 
> > > change to cases where the `-xobjective-c++-header` originates from the 
> > > fallback command.
> > > 
> > > However, as mentioned 
> > > [here](https://github.com/clangd/clangd/issues/1568#issuecomment-1493236437),
> > >  when I tested this I found that `-xobjective-c++-header` did not make 
> > > any difference (compared to `-xc++-header` or  `-xc++`) in the include 
> > > paths returned by gcc. In other words, in gcc's include directory 
> > > structure there are no objc-specific directories. This made me think this 
> > > simpler fix would be appropriate.
> > > 
> > > > if the user is relying on fallback commands with an overwrite of 
> > > > `Compiler:` in the config && --query-driver globs, would it be too much 
> > > > of a hassle to expect them to have a `CompileFlags: Add: ...` block too?
> > > 
> > > You're right, adding a section like this to the config does seem to be a 
> > > viable workaround:
> > > 
> > > ```
> > > ---
> > > 
> > > If:
> > >   PathMatch: *\.h
> > > 
> > > CompileFlags:
> > >   Add: [-xc++-header]
> > > ```
> > > 
> > > But I think it would still be nice to fix this in clangd, as being foiled 
> > > by objective-c support not being installed is a very unexpected failure 
> > > mode for a user whose project does not involve objective-c at all.
> > > 
> > > For what it's worth, I don't think this kind of setup is uncommon. A 
> > > common scenario seems to be a casual user playing around with a small 
> > > project (hence, doesn't have a build system or compile_commands.json), on 
> > > a platform where --query-driver is needed to find the standard library 
> > > headers (most commonly, MinGW on Windows).
> > > However, as mentioned 
> > > [here](https://github.com/clangd/clangd/issues/1568#issuecomment-1493236437),
> > >  when I tested this I found that `-xobjective-c++-header` did not make 
> > > any difference (compared to `-xc++-header` or  `-xc++`) in the include 
> > > paths returned by gcc. In other words, in gcc's include directory 
> > > structure there are no objc-specific directories.
> > 
> > Well, that's definitely re-assuring, but I am not sure if it's enough to 
> > say it'll work that way with all gcc's or when there are other/certain 
> > "system" libraries installed. As in theory objc compilation should at least 
> > add some framework search paths and what not by default, no?
> > 
> > > But I think it would still be nice to fix this in clangd, as being foiled 
> > > by objective-c support not being installed is a very unexpected failure 
> > > mode for a user whose project does not involve objective-c at all.
> > 
> > Completely agree, but we're only showing that to people that already 
> > fiddled with clangd internals. So I don't think that as  unacceptable.
> >  
> > > For what it's worth, I don't think this kind of setup is uncommon. A 
> > > common scenario seems to be a casual user playing around with a small 
> > > project (hence, doesn't have a build system or compile_commands.json), on 
> > > a platform where --query-driver is needed to find the standard library 
> > > headers (most commonly, MinGW on Windows).
> > 
> > I think instead of trying to make things work with query-driver in such 
> > setups, we should try to make sure things work out-of-the-box in mingw (and 
> > other toolchain) setups. I believe people not using query-driver in such 
> > vanilla installation is way more common than people using query-driver and 
> > `CompileFlags.Compiler` override. Also this will probably make sure 

[PATCH] D150021: [RISCV] Make zve32f imply F and zve64d imply D.

2023-05-07 Thread Craig Topper 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 rG728b8a139804: [RISCV] Make zve32f imply F and zve64d imply 
D. (authored by craig.topper).

Changed prior to commit:
  https://reviews.llvm.org/D150021?vs=520038=520150#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150021

Files:
  clang/test/Driver/riscv-arch.c
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td

Index: llvm/lib/Target/RISCV/RISCVFeatures.td
===
--- llvm/lib/Target/RISCV/RISCVFeatures.td
+++ llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -397,7 +397,7 @@
 : SubtargetFeature<"zve32f", "HasStdExtZve32f", "true",
"'Zve32f' (Vector Extensions for Embedded Processors "
"with maximal 32 EEW and F extension)",
-   [FeatureStdExtZve32x]>;
+   [FeatureStdExtZve32x, FeatureStdExtF]>;
 
 def FeatureStdExtZve64x
 : SubtargetFeature<"zve64x", "HasStdExtZve64x", "true",
@@ -415,13 +415,12 @@
 : SubtargetFeature<"zve64d", "HasStdExtZve64d", "true",
"'Zve64d' (Vector Extensions for Embedded Processors "
"with maximal 64 EEW, F and D extension)",
-   [FeatureStdExtZve64f]>;
+   [FeatureStdExtZve64f, FeatureStdExtD]>;
 
 def FeatureStdExtV
 : SubtargetFeature<"v", "HasStdExtV", "true",
"'V' (Vector Extension for Application Processors)",
-   [FeatureStdExtZvl128b, FeatureStdExtZve64d,
-FeatureStdExtF, FeatureStdExtD]>;
+   [FeatureStdExtZvl128b, FeatureStdExtZve64d]>;
 
 def HasVInstructions: Predicate<"Subtarget->hasVInstructions()">,
   AssemblerPredicate<
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -856,10 +856,7 @@
   bool HasD = Exts.count("d") != 0;
   bool HasF = Exts.count("f") != 0;
   bool HasZfinx = Exts.count("zfinx") != 0;
-  bool HasZdinx = Exts.count("zdinx") != 0;
   bool HasVector = Exts.count("zve32x") != 0;
-  bool HasZve32f = Exts.count("zve32f") != 0;
-  bool HasZve64d = Exts.count("zve64d") != 0;
   bool HasZvl = MinVLen != 0;
   bool HasZcmt = Exts.count("zcmt") != 0;
   bool HasZcd = Exts.count("zcd") != 0;
@@ -868,22 +865,10 @@
 return createStringError(errc::invalid_argument,
  "'f' and 'zfinx' extensions are incompatible");
 
-  if (HasZve32f && !HasF && !HasZfinx)
+  if (Exts.count("zvfh") && !Exts.count("zfh") && !Exts.count("zfhmin"))
 return createStringError(
 errc::invalid_argument,
-"'zve32f' requires 'f' or 'zfinx' extension to also be specified");
-
-  if (HasZve64d && !HasD && !HasZdinx)
-return createStringError(
-errc::invalid_argument,
-"'zve64d' requires 'd' or 'zdinx' extension to also be specified");
-
-  if (Exts.count("zvfh") && !Exts.count("zfh") && !Exts.count("zfhmin") &&
-  !Exts.count("zhinx") && !Exts.count("zhinxmin"))
-return createStringError(
-errc::invalid_argument,
-"'zvfh' requires 'zfh', 'zfhmin', 'zhinx' or 'zhinxmin' extension to "
-"also be specified");
+"'zvfh' requires 'zfh' or 'zfhmin extension to also be specified");
 
   if (HasZvl && !HasVector)
 return createStringError(
@@ -949,9 +934,9 @@
 static const char *ImpliedExtsZkn[] = {"zbkb", "zbkc", "zbkx",
"zkne", "zknd", "zknh"};
 static const char *ImpliedExtsZks[] = {"zbkb", "zbkc", "zbkx", "zksed", "zksh"};
-static const char *ImpliedExtsZve32f[] = {"zve32x"};
+static const char *ImpliedExtsZve32f[] = {"zve32x", "f"};
 static const char *ImpliedExtsZve32x[] = {"zvl32b", "zicsr"};
-static const char *ImpliedExtsZve64d[] = {"zve64f"};
+static const char *ImpliedExtsZve64d[] = {"zve64f", "d"};
 static const char *ImpliedExtsZve64f[] = {"zve64x", "zve32f"};
 static const char *ImpliedExtsZve64x[] = {"zve32x", "zvl64b"};
 static const char *ImpliedExtsZvfh[] = {"zve32f"};
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -176,6 +176,8 @@
   RISCVTargetParser.h interface. Similar for ``parseTuneCPUkind`` and
   ``checkTuneCPUKind``.
 * Add sifive-x280 processor.
+* Zve32f is no longer allowed with Zfinx. Zve64d is no longer allowed with
+  Zdinx.
 
 Changes to the WebAssembly Backend
 --
Index: clang/test/Driver/riscv-arch.c
===
--- 

[clang] 728b8a1 - [RISCV] Make zve32f imply F and zve64d imply D.

2023-05-07 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2023-05-06T23:17:16-07:00
New Revision: 728b8a139804db4fd9bce1ac7fa3dcbaf4dc316c

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

LOG: [RISCV] Make zve32f imply F and zve64d imply D.

The 1.0 vector spec PDF has text that says that Zve32f is compatible
with F or Zfinx and that Zve64d is compatible with D and Zdinx.
The references to *inx were removed from the spec in the github repository in
October 2021. The 1.0 pdf was made in September 2021.

Relevant commit 
https://github.com/riscv/riscv-v-spec/commit/6fedb869e213da03f36092d661d14911a2f9d2c6

Reviewed By: jacquesguan

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

Added: 


Modified: 
clang/test/Driver/riscv-arch.c
llvm/docs/ReleaseNotes.rst
llvm/lib/Support/RISCVISAInfo.cpp
llvm/lib/Target/RISCV/RISCVFeatures.td

Removed: 




diff  --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index 5b9a45deb06a2..848fc14dc95e7 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -218,16 +218,6 @@
 // RV32-ORDER: error: invalid arch name 'rv32imcq',
 // RV32-ORDER: standard user-level extension not given in canonical order 'q'
 
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32izve32f -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZVE32F-ER %s
-// RV32-ZVE32F-ER: error: invalid arch name 'rv32izve32f',
-// RV32-ZVE32F-ER: 'zve32f' requires 'f' or 'zfinx' extension to also be 
specified
-
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ifzve64d -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZVE64D-ER %s
-// RV32-ZVE64D-ER: error: invalid arch name 'rv32ifzve64d',
-// RV32-ZVE64D-ER: 'zve64d' requires 'd' or 'zdinx' extension to also be 
specified
-
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32izvl64b -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZVL64B-ER %s
 // RV32-ZVL64B-ER: error: invalid arch name 'rv32izvl64b',
@@ -498,10 +488,6 @@
 // RV32-ZVE32X-GOODVERS: "-target-feature" "+zve32x"
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32izve32f -### %s -c 2>&1 
| \
-// RUN:   FileCheck -check-prefix=RV32-ZVE32F-REQUIRE-F %s
-// RV32-ZVE32F-REQUIRE-F: error: invalid arch name 'rv32izve32f', 'zve32f' 
requires 'f' or 'zfinx' extension to also be specified
-
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ifzve32f -### %s -c 
2>&1 | \
 // RUN:   FileCheck -check-prefix=RV32-ZVE32F-GOOD %s
 // RV32-ZVE32F-GOOD: "-target-feature" "+zve32f"
 
@@ -510,18 +496,10 @@
 // RV32-ZVE64X: "-target-feature" "+zve64x"
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32izve64f -### %s -c 2>&1 
| \
-// RUN:   FileCheck -check-prefix=RV32-ZVE64F-REQUIRE-F %s
-// RV32-ZVE64F-REQUIRE-F: error: invalid arch name 'rv32izve64f', 'zve32f' 
requires 'f' or 'zfinx' extension to also be specified
-
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ifzve64f -### %s -c 
2>&1 | \
 // RUN:   FileCheck -check-prefix=RV32-ZVE64F-GOOD %s
 // RV32-ZVE64F-GOOD: "-target-feature" "+zve64f"
 
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ifzve64d -### %s -c 
2>&1 | \
-// RUN:   FileCheck -check-prefix=RV32-ZVE64D-REQUIRE-D %s
-// RV32-ZVE64D-REQUIRE-D: error: invalid arch name 'rv32ifzve64d', 'zve64d' 
requires 'd' or 'zdinx' extension to also be specified
-
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ifdzve64d -### %s -c 
2>&1 | \
+// RUN: %clang --target=riscv32-unknown-elf -march=rv32izve64d -### %s -c 2>&1 
| \
 // RUN:   FileCheck -check-prefix=RV32-ZVE64D-GOOD %s
 // RV32-ZVE64D-GOOD: "-target-feature" "+zve64d"
 

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 845cee9e75455..2be1febb44a7a 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -176,6 +176,8 @@ Changes to the RISC-V Backend
   RISCVTargetParser.h interface. Similar for ``parseTuneCPUkind`` and
   ``checkTuneCPUKind``.
 * Add sifive-x280 processor.
+* Zve32f is no longer allowed with Zfinx. Zve64d is no longer allowed with
+  Zdinx.
 
 Changes to the WebAssembly Backend
 --

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp 
b/llvm/lib/Support/RISCVISAInfo.cpp
index d25dc2b0381d1..097dffd78205a 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -856,10 +856,7 @@ Error RISCVISAInfo::checkDependency() {
   bool HasD = Exts.count("d") != 0;
   bool HasF = Exts.count("f") != 0;
   bool HasZfinx = Exts.count("zfinx") != 0;
-  bool HasZdinx = Exts.count("zdinx") != 0;
   bool HasVector = Exts.count("zve32x") != 0;
-  bool HasZve32f = Exts.count("zve32f") != 0;
-  bool HasZve64d = Exts.count("zve64d") != 0;
   bool HasZvl = MinVLen != 0;
   bool 

[PATCH] D148457: [clangd] Support macro evaluation on hover

2023-05-07 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

Thanks for the update, and thank you Sam for the suggestions!

In D148457#4307402 , @sammccall wrote:

> A couple of ideas:
>
> - special-case alignof
> - (generalization) allow partial selection via macros that expand to a single 
> token
> - (generalization) allow partial selection as long as it's of a single node - 
> the common ancestor is partially selected and no children are

I was going to suggest a variant of the second option, where if a macro expands 
to a single token, the behaviour is the same as if the expanded token had been 
written in the source and that's what had been hovered over.

However, I don't have any strong concerns about the additional scenarios 
supported by the third option, so since this is what was implemented, let's go 
with it. We can always tweak this in the future if necessary.

---

Circling back to my original concern:

In D148457#4297847 , @nridge wrote:

> the `Definition` field of the hover (which shows the tokens of the macro 
> expansion) and the `Value` and `Type` fields (which show the value and type 
> of a larger expression) could be out of sync and confusing

the original selection being "partial" vs. "complete" is just one part of this 
concern. Another part is the loop in `visitExprFromSelectionTree` that can 
choose a larger enclosing expression than the original selection (even if it's 
"complete"), so that we can get a `Definition` for a smaller expression but a 
`Value` for a larger one.

However, I discovered that this sort of discrepancy can already arise without 
macros, where the same loop means `Type` is given for a smaller expression and 
`Value` for a larger one. I filed https://github.com/clangd/clangd/issues/1622 
for this with an example.

Since this is a pre-existing issue, I think it's fine not to worry about it in 
this patch. Perhaps in a follow-up patch we can find a solution that addresses 
both the macro and non-macro cases.




Comment at: clang-tools-extra/clangd/Hover.cpp:471
+const SelectionTree::Node *
+visitExprFromSelectionTree(const SelectionTree::Node *N, const ASTContext ,
+   llvm::function_ref CB) {

Instead of generalizing `printExprValue` to take an arbitrary callback, can we 
modify the original function to return both the `Expr*` and its printed value 
(grouped in a pair or small struct)? And then our new call site can call 
`getType()` on the `Expr*`.

(I realize this would be a slight change in semantics as the callback for the 
macro codepath currently exits the loop as soon as we have a type... but I 
think there's value in keeping the behaviour consistent between the macro and 
non-macro cases.)



Comment at: clang-tools-extra/clangd/Hover.cpp:705
+
+  // If macro expands to one single token, rule out punctuator or digraph.
+  // E.g., for the case `array L_BRACKET 42 R_BRACKET;` where L_BRACKET and

This check for punctuators actually makes the behaviour **more strict** for 
macros than for non-macros in some cases.

For example:

```
#define PLUS +

constexpr int A = 2 + 3;  // hover over `+` shows `Value = 5`
constexpr int B = 2 PLUS 3;  // hover over `PLUS` does not show `Value`
```

I don't think this is a particularly important use case (it's a bit of a hack 
to get the expression's value this way, much better would be to select the 
entire expression you want to evaluate in the editor, but unfortunately LSP 
only sends a single position in `textDocument/hover`, not a full range...), but 
perhaps we could consider relaxing this in the future. (I'm thinking, if we 
switched to "allow partial selection via macros that expand to a single token", 
we could probably drop this condition.)



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:3832
+  };
+  Alig$3^nOf(Y);
+)cpp",

I guess with this style of test you can simplify the `$3^` to `^`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148457

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