[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[wanglei via Phabricator via cfe-commits]

wangleiat marked 2 inline comments as done.
wangleiat added a comment.

Thanks. @SixWeining


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[wanglei via Phabricator via cfe-commits]

wangleiat updated this revision to Diff 466349.
wangleiat added a comment.

Address @SixWeining's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

Files:
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/atomics.c


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: clang/lib/Basic/Targets/LoongArch.h
===
--- clang/lib/Basic/Targets/LoongArch.h
+++ clang/lib/Basic/Targets/LoongArch.h
@@ -79,6 +79,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -100,6 +103,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  }
 };
 } // end namespace targets
 } // end namespace clang


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, 

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Lu Weining via Phabricator via cfe-commits]

SixWeining accepted this revision.
SixWeining added inline comments.



Comment at: clang/test/CodeGen/LoongArch/atomics.c:2
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \

Nit: --



Comment at: clang/test/CodeGen/LoongArch/atomics.c:4
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA64
+

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Since this option doesn't work for empty functions yet, we should either add a 
note in the documentation or qualify the scope with "non-empty" as suggested in 
my comments.




Comment at: clang/docs/ReleaseNotes.rst:528
 
+- Add `RemoveSemicolon` option for removing unnecessary `;` after a function 
definition.
 

IMO "unnecessary" is unnecessary. Add "non-empty" if we want to be exact.



Comment at: clang/include/clang/Format/Format.h:3058
 
+  /// Remove redundant semicolons after the closing brace of a function
+  /// \warning

Now I feel it's probably better to not add "redundant".



Comment at: clang/include/clang/Format/Format.h:3068
+  ///   int max(int a, int b)  int max(int a, int b)
+  ///   {  {
+  /// return a > b ? a : b;return a > b ? a : 
b;

Nit: unwrap the braces.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:992-997
+  // When this is a function block and there is an unnecessary semicolon
+  // afterwards then mark it as optional (so the RemoveSemi pass can get rid of
+  // it later).
+  if (MunchSemi && FormatTok->is(tok::semi)) {
 nextToken();
+  }

Move the comments to line 965 and elide the braces.



Comment at: clang/unittests/Format/FormatTest.cpp:26773
+   Style);
+
+  // These tests are here to show a problem that may not be easily

Add the test case if it will pass.



Comment at: clang/unittests/Format/FormatTest.cpp:26780-26786
+  verifyFormat("void main(){};", "void main() {};", Style);
+  verifyFormat("void foo(){}; //\n"
+   ";\n"
+   "int bar;",
+   "void foo(){}; //\n"
+   "; int bar;",
+   Style);

And edit the comments above.


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

https://reviews.llvm.org/D135466

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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[wanglei via Phabricator via cfe-commits]

wangleiat created this revision.
wangleiat added reviewers: SixWeining, xen0n, xry111, gonglingqin, MaskRay.
Herald added a subscriber: StephenFan.
Herald added a project: All.
wangleiat 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/D135526

Files:
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/atomics.c


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: clang/lib/Basic/Targets/LoongArch.h
===
--- clang/lib/Basic/Targets/LoongArch.h
+++ clang/lib/Basic/Targets/LoongArch.h
@@ -79,6 +79,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -100,6 +103,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  }
 };
 } // end namespace targets
 } // end namespace clang


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // 

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3844605 , @ChuanqiXu wrote:

> In D134267#3815453 , @dblaikie 
> wrote:
>
>>> This also tries to fix the problem I raised a year ago: 
>>> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144
>>
>> I think this thread is fairly different from what this patch is proposing.
>>
>> @rsmith for discussion of adding effectively implicit modules support for 
>> C++20 modules - this seems like a pretty significant design choice, but 
>> something we (you and I, SG15, etc) have discussed before in terms of "how 
>> do we support 'Hello, World.'" situations without requiring something that 
>> amounts to a build system even for the simplest C++20 modules programs. 
>> Maybe this is it? But it surprises me a bit, and I don't think the linked 
>> thread is sufficiently relevant to draw conclusions about this direction.
>
> I don't understand your point a lot. From my point, the intention of the 
> thread 
> (https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144)
>  are asking why the command line interfaces of clang are clearly more complex 
> than gcc. And @rsmith replies that:
>
>> Currently, Clang lacks two important features here:
>>
>> 1. Produce a .pcm file and a .o file from a single compilation action.
>
> in 
> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144/12.
>  And I think this is what this patch tries to do: (generate .pcm file and .o 
> file in one single compilation action).
>
> In fact, this doesn't change the behavior a lot. Previously, when we tried to 
> compile a `*.cppm` directly to `*.o`, the compiler will generate `*.pcm` 
> automatically too! Although the `*.pcm` file is only generated in `/tmp/` 
> files by default. And what this patch did is only to change the default 
> location for `*.pcm` files from `/tmp` to `module cache path`. And it 
> wouldn't affect the original 2 phase compilation model.



In D134267#3845177 , @dblaikie wrote:

> In D134267#3844605 , @ChuanqiXu 
> wrote:
>
>> In D134267#3815453 , @dblaikie 
>> wrote:
>>
 This also tries to fix the problem I raised a year ago: 
 https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144
>>>
>>> I think this thread is fairly different from what this patch is proposing.
>>>
>>> @rsmith for discussion of adding effectively implicit modules support for 
>>> C++20 modules - this seems like a pretty significant design choice, but 
>>> something we (you and I, SG15, etc) have discussed before in terms of "how 
>>> do we support 'Hello, World.'" situations without requiring something that 
>>> amounts to a build system even for the simplest C++20 modules programs. 
>>> Maybe this is it? But it surprises me a bit, and I don't think the linked 
>>> thread is sufficiently relevant to draw conclusions about this direction.
>>
>> I don't understand your point a lot. From my point, the intention of the 
>> thread 
>> (https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144)
>>  are asking why the command line interfaces of clang are clearly more 
>> complex than gcc. And @rsmith replies that:
>>
>>> Currently, Clang lacks two important features here:
>>>
>>> 1. Produce a .pcm file and a .o file from a single compilation action.
>>
>> in 
>> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144/12.
>>  And I think this is what this patch tries to do: (generate .pcm file and .o 
>> file in one single compilation action).
>>
>> In fact, this doesn't change the behavior a lot. Previously, when we tried 
>> to compile a `*.cppm` directly to `*.o`, the compiler will generate `*.pcm` 
>> automatically too! Although the `*.pcm` file is only generated in `/tmp/` 
>> files by default. And what this patch did is only to change the default 
>> location for `*.pcm` files from `/tmp` to `module cache path`. And it 
>> wouldn't affect the original 2 phase compilation model.
>>
>> BTW, this can simplify the use of modules significantly, see 
>> https://reviews.llvm.org/D135507#change-gpsoCkM1g61J for example.
>
> The use of a module cache path I think is a pretty major difference between 
> what was discussed on the discourse thread and what's being proposed here - a 
> module "cache" is what starts to touch near Clang's old implicit modules that 
> has real problems in terms of parallelism, etc. (granted what you're 
> proposing here doesn't go all the way towards that - it doesn't have implicit 
> actions to generate the pcm, at least)
>
> Is this what GCC Supports? I'd expect something more like Split DWARF, where 
> the .pcm (or .dwo file) is placed next to the .o file 

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D134267#3844605 , @ChuanqiXu wrote:

> In D134267#3815453 , @dblaikie 
> wrote:
>
>>> This also tries to fix the problem I raised a year ago: 
>>> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144
>>
>> I think this thread is fairly different from what this patch is proposing.
>>
>> @rsmith for discussion of adding effectively implicit modules support for 
>> C++20 modules - this seems like a pretty significant design choice, but 
>> something we (you and I, SG15, etc) have discussed before in terms of "how 
>> do we support 'Hello, World.'" situations without requiring something that 
>> amounts to a build system even for the simplest C++20 modules programs. 
>> Maybe this is it? But it surprises me a bit, and I don't think the linked 
>> thread is sufficiently relevant to draw conclusions about this direction.
>
> I don't understand your point a lot. From my point, the intention of the 
> thread 
> (https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144)
>  are asking why the command line interfaces of clang are clearly more complex 
> than gcc. And @rsmith replies that:
>
>> Currently, Clang lacks two important features here:
>>
>> 1. Produce a .pcm file and a .o file from a single compilation action.
>
> in 
> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144/12.
>  And I think this is what this patch tries to do: (generate .pcm file and .o 
> file in one single compilation action).
>
> In fact, this doesn't change the behavior a lot. Previously, when we tried to 
> compile a `*.cppm` directly to `*.o`, the compiler will generate `*.pcm` 
> automatically too! Although the `*.pcm` file is only generated in `/tmp/` 
> files by default. And what this patch did is only to change the default 
> location for `*.pcm` files from `/tmp` to `module cache path`. And it 
> wouldn't affect the original 2 phase compilation model.
>
> BTW, this can simplify the use of modules significantly, see 
> https://reviews.llvm.org/D135507#change-gpsoCkM1g61J for example.

The use of a module cache path I think is a pretty major difference between 
what was discussed on the discourse thread and what's being proposed here - a 
module "cache" is what starts to touch near Clang's old implicit modules that 
has real problems in terms of parallelism, etc. (granted what you're proposing 
here doesn't go all the way towards that - it doesn't have implicit actions to 
generate the pcm, at least)

Is this what GCC Supports? I'd expect something more like Split DWARF, where 
the .pcm (or .dwo file) is placed next to the .o file - not in some separate 
directory (what sort of naming scheme would be used to ensure that paths in 
that directory are unique?).


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

https://reviews.llvm.org/D134267

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

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/include/clang/AST/Type.h:1836
+// The index of the template parameter this substitution represents.
+unsigned Index : 15;
+

erichkeane wrote:
> Is it a problem for this to be a different number of bits used to represent 
> the number of template parameters?  I would expect we would want to make them 
> all them the same size (else we have an additional limit on the size of 
> parameters).
Per documentation reasons, it would certainly be simpler if they were the same 
size.

But they are not the same thing. Index is for indexing into arguments bound to 
non-pack parameters,
while PackIndex is for indexing arguments within a pack.

The only way Index could reach such huge values, would be if there were a 
corresponding huge amount of template parameters. For a pack that is not the 
case.

So packs are much more efficient for handling large amounts of arguments, so it 
makes sense to have a higher limit for them.

But how much that should be in practice is hard to tell.

I just can't see that 2^15 would not be enough for Index.

For PackIndex, 2^16 seems reasonable to me, but in the worst case someone 
complains, we can just store a larger PackIndex in a tail allocated field, 
while we keep storing small ones in the TypeBitfields.



Comment at: clang/include/clang/AST/Type.h:1838
 /// metaprogramming we'd prefer to keep it as large as possible.
-/// At the moment it has been left as a non-bitfield since this type
-/// safely fits in 64 bits as an unsigned, so there is no reason to
-/// introduce the performance impact of a bitfield.
-unsigned NumArgs;
+unsigned NumArgs : 16;
   };

erichkeane wrote:
> mizvekov wrote:
> > davrec wrote:
> > > I can't imagine that limiting template arg index to 16 bits from 32 could 
> > > be all that limiting, but given the comment in the original have you 
> > > tested/confirmed that this is acceptable?
> > Not yet, we will begin performance testing soon. But I am not concerned 
> > about this one, as it's easy to get around this limitation by not using the 
> > bitfields.
> Did the testing here result in finding anyone who used this?  I'm sure libcxx 
> or some of the boost libraries do a lot of MP on large sizes (though I note 
> libcxx seems to have passed pre-commit).
Not on anything LLVM pre-commit as you saw.

I am not sure how much we should be concerned about here.
This is still much larger than implimits. If this hits anyone, we will just 
need to add a test case for a reasonable limit and extend this node to hold 
large values in tail-allocated extra fields.



Comment at: clang/include/clang/Sema/Template.h:80
+struct ArgumentListLevel {
+  Decl *AssociatedDecl;
+  ArgList Args;

davrec wrote:
> mizvekov wrote:
> > davrec wrote:
> > > Actually I think this one should be changed back to `ReplacedDecl` :)
> > > ReplacedDecl makes perfect sense in MLTAL, AssociatedDecl seems to make 
> > > better sense in STTPT.
> > I would be against introducing another term to refer to the same thing...
> The reason we need this unfortunately vague term "AssociatedDecl" in STTPT is 
> because it can either be a template/template-like declaration *or* a 
> TemplateTypeParmDecl.  But here in MLTAL, it won't be a TTPD, will it?  It 
> will always be the parent template/template-like declaration, right?  So 
> there is no need for vagueness.  `ReplacedDecl` or `ParentTemplate` or 
> something like that seems more appropriate.  
No, it can be the TTPD which is used to represent the invented template for a 
requires substitution.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626
+  VisitDeclaratorDecl(D);
+  Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName());
+  Record.push_back(D->getIdentifierNamespace());
+

