[PATCH] D156045: [clang][Interp] Enable existing source_location tests

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

@tbaeder Now it's working correctly. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

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

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

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

tbaeder added a comment.

@hazohelet Thanks for noticing. Should be fixed via 
https://github.com/llvm/llvm-project/commit/f8074942c5a6f266abb1c4d1465a4d8115b22d43.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

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

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

I was rebasing D155064  locally and saw test 
failures for the new interpreter in this file.
The cause seems to me that the new interpreter is not handling some cases of 
`SourceLocExpr` correctly when they appear inside constant-evaluated or 
immediate-function context.
e.g.

  struct A {
    int n = __builtin_LINE();
  };
  struct B {
    A a = {};
  };
  #line 100
  consteval void f() {
    constexpr B c = {};
    static_assert(c.a.n == 101, "");
  }

Live demo: https://godbolt.org/z/9Y7bzj56G

D155064  pushes constant-evaluated context 
against initializers of `constexpr` variables, so this bug appears in the test 
file.
I'm not in any hurry to push D155064  
upstream, but if it might take several weeks for this bug to be fixed, I'd like 
this change to be reverted until it gets fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

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

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7e5809e7e7bc: [clang][Interp] Enable existing 
source_location tests (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

Files:
  clang/test/SemaCXX/source_location.cpp


Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -3,6 +3,12 @@
 // RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT 
-fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS 
-fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS 
-DUSE_CONSTEVAL -fexceptions -verify %s
+//
+/// FIXME: The -DPAREN_INIT one is missing for the new interpreter.
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions 
-fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions 
-fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS 
-fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS 
-DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter 
-DNEW_INTERP -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -755,7 +761,7 @@
 [b = SL::current().line()] { return b; }()) {
   return a;
 }
-#ifdef USE_CONSTEVAL
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );


Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -3,6 +3,12 @@
 // RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT -fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -verify %s
+//
+/// FIXME: The -DPAREN_INIT one is missing for the new interpreter.
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -755,7 +761,7 @@
 [b = SL::current().line()] { return b; }()) {
   return a;
 }
-#ifdef USE_CONSTEVAL
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

[Corentin Jabot via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/source_location.cpp:764-768
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );
 #endif

tbaeder wrote:
> The comment block just before this `#if` seems to say that the new 
> interpreter is correct here...?
Yes, which is interesting. Maybe modifying the existing interpreter properly 
would be doable then? I'll have to look into that...
I'm surprised it's only an evaluation issue 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

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

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

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

tbaeder added inline comments.



Comment at: clang/test/SemaCXX/source_location.cpp:764-768
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );
 #endif

The comment block just before this `#if` seems to say that the new interpreter 
is correct here...?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156045

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

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

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

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

As requested.

This depends on a few other patches though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156045

Files:
  clang/test/SemaCXX/source_location.cpp


Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -3,6 +3,12 @@
 // RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT 
-fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS 
-fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS 
-DUSE_CONSTEVAL -fexceptions -verify %s
+//
+/// FIXME: The -DPAREN_INIT one is missing for the new interpreter.
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions 
-fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions 
-fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS 
-fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS 
-DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter 
-DNEW_INTERP -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -755,7 +761,7 @@
 [b = SL::current().line()] { return b; }()) {
   return a;
 }
-#ifdef USE_CONSTEVAL
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );


Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -3,6 +3,12 @@
 // RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT -fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -verify %s
+//
+/// FIXME: The -DPAREN_INIT one is missing for the new interpreter.
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -755,7 +761,7 @@
 [b = SL::current().line()] { return b; }()) {
   return a;
 }
-#ifdef USE_CONSTEVAL
+#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP)
 static_assert(test_init_capture() == __LINE__ - 4);
 #else
 static_assert(test_init_capture() == __LINE__ );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits