ilya-biryukov updated this revision to Diff 456663.
ilya-biryukov added a comment.

- Special-case the std::source_location::current calls, delay until codegen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/SemaCXX/source_location.cpp
  clang/test/SemaCXX/source_location_consteval.cpp

Index: clang/test/SemaCXX/source_location_consteval.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/source_location_consteval.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+namespace std {
+class source_location {
+  struct __impl;
+
+public:
+  static consteval source_location current(const __impl *__p = __builtin_source_location()) noexcept {
+    source_location __loc;
+    __loc.__m_impl = __p;
+    return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_impl ? __m_impl->_M_line : 0; }
+  constexpr unsigned int column() const noexcept { return __m_impl ? __m_impl->_M_column : 0; }
+  constexpr const char *file() const noexcept { return __m_impl ? __m_impl->_M_file_name : ""; }
+  constexpr const char *function() const noexcept { return __m_impl ? __m_impl->_M_function_name : ""; }
+
+private:
+  // Note: The type name "std::source_location::__impl", and its constituent
+  // field-names are required by __builtin_source_location().
+  struct __impl {
+    const char *_M_file_name;
+    const char *_M_function_name;
+    unsigned _M_line;
+    unsigned _M_column;
+  };
+  const __impl *__m_impl = nullptr;
+
+public:
+  using public_impl_alias = __impl;
+};
+} // namespace std
+
+using SL = std::source_location;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+    if (*LHS != *RHS)
+      return false;
+    ++LHS;
+    ++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+constexpr SL get_sl(SL l = SL::current()) { return l; }
+SL get_sl_not_const(SL l = SL::current()) { return l; }
+
+#line 700 "CheckDefaultArg.h"
+constexpr SL l = get_sl();
+static_assert(l.line() == 700);
+static_assert(is_equal(l.file(), "CheckDefaultArg.h"));
+
+
+consteval SL get_sl_rec_consteval(SL l = get_sl()) { return l; }
+constexpr SL get_sl_rec_constexpr(SL l = get_sl()) { return l; }
+
+static_assert(get_sl_rec_constexpr().line() == __LINE__);
+static_assert(get_sl_rec_consteval().line() == __LINE__);
+
+int test() {
+  static_assert(is_equal(get_sl().function(), __PRETTY_FUNCTION__));
+  static_assert(get_sl().line() ==  __LINE__);
+  return get_sl().line() + get_sl_not_const().line();
+}
Index: clang/test/SemaCXX/source_location.cpp
===================================================================
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++20 -fcxx-exceptions -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -13,7 +13,7 @@
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
+  static consteval source_location current(const __impl *__p = __builtin_source_location()) noexcept {
     source_location __loc;
     __loc.__m_impl = __p;
     return __loc;
@@ -364,8 +364,8 @@
 template <class T>
 void func_template_tests() {
   constexpr auto P = test_func_template(42);
-  //static_assert(is_equal(P.first.function(), __func__), "");
-  //static_assert(!is_equal(P.second.function(), __func__), "");
+  static_assert(is_equal(P.first.function(), __PRETTY_FUNCTION__), "");
+  static_assert(!is_equal(P.second.function(), __PRETTY_FUNCTION__), "");
 }
 template void func_template_tests<int>();
 
Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===================================================================
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -no-opaque-pointers -std=c++2a -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
+// RUN: %clang_cc1 -no-opaque-pointers -std=c++20 -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
 
 // This needs to be performed before #line directives which alter filename
-// RUN: %clang_cc1 -no-opaque-pointers -fno-file-reproducible -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-PREFIX-MAP
+// RUN: %clang_cc1 -no-opaque-pointers -std=c++20 -fno-file-reproducible -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-PREFIX-MAP
 //
 // CHECK-PREFIX-MAP: /UNLIKELY/PATH{{/|\\\\}}builtin-source-location.cpp
 void testRemap() {
@@ -13,13 +13,15 @@
 namespace std {
 class source_location {
 public:
-  static constexpr source_location current(const void *__p = __builtin_source_location()) noexcept {
+  static consteval source_location current(const void *__p = __builtin_source_location()) noexcept {
     source_location __loc;
     __loc.__m_impl = static_cast<const __impl *>(__p);
     return __loc;
   }
   static source_location bad_current(const void *__p = __builtin_source_location()) noexcept {
-    return current(__p);
+    source_location __loc;
+    __loc.__m_impl = static_cast<const __impl *>(__p);
+    return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -79,7 +81,7 @@
 // CHECK-LOCAL-ONE-DAG: @[[IMPL:.*]] = private unnamed_addr constant %"struct.std::source_location::__impl" { {{[^@]*}}@[[FILE]], {{[^@]*}}@[[FUNC]], {{.*}} i32 2100, i32 {{[0-9]+}} }, align 8
 //
 // CHECK-LOCAL-ONE:  define {{.*}} @test_function
-// CHECK-LOCAL-ONE:  call %"struct.std::source_location::__impl"* @_ZNSt15source_location7currentEPKv({{.*}} @[[IMPL]]
+// CHECK-LOCAL-ONE:  store %"struct.std::source_location::__impl"* @.constant.{{.*}}, %"struct.std::source_location::__impl"** %0, align 8
 #line 2100 "test_current.cpp"
   SL local = SL::current();
 }
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4492,7 +4492,7 @@
   Expr *UninstExpr = Param->getUninstantiatedDefaultArg();
 
   EnterExpressionEvaluationContext EvalContext(
-      *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+      *this, ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, Param);
 
   // Instantiate the expression.
   //
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -26,6 +26,7 @@
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConcept.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
@@ -2464,6 +2465,10 @@
       // and non-defining declarations.
       Sema::ContextRAII SavedContext(*this, OwningFunc);
       LocalInstantiationScope Local(*this, true);
+      EnterExpressionEvaluationContext EvalContext(
+          *this, ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed,
+          Sema::ReuseLambdaContextDecl);
+
       ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
       if (NewArg.isUsable()) {
         // It would be nice if we still had this.
@@ -3033,7 +3038,8 @@
   // we don't have a scope.
   ContextRAII SavedContext(*this, Instantiation->getParent());
   EnterExpressionEvaluationContext EvalContext(
-      *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+      *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, nullptr,
+      ExpressionEvaluationContextRecord::EK_Other, Sema::DelaySourceLocation);
 
   LocalInstantiationScope Scope(*this, true);
 
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5848,6 +5848,23 @@
       ArraySubscriptExpr(LHSExp, RHSExp, ResultType, VK, OK, RLoc);
 }
 
+static bool IsStdSourceLocationCurrent(Decl *D) {
+  auto *F = D ? dyn_cast<FunctionDecl>(D) : nullptr;
+  auto *FuncName = F ? F->getIdentifier() : nullptr;
+  if (!FuncName || !FuncName->isStr("current") || !F->isConsteval())
+    return false;
+  auto *Cls = dyn_cast<RecordDecl>(F->getDeclContext());
+  auto *ClsName = Cls ? Cls->getIdentifier() : nullptr;
+  return ClsName && ClsName->isStr("source_location") &&
+         Cls->isInStdNamespace();
+}
+
+static bool IsCallToStdSourceLocationCurrent(Expr *E) {
+  CallExpr *Call = dyn_cast<CallExpr>(E);
+  auto *Callee = Call ? Call->getCalleeDecl() : nullptr;
+  return Callee && IsStdSourceLocationCurrent(Callee);
+}
+
 bool Sema::CheckCXXDefaultArgExpr(SourceLocation CallLoc, FunctionDecl *FD,
                                   ParmVarDecl *Param) {
   if (Param->hasUnparsedDefaultArg()) {
@@ -17395,10 +17412,10 @@
   return TransformToPE(*this).TransformType(TInfo);
 }
 
-void
-Sema::PushExpressionEvaluationContext(
+void Sema::PushExpressionEvaluationContext(
     ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl,
-    ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
+    ExpressionEvaluationContextRecord::ExpressionKind ExprContext,
+    SourceLocationCurrentBehavior_t SourceLocBehavior) {
   ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
                                 LambdaContextDecl, ExprContext);
 
@@ -17411,6 +17428,10 @@
   ExprEvalContexts.back().InImmediateFunctionContext =
       ExprEvalContexts[ExprEvalContexts.size() - 2]
           .isImmediateFunctionContext();
+  if (NewContext == ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed ||
+      SourceLocBehavior == DelaySourceLocation) {
+    ExprEvalContexts.back().DelaySourceLocationCurrentCalls = true;
+  }
 
   Cleanup.reset();
   if (!MaybeODRUseExprs.empty())
@@ -17519,11 +17540,18 @@
 }
 
 static void EvaluateAndDiagnoseImmediateInvocation(
-    Sema &SemaRef, Sema::ImmediateInvocationCandidate Candidate) {
+    Sema &SemaRef, Sema::ImmediateInvocationCandidate Candidate,
+    bool DelaySourceLocCalls) {
   llvm::SmallVector<PartialDiagnosticAt, 8> Notes;
   Expr::EvalResult Eval;
   Eval.Diag = &Notes;
   ConstantExpr *CE = Candidate.getPointer();
+  // [support.srcloc.cons]p2.
+  // Any call to current that appears as a default argument ([dcl.fct.default]),
+  // or as a subexpression thereof, should correspond to the location of the
+  // invocation of the function that uses the default argument ([expr.call]).
+  if (DelaySourceLocCalls && IsCallToStdSourceLocationCurrent(CE->getSubExpr()))
+    return; // Postpone evaluation until we see the context.
   bool Result = CE->EvaluateAsConstantExpr(
       Eval, SemaRef.getASTContext(), ConstantExprKind::ImmediateInvocation);
   if (!Result || !Notes.empty()) {
@@ -17664,7 +17692,8 @@
   }
   for (auto CE : Rec.ImmediateInvocationCandidates)
     if (!CE.getInt())
-      EvaluateAndDiagnoseImmediateInvocation(SemaRef, CE);
+      EvaluateAndDiagnoseImmediateInvocation(
+          SemaRef, CE, Rec.shouldDelayCallsToStdSourceLocationCurrent());
   for (auto DR : Rec.ReferenceToConsteval) {
     auto *FD = cast<FunctionDecl>(DR->getDecl());
     SemaRef.Diag(DR->getBeginLoc(), diag::err_invalid_consteval_take_address)
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1532,7 +1532,8 @@
 
     EnterExpressionEvaluationContext PotentiallyDiscarded(
         Actions, Context, nullptr,
-        Sema::ExpressionEvaluationContextRecord::EK_Other, ShouldEnter);
+        Sema::ExpressionEvaluationContextRecord::EK_Other,
+        Sema::EvaluateSourceLocation, ShouldEnter);
     ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc);
   }
 
@@ -1577,7 +1578,8 @@
 
     EnterExpressionEvaluationContext PotentiallyDiscarded(
         Actions, Context, nullptr,
-        Sema::ExpressionEvaluationContextRecord::EK_Other, ShouldEnter);
+        Sema::ExpressionEvaluationContextRecord::EK_Other,
+        Sema::EvaluateSourceLocation, ShouldEnter);
     ElseStmt = ParseStatement();
 
     if (ElseStmt.isUsable())
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3147,7 +3147,9 @@
          "Data member initializer not starting with '=' or '{'");
 
   EnterExpressionEvaluationContext Context(
-      Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, D);
+      Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, D,
+      Sema::ExpressionEvaluationContextRecord::EK_Other,
+      Sema::DelaySourceLocation);
   if (TryConsumeToken(tok::equal, EqualLoc)) {
     if (Tok.is(tok::kw_delete)) {
       // In principle, an initializer of '= delete p;' is legal, but it will
Index: clang/lib/CodeGen/CGExprConstant.cpp
===================================================================
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
@@ -1392,14 +1393,27 @@
 }
 
 llvm::Constant *ConstantEmitter::tryEmitConstantExpr(const ConstantExpr *CE) {
-  if (!CE->hasAPValueResult())
-    return nullptr;
+  APValue Result;
+  if (CE->hasAPValueResult()) {
+    Result = CE->getAPValueResult();
+  } else {
+    if (!CE->isImmediateInvocation())
+      return nullptr;
+    // Handle delayed immediate invocations.
+    // These are rare, std::source_location::current() is an example.
+    Expr::EvalResult Eval;
+    if (!CE->EvaluateAsConstantExpr(Eval, CGM.getContext(),
+                                    ConstantExprKind::ImmediateInvocation) ||
+        Eval.HasSideEffects)
+      return nullptr;
+    Result = std::move(Eval.Val);
+  }
 
   QualType RetType = CE->getType();
   if (CE->isGLValue())
     RetType = CGM.getContext().getLValueReferenceType(RetType);
 
-  return emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
+  return emitAbstract(CE->getBeginLoc(), Result, RetType);
 }
 
 llvm::Constant *
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1326,6 +1326,10 @@
     // an immediate function context, so they need to be tracked independently.
     bool InDiscardedStatement;
     bool InImmediateFunctionContext;
+    // Set to true in contexts where the standard requires to avoid evaluating
+    // immediate calls to std::source_location::current(). These are default
+    // arguments and class field initializers.
+    bool DelaySourceLocationCurrentCalls;
 
     ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
                                       unsigned NumCleanupObjects,
@@ -1335,7 +1339,8 @@
         : Context(Context), ParentCleanup(ParentCleanup),
           NumCleanupObjects(NumCleanupObjects), NumTypos(0),
           ManglingContextDecl(ManglingContextDecl), ExprContext(ExprContext),
-          InDiscardedStatement(false), InImmediateFunctionContext(false) {}
+          InDiscardedStatement(false), InImmediateFunctionContext(false),
+          DelaySourceLocationCurrentCalls(false) {}
 
     bool isUnevaluated() const {
       return Context == ExpressionEvaluationContext::Unevaluated ||
@@ -1360,6 +1365,10 @@
                   ExpressionEvaluationContext::ImmediateFunctionContext &&
               InDiscardedStatement);
     }
+
+    bool shouldDelayCallsToStdSourceLocationCurrent() const {
+      return DelaySourceLocationCurrentCalls;
+    }
   };
 
   /// A stack of expression evaluation contexts.
@@ -5262,10 +5271,16 @@
   void DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
                              ArrayRef<Expr *> Args);
 
+  enum SourceLocationCurrentBehavior_t {
+    EvaluateSourceLocation,
+    DelaySourceLocation
+  };
   void PushExpressionEvaluationContext(
       ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl = nullptr,
       ExpressionEvaluationContextRecord::ExpressionKind Type =
-          ExpressionEvaluationContextRecord::EK_Other);
+          ExpressionEvaluationContextRecord::EK_Other,
+      SourceLocationCurrentBehavior_t SourceLocBehavior =
+          EvaluateSourceLocation);
   enum ReuseLambdaContextDecl_t { ReuseLambdaContextDecl };
   void PushExpressionEvaluationContext(
       ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t,
@@ -9436,6 +9451,12 @@
     return ExprEvalContexts.back().isImmediateFunctionContext();
   }
 
+  bool shouldDelayCallsToStdSourceLocationCurrent() const {
+    assert(!ExprEvalContexts.empty() &&
+           "Must be in an expression evaluation context");
+    return ExprEvalContexts.back().shouldDelayCallsToStdSourceLocationCurrent();
+  }
+
   /// RAII class used to determine whether SFINAE has
   /// trapped any errors that occur during template argument
   /// deduction.
@@ -13558,11 +13579,13 @@
       Decl *LambdaContextDecl = nullptr,
       Sema::ExpressionEvaluationContextRecord::ExpressionKind ExprContext =
           Sema::ExpressionEvaluationContextRecord::EK_Other,
+      Sema::SourceLocationCurrentBehavior_t SourceLocBehavior =
+          Sema::EvaluateSourceLocation,
       bool ShouldEnter = true)
       : Actions(Actions), Entered(ShouldEnter) {
     if (Entered)
       Actions.PushExpressionEvaluationContext(NewContext, LambdaContextDecl,
-                                              ExprContext);
+                                              ExprContext, SourceLocBehavior);
   }
   EnterExpressionEvaluationContext(
       Sema &Actions, Sema::ExpressionEvaluationContext NewContext,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D129488: [Sema] Fix ... Ilya Biryukov via Phabricator via cfe-commits

Reply via email to