ChuanqiXu wrote:
> ChuanqiXu wrote:
> > mizvekov wrote:
> > > ChuanqiXu wrote:
> > > > mizvekov wrote:
> > > > > ChuanqiXu wrote:
> > > > > > mizvekov wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > mizvekov wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > I still don't get the reason for the move. What's the 
> > > > > > > > > > > > benefit? Or why is it necessary?
> > > > > > > > > > > Yeah, now the type can reference the template decl, so 
> > > > > > > > > > > without moving this, it can happen during import of the 
> > > > > > > > > > > type that we try to read this function template bits 
> > > > > > > > > > > without having imported them yet.
> > > > > > > > > > Oh, I guess I met the problem before (D129748 ) and I made 
> > > > > > > > > > a workaround for it (https://reviews.llvm.org/D130331). If 
> > > > > > > > > > I understood right, the patch will solve that problem. I'll 
> > > > > > > > > > check it out later.
> > > > > > > > 

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 466318.
cjdb retitled this revision from "[clang] adds 
`__reference_constructs_from_temporary` and 
`__reference_converts_from_temporary`" to "[clang] adds 
`__reference_constructs_from_temporary`".
cjdb added a comment.

discards work on `__reference_converts_from_temporary` for now. This feature 
isn't as trivial to implement as `__reference_constructs_from_temporary`, so 
it's deserving of its own commit. The two features are used exclusively, so 
it's not like adding one without the other will lead to an incomplete standard 
type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1419,10 +1419,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2535,6 +2535,55 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
+void reference_converts_from_temporary() {
+  (void)__reference_converts_from_temporary(int, int); // expected-error{{'__reference_converts_from_temporary' is a compiler-reserved identifier for a future feature}}
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -29,6 +29,7 

[PATCH] D135339: [clang] makes `__is_destructible` KEYALL instead of KEYMS

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 466316.
cjdb retitled this revision from "[clang] makes `__is_destructible` KEYCXX 
instead of KEYMS" to "[clang] makes `__is_destructible` KEYALL instead of 
KEYMS".
cjdb edited the summary of this revision.
cjdb added a comment.

applies feedback, rebases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135339

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/test/SemaCXX/type-traits-ms-extensions.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
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins %s
 
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
@@ -414,23 +414,12 @@
 template<>
 struct PotentiallyFinal final { };
 
-struct SealedClass sealed {
-};
 
-template
-struct PotentiallySealed { };
 
-template
-struct PotentiallySealed sealed { };
 
-template<>
-struct PotentiallySealed sealed { };
 
 void is_final()
 {
-	{ int arr[T(__is_final(SealedClass))]; }
-	{ int arr[T(__is_final(PotentiallySealed))]; }
-	{ int arr[T(__is_final(PotentiallySealed))]; }
 	{ int arr[T(__is_final(FinalClass))]; }
 	{ int arr[T(__is_final(PotentiallyFinal))]; }
 	{ int arr[T(__is_final(PotentiallyFinal))]; }
@@ -445,32 +434,8 @@
 	{ int arr[F(__is_final(cvoid))]; }
 	{ int arr[F(__is_final(IntArNB))]; }
 	{ int arr[F(__is_final(HasAnonymousUnion))]; }
-	{ int arr[F(__is_final(PotentiallyFinal))]; }
-	{ int arr[F(__is_final(PotentiallySealed))]; }
 }
 
-void is_sealed()
-{
-	{ int arr[T(__is_sealed(SealedClass))]; }
-	{ int arr[T(__is_sealed(PotentiallySealed))]; }
-	{ int arr[T(__is_sealed(PotentiallySealed))]; }
-	{ int arr[T(__is_sealed(FinalClass))]; }
-	{ int arr[T(__is_sealed(PotentiallyFinal))]; }
-	{ int arr[T(__is_sealed(PotentiallyFinal))]; }
-
-	{ int arr[F(__is_sealed(int))]; }
-	{ int arr[F(__is_sealed(Union))]; }
-	{ int arr[F(__is_sealed(Int))]; }
-	{ int arr[F(__is_sealed(IntAr))]; }
-	{ int arr[F(__is_sealed(UnionAr))]; }
-	{ int arr[F(__is_sealed(Derives))]; }
-	{ int arr[F(__is_sealed(ClassType))]; }
-	{ int arr[F(__is_sealed(cvoid))]; }
-	{ int arr[F(__is_sealed(IntArNB))]; }
-	{ int arr[F(__is_sealed(HasAnonymousUnion))]; }
-	{ int arr[F(__is_sealed(PotentiallyFinal))]; }
-	{ int arr[F(__is_sealed(PotentiallySealed))]; }
-}
 
 typedef HasVirt Polymorph;
 struct InheritPolymorph : Polymorph {};
Index: clang/test/SemaCXX/type-traits-ms-extensions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-traits-ms-extensions.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s -Wno-c++17-extensions
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s -Wno-c++17-extensions
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -x c -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu11 -fblocks -Wno-deprecated-builtins -fms-extensions -Wno-microsoft %s
+
+#ifdef __cplusplus
+
+// expected-no-diagnostics
+
+using Int = int;
+
+struct NonPOD { NonPOD(int); };
+enum Enum { EV };
+struct POD { Enum e; int i; float f; NonPOD* p; };
+struct Derives : POD {};
+using ClassType = Derives;
+
+union Union { int i; float f; };
+
+struct HasAnonymousUnion {
+  union {
+int i;
+float f;
+  };
+};
+
+struct FinalClass final {
+};
+
+template
+struct PotentiallyFinal { };
+
+template
+struct PotentiallyFinal final { };
+
+template<>
+struct PotentiallyFinal final { };
+
+struct SealedClass sealed {
+};
+
+template
+struct PotentiallySealed { };
+
+template
+struct PotentiallySealed sealed { };
+
+template<>
+struct PotentiallySealed sealed { };
+
+void is_final() {
+  

[PATCH] D135177: [clang] adds `__is_scoped_enum`, `__is_nullptr`, and `__is_referenceable`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 466315.
cjdb marked 5 inline comments as done.
cjdb added a comment.

applies feedback, rebases, adds documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135177

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.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
@@ -345,11 +345,19 @@
 }
 
 typedef Enum EnumType;
+typedef EnumClass EnumClassType;
 
 void is_enum()
 {
   { int arr[T(__is_enum(Enum))]; }
   { int arr[T(__is_enum(EnumType))]; }
+  { int arr[T(__is_enum(SignedEnum))]; }
+  { int arr[T(__is_enum(UnsignedEnum))]; }
+
+  { int arr[T(__is_enum(EnumClass))]; }
+  { int arr[T(__is_enum(EnumClassType))]; }
+  { int arr[T(__is_enum(SignedEnumClass))]; }
+  { int arr[T(__is_enum(UnsignedEnumClass))]; }
 
   { int arr[F(__is_enum(int))]; }
   { int arr[F(__is_enum(Union))]; }
@@ -361,6 +369,37 @@
   { int arr[F(__is_enum(cvoid))]; }
   { int arr[F(__is_enum(IntArNB))]; }
   { int arr[F(__is_enum(HasAnonymousUnion))]; }
+  { int arr[F(__is_enum(AnIncompleteType))]; }
+  { int arr[F(__is_enum(AnIncompleteTypeAr))]; }
+  { int arr[F(__is_enum(AnIncompleteTypeArMB))]; }
+  { int arr[F(__is_enum(AnIncompleteTypeArNB))]; }
+}
+
+void is_scoped_enum() {
+  static_assert(!__is_scoped_enum(Enum), "");
+  static_assert(!__is_scoped_enum(EnumType), "");
+  static_assert(!__is_scoped_enum(SignedEnum), "");
+  static_assert(!__is_scoped_enum(UnsignedEnum), "");
+
+  static_assert(__is_scoped_enum(EnumClass), "");
+  static_assert(__is_scoped_enum(EnumClassType), "");
+  static_assert(__is_scoped_enum(SignedEnumClass), "");
+  static_assert(__is_scoped_enum(UnsignedEnumClass), "");
+
+  static_assert(!__is_scoped_enum(int), "");
+  static_assert(!__is_scoped_enum(Union), "");
+  static_assert(!__is_scoped_enum(Int), "");
+  static_assert(!__is_scoped_enum(IntAr), "");
+  static_assert(!__is_scoped_enum(UnionAr), "");
+  static_assert(!__is_scoped_enum(Derives), "");
+  static_assert(!__is_scoped_enum(ClassType), "");
+  static_assert(!__is_scoped_enum(cvoid), "");
+  static_assert(!__is_scoped_enum(IntArNB), "");
+  static_assert(!__is_scoped_enum(HasAnonymousUnion), "");
+  static_assert(!__is_scoped_enum(AnIncompleteType), "");
+  static_assert(!__is_scoped_enum(AnIncompleteTypeAr), "");
+  static_assert(!__is_scoped_enum(AnIncompleteTypeArMB), "");
+  static_assert(!__is_scoped_enum(AnIncompleteTypeArNB), "");
 }
 
 struct FinalClass final {
@@ -766,6 +805,36 @@
   (void)__is_unbounded_array(decltype(t32)); // expected-error{{variable length arrays are not supported for '__is_unbounded_array'}}
 }
 
+void is_referenceable() {
+  static_assert(__is_referenceable(int), "");
+  static_assert(__is_referenceable(const int), "");
+  static_assert(__is_referenceable(volatile int), "");
+  static_assert(__is_referenceable(const volatile int), "");
+  static_assert(__is_referenceable(int *), "");
+  static_assert(__is_referenceable(int &), "");
+  static_assert(__is_referenceable(int &&), "");
+  static_assert(__is_referenceable(int (*)()), "");
+  static_assert(__is_referenceable(int (&)()), "");
+  static_assert(__is_referenceable(int(&&)()), "");
+  static_assert(__is_referenceable(IntAr), "");
+  static_assert(__is_referenceable(IntArNB), "");
+  static_assert(__is_referenceable(decltype(nullptr)), "");
+  static_assert(__is_referenceable(Empty), "");
+  static_assert(__is_referenceable(Union), "");
+  static_assert(__is_referenceable(Derives), "");
+  static_assert(__is_referenceable(Enum), "");
+  static_assert(__is_referenceable(EnumClass), "");
+  static_assert(__is_referenceable(int Empty::*), "");
+  static_assert(__is_referenceable(int(Empty::*)()), "");
+  static_assert(__is_referenceable(AnIncompleteType), "");
+  static_assert(__is_referenceable(struct AnIncompleteType), "");
+
+  using function_type = void(int);
+  static_assert(__is_referenceable(function_type), "");
+
+  static_assert(!__is_referenceable(void), "");
+}
+
 template  void tmpl_func(T&) {}
 
 template  struct type_wrapper {
@@ -998,6 +1067,42 @@
   int t34[F(__is_pointer(void (StructWithMembers::*) ()))];
 }
 
+void is_null_pointer() {
+  StructWithMembers x;
+
+  static_assert(__is_nullptr(decltype(nullptr)), "");
+  static_assert(!__is_nullptr(void *), "");
+  static_assert(!__is_nullptr(cvoid *), "");
+  static_assert(!__is_nullptr(cvoid *), "");
+  static_assert(!__is_nullptr(char *), "");
+  static_assert(!__is_nullptr(int *), "");
+  static_assert(!__is_nullptr(int **), "");
+  static_assert(!__is_nullptr(ClassType *), "");
+  static_assert(!__is_nullptr(Derives *), "");
+  

[PATCH] D135175: [clang] adds `__is_bounded_array` and `__is_unbounded_array` as builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 466314.
cjdb marked 4 inline comments as done.
cjdb added a comment.

applies requested changes, adds documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135175

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.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
@@ -702,6 +702,70 @@
   int t31[F(__is_array(cvoid*))];
 }
 
+void is_bounded_array(int n) {
+  static_assert(__is_bounded_array(IntAr), "");
+  static_assert(!__is_bounded_array(IntArNB), "");
+  static_assert(__is_bounded_array(UnionAr), "");
+
+  static_assert(!__is_bounded_array(void), "");
+  static_assert(!__is_bounded_array(cvoid), "");
+  static_assert(!__is_bounded_array(float), "");
+  static_assert(!__is_bounded_array(double), "");
+  static_assert(!__is_bounded_array(long double), "");
+  static_assert(!__is_bounded_array(bool), "");
+  static_assert(!__is_bounded_array(char), "");
+  static_assert(!__is_bounded_array(signed char), "");
+  static_assert(!__is_bounded_array(unsigned char), "");
+  static_assert(!__is_bounded_array(wchar_t), "");
+  static_assert(!__is_bounded_array(short), "");
+  static_assert(!__is_bounded_array(unsigned short), "");
+  static_assert(!__is_bounded_array(int), "");
+  static_assert(!__is_bounded_array(unsigned int), "");
+  static_assert(!__is_bounded_array(long), "");
+  static_assert(!__is_bounded_array(unsigned long), "");
+  static_assert(!__is_bounded_array(Union), "");
+  static_assert(!__is_bounded_array(Derives), "");
+  static_assert(!__is_bounded_array(ClassType), "");
+  static_assert(!__is_bounded_array(Enum), "");
+  static_assert(!__is_bounded_array(void *), "");
+  static_assert(!__is_bounded_array(cvoid *), "");
+
+  int t32[n];
+  (void)__is_bounded_array(decltype(t32)); // expected-error{{variable length arrays are not supported for '__is_bounded_array'}}
+}
+
+void is_unbounded_array(int n) {
+  static_assert(!__is_unbounded_array(IntAr), "");
+  static_assert(__is_unbounded_array(IntArNB), "");
+  static_assert(!__is_unbounded_array(UnionAr), "");
+
+  static_assert(!__is_unbounded_array(void), "");
+  static_assert(!__is_unbounded_array(cvoid), "");
+  static_assert(!__is_unbounded_array(float), "");
+  static_assert(!__is_unbounded_array(double), "");
+  static_assert(!__is_unbounded_array(long double), "");
+  static_assert(!__is_unbounded_array(bool), "");
+  static_assert(!__is_unbounded_array(char), "");
+  static_assert(!__is_unbounded_array(signed char), "");
+  static_assert(!__is_unbounded_array(unsigned char), "");
+  static_assert(!__is_unbounded_array(wchar_t), "");
+  static_assert(!__is_unbounded_array(short), "");
+  static_assert(!__is_unbounded_array(unsigned short), "");
+  static_assert(!__is_unbounded_array(int), "");
+  static_assert(!__is_unbounded_array(unsigned int), "");
+  static_assert(!__is_unbounded_array(long), "");
+  static_assert(!__is_unbounded_array(unsigned long), "");
+  static_assert(!__is_unbounded_array(Union), "");
+  static_assert(!__is_unbounded_array(Derives), "");
+  static_assert(!__is_unbounded_array(ClassType), "");
+  static_assert(!__is_unbounded_array(Enum), "");
+  static_assert(!__is_unbounded_array(void *), "");
+  static_assert(!__is_unbounded_array(cvoid *), "");
+
+  int t32[n];
+  (void)__is_unbounded_array(decltype(t32)); // expected-error{{variable length arrays are not supported for '__is_unbounded_array'}}
+}
+
 template  void tmpl_func(T&) {}
 
 template  struct type_wrapper {
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2627,12 +2627,10 @@
 
   if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
 // CUDA device code and some other targets don't support VLAs.
-targetDiag(Loc, (getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
-? diag::err_cuda_vla
-: diag::err_vla_unsupported)
-<< ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
-? CurrentCUDATarget()
-: CFT_InvalidTarget);
+bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice);
+targetDiag(Loc,
+   IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported)
+<< (IsCUDADevice ? CurrentCUDATarget() : 0);
   }
 
   // If this is not C99, diagnose array size modifiers on non-VLAs.
Index: clang/lib/Sema/SemaExprCXX.cpp

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

After 4 weeks' study, I think the comment didn't need to be changed, sorry to 
have bring your so much trouble.
During this valuable process of studying, I grow up a lot. I learned to read 
the C++ standard, and compare the standard to its implementation.
In my case, the "user-defined conversion" is the variable "Candidate", the 
"second standard conversion sequence" is the object member  
"Candidate.FinalConversion".
The only pity during my study is that I can't find a example code to let Clang 
(even with commit cba72b1f620fd) hit the code below above comment.

I am going to close this thread.

Thanks for reviewing my patch, and for your guidance.
Cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked an inline comment as done.
AntonBikineev added a comment.

Thanks everybody!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b85e765000d: [PGO] Consider parent context when weighing 
branches with likelyhood. (authored by AntonBikineev).

Changed prior to commit:
  https://reviews.llvm.org/D134456?vs=462250=466313#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
  clang/test/Profile/cxx-never-executed-branch.cpp


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o 
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - 
-emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | 
FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,20 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, differentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] attribute if the branch is never
+  // executed,
+  // - assuming the profile is poor, preserving the attribute may still be
+  // beneficial.
+  // As an approximation, preserve the attribute only if both the branch and 
the
+  // parent context were not executed.
   Stmt::Likelihood LH = Stmt::LH_None;
-  uint64_t Count = getProfileCount(S.getThen());
-  if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+  uint64_t ThenCount = getProfileCount(S.getThen());
+  if (!ThenCount && !getCurrentProfileCount() &&
+  CGM.getCodeGenOpts().OptimizationLevel)
 LH = Stmt::getLikelihood(S.getThen(), S.getElse());
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,20 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the 

[clang] 7b85e76 - [PGO] Consider parent context when weighing branches with likelyhood.

[Anton Bikineev via cfe-commits]

Author: Anton Bikineev
Date: 2022-10-08T23:49:27+02:00
New Revision: 7b85e765000df36fcc6a5191dec9a28f444245ba

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

LOG: [PGO] Consider parent context when weighing branches with likelyhood.

Generally, with PGO enabled the C++20 likelyhood attributes shall be
dropped assuming the profile has a good coverage. However, currently
this is not the case for the following code:

 if (always_false()) [[likely]] {
   ...
 }

The patch fixes this and drops the attribute, if the parent context was
executed in the profile. The patch still preserves the attribute, if the
parent context was not executed, e.g. to support the cases when the
profile has insufficient coverage.

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

Added: 
clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
clang/test/Profile/cxx-never-executed-branch.cpp

Modified: 
clang/lib/CodeGen/CGStmt.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 9935fcc0d3ea2..ebbc79cc8b4b6 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,20 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, 
diff erentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] attribute if the branch is never
+  // executed,
+  // - assuming the profile is poor, preserving the attribute may still be
+  // beneficial.
+  // As an approximation, preserve the attribute only if both the branch and 
the
+  // parent context were not executed.
   Stmt::Likelihood LH = Stmt::LH_None;
-  uint64_t Count = getProfileCount(S.getThen());
-  if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+  uint64_t ThenCount = getProfileCount(S.getThen());
+  if (!ThenCount && !getCurrentProfileCount() &&
+  CGM.getCodeGenOpts().OptimizationLevel)
 LH = Stmt::getLikelihood(S.getThen(), S.getElse());
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);

diff  --git a/clang/test/Profile/Inputs/cxx-never-executed-branch.proftext 
b/clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
new file mode 100644
index 0..64a55b3e52b37
--- /dev/null
+++ b/clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0

diff  --git a/clang/test/Profile/cxx-never-executed-branch.cpp 
b/clang/test/Profile/cxx-never-executed-branch.cpp
new file mode 100644
index 0..812f65f3996d3
--- /dev/null
+++ b/clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o 
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - 
-emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | 
FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}



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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

I've found a strange scenario.

The following conversions are allowed

  double *a[2][3];
  double const * const (*ap)[3] = a; // OK
  double * const (*ap1)[] = a;   // OK since C++20

However if the same conversion is supposed to be performed in a `catch()` 
statement, it's not happening and the thrown object is not caught.
See it on godbolt .

Quoteing cppreference :

  When an exception is thrown by any statement in compound-statement, the 
exception object of type E is matched against the types of the formal 
parameters T of each catch-clause in handler-seq, in the order in which the 
catch clauses are listed. The exception is a match if any of the following is 
true:
  
  ...
  - T is (possibly cv-qualified) U or const U& (since C++14), and U is a 
pointer or pointer to member type, and E is also a pointer or pointer to member 
type that is implicitly convertible to U by one or more of
- a standard pointer conversion other than one to a private, protected, or 
ambiguous base class
- **a qualification conversion**
- a function pointer conversion (since C++17)
  ...

Checking the quality conversion related part of cppreference 

 lists the examples I quoted before as performable conversions.


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

https://reviews.llvm.org/D135495

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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs updated this revision to Diff 466312.
isuckatcs marked 2 inline comments as done.
isuckatcs added a comment.

Addressed comments and added support for multi-level pointers too.


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

https://reviews.llvm.org/D135495

Files:
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.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
@@ -101,6 +101,126 @@
   }
 }
 
+void throw_catch_pointer_c() noexcept {
+  try {
+int a = 1;
+throw 
+  } catch(const int *) {}
+}
+
+void throw_catch_pointer_v() noexcept {
+  try {
+int a = 1;
+throw 
+  } catch(volatile int *) {}
+}
+
+void throw_catch_pointer_cv() noexcept {
+  try {
+int a = 1;
+throw 
+  } catch(const volatile int *) {}
+}
+
+void throw_catch_multi_ptr_1() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_multi_ptr_1' which should not throw exceptions
+  try {
+char **p = 0;
+throw p;
+  } catch (const char **) {
+  }
+}
+
+void throw_catch_multi_ptr_2() noexcept {
+  try {
+char **p = 0;
+throw p;
+  } catch (const char *const *) {
+  }
+}
+
+void throw_catch_multi_ptr_3() noexcept {
+  try {
+char **p = 0;
+throw p;
+  } catch (volatile char *const *) {
+  }
+}
+
+void throw_catch_multi_ptr_4() noexcept {
+  try {
+char **p = 0;
+throw p;
+  } catch (volatile const char *const *) {
+  }
+}
+
+// FIXME: In this case 'a' is convertible to the handler and should be caught
+// but in reality it's thrown. Note that clang doesn't report a warning for 
+// this either.
+void throw_catch_multi_ptr_5() noexcept {
+  try {
+double *a[2][3];
+throw a;
+  } catch (double *(*)[3]) {
+  }
+}
+
+
+void throw_c_catch_pointer() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer' which should not throw exceptions
+  try {
+int a = 1;
+const int *p = 
+throw p;
+  } catch(int *) {}
+}
+
+void throw_c_catch_pointer_v() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer_v' which should not throw exceptions
+  try {
+int a = 1;
+const int *p = 
+throw p;
+  } catch(volatile int *) {}
+}
+
+void throw_v_catch_pointer() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer' which should not throw exceptions
+  try {
+int a = 1;
+volatile int *p = 
+throw p;
+  } catch(int *) {}
+}
+
+void throw_v_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer_c' which should not throw exceptions
+  try {
+int a = 1;
+volatile int *p = 
+throw p;
+  } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_c' which should not throw exceptions
+  try {
+int a = 1;
+const volatile int *p = 
+throw p;
+  } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_v() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_v' which should not throw exceptions
+  try {
+int a = 1;
+const volatile int *p = 
+throw p;
+  } catch(volatile int *) {}
+}
+
 class base {};
 class derived: public base {};
 
@@ -112,6 +232,24 @@
   }
 }
 
+void throw_derived_catch_base_ptr_c() noexcept {
+  try {
+derived d;
+throw  
+  } catch(const base *) {
+  }
+}
+
+void throw_derived_catch_base_ptr() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ptr' which should not throw exceptions
+  try {
+derived d;
+const derived *p = 
+throw p; 
+  } catch(base *) {
+  }
+}
+
 void try_nested_try(int n) noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
   try {
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -56,11 +56,108 @@
   [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; });
 }
 
+// Checks if T1 is convertible to T2.
+static bool isMultiLevelConvertiblePointer(QualType P1, QualType P2,
+   unsigned 

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D134454#3844627 , @zixuan-wu wrote:

> It's fine for CSKY to use config file. I only have 2 points.

Thanks. It'll be nice for CSKY to move away from changing `computeSysRoot`.

> 1. I agree sysroot should be separated from GCC because sysroot is not 
> dependent to GCC and there is even not gcc when we use llvm runtime. This 
> rule should also apply to multilib logic. Sysroot can detect multilib path 
> individually.
> 2. From user view, it's not easy to add sysroot or config file path manually, 
> so it needs good way to enable this functionality.

There is a good summary on the recent configuration file improvements: 
https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/
The proper way is to let the system GCC or a similar package to provide a Clang 
configuration file.


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

https://reviews.llvm.org/D134454

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

[PATCH] D135519: [RISCV] Remove some vsetvli intrinsics under Zve32*.

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

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

Looks good to me. I think this should solve 
https://github.com/riscv/riscv-v-spec/issues/832.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135519

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

[PATCH] D131963: [libc++] Add custom clang-tidy checks

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D131963#3831786 , @smeenai wrote:

> In D131963#3811811 , @ldionne wrote:
>
>> I'd be fine with this as-is if it works in the CI.
>>
>> IIUC, the current blocker for this is that the `ClangConfig.cmake` installed 
>> by LLVM is not robust to the dev packages missing. If you do 
>> `find_package(Clang 16.0)`, it will error out if the dev packages are not 
>> present, since it will try to reference static libraries (and other 
>> artifacts) that don't exist and try to create IMPORTED targets for those. 
>> @smeenai @beanz do you know more about that, or do you know someone that 
>> does? Do you know who set up the CMake config files so that 
>> `find_package(Clang)` would work in the first place? I'd also welcome your 
>> input on the `ClangConfigVersion.cmake.in` changes.
>>
>> Just for the context, what we're trying to do here is simply use 
>> `clang-tidy`'s development packages from the libc++ build to add our own 
>> custom clang-tidy checks.
>>
>> Accepting because this LGTM, although we do have some stuff to resolve 
>> before this can be shipped.
>
> IIRC, the intended solution was to use `LLVM_DISTRIBUTION_COMPONENTS` 
> (https://llvm.org/docs/BuildingADistribution.html). When you use that option, 
> the generated CMake package files (at least in the install tree; I'm not sure 
> about the ones in the build tree) should only contain imported targets that 
> were part of your distribution. Multi-distribution support extends this even 
> further, where the file defining the imported targets for a distribution is 
> only imported if it's present, so things should work as expected both with 
> and without the dev packages. Is that workable for your use case?

That's a thing the vendor has to change, right? If we only get the targets 
which are actually available it should work just fine.




Comment at: libcxx/test/tools/clang_tidy_checks/robust_against_adl.cpp:22
+AST_MATCHER(clang::UnresolvedLookupExpr, isCustomizationPoint) {
+  // TODO: Are make_error_code and make_error_condition actually customization 
points?
+  return std::ranges::any_of(

jwakely wrote:
> jwakely wrote:
> > Mordante wrote:
> > > ldionne wrote:
> > > > This is funny, I actually saw a LWG issue about that go by a few weeks 
> > > > ago. I'll try to get more details.
> > > @ldionne @jwakely posted this bug report about it 
> > > https://github.com/llvm/llvm-project/issues/57614
> > They're definitely customization points. If they weren't, specializing 
> > `is_error_code_enum` and `is_error_condition_enum` would be completely 
> > useless and serve no purpose.
> > 
> > LWG confirmed that and [LWG 3629](https://wg21.link/lwg3629) is Tentatively 
> > Ready now.
> I created https://reviews.llvm.org/D134943 to fix those customization points.
Thanks for the info and patch @jwakely!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131963

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

[PATCH] D135519: [RISCV] Remove some vsetvli intrinsics under Zve32*.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: kito-cheng, eopXD, frasercrmck, rogfer01, reames.
Herald added subscribers: sunshaoce, VincentWu, StephenFan, vkmr, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, 
shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead.
Herald added a project: clang.

Zve32* does not support SEW=64. Or any LMUL smaller than 32/SEW.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135519

Files:
  clang/include/clang/Basic/riscv_vector.td


Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -1620,7 +1620,6 @@
 // and LMUL.
 let HeaderCode =
 [{
-#define vsetvl_e8mf8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 5)
 #define vsetvl_e8mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 6)
 #define vsetvl_e8mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 7)
 #define vsetvl_e8m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 0)
@@ -1628,25 +1627,28 @@
 #define vsetvl_e8m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 2)
 #define vsetvl_e8m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 3)
 
-#define vsetvl_e16mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 6)
 #define vsetvl_e16mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 7)
 #define vsetvl_e16m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 0)
 #define vsetvl_e16m2(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 1)
 #define vsetvl_e16m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 2)
 #define vsetvl_e16m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 3)
 
-#define vsetvl_e32mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 7)
 #define vsetvl_e32m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 0)
 #define vsetvl_e32m2(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 1)
 #define vsetvl_e32m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 2)
 #define vsetvl_e32m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 3)
 
+#if __riscv_v_elen >= 64
+#define vsetvl_e8mf8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 5)
+#define vsetvl_e16mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 6)
+#define vsetvl_e32mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 7)
+
 #define vsetvl_e64m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 0)
 #define vsetvl_e64m2(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 1)
 #define vsetvl_e64m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 2)
 #define vsetvl_e64m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 3)
+#endif
 
-#define vsetvlmax_e8mf8() __builtin_rvv_vsetvlimax(0, 5)
 #define vsetvlmax_e8mf4() __builtin_rvv_vsetvlimax(0, 6)
 #define vsetvlmax_e8mf2() __builtin_rvv_vsetvlimax(0, 7)
 #define vsetvlmax_e8m1() __builtin_rvv_vsetvlimax(0, 0)
@@ -1654,23 +1656,28 @@
 #define vsetvlmax_e8m4() __builtin_rvv_vsetvlimax(0, 2)
 #define vsetvlmax_e8m8() __builtin_rvv_vsetvlimax(0, 3)
 
-#define vsetvlmax_e16mf4() __builtin_rvv_vsetvlimax(1, 6)
 #define vsetvlmax_e16mf2() __builtin_rvv_vsetvlimax(1, 7)
 #define vsetvlmax_e16m1() __builtin_rvv_vsetvlimax(1, 0)
 #define vsetvlmax_e16m2() __builtin_rvv_vsetvlimax(1, 1)
 #define vsetvlmax_e16m4() __builtin_rvv_vsetvlimax(1, 2)
 #define vsetvlmax_e16m8() __builtin_rvv_vsetvlimax(1, 3)
 
-#define vsetvlmax_e32mf2() __builtin_rvv_vsetvlimax(2, 7)
 #define vsetvlmax_e32m1() __builtin_rvv_vsetvlimax(2, 0)
 #define vsetvlmax_e32m2() __builtin_rvv_vsetvlimax(2, 1)
 #define vsetvlmax_e32m4() __builtin_rvv_vsetvlimax(2, 2)
 #define vsetvlmax_e32m8() __builtin_rvv_vsetvlimax(2, 3)
 
+#if __riscv_v_elen >= 64
+#define vsetvlmax_e8mf8() __builtin_rvv_vsetvlimax(0, 5)
+#define vsetvlmax_e16mf4() __builtin_rvv_vsetvlimax(1, 6)
+#define vsetvlmax_e32mf2() __builtin_rvv_vsetvlimax(2, 7)
+
 #define vsetvlmax_e64m1() __builtin_rvv_vsetvlimax(3, 0)
 #define vsetvlmax_e64m2() __builtin_rvv_vsetvlimax(3, 1)
 #define vsetvlmax_e64m4() __builtin_rvv_vsetvlimax(3, 2)
 #define vsetvlmax_e64m8() __builtin_rvv_vsetvlimax(3, 3)
+#endif
+
 }] in
 def vsetvl_macro: RVVHeader;
 


Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -1620,7 +1620,6 @@
 // and LMUL.
 let HeaderCode =
 [{
-#define vsetvl_e8mf8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 5)
 #define vsetvl_e8mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 6)
 #define vsetvl_e8mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 7)
 #define vsetvl_e8m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 0)
@@ -1628,25 +1627,28 @@
 #define vsetvl_e8m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 2)
 #define vsetvl_e8m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 3)

[PATCH] D135518: [clangAST] support TypeLoc in TextNodeDumper

[Tobias Ribizel via Phabricator via cfe-commits]

upsj created this revision.
upsj added a reviewer: aaron.ballman.
Herald added a project: All.
upsj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

TextNodeDumper currently doesn't support printing the `TypeLoc` hierarchy, 
which makes navigating `clang-query` output involving `TypeLoc` hard to 
understand.
This diff replicates the `Type` hierarchy as well as possible, relying on 
`Type` visitors for printing type-specific info.
The only exception where this doesn't work is most sugared types and any kinds 
of parameters (function or template), for which the `TypeLoc` is not easily 
accessible.

Example `clang-query` output:

  clang-query> set output dump
  clang-query> match typeLoc(loc(type().bind("type")))
  ...
  Binding for "root":
  QualifiedTypeLoc  
'matrix::Dense::type> *const' const
  `-PointerTypeLoc  'matrix::Dense::type> *'
`-ElaboratedTypeLoc  'matrix::Dense::type>'
  `-TemplateSpecializationTypeLoc  'Dense::type>' Dense
  
  Binding for "type":
  PointerType 0x261f2b0 'matrix::Dense::type> *' 
dependent
  `-ElaboratedType 0x261f250 'matrix::Dense::type>' sugar dependent
`-TemplateSpecializationType 0x261f210 'Dense::type>' dependent Dense
  `-TemplateArgument type 'typename remove_complex_t::type'
`-DependentNameType 0x261f190 'typename remove_complex_t::type' 
dependent


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135518

Files:
  clang/include/clang/AST/ASTDumperUtils.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypeLoc.cpp

Index: clang/lib/AST/TypeLoc.cpp
===
--- clang/lib/AST/TypeLoc.cpp
+++ clang/lib/AST/TypeLoc.cpp
@@ -57,6 +57,22 @@
 
 namespace {
 
+class TypeNamer : public TypeLocVisitor {
+public:
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  const char *Visit##CLASS##TypeLoc(CLASS##TypeLoc TyLoc) { return #CLASS; }
+#include "clang/AST/TypeLocNodes.def"
+};
+
+} // namespace
+
+const char *TypeLoc::getTypeLocClassName() const {
+  return TypeNamer().Visit(*this);
+}
+
+namespace {
+
 class TypeAligner : public TypeLocVisitor {
 public:
 #define ABSTRACT_TYPELOC(CLASS, PARENT)
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/Type.h"
+#include "clang/AST/TypeLocVisitor.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
@@ -239,6 +240,18 @@
   OS << " " << T.split().Quals.getAsString();
 }
 
+void TextNodeDumper::Visit(TypeLoc TL) {
+  {
+ColorScope Color(OS, ShowColors, TypeLocColor);
+OS << TL.getTypeLocClassName() << "TypeLoc";
+  }
+  dumpSourceRange(TL.getSourceRange());
+  OS << " ";
+  dumpBareType(TL.getType(), false);
+
+  TypeLocVisitor::Visit(TL);
+}
+
 void TextNodeDumper::Visit(const Decl *D) {
   if (!D) {
 ColorScope Color(OS, ShowColors, NullColor);
@@ -1622,6 +1635,118 @@
 OS << " expansions " << *N;
 }
 
+void TextNodeDumper::VisitQualifiedTypeLoc(QualifiedTypeLoc TL) {
+  OS << " " << TL.getType().split().Quals.getAsString();
+}
+
+void TextNodeDumper::VisitRValueReferenceTypeLoc(ReferenceTypeLoc TL) {
+  VisitReferenceType(dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitArrayTypeLoc(ArrayTypeLoc TL) {
+  VisitArrayType(dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitConstantArrayTypeLoc(ConstantArrayTypeLoc TL) {
+  VisitConstantArrayType(
+  dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitVariableArrayTypeLoc(VariableArrayTypeLoc TL) {
+  VisitVariableArrayType(
+  dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitDependentSizedArrayTypeLoc(
+DependentSizedArrayTypeLoc TL) {
+  VisitDependentSizedArrayType(
+  dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitDependentSizedExtVectorTypeLoc(
+DependentSizedExtVectorTypeLoc TL) {
+  VisitDependentSizedExtVectorType(
+  dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitVectorTypeLoc(VectorTypeLoc TL) {
+  VisitVectorType(dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitFunctionTypeLoc(FunctionTypeLoc TL) {
+  VisitFunctionType(dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitFunctionProtoTypeLoc(FunctionProtoTypeLoc TL) {
+  VisitFunctionProtoType(
+  dyn_cast(TL.getType().getTypePtr()));
+}
+
+void TextNodeDumper::VisitUnresolvedUsingTypeLoc(UnresolvedUsingTypeLoc TL) {
+  VisitUnresolvedUsingType(
+  

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

Should cxx_status.html be updated as well? It's listed there as a C++2b paper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134529

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

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:169
+// For renaming, we're only interested in foo's declaration, so drop the other 
one
+void filterUsingDecl(llvm::DenseSet& Decls) {
+  // There should never be more than one UsingDecl here, 

you might want to call this `filterRenameTargets` or something, with the idea 
that we may add restrictions other than UsingDecl in future



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:170
+void filterUsingDecl(llvm::DenseSet& Decls) {
+  // There should never be more than one UsingDecl here, 
+  // otherwise the rename would be ambiguos anyway.

nit: `llvm::erase_if(Decls, [](const NamedDecl *D) { ... })`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135489

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

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bb34cc462ff: [clangd] FindTarget: UsingEnumDecl is not an 
alias (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+report(USD->getIntroducer(), Flags | Rel::Alias);
   // Shadow decls are synthetic and not themselves interesting.
   // Record the underlying decl instead, if allowed.
   D = USD->getTargetDecl();


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+

[clang-tools-extra] 2bb34cc - [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-10-08T19:29:35+02:00
New Revision: 2bb34cc462ff857576a8145edc89ad2bdd1f4396

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

LOG: [clangd] FindTarget: UsingEnumDecl is not an alias

Unlike UsingDecl it doesn't name the UsingShadowDecls it emits, so it doesn't
make sense to consider them the same thing. Don't consider the UsingEnumDecl
a target when the UsingShadowDecl is referenced.

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 17be646c7d7c5..dea5cb7f50288 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@ struct TargetFinder {
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@ struct TargetFinder {
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+report(USD->getIntroducer(), Flags | Rel::Alias);
   // Shadow decls are synthetic and not themselves interesting.
   // Record the underlying decl instead, if allowed.
   D = USD->getTargetDecl();

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 6656ed9847233..fd25e8059e816 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@ TEST_F(TargetDeclTest, UsingDecl) {
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {



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

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D135506#3844919 , @tom-anders 
wrote:

> lgtm, thanks! With this change in place, we could probably ajdust 
> `pickDeclToUse` in Hover.cpp (introduced in https://reviews.llvm.org/D133664) 
> to also check for `UsingDecl` instead of `BaseUsingDecl`, right?

Yes, I think so!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3779
+  return a>b?a:b;return a>b?a:b;
+}; };
+

owenpan wrote:
> lahwaacz wrote:
> > There is an extra `;` in the `true` case ;-)
> > 
> If this file was generated from the Format.h header below, we have a bug in 
> the dump_format_style.py script.
I wonder if I didn't do a final regeneration, lets check when I update



Comment at: clang/include/clang/Format/Format.h:3076
+  /// \version 16
+  bool RemoveSemiColons;
+

owenpan wrote:
> HazardyKnusperkeks wrote:
> > The name is a bit hard, just from that it's not clear that this option is 
> > harmless (modulo bugs). Maybe something like the said warning 
> > `RemoveExtraSemiColons`?
> I'm ok with the short name, which makes the user to look it up in the 
> documentation. The alternative would be something like 
> `RemoveSemicolonAfterFunctionBody`.
I feel if we called it `RemoveSemicolonAfterFunctionBody` then adding extra use 
cases would then require a new option.. in the same way, `RemoveBracesLLVM` 
doesn't remove all Braces.. can we go with @owenpan suggestion of 
`RemoveSemicolon` I could go for

`RemoveExtraSemicolon`
`RemoveExtraSemicolons`

But I don't feel `Extra` adds anything extra, if you pardon the pune.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:843
+bool CanContainBracedList, TokenType NextLBracesType,
+bool IsFunctionBlock) {
   auto HandleVerilogBlockLabel = [this]() {

owenpan wrote:
> We don't need to add the parameter. See below.
Oh! thank goodness, I hated having to add that..



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:923
   }
 
   auto RemoveBraces = [=]() mutable {

owenpan wrote:
> So that we don't need to add a parameter to `parseBlock`.
I don't know why I didn't see this! much better approach



Comment at: clang/unittests/Format/FormatTest.cpp:26755
+   "class Foo {\n"
+   "  int getSomething() const { return something; };\n"
+   "};",

HazardyKnusperkeks wrote:
> What happens with 2 (or more) semicolons?
I'll add the test but with @owenpan suggestion it handles it


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

https://reviews.llvm.org/D135466

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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 466291.
MyDeveloperDay marked 16 inline comments as done.
MyDeveloperDay added a comment.

- Switch to the looping method and remove the need to change parseBlock (phew!) 
thanks @owenpan for the guidance
- Add tests for double `;;`
- Change the name to RemoveSemicolon


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

https://reviews.llvm.org/D135466

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20281,6 +20281,7 @@
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(ReflowComments);
   CHECK_PARSE_BOOL(RemoveBracesLLVM);
+  CHECK_PARSE_BOOL(RemoveSemicolon);
   CHECK_PARSE_BOOL(SortUsingDeclarations);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
@@ -26740,6 +26741,51 @@
"inline bool var = is_integral_v && is_signed_v;");
 }
 
+TEST_F(FormatTest, RemoveSemicolon) {
+  FormatStyle Style = getLLVMStyle();
+  Style.RemoveSemicolon = true;
+
+  verifyFormat("int max(int a, int b) { return a > b ? a : b; }",
+   "int max(int a, int b) { return a > b ? a : b; };", Style);
+
+  verifyFormat("int max(int a, int b) { return a > b ? a : b; }",
+   "int max(int a, int b) { return a > b ? a : b; };;", Style);
+
+  verifyFormat("class Foo {\n"
+   "  int getSomething() const { return something; }\n"
+   "};",
+   "class Foo {\n"
+   "  int getSomething() const { return something; };\n"
+   "};",
+   Style);
+
+  verifyFormat("class Foo {\n"
+   "  int getSomething() const { return something; }\n"
+   "};",
+   "class Foo {\n"
+   "  int getSomething() const { return something; };;\n"
+   "};",
+   Style);
+
+  verifyFormat("for (;;) {\n"
+   "}",
+   Style);
+
+  // These tests are here to show a problem that may not be easily
+  // solved, our implementation to remove semicolons is only as good
+  // as our FunctionLBrace detection and this fails for empty braces
+  // because we can't distringuish this from a bracelist
+  // when we fix that..then these tests can be corrected to remove the ;
+  // and to add a space between the `)` and `{`
+  verifyFormat("void main(){};", "void main() {};", Style);
+  verifyFormat("void foo(){}; //\n"
+   ";\n"
+   "int bar;",
+   "void foo(){}; //\n"
+   "; int bar;",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -920,6 +920,9 @@
 return IfLBrace;
   }
 
+  const bool IsFunctionRBrace =
+  FormatTok->is(tok::r_brace) && Tok->is(TT_FunctionLBrace);
+
   auto RemoveBraces = [=]() mutable {
 if (!SimpleBlock)
   return false;
@@ -959,6 +962,14 @@
 
   // Munch the closing brace.
   nextToken(/*LevelDifference=*/-AddLevels);
+
+  if (Style.RemoveSemicolon && IsFunctionRBrace) {
+while (FormatTok->is(tok::semi)) {
+  FormatTok->Optional = true;
+  nextToken();
+}
+  }
+
   HandleVerilogBlockLabel();
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
@@ -978,8 +989,12 @@
 parseStructuralElement();
   }
 
-  if (MunchSemi && FormatTok->is(tok::semi))
+  // When this is a function block and there is an unnecessary semicolon
+  // afterwards then mark it as optional (so the RemoveSemi pass can get rid of
+  // it later).
+  if (MunchSemi && FormatTok->is(tok::semi)) {
 nextToken();
+  }
 
   if (PPStartHash == PPEndHash) {
 Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -852,6 +852,7 @@
 IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM);
+IO.mapOptional("RemoveSemicolon", Style.RemoveSemicolon);
 IO.mapOptional("RequiresClausePosition", Style.RequiresClausePosition);
 IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
 IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
@@ -1297,6 +1298,7 @@
   LLVMStyle.UseTab = FormatStyle::UT_Never;
   LLVMStyle.ReflowComments = true;
   

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

[Tom Praschan via Phabricator via cfe-commits]

tom-anders updated this revision to Diff 466292.
tom-anders added a comment.

Add additional tests, only handle UsingDecl instead of BaseUsingDecl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135489

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -817,6 +817,29 @@
   char [[da^ta]];
 } @end
   )cpp",
+
+  // Issue 170: Rename symbol introduced by UsingDecl
+  R"cpp(
+namespace ns { void [[f^oo]](); } 
+
+using ns::[[f^oo]];
+
+void f() {
+[[f^oo]]();
+auto p = &[[f^oo]];
+}
+  )cpp",
+
+  // Issue 170: using decl that imports multiple overloads
+  // -> Only the overload under the cursor is renamed
+  R"cpp(
+namespace ns { int [[^foo]](int); char foo(char); }
+using ns::[[foo]];
+void f() {
+  [[^foo]](42);
+  foo('x');
+}
+  )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
@@ -1062,6 +1085,14 @@
 };
   )cpp",
"no symbol", false},
+
+  {R"cpp(// FIXME we probably want to rename both overloads here,
+ // but renaming currently assumes there's only a 
+ // single canonical declaration.
+namespace ns { int foo(int); char foo(char); }
+using ns::^foo;
+  )cpp",
+   "there are multiple symbols at the given location", !HeaderFile},
   };
 
   for (const auto& Case : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -133,6 +133,10 @@
 if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
   return canonicalRenameDecl(OriginalVD);
   }
+  if (const auto *UD = dyn_cast(D)) {
+if (const auto* TargetDecl = UD->getTargetDecl()) 
+  return canonicalRenameDecl(TargetDecl);
+  }
   return dyn_cast(D->getCanonicalDecl());
 }
 
@@ -157,6 +161,21 @@
   return Result;
 }
 
+// For something like 
+// namespace ns { void foo(); }
+// void bar() { using ns::f^oo; foo(); }
+// locateDeclAt() will return a UsingDecl and foo's actual declaration.
+// For renaming, we're only interested in foo's declaration, so drop the other 
one
+void filterUsingDecl(llvm::DenseSet& Decls) {
+  // There should never be more than one UsingDecl here, 
+  // otherwise the rename would be ambiguos anyway.
+  auto UD = std::find_if(Decls.begin(), Decls.end(), 
+ [](const NamedDecl* D) { return 
llvm::isa(D); });
+  if (UD != Decls.end()) {
+Decls.erase(UD);
+  }
+}
+
 // By default, we exclude symbols from system headers and protobuf symbols as
 // renaming these symbols would change system/generated files which are 
unlikely
 // to be good candidates for modification.
@@ -737,6 +756,7 @@
 return makeError(ReasonToReject::UnsupportedSymbol);
 
   auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+  filterUsingDecl(DeclsUnderCursor);
   if (DeclsUnderCursor.empty())
 return makeError(ReasonToReject::NoSymbolFound);
   if (DeclsUnderCursor.size() > 1)


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -817,6 +817,29 @@
   char [[da^ta]];
 } @end
   )cpp",
+
+  // Issue 170: Rename symbol introduced by UsingDecl
+  R"cpp(
+namespace ns { void [[f^oo]](); } 
+
+using ns::[[f^oo]];
+
+void f() {
+[[f^oo]]();
+auto p = &[[f^oo]];
+}
+  )cpp",
+
+  // Issue 170: using decl that imports multiple overloads
+  // -> Only the overload under the cursor is renamed
+  R"cpp(
+namespace ns { int [[^foo]](int); char foo(char); }
+using ns::[[foo]];
+void f() {
+  [[^foo]](42);
+  foo('x');
+}
+  )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
@@ -1062,6 +1085,14 @@
 };
   )cpp",
"no symbol", false},
+
+  {R"cpp(// FIXME we probably want to rename both overloads here,
+ // but renaming currently assumes there's only a 
+ // single canonical declaration.
+namespace ns { int foo(int); char foo(char); }
+using ns::^foo;
+  )cpp",
+   "there are multiple symbols at the given location", !HeaderFile},
   

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

[Tom Praschan via Phabricator via cfe-commits]

tom-anders marked 4 inline comments as done.
tom-anders added a comment.

In D135489#3844426 , @sammccall wrote:

>   namespace ns { int foo(int); char foo(char); }
>   using ns::foo;
>   int x = foo(42);
>   char y = foo('x');
>
> What should happen when we rename `foo(int)` on line 1?
>
> - rename both functions
> - rename one function + the usingdecl
> - rename one function and not the usingdecl
> - rename one function and add a second usingdecl
> - return an error
>
> In general the UsingDecl will be in another file and not visible in the AST. 
> Index only knows "there's a reference there". So I think our only real option 
> is to rename one function + the UsingDecl.
> (Assuming we want the common non-overloaded case to work. And assuming we 
> don't want to do something drastic like "silently rename overload sets 
> together in all cases").

Agreed.

> What should happen when we rename `using ns::foo` on line 2?
>
> - rename both functions
> - rename one arbitrarily
> - return an error
>
> Renaming both functions sounds great here. However for now the rename 
> implementation requires a single canonical decl, so we need to return an 
> error for now.
> If there's a single decl, renaming it seems fine.

Agreed, I added a FIXME in the test that we might want to extend 
canonicalRenameDecl to return multiple Decls (Probably a `llvm::DenseSet`?)
There already is a similar issue when renaming virtual methods with 
`size_overridden_methods() > 1`.

> What should happen when we rename `foo(42)` on line 3?
>
> - rename both functions
> - rename one function + the usingdecl
> - rename one function and not the usingdecl
> - rename one function and add a second usingdecl
> - return an error
>
> We *can* rely on the UsingDecl being visible here. However we should be 
> reasonably consistent with the first case, which I think rules out options 1, 
> 3 and 5.
> Splitting the usingdecl is a neat trick, but complicated, so let's start with 
> renaming one + functiondecl and maybe tack that on later.

Sounds good, splitting the using decl would be fancy though, maybe also add a 
FIXME for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135489

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

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Tom Praschan via Phabricator via cfe-commits]

tom-anders accepted this revision.
tom-anders added a comment.
This revision is now accepted and ready to land.

lgtm, thanks! With this change in place, we could probably ajdust 
`pickDeclToUse` in Hover.cpp (introduced in https://reviews.llvm.org/D133664) 
to also check for `UsingDecl` instead of `BaseUsingDecl`, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D135466#3844287 , @owenpan wrote:

> @MyDeveloperDay Cool!
>
> In D135466#3843792 , 
> @HazardyKnusperkeks wrote:
>
>> Maybe even extend the option to other unnecessary semicolons like `int x = 
>> 5;;` or other noop statements, one just has to be careful not to remove the 
>> semicolon when it's the sole if/loop body.
>
> IMO we should keep it simple for now. We can always extend it to an `enum` or 
> `struct` down the road, similar to what I plan to do with `RemoveBracesLLVM`.

Thanks, everyone for all the reviews, I'll work on those.

I tend to agree here, I was really only focused on the extraneous `;` after a 
function, I sort of feel catching a double coding `;;` error or even adding a 
missing `;` after a functions `)` as prettier/vs code seems to do are different 
problem statements.

I certainly don't mind adding the capability to fix those too (in subsequent 
commits), I don't like to add too much in a single commit. (Especially the 
initial one that carries a lot of the infrastructure for a new feature).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135466

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

[PATCH] D135404: [clang-tidy] Add a checker for converting into C++17 variable template type traits

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 466285.
royjacobson added a comment.

Addrees CR comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135404

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-type-traits %t
+
+namespace std {
+
+template 
+struct integral_constant {
+static constexpr inline T value = x;
+};
+
+template 
+struct is_same : integral_constant {};
+
+template 
+struct is_trivially_default_constructible : integral_constant {};
+
+template 
+struct alignment_of : integral_constant {};
+
+}
+
+
+template 
+void f() {
+auto x = std::is_same::value;
+auto y = std::is_trivially_default_constructible::value;
+auto z = std::alignment_of::value;
+
+// CHECK-FIXES:  auto x = std::is_same_v;
+// CHECK-FIXES-NEXT: auto y = std::is_trivially_default_constructible_v;
+// CHECK-FIXES-NEXT: auto z = std::alignment_of_v;
+// Test that we don't get this twice or something weird like that because
+// we're in a dependant context.
+// CHECK-FIXES-NOT:  auto x = std::is_same_v;
+}
+
+void g() {
+f();
+f();
+}
+
+void h() {
+auto x = std::is_same::value;
+auto y = std::is_trivially_default_constructible::value;
+auto z = std::alignment_of::value;
+// CHECK-FIXES:  auto x = std::is_same_v;
+// CHECK-FIXES-NEXT: auto y = std::is_trivially_default_constructible_v;
+// CHECK-FIXES-NEXT: auto z = std::alignment_of_v;
+}
+
+
+template 
+struct NonDiagnosableTemplate : std::integral_constant {};
+
+struct NonDiagnosable1 : std::integral_constant {};
+struct NonDiagnosable2 {
+using value = int;
+};
+
+#define IN_A_MACRO std::is_same::value
+void macro_use() {
+bool a = IN_A_MACRO;
+}
+// Don't fix-it if we use it inside a macro.
+// CHECK-FIXES: #define IN_A_MACRO std::is_same::value
+
+void test_no_diagnostic() {
+auto x = NonDiagnosableTemplate::value;
+auto y = NonDiagnosable1::value;
+auto z = NonDiagnosable2::value(4);
+// CHECK-FIXES:  auto x = NonDiagnosableTemplate::value;
+// CHECK-FIXES-NEXT: auto y = NonDiagnosable1::value;
+// CHECK-FIXES-NEXT: auto z = NonDiagnosable2::value(4);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - modernize-type-traits
+
+modernize-type-traits
+=
+
+
+Convert type traits from the form ``trait<...>::value`` into ``trait_v<...>``
+The template variable traits from C++17 can improve compile times, as in
+modern library implementations they don't instantiate structs.
+
+For example, this:
+
+.. code-block:: c++
+
+  std::is_same::value
+  std::is_integral::value
+
+is replaced by:
+
+.. code-block:: c++
+
+  std::is_same_v
+  std::is_integral_v
+
+Options
+---
+
+.. option:: ValueTypeTraits
+
+Specifies a list of type traits which are used with a static ``::value``
+member for metaprogramming instead of the regular one of traits available
+in the STL. This check assumes that a matching version exists with the
+same name and a ``_v`` suffix.
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -275,6 +275,7 @@
`modernize-replace-random-shuffle `_, "Yes"
`modernize-return-braced-init-list `_, "Yes"
`modernize-shrink-to-fit `_, "Yes"
+   `modernize-type-traits `_, "Yes"
`modernize-unary-static-assert `_, "Yes"
`modernize-use-auto `_, "Yes"
`modernize-use-bool-literals `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,6 +110,13 @@
 
   Warns when a struct or class uses const or reference (lvalue or rvalue) data members.
 
+- New :doc:`modernize-type-traits
+  ` check.
+
+  Finds usages of C++11 style type traits like 

[PATCH] D135404: [clang-tidy] Add a checker for converting into C++17 variable template type traits

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked 2 inline comments as done.
royjacobson added a comment.

In D135404#3842156 , @njames93 wrote:

> Why limit this to just the value type traits, makes sense to also support 
> type type traits.

I'd love to, but I can't match against the `::type` usages with `DeclRefExpr`. 
I think I'd have to match all types in the program or something like that, but 
I'm not sure if even that's enough because of unevaluated contexts etc... So I 
decided to focus on this first.




Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:73
+const auto *RecordQualifier =
+cast_if_present(
+Qualifier->getAsRecordDecl());

njames93 wrote:
> Won't this assert if a pointer is supplied but it isn't a 
> ClassTemplateSpecializationDefl. Wouldn't dyn_cast be needed.
> In any case a test where you access a static variable from a non template 
> class needs adding to make sure no warning is emitter.
Definitely need a dyn_cast here, thanks!



Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:86
+auto CurrentText = tooling::fixit::getText(*MatchedDecl, *Result.Context);
+if (replacement_regex.match(CurrentText))
+  IssuedDiag << FixItHint::CreateReplacement(

njames93 wrote:
> This fixit logic looks cumbersome, just replace the qualifier loc(::)to the 
> declared with an `_v`. But only iff the qualified name loc is not a macro.
I think to do that I have to get the location of the template-id of the 
qualifier from the ClassTemplateSpecializationDecl, but I haven't really 
managed to do that. Or is there maybe a way to get the lexer tokens list of a 
source range or something like that?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst:28
+
+.. option:: ValueTypeTraits
+

njames93 wrote:
> I'm not sure this option would ever be used in the wild as it would override 
> all the default values in the stl.
The intended use case is one-time running of this check over a code base to 
modernize it, and you want to add your own type traits that you define across 
your own libraries. I think that's useful, at least.

Maybe this should just _add_ traits, and not replace them? Is this something 
that clang-tidy does? I basically copied this from the `modernize-use-emplace` 
check.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135404

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

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 466282.

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

https://reviews.llvm.org/D135513

Files:
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -144,11 +144,14 @@
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
 S *s = nullptr;
-return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+   // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'foo()'}}
+  // ref-note {{in call to 'foo()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -168,7 +168,6 @@
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -119,6 +119,9 @@
 
 template  inline bool IsTrue(const T ) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState , APValue );
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1150,6 +1153,38 @@
   }
 }
 
+template ::T>
+static bool Call(InterpState , CodePtr , const Function *Func) {
+  auto *NewFrame = new InterpFrame(S, Func, PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  NewFrame->popArgs();
+  delete NewFrame;
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame;
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  NewFrame->popArgs();
+  delete NewFrame;
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1163,9 +1198,6 @@
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState , APValue );
-
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -53,16 +53,6 @@
   return true;
 }
 
-template ::T>
-static bool Call(InterpState , CodePtr , const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
 static bool CallVoid(InterpState , CodePtr , const Function *Func) {
   APValue VoidResult;
   S.Current = new InterpFrame(S, const_cast(Func), PC);
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/Interp/Function.cpp
+++ clang/lib/AST/Interp/Function.cpp
@@ -30,11 +30,13 @@
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {
+  assert(PC >= getCodeBegin() && "PC does not belong to this function");
+  assert(PC <= getCodeEnd() && "PC Does not belong to this function");
   unsigned Offset = PC - getCodeBegin();
   using Elem = std::pair;
   auto It = llvm::lower_bound(SrcMap, Elem{Offset, {}}, llvm::less_first());
-  if (It == SrcMap.end() || It->first != Offset)
-llvm::report_fatal_error("missing source location");
+  assert(It != SrcMap.end());
+  It--; // We want the offset *before* the given one.
   

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 466281.

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

https://reviews.llvm.org/D135513

Files:
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -144,11 +144,14 @@
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
 S *s = nullptr;
-return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+   // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'foo()'}}
+  // ref-note {{in call to 'foo()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -168,7 +168,6 @@
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -119,6 +119,9 @@
 
 template  inline bool IsTrue(const T ) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState , APValue );
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1150,6 +1153,38 @@
   }
 }
 
+template ::T>
+static bool Call(InterpState , CodePtr , const Function *Func) {
+  auto *NewFrame = new InterpFrame(S, const_cast(Func), PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  NewFrame->popArgs();
+  delete NewFrame;
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame;
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  NewFrame->popArgs();
+  delete NewFrame;
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1163,9 +1198,6 @@
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState , APValue );
-
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -53,22 +53,11 @@
   return true;
 }
 
-template ::T>
-static bool Call(InterpState , CodePtr , const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
-static bool CallVoid(InterpState , CodePtr , const Function *Func) {
+bool CallVoid(InterpState , CodePtr , const Function *Func) {
   APValue VoidResult;
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
+  S.Current = new InterpFrame(S, Func, {});
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());
-
   return Success;
 }
 
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/Interp/Function.cpp
+++ clang/lib/AST/Interp/Function.cpp
@@ -30,11 +30,13 @@
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {
+  assert(PC >= getCodeBegin() && "PC does not belong to this function");
+  assert(PC <= getCodeEnd() && "PC Does not belong to this function");
   unsigned Offset = PC - getCodeBegin();
   using Elem = std::pair;
   auto It = llvm::lower_bound(SrcMap, Elem{Offset, 

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 466280.

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

https://reviews.llvm.org/D135513

Files:
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -144,11 +144,14 @@
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
 S *s = nullptr;
-return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+   // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'foo()'}}
+  // ref-note {{in call to 'foo()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -168,7 +168,6 @@
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -119,6 +119,9 @@
 
 template  inline bool IsTrue(const T ) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState , APValue );
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1150,6 +1153,38 @@
   }
 }
 
+template ::T>
+static bool Call(InterpState , CodePtr , const Function *Func) {
+  auto *NewFrame = new InterpFrame(S, const_cast(Func), PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  NewFrame->popArgs();
+  delete NewFrame;
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame;
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  NewFrame->popArgs();
+  delete NewFrame;
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1163,9 +1198,6 @@
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState , APValue );
-
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -53,22 +53,13 @@
   return true;
 }
 
-template ::T>
-static bool Call(InterpState , CodePtr , const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
-static bool CallVoid(InterpState , CodePtr , const Function *Func) {
+bool CallVoid(InterpState , CodePtr , const Function *Func) {
   APValue VoidResult;
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
+  InterpFrame *before = S.Current;
+  S.Current = new InterpFrame(S, Func, {});
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());
-
+  assert(S.Current == before);
   return Success;
 }
 
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/Interp/Function.cpp
+++ clang/lib/AST/Interp/Function.cpp
@@ -30,11 +30,13 @@
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {
+  assert(PC >= getCodeBegin() && "PC does not belong to this function");
+  assert(PC <= getCodeEnd() && "PC Does not belong to this function");
   unsigned Offset = PC - getCodeBegin();
   using Elem 

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Remove the double Call() implementation to reduce code duplication.
Then fix Function::getSource() so we can diagnose instance pointers being null.

As discussed in https://reviews.llvm.org/D134699 - this fixes the test case 
added there.
The output in this case is not 100% the same as the current constant 
interpreter however.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135513

Files:
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -144,11 +144,14 @@
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
 S *s = nullptr;
-return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+   // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'foo()'}}
+  // ref-note {{in call to 'foo()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -168,7 +168,6 @@
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -119,6 +119,9 @@
 
 template  inline bool IsTrue(const T ) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState , APValue );
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1150,6 +1153,38 @@
   }
 }
 
+template ::T>
+static bool Call(InterpState , CodePtr , const Function *Func) {
+  auto *NewFrame = new InterpFrame(S, const_cast(Func), PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  NewFrame->popArgs();
+  delete NewFrame;
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  [[maybe_unused]] InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame;
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  NewFrame->popArgs();
+  delete NewFrame;
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1163,9 +1198,6 @@
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState , APValue );
-
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -53,22 +53,13 @@
   return true;
 }
 
-template ::T>
-static bool Call(InterpState , CodePtr , const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
-static bool CallVoid(InterpState , CodePtr , const Function *Func) {
+bool CallVoid(InterpState , CodePtr , const Function *Func) {
   APValue VoidResult;
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
+  InterpFrame *before = S.Current;
+  S.Current = new InterpFrame(S, Func, {});
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());

[PATCH] D135177: [clang] adds `__is_scoped_enum`, `__is_nullptr`, and `__is_referenceable`

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: clang/include/clang/Basic/TokenKinds.def:525
+TYPE_TRAIT_1(__is_scoped_enum, IsScopedEnum, KEYCXX)
+TYPE_TRAIT_1(__is_referenceable, IsReferenceable, KEYCXX)
 TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX)

Shouldn't these traits be documented or is the documentation generated 
automatically from the def files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135177

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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:81
+   isBaseOf(TPointeeUQTy, BPointeeUQTy)) &&
+  (TCVR == 0 || (BCVR ^ TCVR) == 0 || (BCVR & TCVR) > BCVR)) {
+TypesToDelete.push_back(T);

This harms readability and it doesn't cover extended qualifiers. I believe 
Qualifiers::isStrictSuperSet would be a better candidate to use here.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp:105
+void throw_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown 
in function 'throw_catch_pointer_c' which should not throw exceptions
+  try {

These check not lines are useless and should be removed, same goes below.


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

https://reviews.llvm.org/D135495

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

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Zhi-Hao Ye via Phabricator via cfe-commits]

Vigilans accepted this revision.
Vigilans added a comment.

In D129443#3844805 , 
@HazardyKnusperkeks wrote:

> In D129443#3844535 , @owenpan wrote:
>
>> In D129443#3842427 , 
>> @HazardyKnusperkeks wrote:
>>
>>> We can //fix// that regression, and I will adopt this change for my company 
>>> to keep the requires expressions where they are. :)
>>
>> Then we have three options:
>>
>> 1. Only fix the "regression".
>> 2. Add the option and make `OuterScope` the default.
>> 3. Add the option and make `Keyword` the default.
>>
>> @HazardyKnusperkeks I will defer to your choice. :)
>
> Add the option, and then either way we will have a regression to somebody. 
> `OuterScope` will regress to version 15. `Keyword` up to 14. So it should be 
> the most reasonable to make `OuterScope` the default, since the early 
> adapters will most be more open to change their configuration.
>
> @rymiel please add a change log note about that changed default.

I am fine with either default option with their persuasive reasons:

1. Default to `Keyword`: Interpreted as `Formally documented formatting 
behavior`, and the options arrangement looks more consistent with 
`LambdaBodyIndentation` (`Signature` (default) and `OuterScope`).
2. Default to `OuterScope`: Interpreted as `Fix the regression`, and the style 
looks more consistent with example codes in common concept tutorials (the 
expression is indented the same level as `concept` keyword, not `requires` 
keyword).

For consistency with `clang-format` current behaviors and options, we may 
prefer `Keyword`; For consistency with common current tutorial code, we may 
prefer `OuterScope`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129443

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

[PATCH] D133853: [AST] Add msvc-specific C++11 attributes

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D133853#3808674 , @aaron.ballman 
wrote:

> Now I'm wondering why the attribute exists at all. If it's functionally 
> equivalent to `constexpr` as a keyword, what are the use cases for the 
> attribute?

It appears that `[[msvc::constexpr]]` does not make a function `constexpr`, but 
if `[[msvc::constexpr]]` is used in a function definition //and// in a call to 
that function, then the annotated function call can be evaluated during 
constant evaluation: https://godbolt.org/z/3MPTsz6Yn

Apparently this is used to implement constexpr `std::construct_at`, which needs 
to call placement `operator new`, but the latter is not `constexpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133853

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

[clang-tools-extra] a8fbd11 - [clang-tidy] Ignore concepts in `misc-redundant-expression`

[Evgeny Shulgin via cfe-commits]

Author: Evgeny Shulgin
Date: 2022-10-08T10:37:16Z
New Revision: a8fbd1157debfe5872d4a34babef3aab75732aa7

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

LOG: [clang-tidy] Ignore concepts in `misc-redundant-expression`

The checker should ignore requirement expressions inside concept
definitions, because redundant expressions still make sense here

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

Reviewed By: njames93, aaron.ballman

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp

Modified: 
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index 5d30ad23a8531..e7c79331757a0 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -10,6 +10,7 @@
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -440,6 +441,8 @@ AST_MATCHER(Expr, isIntegerConstantExpr) {
   return Node.isIntegerConstantExpr(Finder->getASTContext());
 }
 
+AST_MATCHER(Expr, isRequiresExpr) { return isa(Node); }
+
 AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
   return areEquivalentExpr(Node.getLHS(), Node.getRHS());
 }
@@ -858,6 +861,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
 
   const auto BannedIntegerLiteral =
   integerLiteral(expandedByMacro(KnownBannedMacroNames));
+  const auto BannedAncestor = expr(isRequiresExpr());
 
   // Binary with equivalent operands, like (X != 2 && X != 2).
   Finder->addMatcher(
@@ -873,7 +877,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
unless(hasType(realFloatingPointType())),
unless(hasEitherOperand(hasType(realFloatingPointType(,
unless(hasLHS(AnyLiteralExpr)),
-   unless(hasDescendant(BannedIntegerLiteral)))
+   unless(hasDescendant(BannedIntegerLiteral)),
+   unless(hasAncestor(BannedAncestor)))
.bind("binary")),
   this);
 
@@ -886,7 +891,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
  unless(isInTemplateInstantiation()),
  unless(binaryOperatorIsInMacro()),
  // TODO: if the banned macros are themselves duplicated
- unless(hasDescendant(BannedIntegerLiteral)))
+ unless(hasDescendant(BannedIntegerLiteral)),
+ unless(hasAncestor(BannedAncestor)))
   .bind("nested-duplicates"),
   this);
 
@@ -896,7 +902,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
conditionalOperator(expressionsAreEquivalent(),
// Filter noisy false positives.
unless(conditionalOperatorIsInMacro()),
-   unless(isInTemplateInstantiation()))
+   unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("cond")),
   this);
 
@@ -909,7 +916,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
 ">=", "&&", "||", "="),
