[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

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

tbaeder closed this revision.
tbaeder added a comment.

This has been pushed already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156042

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

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Reverted with D155568 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156042

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

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

[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 rGd425720aed48: [clang][Interp] Implement __builtin_strlen 
(authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D156042?vs=543414&id=551084#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156042

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -1,9 +1,11 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -triple avr -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -triple avr -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
@@ -40,6 +42,70 @@
 // ref-note {{dereferenced one-past-the-end}}
 }
 
+/// Copied from constant-expression-cxx11.cpp
+namespace strlen {
+constexpr const char *a = "foo\0quux";
+  constexpr char b[] = "foo\0quux";
+  constexpr int f() { return 'u'; }
+  constexpr char c[] = { 'f', 'o', 'o', 0, 'q', f(), 'u', 'x', 0 };
+
+  static_assert(__builtin_strlen("foo") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+
+  constexpr bool check(const char *p) {
+return __builtin_strlen(p) == 3 &&
+   __builtin_strlen(p + 1) == 2 &&
+   __builtin_strlen(p + 2) == 1 &&
+   __builtin_strlen(p + 3) == 0 &&
+   __builtin_strlen(p + 4) == 4 &&
+   __builtin_strlen(p + 5) == 3 &&
+   __builtin_strlen(p + 6) == 2 &&
+   __builtin_strlen(p + 7) == 1 &&
+   __builtin_strlen(p + 8) == 0;
+  }
+
+  static_assert(check(a), "");
+  static_assert(check(b), "");
+  static_assert(check(c), "");
+
+  constexpr int over1 = __builtin_strlen(a + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over2 = __builtin_strlen(b + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over3 = __builtin_strlen(c + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+
+  constexpr int under1 = __builtin_strlen(a - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under2 = __builtin_strlen(b - 1); // expected-error {{constant expression}} \
+  

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

[Corentin Jabot via Phabricator via cfe-commits]

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

LGTM, modulo suggestion




Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:63-69
+  if (SizeTWidth == 64)
+S.Stk.push>(Integral<64, false>::from(Val));
+  else if (SizeTWidth == 32)
+S.Stk.push>(Integral<32, false>::from(Val));
+  else
+llvm_unreachable("size_t isn't 64 or 32 bit?");
+}

Maybe we could have
```
pushSignedInteger(InterpState &S, unsigned bit width, int64_t Val);
pushUnsignedInteger(InterpState &S, unsigned bit width, int64_t Val)
```
to simplify pushSizeT and pushInt


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

https://reviews.llvm.org/D156042

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

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

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

tbaeder updated this revision to Diff 543414.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D156042

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
@@ -38,6 +40,70 @@
 // ref-note {{dereferenced one-past-the-end}}
 }
 
+/// Copied from constant-expression-cxx11.cpp
+namespace strlen {
+constexpr const char *a = "foo\0quux";
+  constexpr char b[] = "foo\0quux";
+  constexpr int f() { return 'u'; }
+  constexpr char c[] = { 'f', 'o', 'o', 0, 'q', f(), 'u', 'x', 0 };
+
+  static_assert(__builtin_strlen("foo") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+
+  constexpr bool check(const char *p) {
+return __builtin_strlen(p) == 3 &&
+   __builtin_strlen(p + 1) == 2 &&
+   __builtin_strlen(p + 2) == 1 &&
+   __builtin_strlen(p + 3) == 0 &&
+   __builtin_strlen(p + 4) == 4 &&
+   __builtin_strlen(p + 5) == 3 &&
+   __builtin_strlen(p + 6) == 2 &&
+   __builtin_strlen(p + 7) == 1 &&
+   __builtin_strlen(p + 8) == 0;
+  }
+
+  static_assert(check(a), "");
+  static_assert(check(b), "");
+  static_assert(check(c), "");
+
+  constexpr int over1 = __builtin_strlen(a + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over2 = __builtin_strlen(b + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over3 = __builtin_strlen(c + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+
+  constexpr int under1 = __builtin_strlen(a - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under2 = __builtin_strlen(b - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under3 = __builtin_strlen(c - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+
+  constexpr char d[]

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

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

tbaeder marked an inline comment as done.
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:114
+
+  // TODO: Push platform-dependent size_t.
+  S.Stk.push>(Integral<64, false>::from(Len));

cor3ntin wrote:
> Why not do that in this patch?
I was trying to keep the patch small but here it is.


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

https://reviews.llvm.org/D156042

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

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

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

tbaeder updated this revision to Diff 543317.

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

https://reviews.llvm.org/D156042

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
@@ -38,6 +40,70 @@
 // ref-note {{dereferenced one-past-the-end}}
 }
 
+/// Copied from constant-expression-cxx11.cpp
+namespace strlen {
+constexpr const char *a = "foo\0quux";
+  constexpr char b[] = "foo\0quux";
+  constexpr int f() { return 'u'; }
+  constexpr char c[] = { 'f', 'o', 'o', 0, 'q', f(), 'u', 'x', 0 };
+
+  static_assert(__builtin_strlen("foo") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+
+  constexpr bool check(const char *p) {
+return __builtin_strlen(p) == 3 &&
+   __builtin_strlen(p + 1) == 2 &&
+   __builtin_strlen(p + 2) == 1 &&
+   __builtin_strlen(p + 3) == 0 &&
+   __builtin_strlen(p + 4) == 4 &&
+   __builtin_strlen(p + 5) == 3 &&
+   __builtin_strlen(p + 6) == 2 &&
+   __builtin_strlen(p + 7) == 1 &&
+   __builtin_strlen(p + 8) == 0;
+  }
+
+  static_assert(check(a), "");
+  static_assert(check(b), "");
+  static_assert(check(c), "");
+
+  constexpr int over1 = __builtin_strlen(a + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over2 = __builtin_strlen(b + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over3 = __builtin_strlen(c + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+
+  constexpr int under1 = __builtin_strlen(a - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under2 = __builtin_strlen(b - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under3 = __builtin_strlen(c - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+
+  constexpr char d[] = { 'f', 'o', 'o' }; // no nul terminator

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:101-112
+  for (;;) {
+const Pointer &ElemPtr = StrPtr.atIndex(I);
+
+if (!CheckRange(S, OpPC, ElemPtr, AK_Read))
+  return false;
+
+uint8_t Val = ElemPtr.deref();

maybe a do while would be cleaner



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:114
+
+  // TODO: Push platform-dependent size_t.
+  S.Stk.push>(Integral<64, false>::from(Len));

Why not do that in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156042

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

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

[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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156042

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
@@ -38,6 +38,70 @@
 // ref-note {{dereferenced one-past-the-end}}
 }
 
+/// Copied from constant-expression-cxx11.cpp
+namespace strlen {
+constexpr const char *a = "foo\0quux";
+  constexpr char b[] = "foo\0quux";
+  constexpr int f() { return 'u'; }
+  constexpr char c[] = { 'f', 'o', 'o', 0, 'q', f(), 'u', 'x', 0 };
+
+  static_assert(__builtin_strlen("foo") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+
+  constexpr bool check(const char *p) {
+return __builtin_strlen(p) == 3 &&
+   __builtin_strlen(p + 1) == 2 &&
+   __builtin_strlen(p + 2) == 1 &&
+   __builtin_strlen(p + 3) == 0 &&
+   __builtin_strlen(p + 4) == 4 &&
+   __builtin_strlen(p + 5) == 3 &&
+   __builtin_strlen(p + 6) == 2 &&
+   __builtin_strlen(p + 7) == 1 &&
+   __builtin_strlen(p + 8) == 0;
+  }
+
+  static_assert(check(a), "");
+  static_assert(check(b), "");
+  static_assert(check(c), "");
+
+  constexpr int over1 = __builtin_strlen(a + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over2 = __builtin_strlen(b + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+  constexpr int over3 = __builtin_strlen(c + 9); // expected-error {{constant expression}} \
+ // expected-note {{one-past-the-end}} \
+ // expected-note {{in call to}} \
+ // ref-error {{constant expression}} \
+ // ref-note {{one-past-the-end}}
+
+  constexpr int under1 = __builtin_strlen(a - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under2 = __builtin_strlen(b - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+  constexpr int under3 = __builtin_strlen(c - 1); // expected-error {{constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+
+  constexpr char d[] = { 'f', 'o', 'o' }; // no nul terminator.
+  constexpr int bad = __builtin_strlen(d); // ex