Mordante marked 2 inline comments as done.
Mordante added inline comments.
================
Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -fsyntax-only
-ast-dump %s | FileCheck %s
+
----------------
rsmith wrote:
> I don't think we really need a dedicated AST test for this. Such tests create
> a maintenance burden, and they don't really capture what we care about here:
> that all non-zero values are correctly converted to the `true` value of the
> enumeration type.
I'll remove the test.
================
Comment at: clang/test/CodeGen/enum-bool.cpp:7-14
+// CHECK: ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381A1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %x.addr = alloca i32, align 4
+// CHECK-NEXT: store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT: %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT: ret i32 %0
----------------
rsmith wrote:
> Some general guidance for writing IR testcases:
>
> - Don't test things that aren't relevant to the test case; doing so will
> make the test brittle as IR generation changes. In particular, don't test the
> function attribute comments, the `#0` introducing function metadata,
> instruction names, alignments.
> - Use `CHECK-LABEL` for each function definition to improve matching
> semantics and diagnostics on mismatches. (The `CHECK-LABEL`s are checked
> first, then the input is sliced up into pieces between them, and those pieces
> are checked independently.)
> - Don't use `CHECK-NEXT` unless it's relevant to your test that no other
> instructions appear in between.
Thanks for the information, I'll reduce the test case.
================
Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:597-609
+
+namespace dr2338 {
+namespace B {
+enum E : bool { Zero, One };
+consteval E c(int x) { return (E)x; }
+static_assert(static_cast<int>(c(2)) == 1);
+} // namespace B
----------------
rsmith wrote:
> This DR test should go in `test/CXX/drs/dr23xx.cpp`, along with a suitable
> "magic comment" `// dr2338: 12` to indicate this DR is implemented in Clang
> 12 onwards. (We have tooling that generates the
> `clang.llvm.org/cxx_dr_status.html` page from those magic comments.)
>
> I don't think this has anything to do with `consteval`; a more-reduced test
> should work just as well (eg, `static_assert((int)(E)2 == 1, "");`) and would
> allow us to test this in C++11 onwards, not only in C++20.
I thought the magic comment could be done in any test file. It'll move the test
and add the comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85612/new/
https://reviews.llvm.org/D85612
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