parametersAreEquivalent(),
// Filter noisy false positives.
-   unless(isMacro()), unless(isInTemplateInstantiation()))
+   unless(isMacro()), unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("call")),
   this);
 
@@ -919,7 +927,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
   hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"),
   nestedParametersAreEquivalent(), argumentCountIs(2),
   // Filter noisy false positives.
-  unless(isMacro()), unless(isInTemplateInstantiation()))
+  unless(isMacro()), unless(isInTemplateInstantiation()),
+  unless(hasAncestor(BannedAncestor)))
   .bind("nested-duplicates"),
   this);
 
@@ -936,7 +945,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

[Evgeny Shulgin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8fbd1157deb: [clang-tidy] Ignore concepts in 
`misc-redundant-expression` (authored by Izaron).

Changed prior to commit:
  https://reviews.llvm.org/D122078?vs=466271=466273#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122078

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s -checks=-*,misc-redundant-expression -- -std=c++20 | count 0
+
+namespace concepts {
+// redundant expressions inside concepts make sense, ignore them
+template 
+concept TestConcept = requires(I i) {
+  {i - i};
+  {i && i};
+  {i ? i : i};
+};
+} // namespace concepts
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -164,6 +164,12 @@
   :doc:`readability-simplify-boolean-expr `
   when using a C++23 ``if consteval`` statement.
 
+- Improved :doc:`misc-redundant-expression `
+  check.
+
+  The check now skips concept definitions since redundant expressions still make sense
+  inside them.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -10,6 +10,7 @@
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -440,6 +441,8 @@
   return Node.isIntegerConstantExpr(Finder->getASTContext());
 }
 
