[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::is_within_lifetime #91895

MitalAshok · 2024-05-12T18:58:49Z

This new builtin function is declared consteval. Support for -fexperimental-new-constant-interpreter will be added in a later patch.

github-actions · 2024-05-12T19:01:48Z

✅ With the latest revision this PR passed the C/C++ code formatter.

MitalAshok · 2024-05-12T19:03:24Z

This is on top of #91894

P2641R4

Currently, this doesn't strictly check "whose complete object's lifetime began within E". A bunch of the static_asserts I have for objects under construction should be ill-formed instead of false, but some of them should be true.

cor3ntin

This looks great.
Most of the changes to DiagnosticASTKinds.td could be done as a nice NFC cleanup PR.
My only real concern is the way we reject function pointer.
We also might want tests for member function pointers.

Are VLAs something we need to care about here @AaronBallman ?

We are missing release note + change to LanguageExtentions.rst

cor3ntin · 2024-08-05T14:03:30Z

clang/lib/AST/ExprConstant.cpp

+  // assert(Info.InConstantContext && "Call to consteval builtin not in constant
+  // context?");
+  assert(E->getBuiltinCallee() == Builtin::BI__builtin_is_within_lifetime);


Why is that commented? It certainly looks like something that should not be constant folded

It was being called from here:

llvm-project/clang/lib/Sema/SemaChecking.cpp

Line 12702 in b368404

CheckForIntOverflow(E);

I've now made it so it explicitly does not work in those sorts of cases

cor3ntin · 2024-08-05T14:25:32Z

clang/lib/Sema/SemaChecking.cpp

+  // A call to this function is always ill-formed if the type is not a pointer
+  // to an object type. There is no Mandates: to that effect, so we can only
+  // issue an error if it is actually evaluated as part of a constant evaluation
+  // (e.g., `false ? true :


I think this is not quite true.
The function is defined as consteval bool is_within_lifetime(const T* p) noexcept;
The const does a lot of heavy lifting there.
https://compiler-explorer.com/z/hdvfWEbYj

The const reject function pointer types (https://eel.is/c++draft/conv.qual#5)
So I think this should be handled there, rather than in the constexpr evaluator

You can write std::is_within_lifetime<int()>(&function) (https://compiler-explorer.com/z/GYn1sEjd8). It's not too much of a hassle to accept function pointers and fail them later since they have the same representation as object pointers during constant evaluation

cor3ntin · 2024-08-11T11:10:57Z

https://cplusplus.github.io/LWG/issue4138

MitalAshok · 2024-08-11T11:25:41Z

@cor3ntin A void* can be a pointer to an object but void* is not a pointer-to-object type. is_object_v<T> -> is_object_v<T> || is_void_v<T> or !is_function_v<T>

AaronBallman · 2024-08-12T13:08:08Z

Are VLAs something we need to care about here @AaronBallman ?

VLAs are always something we need to care about thanks to exposing them in C++. :-D We should add some test coverage to ensure the behavior is reasonable.

Other test cases to consider: lifetime around this in constructors and destructors, lifetime started via placement new/explicit destructor calls, and dangling references.

Also, we should update the status page, and what's the plan for the feature test macro?

MitalAshok · 2024-08-12T13:53:22Z

The feature test macro (__cpp_lib_is_within_lifetime) should be defined by the standard library in <type_traits>/<version> (so libc++ or libstdc++).
All that changes for Clang with this patch is __has_builtin(__builtin_is_within_lifetime). P2641R4 doesn't appear to be in cxx_status. It's on the libc++ C++26 status page, which can be updated when this is exposed as std::is_within_lifetime.

I'll add VLA tests. I'm surprised they are available at all during constant evaluation. I will make it just error on VLA types, since those can't be passed to the template function is_within_lifetime(const auto*). VLA objects will still be supported the same as any other object if they are used after being cast to void*.

I also don't think the current behaviour before/during constructors and during/after destructors is correct, and I have to add more tests (for basically every scenario in [basic.life]).

MitalAshok · 2024-08-12T21:01:00Z

Hmm. On reading https://eel.is/c++draft/meta.const.eval#4 again, it says:

Remarks: During the evaluation of an expression E as a core constant expression, a call to this function is ill-formed unless p points to an object that is usable in constant expressions or whose complete object's lifetime began within E.

So calling it inside constructors before the complete-object's lifetime has began (https://eel.is/c++draft/basic.life#1.2) should be ill-formed (instead of the current behaviour of false)

But I'm unsure about dangling pointers. The complete object's lifetime had begun, but it also ended. And https://cplusplus.github.io/CWG/issues/2822.html seems to make it implementation defined to use the pointer

hanickadot · 2024-08-19T18:01:44Z

clang/lib/AST/ExprConstant.cpp

@@ -3760,6 +3765,9 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
    if ((O->isAbsent() && !(handler.AccessKind == AK_Construct && I == N)) ||
        (O->isIndeterminate() &&
         !isValidIndeterminateAccess(handler.AccessKind))) {
+      // Object has ended lifetime since pointer was formed
+      if (handler.AccessKind == AK_IsWithinLifetime)
+        return false;


AFAIK you can't use bool here, it will be problem when findSubobject will be used with non-compatible return type

There are already other return false;/return true; returns in findSubobject, so findSubobject::result_type must be constructible from bool

hanickadot · 2024-08-19T18:01:52Z

clang/lib/AST/ExprConstant.cpp

@@ -3927,6 +3935,9 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
            // Placement new onto an inactive union member makes it active.
            O->setUnion(Field, APValue());
          } else {
+            // Pointer to/into inactive union member: Not within lifetime
+            if (handler.AccessKind == AK_IsWithinLifetime)
+              return false;


hanickadot · 2024-08-19T18:11:46Z

clang/lib/AST/ExprConstant.cpp

+  if (!CO)
+    return false;
+  IsWithinLifetimeHandler handler{Info};
+  return findSubobject(Info, E, CO, Val.getLValueDesignator(), handler);


associated with previous comment ... you are returning false but .failure() will return std::nullopt, so be aware :)

Yeah, it's intentional: std::nullopt -> this call was ill-formed, false -> this is a well-formed call and results in false, true -> this is a well-formed call that results in true. I especially want the second one to return false instead of nullopt

… from llvm#91895 as I thought I have an error)

…is_within_lifetime Squashed all previous commits (no longer in draft)

llvmbot · 2024-08-20T12:30:14Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Mital Ashok (MitalAshok)

Changes

P2641R4

This new builtin function is declared consteval. Support for -fexperimental-new-constant-interpreter will be added in a later patch.

Patch is 38.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91895.diff

11 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3)
(modified) clang/include/clang/Basic/Builtins.td (+6)
(modified) clang/include/clang/Basic/DiagnosticASTKinds.td (+9-3)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/lib/AST/ByteCode/State.h (+1)
(modified) clang/lib/AST/ExprConstant.cpp (+105-3)
(modified) clang/lib/CodeGen/CGBuiltin.cpp (+3)
(modified) clang/lib/Sema/SemaChecking.cpp (+36)
(modified) clang/lib/Sema/SemaExpr.cpp (+2-1)
(added) clang/test/SemaCXX/builtin-is-within-lifetime.cpp (+435)
(added) clang/test/SemaCXX/consteval-builtin.cpp (+93)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 249249971dec7c..131f31f45ea28b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -131,6 +131,9 @@ C++2c Feature Support
 
 - Implemented `P2893R3 Variadic Friends <https://wg21.link/P2893>`_
 
+- Added the ``__builtin_is_within_lifetime`` builtin, which supports
+  `P2641R4 Checking if a union alternative is active <https://wg21.link/p2641r4>`_
+
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 036366cdadf4aa..e2cbf2ac69107d 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -934,6 +934,12 @@ def IsConstantEvaluated : LangBuiltin<"CXX_LANG"> {
   let Prototype = "bool()";
 }
 
+def IsWithinLifetime : LangBuiltin<"CXX_LANG"> {
+  let Spellings = ["__builtin_is_within_lifetime"];
+  let Attributes = [NoThrow, CustomTypeChecking, Consteval];
+  let Prototype = "bool(void*)";
+}
+
 // GCC exception builtins
 def EHReturn : Builtin {
   let Spellings = ["__builtin_eh_return"];
diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index f317c5ac44f32b..581640b433568d 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -169,14 +169,14 @@ def note_constexpr_this : Note<
 def access_kind : TextSubstitution<
   "%select{read of|read of|assignment to|increment of|decrement of|"
   "member call on|dynamic_cast of|typeid applied to|construction of|"
-  "destruction of}0">;
+  "destruction of|read of}0">;
 def access_kind_subobject : TextSubstitution<
   "%select{read of|read of|assignment to|increment of|decrement of|"
   "member call on|dynamic_cast of|typeid applied to|"
-  "construction of subobject of|destruction of}0">;
+  "construction of subobject of|destruction of|read of}0">;
 def access_kind_volatile : TextSubstitution<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "<ERROR>|<ERROR>|<ERROR>|<ERROR>|<ERROR>}0">;
+  "<ERROR>|<ERROR>|<ERROR>|<ERROR>|<ERROR>|<ERROR>}0">;
 def note_constexpr_lifetime_ended : Note<
   "%sub{access_kind}0 %select{temporary|variable}1 whose "
   "%plural{8:storage duration|:lifetime}0 has ended">;
@@ -408,6 +408,12 @@ def warn_is_constant_evaluated_always_true_constexpr : Warning<
   "'%0' will always evaluate to 'true' in a manifestly constant-evaluated expression">,
   InGroup<DiagGroup<"constant-evaluated">>;
 
+def err_invalid_is_within_lifetime : Note<
+  "'%0' cannot be called with "
+  "%select{a null pointer|a function pointer|a one-past-the-end pointer|"
+  "a pointer to an object whose lifetime has not yet begun}1"
+>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4b6aadd635786a..e50ea9bf20763c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12190,6 +12190,9 @@ def err_builtin_launder_invalid_arg : Error<
   "%select{non-pointer|function pointer|void pointer}0 argument to "
   "'__builtin_launder' is not allowed">;
 
+def err_builtin_is_within_lifetime_invalid_arg : Error<
+  "non-pointer argument to '__builtin_is_within_lifetime' is not allowed">;
+
 def err_builtin_invalid_arg_type: Error <
   "%ordinal0 argument must be "
   "%select{a vector, integer or floating point type|a matrix|"
diff --git a/clang/lib/AST/ByteCode/State.h b/clang/lib/AST/ByteCode/State.h
index 44d6c037c5ad95..c163fd29222252 100644
--- a/clang/lib/AST/ByteCode/State.h
+++ b/clang/lib/AST/ByteCode/State.h
@@ -34,6 +34,7 @@ enum AccessKinds {
   AK_TypeId,
   AK_Construct,
   AK_Destroy,
+  AK_IsWithinLifetime,
 };
 
 /// The order of this enum is important for diagnostics.
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5540f58b526705..9f12fcf9ad7dce 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1522,7 +1522,8 @@ CallStackFrame::~CallStackFrame() {
 }
 
 static bool isRead(AccessKinds AK) {
-  return AK == AK_Read || AK == AK_ReadObjectRepresentation;
+  return AK == AK_Read || AK == AK_ReadObjectRepresentation ||
+         AK == AK_IsWithinLifetime;
 }
 
 static bool isModification(AccessKinds AK) {
@@ -1532,6 +1533,7 @@ static bool isModification(AccessKinds AK) {
   case AK_MemberCall:
   case AK_DynamicCast:
   case AK_TypeId:
+  case AK_IsWithinLifetime:
     return false;
   case AK_Assign:
   case AK_Increment:
@@ -1549,7 +1551,8 @@ static bool isAnyAccess(AccessKinds AK) {
 
 /// Is this an access per the C++ definition?
 static bool isFormalAccess(AccessKinds AK) {
-  return isAnyAccess(AK) && AK != AK_Construct && AK != AK_Destroy;
+  return isAnyAccess(AK) && AK != AK_Construct && AK != AK_Destroy &&
+         AK != AK_IsWithinLifetime;
 }
 
 /// Is this kind of axcess valid on an indeterminate object value?
@@ -1561,6 +1564,7 @@ static bool isValidIndeterminateAccess(AccessKinds AK) {
     // These need the object's value.
     return false;
 
+  case AK_IsWithinLifetime:
   case AK_ReadObjectRepresentation:
   case AK_Assign:
   case AK_Construct:
@@ -3707,7 +3711,8 @@ struct CompleteObject {
     // In C++14 onwards, it is permitted to read a mutable member whose
     // lifetime began within the evaluation.
     // FIXME: Should we also allow this in C++11?
-    if (!Info.getLangOpts().CPlusPlus14)
+    if (!Info.getLangOpts().CPlusPlus14 &&
+        AK != AccessKinds::AK_IsWithinLifetime)
       return false;
     return lifetimeStartedInEvaluation(Info, Base, /*MutableSubobject*/true);
   }
@@ -3760,6 +3765,12 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
     if ((O->isAbsent() && !(handler.AccessKind == AK_Construct && I == N)) ||
         (O->isIndeterminate() &&
          !isValidIndeterminateAccess(handler.AccessKind))) {
+      // Object has ended lifetime.
+      // If I is non-zero, some subobject (member or array element) of a
+      // complete object has ended its lifetime, so this is valid for
+      // IsWithinLifetime, resulting in false.
+      if (I != 0 && handler.AccessKind == AK_IsWithinLifetime)
+        return false;
       if (!Info.checkingPotentialConstantExpression())
         Info.FFDiag(E, diag::note_constexpr_access_uninit)
             << handler.AccessKind << O->isIndeterminate()
@@ -3927,6 +3938,9 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
             // Placement new onto an inactive union member makes it active.
             O->setUnion(Field, APValue());
           } else {
+            // Pointer to/into inactive union member: Not within lifetime
+            if (handler.AccessKind == AK_IsWithinLifetime)
+              return false;
             // FIXME: If O->getUnionValue() is absent, report that there's no
             // active union member rather than reporting the prior active union
             // member. We'll need to fix nullptr_t to not use APValue() as its
@@ -11667,6 +11681,9 @@ class IntExprEvaluator
 
   bool ZeroInitialization(const Expr *E) { return Success(0, E); }
 
+  friend std::optional<bool> EvaluateBuiltinIsWithinLifetime(IntExprEvaluator &,
+                                                             const CallExpr *);
+
   //===--------------------------------------------------------------------===//
   //                            Visitor Methods
   //===--------------------------------------------------------------------===//
@@ -12722,6 +12739,11 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     return Success(Info.InConstantContext, E);
   }
 
+  case Builtin::BI__builtin_is_within_lifetime:
+    if (auto result = EvaluateBuiltinIsWithinLifetime(*this, E))
+      return Success(*result, E);
+    return false;
+
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
   case Builtin::BI__builtin_ctzll:
@@ -17310,3 +17332,83 @@ bool Expr::tryEvaluateStrLen(uint64_t &Result, ASTContext &Ctx) const {
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);
   return EvaluateBuiltinStrLen(this, Result, Info);
 }
+
+namespace {
+struct IsWithinLifetimeHandler {
+  EvalInfo &Info;
+  static constexpr AccessKinds AccessKind = AccessKinds::AK_IsWithinLifetime;
+  using result_type = std::optional<bool>;
+  std::optional<bool> failed() { return std::nullopt; }
+  template <typename T>
+  std::optional<bool> found(T &Subobj, QualType SubobjType) {
+    return true;
+  }
+};
+
+std::optional<bool> EvaluateBuiltinIsWithinLifetime(IntExprEvaluator &IEE,
+                                                    const CallExpr *E) {
+  EvalInfo &Info = IEE.Info;
+  // Sometimes this is called during some sorts of constant folding / early
+  // evaluation. These are meant for non-constant expressions and are not
+  // necessary since this consteval builtin will never be evaluated at runtime.
+  // Just fail to evaluate when not in a constant context.
+  if (!Info.InConstantContext)
+    return std::nullopt;
+  assert(E->getBuiltinCallee() == Builtin::BI__builtin_is_within_lifetime);
+  const Expr *Arg = E->getArg(0);
+  if (Arg->isValueDependent())
+    return std::nullopt;
+  LValue Val;
+  if (!EvaluatePointer(Arg, Val, Info))
+    return std::nullopt;
+
+  auto Error = [&](int Diag) {
+    bool CalledFromStd = false;
+    const auto *Callee = Info.CurrentCall->getCallee();
+    if (Callee && Callee->isInStdNamespace()) {
+      const IdentifierInfo *Identifier = Callee->getIdentifier();
+      CalledFromStd = Identifier && Identifier->isStr("is_within_lifetime");
+    }
+    Info.CCEDiag(CalledFromStd ? Info.CurrentCall->getCallRange().getBegin()
+                               : E->getExprLoc(),
+                 diag::err_invalid_is_within_lifetime)
+        << (CalledFromStd ? "std::is_within_lifetime"
+                          : "__builtin_is_within_lifetime")
+        << Diag;
+    return std::nullopt;
+  };
+  // C++2c [meta.const.eval]p4:
+  //   During the evaluation of an expression E as a core constant expression, a
+  //   call to this function is ill-formed unless p points to an object that is
+  //   usable in constant expressions or whose complete object's lifetime began
+  //   within E.
+
+  // Make sure it points to an object
+  // nullptr does not point to an object
+  if (Val.isNullPointer() || Val.getLValueBase().isNull())
+    return Error(0);
+  QualType T = Val.getLValueBase().getType();
+  if (T->isFunctionType())
+    return Error(1);
+  assert(T->isObjectType());
+  // Hypothetical array element is not an object
+  if (Val.getLValueDesignator().isOnePastTheEnd())
+    return Error(2);
+  assert(Val.getLValueDesignator().isValidSubobject() &&
+         "Unchecked case for valid subobject");
+  // All other ill-formed values should have failed EvaluatePointer, so the
+  // object should be a pointer to an object that is usable in a constant
+  // expression or whose complete lifetime began within the expression
+  CompleteObject CO =
+      findCompleteObject(Info, E, AccessKinds::AK_IsWithinLifetime, Val, T);
+  // The lifetime hasn't begun yet if we are still evaluating the
+  // initializer ([basic.life]p(1.2))
+  if (Info.EvaluatingDeclValue && CO.Value == Info.EvaluatingDeclValue)
+    return Error(3);
+
+  if (!CO)
+    return false;
+  IsWithinLifetimeHandler handler{Info};
+  return findSubobject(Info, E, CO, Val.getLValueDesignator(), handler);
+}
+} // namespace
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f424ddaa175400..a69d98b6794a95 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2538,6 +2538,9 @@ static RValue EmitHipStdParUnsupportedBuiltin(CodeGenFunction *CGF,
 RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                                         const CallExpr *E,
                                         ReturnValueSlot ReturnValue) {
+  assert(!getContext().BuiltinInfo.isImmediate(BuiltinID) &&
+         "Should not codegen for consteval builtins");
+
   const FunctionDecl *FD = GD.getDecl()->getAsFunction();
   // See if we can constant fold this builtin.  If so, don't emit it at all.
   // TODO: Extend this handling to all builtin calls that we can constant-fold.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ee143381cf4f79..1d2fa342429e2c 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1844,6 +1844,40 @@ static ExprResult BuiltinLaunder(Sema &S, CallExpr *TheCall) {
   return TheCall;
 }
 
+static ExprResult BuiltinIsWithinLifetime(Sema &S, CallExpr *TheCall) {
+  if (S.checkArgCount(TheCall, 1))
+    return ExprError();
+
+  ExprResult Arg = S.DefaultFunctionArrayLvalueConversion(TheCall->getArg(0));
+  if (Arg.isInvalid())
+    return ExprError();
+  QualType ParamTy = Arg.get()->getType();
+  TheCall->setArg(0, Arg.get());
+  TheCall->setType(S.Context.BoolTy);
+
+  // A call to this function is always ill-formed if the type is not a pointer
+  // to an object type. There is no Mandates: to that effect, so we can only
+  // issue an error if it is actually evaluated as part of a constant evaluation
+  // (e.g., `false ? true : std::is_within_lifetime<void()>(nullptr);` is fine)
+  // However, `std::is_within_lifetime` will only take pointer types (allow
+  // non-const qualified too)
+  if (const auto *PT = ParamTy->getAs<PointerType>()) {
+    // Disallow VLAs too since those shouldn't be able to
+    // be a template parameter for `std::is_within_lifetime`
+    if (PT->getPointeeType()->isVariableArrayType()) {
+      S.Diag(TheCall->getArg(0)->getExprLoc(), diag::err_vla_unsupported)
+          << 1 << "__builtin_is_within_lifetime";
+      return ExprError();
+    }
+  } else {
+    S.Diag(TheCall->getArg(0)->getExprLoc(),
+           diag::err_builtin_is_within_lifetime_invalid_arg);
+    return ExprError();
+  }
+
+  return TheCall;
+}
+
 // Emit an error and return true if the current object format type is in the
 // list of unsupported types.
 static bool CheckBuiltinTargetNotInUnsupported(
@@ -2276,6 +2310,8 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   }
   case Builtin::BI__builtin_launder:
     return BuiltinLaunder(*this, TheCall);
+  case Builtin::BI__builtin_is_within_lifetime:
+    return BuiltinIsWithinLifetime(*this, TheCall);
   case Builtin::BI__sync_fetch_and_add:
   case Builtin::BI__sync_fetch_and_add_1:
   case Builtin::BI__sync_fetch_and_add_2:
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index f495658fe58641..207d6524156aa2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17577,7 +17577,8 @@ HandleImmediateInvocations(Sema &SemaRef,
         (SemaRef.inTemplateInstantiation() && !ImmediateEscalating)) {
       SemaRef.Diag(DR->getBeginLoc(), diag::err_invalid_consteval_take_address)
           << ND << isa<CXXRecordDecl>(ND) << FD->isConsteval();
-      SemaRef.Diag(ND->getLocation(), diag::note_declared_at);
+      if (!FD->getBuiltinID())
+        SemaRef.Diag(ND->getLocation(), diag::note_declared_at);
       if (auto Context =
               SemaRef.InnermostDeclarationWithDelayedImmediateInvocations()) {
         SemaRef.Diag(Context->Loc, diag::note_invalid_consteval_initializer)
diff --git a/clang/test/SemaCXX/builtin-is-within-lifetime.cpp b/clang/test/SemaCXX/builtin-is-within-lifetime.cpp
new file mode 100644
index 00000000000000..049a84bc13eac8
--- /dev/null
+++ b/clang/test/SemaCXX/builtin-is-within-lifetime.cpp
@@ -0,0 +1,435 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-unused %s -verify=expected,cxx20 -Wno-vla-cxx-extension
+// RUN: %clang_cc1 -std=c++23 -Wno-unused %s -verify=expected,sincecxx23 -Wno-vla-cxx-extension
+// RUN: %clang_cc1 -std=c++26 -Wno-unused %s -verify=expected,sincecxx23 -Wno-vla-cxx-extension
+// RUN: %clang_cc1 -std=c++26 -DINLINE_NAMESPACE -Wno-unused %s -verify=expected,sincecxx23 -Wno-vla-cxx-extension
+
+inline constexpr void* operator new(__SIZE_TYPE__, void* p) noexcept { return p; }
+namespace std {
+template<typename T, typename... Args>
+constexpr T* construct_at(T* p, Args&&... args) { return ::new((void*)p) T(static_cast<Args&&>(args)...); }
+template<typename T>
+constexpr void destroy_at(T* p) { p->~T(); }
+template<typename T>
+struct allocator {
+  constexpr T* allocate(__SIZE_TYPE__ n) { return static_cast<T*>(::operator new(n * sizeof(T))); }
+  constexpr void deallocate(T* p, __SIZE_TYPE__) { ::operator delete(p); }
+};
+using nullptr_t = decltype(nullptr);
+template<typename T, T v>
+struct integral_constant { static constexpr T value = v; };
+template<bool v>
+using bool_constant = integral_constant<bool, v>;
+using true_type = bool_constant<true>;
+using false_type = bool_constant<false>;
+#ifdef INLINE_NAMESPACE
+inline namespace __1 {
+#endif
+template<typename T>
+consteval bool is_within_lifetime(const T* p) noexcept {
+  return __builtin_is_within_lifetime(p);
+}
+#ifdef INLINE_NAMESPACE
+}
+#endif
+}
+
+consteval bool test_union(int& i, char& c) {
+  if (__builtin_is_within_lifetime(&i) || __builtin_is_within_lifetime(&c))
+    return false;
+  std::construct_at(&c, 1);
+  if (__builtin_is_within_lifetime(&i) || !__builtin_is_within_lifetime(&c))
+    return false;
+  std::construct_at(&i, 3);
+  if (!__builtin_is_within_lifetime(&i) || __builtin_is_within_lifetime(&c))
+    return false;
+  return true;
+}
+
+static_assert([]{
+  union { int i; char c; } u;
+  return test_union(u.i, u.c);
+}());
+static_assert([]{
+  union { int i; char c; };
+  return test_union(i, c);
+}());
+static_assert([]{
+  struct { union { int i; char c; }; } u;
+  return test_union(u.i, u.c);
+}());
+static_assert([]{
+  struct { union { int i; char c; } u; } r;
+  return test_union(r.u.i, r.u.c);
+}());
+
+consteval bool test_nested() {
+  union {
+    union { int i; char c; } u;
+    long l;
+  };
+  if (__builtin_is_within_lifetime(&l) || __builtin_is_within_lifetime(&u) || __builtin_is_within_lifetime(&u.i) || __builtin_is_within_lifetime(&u.c))
+    return false;
+  std::construct_at(&l);
+  if (!__builtin_is_within_lifetime(&l) || __builtin_is_within_lifetime(&u) || __builtin_is_within_lifetime(&u.i) || __builtin_is_within_lifetime(&u.c))
+    return false;
+  std::construct_at(&u);
+  std::construct_at(&u.i);
+  if (__builtin_is_within_lifetime(&l) || !__builtin_is_within_lifetime(&u) || !__builtin_is_within_lifetime(&u.i) || __builtin_is_within_lifetime(&u.c))
+    return false;
+  std::construct_at(&u.c);
+  if (__builtin_is_within_lifetime(&l) || !__builtin_is_within_lifetime(&u) || __builtin_is_within_lifetime(&u.i) || !__builtin_is_within_lifetime(&u.c))
+    return false;
+  return true;
+}
+static_assert(test_nested());
+
+consteval bool test_dynamic(bool read_after_deallocate) {
+  std::allocator<int> a;
+  int* p = a.allocate(1);
+  // a.allocate starts the lifetime of an array,
+  // the complete object of *p has started its lifetime
+  if (__builtin_is_within_lifetime(p))
+    return false;
+  std::construct_at(p);
+  if (!__builtin_is_within_lifetime(p))
+    return false;
+  std::destroy_at(p);
+  if (__builtin_is_within_lifetime(p))
+    return false;
+  a.deallocate(p, 1);
+  if (read_after_deallocate)
+    __builtin_is_within_lifetime(p); // expected-note {{read of heap allocated object that has been deleted}}
+  return true;
+}
+static_assert(test_dynamic(false));
+static_assert(test_dynamic(true));
+// expected-error@-1 {{static assertion expression is not an integral constant expression}}
+//   expected-note@-2 {{in call to 'test_dynamic(true)'}}
+
+consteval bool test_automatic(int read_dangling) {
+  int* p;
+  {
+    int x = 0;
+    p = &x;
+    if (!__builtin_is_within_lifetime(p))
+      return false;
+  }
+  {
+    int x = 0;
+    if (read_dangling == 1)
+      __builtin_is_within_lifetime(p); // expected-note {{read of object outside its lifetime is not allowed in a constant expression}}
+  }
+  if (read_dangling == 2)
+    __builtin_is_within_lifetime(p); // expected-note {{read of object outside its lifetime is not allowed in a constant expression}}
+  {
+    ...
[truncated]

cor3ntin · 2024-08-20T12:49:43Z

clang/lib/AST/ExprConstant.cpp

+  if (T->isFunctionType())
+    return Error(1);


I really want to see that be a sema check rather than a constant evaluation error (I am happy keeping the void* case for now as LWG is still talking about it)

cor3ntin · 2024-08-20T13:41:53Z

clang/include/clang/Basic/DiagnosticASTKinds.td

 def access_kind_subobject : TextSubstitution<
  "%select{read of|read of|assignment to|increment of|decrement of|"
  "member call on|dynamic_cast of|typeid applied to|"
-  "construction of subobject of|destruction of}0">;
+  "construction of subobject of|destruction of|read of}0">;


read of is already there (0)

This is for the messages shown in places like this:

constexpr struct { mutable int i } s; static_assert(__builtin_is_within_lifetime(&s.i)); // read of mutable member 'i' is not allowed in a constant expression

This is the same message as AK_Read and AK_ReadObjectRepresentation because is_within_lifetime has basically the same restrictions as reading the value of an object (you "read" the value just to check if it is in lifetime).

Open to other suggestions to make it more clear

Oh right. that's good enough then!

cor3ntin · 2024-08-20T13:44:56Z

clang/lib/Sema/SemaChecking.cpp

+  // This function should only be called through `std::is_within_lifetime`,
+  // which requires a pointer type


Not sure this comment make sense, the built in expect a pointer regardless of how it's called

cor3ntin

LGTM

cor3ntin · 2024-08-20T14:06:33Z

clang/include/clang/Basic/DiagnosticASTKinds.td

 def access_kind_subobject : TextSubstitution<
  "%select{read of|read of|assignment to|increment of|decrement of|"
  "member call on|dynamic_cast of|typeid applied to|"
-  "construction of subobject of|destruction of}0">;
+  "construction of subobject of|destruction of|read of}0">;


Oh right. that's good enough then!

cor3ntin · 2024-08-20T14:36:25Z

@MitalAshok will you need me to merge?

MitalAshok · 2024-08-20T14:48:31Z

@cor3ntin I will, yes. But this probably at least needs some libc++ reviewers

cor3ntin · 2024-08-21T21:16:31Z

@ldionne and @philnik777 you have an opinion on that builtin?

ldionne

I did a very cursory review and this seems good to me, the tests seem to cover what the paper was intending to provide.

ldionne · 2024-08-23T14:40:58Z

clang/test/SemaCXX/builtin-is-within-lifetime.cpp

+using false_type = bool_constant<false>;
+template<typename T>
+inline constexpr bool is_function_v = __is_function(T);
+#ifdef INLINE_NAMESPACE


I didn't read super carefully, but can you explain what's the purpose of the inline namespace here?

The diagnostic message goes from '__builtin_is_within_lifetime' -> 'std::is_within_lifetime' if the builtin is called from std::is_within_lifetime (See the last few tests in this file), and I'm checking that still works if the function is declared in an inline namespace (and libc++ might put it in an inline versioning namespace)

Ack, thanks.

…is_within_lifetime (llvm#91895) [P2641R4](https://wg21.link/P2641R4) This new builtin function is declared `consteval`. Support for `-fexperimental-new-constant-interpreter` will be added in a later patch. --------- Co-authored-by: cor3ntin <[email protected]>

MitalAshok marked this pull request as ready for review May 20, 2024 12:32

MitalAshok requested a review from tbaederr as a code owner May 20, 2024 12:32

cor3ntin reviewed Aug 5, 2024

View reviewed changes

MitalAshok mentioned this pull request Aug 5, 2024

[NFC] Deduplicate clang::AccessKinds to diagnostic strings #102030

Merged

MitalAshok marked this pull request as draft August 5, 2024 18:11

AaronBallman requested a review from philnik777 August 12, 2024 13:07

hanickadot reviewed Aug 19, 2024

View reviewed changes

hanickadot added a commit to hanickadot/llvm-project that referenced this pull request Aug 19, 2024

is_within_lifetime in stdlib + constexpr print (implementation steals…

e79cf90

… from llvm#91895 as I thought I have an error)

MitalAshok force-pushed the builtin-is-within-lifetime branch from 70f050b to c91517c Compare August 20, 2024 12:24

[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::…

f602189

…is_within_lifetime Squashed all previous commits (no longer in draft)

MitalAshok force-pushed the builtin-is-within-lifetime branch from c91517c to f602189 Compare August 20, 2024 12:26

Add release note

05a2514

MitalAshok marked this pull request as ready for review August 20, 2024 12:29

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Aug 20, 2024

cor3ntin reviewed Aug 20, 2024

View reviewed changes

LWG4138: Disallow function pointers

09a175b

cor3ntin reviewed Aug 20, 2024

View reviewed changes

Reword comments

70fc707

cor3ntin approved these changes Aug 20, 2024

View reviewed changes

ldionne reviewed Aug 23, 2024

View reviewed changes

Merge branch 'main' into builtin-is-within-lifetime

f4a3a18

cor3ntin merged commit 2a07509 into llvm:main Sep 5, 2024
9 checks passed

Zingam mentioned this pull request Sep 5, 2024

P2641R4: Checking if a union alternative is active #105381

Open

MitalAshok mentioned this pull request Sep 5, 2024

[libc++] P2641R4: Checking if a union alternative is active (std::is_within_lifetime) #107450

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::is_within_lifetime #91895

[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::is_within_lifetime #91895

MitalAshok commented May 12, 2024 •

edited

Loading

github-actions bot commented May 12, 2024 •

edited

Loading

MitalAshok commented May 12, 2024

cor3ntin left a comment

cor3ntin Aug 5, 2024

MitalAshok Aug 12, 2024

cor3ntin Aug 5, 2024

MitalAshok Aug 5, 2024

cor3ntin commented Aug 11, 2024

MitalAshok commented Aug 11, 2024

AaronBallman commented Aug 12, 2024

MitalAshok commented Aug 12, 2024

MitalAshok commented Aug 12, 2024

hanickadot Aug 19, 2024

MitalAshok Aug 20, 2024

hanickadot Aug 19, 2024

hanickadot Aug 19, 2024

MitalAshok Aug 20, 2024

llvmbot commented Aug 20, 2024 •

edited

Loading

cor3ntin Aug 20, 2024

cor3ntin Aug 20, 2024

MitalAshok Aug 20, 2024

cor3ntin Aug 20, 2024

cor3ntin Aug 20, 2024

cor3ntin left a comment

cor3ntin Aug 20, 2024

cor3ntin commented Aug 20, 2024

MitalAshok commented Aug 20, 2024

cor3ntin commented Aug 21, 2024

ldionne left a comment

ldionne Aug 23, 2024

MitalAshok Aug 23, 2024

ldionne Aug 23, 2024

		// This function should only be called through `std::is_within_lifetime`,
		// which requires a pointer type

[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::is_within_lifetime #91895

[Clang] Add __builtin_is_within_lifetime to implement P2641R4's std::is_within_lifetime #91895

Conversation

MitalAshok commented May 12, 2024 • edited Loading

github-actions bot commented May 12, 2024 • edited Loading

MitalAshok commented May 12, 2024

cor3ntin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cor3ntin commented Aug 11, 2024

MitalAshok commented Aug 11, 2024

AaronBallman commented Aug 12, 2024

MitalAshok commented Aug 12, 2024

MitalAshok commented Aug 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llvmbot commented Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cor3ntin commented Aug 20, 2024

MitalAshok commented Aug 20, 2024

cor3ntin commented Aug 21, 2024

ldionne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitalAshok commented May 12, 2024 •

edited

Loading

github-actions bot commented May 12, 2024 •

edited

Loading

llvmbot commented Aug 20, 2024 •

edited

Loading