+AST_MATCHER(Expr, isRequiresExpr) { return isa(Node); }
+
 AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
   return areEquivalentExpr(Node.getLHS(), Node.getRHS());
 }
@@ -858,6 +861,7 @@
 
   const auto BannedIntegerLiteral =
   integerLiteral(expandedByMacro(KnownBannedMacroNames));
+  const auto BannedAncestor = expr(isRequiresExpr());
 
   // Binary with equivalent operands, like (X != 2 && X != 2).
   Finder->addMatcher(
@@ -873,7 +877,8 @@
unless(hasType(realFloatingPointType())),
unless(hasEitherOperand(hasType(realFloatingPointType(,
unless(hasLHS(AnyLiteralExpr)),
-   unless(hasDescendant(BannedIntegerLiteral)))
+   unless(hasDescendant(BannedIntegerLiteral)),
+   unless(hasAncestor(BannedAncestor)))
.bind("binary")),
   this);
 
@@ -886,7 +891,8 @@
  unless(isInTemplateInstantiation()),
  unless(binaryOperatorIsInMacro()),
  // TODO: if the banned macros are themselves duplicated
- unless(hasDescendant(BannedIntegerLiteral)))
+ unless(hasDescendant(BannedIntegerLiteral)),
+ unless(hasAncestor(BannedAncestor)))
   .bind("nested-duplicates"),
   this);
 
@@ -896,7 +902,8 @@
conditionalOperator(expressionsAreEquivalent(),
// Filter noisy false positives.
unless(conditionalOperatorIsInMacro()),
-   unless(isInTemplateInstantiation()))
+   unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("cond")),
   this);
 
@@ -909,7 +916,8 @@
 ">=", "&&", "||", "="),
parametersAreEquivalent(),
// Filter noisy false positives.
-   unless(isMacro()), unless(isInTemplateInstantiation()))
+   unless(isMacro()), unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("call")),
   this);
 
@@ -919,7 +927,8 @@
   hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"),
   nestedParametersAreEquivalent(), argumentCountIs(2),
   // Filter noisy false positives.
-  unless(isMacro()), unless(isInTemplateInstantiation()))
+  unless(isMacro()), unless(isInTemplateInstantiation()),
+  

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a reviewer: Vigilans.
HazardyKnusperkeks added a comment.

In D129443#3844535 , @owenpan wrote:

> In D129443#3842427 , 
> @HazardyKnusperkeks wrote:
>
>> We can //fix// that regression, and I will adopt this change for my company 
>> to keep the requires expressions where they are. :)
>
> Then we have three options:
>
> 1. Only fix the "regression".
> 2. Add the option and make `OuterScope` the default.
> 3. Add the option and make `Keyword` the default.
>
> @HazardyKnusperkeks I will defer to your choice. :)

Add the option, and then either way we will have a regression to somebody. 
`OuterScope` will regress to version 15. `Keyword` up to 14. So it should be 
the most reasonable to make `OuterScope` the default, since the early adapters 
will most be more open to change their configuration.

@rymiel please add a change log note about that changed default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129443

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

[PATCH] D135422: Fix clang-format misattributing preprocessor directives to macros

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

The clang-format tests are (mainly) in `clang/unittests/Format/FormatTest.cpp`, 
please put it there.
You should have a Format-Tests (or similar) build target.


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

https://reviews.llvm.org/D135422

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

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron updated this revision to Diff 466271.
Izaron added a comment.

Removed redundant comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122078

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s -checks=-*,misc-redundant-expression -- -std=c++20 | count 0
+
+namespace concepts {
+// redundant expressions inside concepts make sense, ignore them
+template 
+concept TestConcept = requires(I i) {
+  {i - i};
+  {i && i};
+  {i ? i : i};
+};
+} // namespace concepts
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,12 @@
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- Improved :doc:`misc-redundant-expression `
+  check.
+
+  The check now skips concept definitions since redundant expressions still make sense
+  inside them.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -10,6 +10,7 @@
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -440,6 +441,8 @@
   return Node.isIntegerConstantExpr(Finder->getASTContext());
 }
 
+AST_MATCHER(Expr, isRequiresExpr) { return isa(Node); }
+
 AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
   return areEquivalentExpr(Node.getLHS(), Node.getRHS());
 }
@@ -858,6 +861,7 @@
 
   const auto BannedIntegerLiteral =
   integerLiteral(expandedByMacro(KnownBannedMacroNames));
+  const auto BannedAncestor = expr(isRequiresExpr());
 
   // Binary with equivalent operands, like (X != 2 && X != 2).
   Finder->addMatcher(
@@ -873,7 +877,8 @@
unless(hasType(realFloatingPointType())),
unless(hasEitherOperand(hasType(realFloatingPointType(,
unless(hasLHS(AnyLiteralExpr)),
-   unless(hasDescendant(BannedIntegerLiteral)))
+   unless(hasDescendant(BannedIntegerLiteral)),
+   unless(hasAncestor(BannedAncestor)))
.bind("binary")),
   this);
 
@@ -886,7 +891,8 @@
  unless(isInTemplateInstantiation()),
  unless(binaryOperatorIsInMacro()),
  // TODO: if the banned macros are themselves duplicated
- unless(hasDescendant(BannedIntegerLiteral)))
+ unless(hasDescendant(BannedIntegerLiteral)),
+ unless(hasAncestor(BannedAncestor)))
   .bind("nested-duplicates"),
   this);
 
@@ -896,7 +902,8 @@
conditionalOperator(expressionsAreEquivalent(),
// Filter noisy false positives.
unless(conditionalOperatorIsInMacro()),
-   unless(isInTemplateInstantiation()))
+   unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("cond")),
   this);
 
@@ -909,7 +916,8 @@
 ">=", "&&", "||", "="),
parametersAreEquivalent(),
// Filter noisy false positives.
-   unless(isMacro()), unless(isInTemplateInstantiation()))
+   unless(isMacro()), unless(isInTemplateInstantiation()),
+   unless(hasAncestor(BannedAncestor)))
.bind("call")),
   this);
 
@@ -919,7 +927,8 @@
   hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"),
   nestedParametersAreEquivalent(), argumentCountIs(2),
   // Filter noisy false positives.
-  unless(isMacro()), unless(isInTemplateInstantiation()))
+  unless(isMacro()), unless(isInTemplateInstantiation()),
+  unless(hasAncestor(BannedAncestor)))
   .bind("nested-duplicates"),
   this);
 
@@ -936,7 +945,8 @@

[clang] 0c4f0bf - [C++20] [Modules] Only allow redeclarations in partitions if they are in the same module

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-10-08T17:44:04+08:00
New Revision: 0c4f0bf40d17d516aff54f6cea79b69101085799

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

LOG: [C++20] [Modules] Only allow redeclarations in partitions if they are in 
the same module

Closes https://github.com/llvm/llvm-project/issues/58196.

The root cause for the problem is an oversight in
https://reviews.llvm.org/D127624, which allows the redeclarations in
partitions. However, we took a mistake there that we should only allow
it if the redeclarations in the one same module instead of return
directly if either the redeclaration lives in a partition. The original
implementation makes no sense and I believe it was an oversight.

Added: 
clang/test/Modules/inconsistent-deduction-guide-linkage.cppm

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 760cf7fd805c..1bf959a35178 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1660,9 +1660,11 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl 
*New, NamedDecl *Old) {
 
   // Partitions are part of the module, but a partition could import another
   // module, so verify that the PMIs agree.
-  if (NewM && OldM && (NewM->isModulePartition() || OldM->isModulePartition()))
-return NewM->getPrimaryModuleInterfaceName() ==
-   OldM->getPrimaryModuleInterfaceName();
+  if (NewM && OldM &&
+  (NewM->isModulePartition() || OldM->isModulePartition()) &&
+  NewM->getPrimaryModuleInterfaceName() ==
+  OldM->getPrimaryModuleInterfaceName())
+return false;
 
   bool NewIsModuleInterface = NewM && NewM->isModulePurview();
   bool OldIsModuleInterface = OldM && OldM->isModulePurview();

diff  --git a/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm 
b/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm
new file mode 100644
index ..abcbec07f97d
--- /dev/null
+++ b/clang/test/Modules/inconsistent-deduction-guide-linkage.cppm
@@ -0,0 +1,49 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %t/A.cppm -I%t 
-fprebuilt-module-path=%t -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/D.cppm -I%t -emit-module-interface -o %t/D.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %t/D-part.cppm -I%t 
-fprebuilt-module-path=%t -verify
+
+//--- A.cppm
+module;
+export module baz:A;
+import B;
+#include "C.h"
+
+//--- B.cppm
+module;
+
+#include "C.h"
+export module B;
+
+//--- C.h
+namespace foo {
+  template struct bar { // expected-error {{declaration of 
'bar' in module baz:A follows declaration in the global module}} // 
expected-note {{previous declaration is here}}
+  template bar(T, U);
+  };
+  template bar(T, U) -> bar; // expected-error 
{{declaration of '' in module baz:A follows 
declaration in the global module}} // expected-note {{previous declaration is 
here}}
+}
+
+//--- D.cppm
+// Tests that it is still problematic if they are in one module.
+module;
+#include "E.h"
+export module D;
+
+//--- D-part.cppm
+export module D:part;
+import D;
+#include "E.h"
+
+//--- E.h
+// another file for simpler diagnostics.
+namespace foo {
+  template struct bar { // expected-error {{declaration of 
'bar' in module D:part follows declaration in the global module}} // 
expected-note {{previous declaration is here}}
+  template bar(T, U);
+  };
+  template bar(T, U) -> bar; // expected-error 
{{declaration of '' in module D:part follows 
declaration in the global module}} // expected-note {{previous declaration is 
here}}
+}



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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Daniel Bertalan via Phabricator via cfe-commits]

BertalanD added a comment.

Hi @erichkeane,

This change broke compilation of this program (https://godbolt.org/z/KrWGvcf8h; 
reduced from https://github.com/SerenityOS/ladybird):

  template
  constexpr bool IsSame = false;
  
  template
  constexpr bool IsSame = true;
  
  template
  struct Foo {
  template
  Foo(U&&) requires (!IsSame);
  };
  
  template<>
  struct Foo : Foo {
  using Foo::Foo;
  };
  
  Foo test() { return 0; }



  :18:27: error: invalid reference to function 'Foo': constraints not 
satisfied
  Foo test() { return 0; }
^
  :10:24: note: because substituted constraint expression is 
ill-formed: value of type '' is not contextually convertible to 
'bool'
  Foo(U&&) requires (!IsSame);
 ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs updated this revision to Diff 466269.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.

Addressed inline comment


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

https://reviews.llvm.org/D135495

Files:
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.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
@@ -101,6 +101,84 @@
   }
 }
 
+void throw_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_pointer_c' which should not throw exceptions
+  try {
+int a = 1;
+throw 
+  } catch(const int *) {}
+}
+
+void throw_catch_pointer_v() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_pointer_v' which should not throw exceptions
+  try {
+int a = 1;
+throw 
+  } catch(volatile int *) {}
+}
+
+void throw_catch_pointer_cv() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_pointer_cv' which should not throw exceptions
+  try {
+int a = 1;
+throw 
+  } catch(const volatile int *) {}
+}
+
+void throw_c_catch_pointer() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer' which should not throw exceptions
+  try {
+int a = 1;
+const int *p = 
+throw p;
+  } catch(int *) {}
+}
+
+void throw_c_catch_pointer_v() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer_v' which should not throw exceptions
+  try {
+int a = 1;
+const int *p = 
+throw p;
+  } catch(volatile int *) {}
+}
+
+void throw_v_catch_pointer() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer' which should not throw exceptions
+  try {
+int a = 1;
+volatile int *p = 
+throw p;
+  } catch(int *) {}
+}
+
+void throw_v_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer_c' which should not throw exceptions
+  try {
+int a = 1;
+volatile int *p = 
+throw p;
+  } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_c() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_c' which should not throw exceptions
+  try {
+int a = 1;
+const volatile int *p = 
+throw p;
+  } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_v() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_v' which should not throw exceptions
+  try {
+int a = 1;
+const volatile int *p = 
+throw p;
+  } catch(volatile int *) {}
+}
+
 class base {};
 class derived: public base {};
 
@@ -112,6 +190,25 @@
   }
 }
 
+void throw_derived_catch_base_ptr_c() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ptr_c' which should not throw exceptions
+  try {
+derived d;
+throw  
+  } catch(const base *) {
+  }
+}
+
+void throw_derived_catch_base_ptr() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ptr' which should not throw exceptions
+  try {
+derived d;
+const derived *p = 
+throw p; 
+  } catch(base *) {
+  }
+}
+
 void try_nested_try(int n) noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
   try {
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -61,6 +61,27 @@
   for (const Type *T : ThrownExceptions) {
 if (T == BaseClass || isBaseOf(T, BaseClass))
   TypesToDelete.push_back(T);
+else if (T->isPointerType() && BaseClass->isPointerType()) {
+  QualType BPointeeTy = BaseClass->getAs()->getPointeeType();
+  QualType TPointeeTy = T->getAs()->getPointeeType();
+
+  const Type *BPointeeUQTy = BPointeeTy->getUnqualifiedDesugaredType();
+  const Type *TPointeeUQTy = TPointeeTy->getUnqualifiedDesugaredType();
+
+  unsigned BCVR = BPointeeTy.getCVRQualifiers();
+  unsigned TCVR = TPointeeTy.getCVRQualifiers();
+
+  // In case the unqualified types are the same, the exception will be
+ 

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

[Nathan James via Phabricator via cfe-commits]

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

LGTM, just one small nit.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp:3
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.

This comment isn't strictly necessary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122078

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

[clang] 9974ed8 - [C++20] [Modules] Remove assertion of current module when acting on import

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-10-08T16:44:51+08:00
New Revision: 9974ed804995d2e34be69404e9904c7e03cfbda4

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

LOG: [C++20] [Modules] Remove assertion of current module when acting on import

Closes https://github.com/llvm/llvm-project/issues/58199

Previously, when we act on a import statement, we'll assume there is a
module declaration in the current TU if the command line tells us we're
compiling a module unit. This makes since on valid codes. However, for
invalid codes, it is possible. See
https://github.com/llvm/llvm-project/issues/58199 for example.

This patch removes the assertion. And the assertion is a noop and it
should be safe to remove it.

Added: 
clang/test/Modules/missing-module-declaration.cppm

Modified: 
clang/lib/Sema/SemaModule.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index b205fd914f9d3..4b01f109fc881 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -561,11 +561,6 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
 Diag(ExportLoc, diag::err_export_not_in_module_interface)
 << (!ModuleScopes.empty() &&
 !ModuleScopes.back().ImplicitGlobalModuleFragment);
-  } else if (getLangOpts().isCompilingModule()) {
-Module *ThisModule = PP.getHeaderSearchInfo().lookupModule(
-getLangOpts().CurrentModule, ExportLoc, false, false);
-(void)ThisModule;
-assert(ThisModule && "was expecting a module if building one");
   }
 
   // In some cases we need to know if an entity was present in a directly-

diff  --git a/clang/test/Modules/missing-module-declaration.cppm 
b/clang/test/Modules/missing-module-declaration.cppm
new file mode 100644
index 0..d52f6639fe4f6
--- /dev/null
+++ b/clang/test/Modules/missing-module-declaration.cppm
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -I%t -fprebuilt-module-path=%t 
-emit-module-interface -verify
+
+//--- A.cppm
+import B; // expected-error{{missing 'export module' declaration in module 
interface unit}}
+
+//--- B.cppm
+module;
+export module B;



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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:963
   nextToken(/*LevelDifference=*/-AddLevels);
   HandleVerilogBlockLabel();
 

lahwaacz wrote:
> owenpan wrote:
> > The `while` loop would address 
> > https://reviews.llvm.org/D135466#inline-1306331 but is optional IMO because 
> > it wouldn't distinguish `};;` and the following:
> > ```
> > };
> > ;
> > ```
> Does it matter? If you have `struct foo {};;` and format it with e.g. 
> `clang-format --style=LLVM`, you'll get
> ```
> struct foo {};
> ;
> ```
> so IMO these _should_ be indistinguishible.
It might matter when removing the semicolons. We can keep the loop if we have 
sufficient coverage in test cases. For example, we must ensure that
```
void foo() {}; //
; int bar;
```
is not formatted to
```
void foo() {} // int bar;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135466

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

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:446
+  if (Optional T = classify(LitType)) {
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);

aaron.ballman wrote:
> Should this be concerned about the sign of the promoted literal?
Switched to `emitConst()` that takes into account signedness.



Comment at: clang/test/AST/Interp/arrays.cpp:135
+  static_assert(u32[1] == U'b', "");
+};
+

aaron.ballman wrote:
> I think you need a more coverage for character literals. Some test cases that 
> are interesting: multichar literals (`'abcd'`), character literals with UCNs 
> (`'\u'`), character literals with numeric escapes (`'\xFF'`). I'm 
> especially interested in seeing whether we handle integer promotions 
> properly, especially when promoting to `unsigned int`.
I added two more test cases but I'm generally not that familiar with character  
literal edge cases and integer promotion, so if you have concrete test cases in 
mind, that would be great :)


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

https://reviews.llvm.org/D135366

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

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 466262.

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

https://reviews.llvm.org/D135366

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/arrays.cpp


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -118,6 +118,29 @@
// expected-note {{cannot refer to 
element -2 of array of 10}}
 };
 
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 'a', "");
+  static_assert(S[1] == 'b', "");
+  static_assert(S[2] == 'c', "");
+  static_assert(S[3] == '\0', "");
+
+  static_assert("foobar"[2] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+
+  constexpr char32_t c = U'\U0001F60E';
+  static_assert(c == U'');
+
+  constexpr char k = -1;
+  static_assert(k == -1);
+
+};
+
 #if 0
 namespace fillers {
   constexpr int k[3] = {1337};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -90,6 +90,8 @@
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
+  bool VisitStringLiteral(const StringLiteral *E);
+  bool VisitCharacterLiteral(const CharacterLiteral *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
@@ -433,6 +433,21 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitStringLiteral(const StringLiteral *E) {
+  unsigned StringIndex = P.createGlobalString(E);
+  return this->emitGetPtrGlobal(StringIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitCharacterLiteral(
+const CharacterLiteral *E) {
+  if (Optional T = classify(E->getType()))
+return this->emitConst(E, E->getValue());
+
+  return this->bail(E);
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) 
{
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -118,6 +118,29 @@
// expected-note {{cannot refer to element -2 of array of 10}}
 };
 
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 'a', "");
+  static_assert(S[1] == 'b', "");
+  static_assert(S[2] == 'c', "");
+  static_assert(S[3] == '\0', "");
+
+  static_assert("foobar"[2] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+
+  constexpr char32_t c = U'\U0001F60E';
+  static_assert(c == U'😎');
+
+  constexpr char k = -1;
+  static_assert(k == -1);
+
+};
+
 #if 0
 namespace fillers {
   constexpr int k[3] = {1337};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -90,6 +90,8 @@
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
+  bool VisitStringLiteral(const StringLiteral *E);
+  bool VisitCharacterLiteral(const CharacterLiteral *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
@@ -433,6 +433,21 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitStringLiteral(const StringLiteral *E) {
+  unsigned StringIndex = P.createGlobalString(E);
+  return this->emitGetPtrGlobal(StringIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitCharacterLiteral(
+const CharacterLiteral *E) {
+  if (Optional T = classify(E->getType()))
+return this->emitConst(E, E->getValue());
+
+  return this->bail(E);
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D135433: [clang][Interp] Implement while and do-while loops

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 466259.

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

https://reviews.llvm.org/D135433

Files:
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/test/AST/Interp/loops.cpp

Index: clang/test/AST/Interp/loops.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/loops.cpp
@@ -0,0 +1,173 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -std=c++14 -verify=ref %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify=expected-cpp20 %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref %s
+
+// ref-no-diagnostics
+// expected-no-diagnostics
+
+namespace WhileLoop {
+  constexpr int f() {
+int i = 0;
+while(false) {
+  i = i + 1;
+}
+return i;
+  }
+  static_assert(f() == 0, "");
+
+
+  constexpr int f2() {
+int i = 0;
+while(i != 5) {
+  i = i + 1;
+}
+return i;
+  }
+  static_assert(f2() == 5, "");
+
+  constexpr int f3() {
+int i = 0;
+while(true) {
+  i = i + 1;
+
+  if (i == 5)
+break;
+}
+return i;
+  }
+  static_assert(f3() == 5, "");
+
+  constexpr int f4() {
+int i = 0;
+while(i != 5) {
+
+  i = i + 1;
+  continue;
+  i = i - 1;
+}
+return i;
+  }
+  static_assert(f4() == 5, "");
+
+
+  constexpr int f5(bool b) {
+int i = 0;
+
+while(true) {
+  if (!b) {
+if (i == 5)
+  break;
+  }
+
+  if (b) {
+while (i != 10) {
+  i = i + 1;
+  if (i == 8)
+break;
+
+  continue;
+}
+  }
+
+  if (b)
+break;
+
+  i = i + 1;
+  continue;
+}
+
+return i;
+  }
+  static_assert(f5(true) == 8, "");
+  static_assert(f5(false) == 5, "");
+};
+
+namespace DoWhileLoop {
+
+  constexpr int f() {
+int i = 0;
+do {
+  i = i + 1;
+} while(false);
+return i;
+  }
+  static_assert(f() == 1, "");
+
+  constexpr int f2() {
+int i = 0;
+do {
+  i = i + 1;
+} while(i != 5);
+return i;
+  }
+  static_assert(f2() == 5, "");
+
+
+  constexpr int f3() {
+int i = 0;
+do {
+  i = i + 1;
+  if (i == 5)
+break;
+} while(true);
+return i;
+  }
+  static_assert(f3() == 5, "");
+
+  constexpr int f4() {
+int i = 0;
+do {
+  i = i + 1;
+  continue;
+  i = i - 1;
+} while(i != 5);
+return i;
+  }
+  static_assert(f4() == 5, "");
+
+  constexpr int f5(bool b) {
+int i = 0;
+
+do {
+  if (!b) {
+if (i == 5)
+  break;
+  }
+
+  if (b) {
+do {
+  i = i + 1;
+  if (i == 8)
+break;
+
+  continue;
+} while (i != 10);
+  }
+
+  if (b)
+break;
+
+  i = i + 1;
+  continue;
+} while(true);
+
+return i;
+  }
+  static_assert(f5(true) == 8, "");
+  static_assert(f5(false) == 5, "");
+
+  /// FIXME: This should be accepted in C++20 but is currently being rejected
+  ///   because the variable declaration doesn't have an initializier.
+#if __cplusplus >= 202002L
+  constexpr int f6() {
+int i;
+do {
+  i = 5;
+  break;
+} while (true);
+return i;
+  }
+  static_assert(f6() == 5, ""); // expected-cpp20-error {{not an integral constant}}
+#endif
+};
Index: clang/lib/AST/Interp/ByteCodeStmtGen.h
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.h
+++ clang/lib/AST/Interp/ByteCodeStmtGen.h
@@ -58,6 +58,10 @@
   bool visitDeclStmt(const DeclStmt *DS);
   bool visitReturnStmt(const ReturnStmt *RS);
   bool visitIfStmt(const IfStmt *IS);
+  bool visitWhileStmt(const WhileStmt *S);
+  bool visitDoStmt(const DoStmt *S);
+  bool visitBreakStmt(const BreakStmt *S);
+  bool visitContinueStmt(const ContinueStmt *S);
 
   /// Compiles a variable declaration.
   bool visitVarDecl(const VarDecl *VD);
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -167,6 +167,14 @@
 return visitReturnStmt(cast(S));
   case Stmt::IfStmtClass:
 return visitIfStmt(cast(S));
+  case Stmt::WhileStmtClass:
+return visitWhileStmt(cast(S));
+  case Stmt::DoStmtClass:
+return visitDoStmt(cast(S));
+  case Stmt::BreakStmtClass:
+return visitBreakStmt(cast(S));
+  case Stmt::ContinueStmtClass:
+return visitContinueStmt(cast(S));
   case Stmt::NullStmtClass:
 return true;
   default: {
@@ -276,6 +284,69 @@
   return true;
 }
 
+template 
+bool ByteCodeStmtGen::visitWhileStmt(const WhileStmt *S) {
+  const Expr *Cond = S->getCond();
+  const Stmt *Body = S->getBody();
+
+  LabelTy CondLabel = this->getLabel(); // Label before the condition.
+  LabelTy EndLabel = this->getLabel();  // 

[clang] 3b276a0 - [ARM64EC][clang-cl] Add arm64EC test; NFC

[via cfe-commits]

Author: chenglin.bi
Date: 2022-10-08T14:47:50+08:00
New Revision: 3b276a0decea1adcb822d5110f25f7256a6ade6c

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

LOG: [ARM64EC][clang-cl] Add arm64EC test; NFC

Added: 


Modified: 
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6c768fbb7100..423799ef9963 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -782,8 +782,11 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
-// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
-// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck 
--check-prefix=ARM64EC %s 
 // ARM64EC-NOT: /arm64EC has been overridden by specified target
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  
-### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC_OVERRIDE %s
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified 
target: x86_64-pc-windows-msvc; option ignored
 
 void f(void) { }



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

[PATCH] D135433: [clang][Interp] Implement while and do-while loops

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/test/AST/Interp/loops.cpp:93
+  }
+  static_assert(f4() == 5, "");
+};

aaron.ballman wrote:
> Can you also add some tests that use nested loops with multiple levels of 
> `break` and `continue` use?
> 
> Also, I think it might be useful to show the jump statements causing code to 
> be an invalid constant expression, as in: 
> ```
> constexpr int foo() {
>   int i;
> 
>   do {
> break;
> i = 12;
>   } while (1);
> 
>   return i;
> }
> 
> constexpr int f = foo();
> ```
I can add a commented-out test case for that, //but// it is currently being 
rejected for the wrong reasons. I investigated that in another case already and 
it will simply be rejected because the variable declaration does not have an 
initializer. Not because we're reading an uninitialized variable.


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

https://reviews.llvm.org/D135433

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

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[Jakub Klinkovský via Phabricator via cfe-commits]

lahwaacz added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:963
   nextToken(/*LevelDifference=*/-AddLevels);
   HandleVerilogBlockLabel();
 

owenpan wrote:
> The `while` loop would address 
> https://reviews.llvm.org/D135466#inline-1306331 but is optional IMO because 
> it wouldn't distinguish `};;` and the following:
> ```
> };
> ;
> ```
Does it matter? If you have `struct foo {};;` and format it with e.g. 
`clang-format --style=LLVM`, you'll get
```
struct foo {};
;
```
so IMO these _should_ be indistinguishible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135466

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