[PATCH] D136549: [clang] Fix time profile in "isIntegerConstantExpr"

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron created this revision.
Izaron added reviewers: dyung, aaron.ballman, jloser.
Herald added a project: All.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The time profiler in `Expr::isIntegerConstantExpr` used to
call `Loc->printToString`, it was inconsistent with other time
profiles in the file and caused segfaults if `Loc` was `nullptr`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136549

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/unittests/Support/TimeProfilerTest.cpp


Index: clang/unittests/Support/TimeProfilerTest.cpp
===================================================================
--- clang/unittests/Support/TimeProfilerTest.cpp
+++ clang/unittests/Support/TimeProfilerTest.cpp
@@ -37,14 +37,14 @@
 
 // Returns true if code compiles successfully.
 // We only parse AST here. This is enough for constexpr evaluation.
-bool compileFromString(StringRef Code) {
+bool compileFromString(StringRef Code, StringRef Standard, StringRef FileName) {
   CompilerInstance Compiler;
   Compiler.createDiagnostics();
 
   auto Invocation = std::make_shared<CompilerInvocation>();
   Invocation->getPreprocessorOpts().addRemappedFile(
-      "test.cc", MemoryBuffer::getMemBuffer(Code).release());
-  const char *Args[] = {"-std=c++20", "test.cc"};
+      FileName, MemoryBuffer::getMemBuffer(Code).release());
+  const char *Args[] = {Standard.data(), FileName.data()};
   CompilerInvocation::CreateFromArgs(*Invocation, Args,
                                      Compiler.getDiagnostics());
   Compiler.setInvocation(std::move(Invocation));
@@ -143,7 +143,7 @@
 
 } // namespace
 
-TEST(TimeProfilerTest, ConstantEvaluation) {
+TEST(TimeProfilerTest, ConstantEvaluationCxx20) {
   constexpr StringRef Code = R"(
 void print(double value);
 
@@ -172,7 +172,7 @@
     )";
 
   setupProfiler();
-  ASSERT_TRUE(compileFromString(Code));
+  ASSERT_TRUE(compileFromString(Code, "-std=c++20", "test.cc"));
   std::string Json = teardownProfiler();
   std::string TraceGraph = buildTraceGraph(Json);
   ASSERT_TRUE(TraceGraph == R"(
@@ -197,3 +197,25 @@
   // NOTE: If this test is failing, run this test with
   // `llvm::errs() << TraceGraph;` and change the assert above.
 }
+
+TEST(TimeProfilerTest, ConstantEvaluationC99) {
+  constexpr StringRef Code = R"(
+struct {
+  short quantval[4]; // 3rd line
+} value;
+    )";
+
+  setupProfiler();
+  ASSERT_TRUE(compileFromString(Code, "-std=c99", "test.c"));
+  std::string Json = teardownProfiler();
+  std::string TraceGraph = buildTraceGraph(Json);
+  ASSERT_TRUE(TraceGraph == R"(
+Frontend
+| isIntegerConstantExpr (<test.c:3:18>)
+| EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)
+| PerformPendingInstantiations
+)");
+
+  // NOTE: If this test is failing, run this test with
+  // `llvm::errs() << TraceGraph;` and change the assert above.
+}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -15905,9 +15905,7 @@
   assert(!isValueDependent() &&
          "Expression evaluator can't be called on a dependent expression.");
 
-  llvm::TimeTraceScope TimeScope("isIntegerConstantExpr", [&] {
-    return Loc->printToString(Ctx.getSourceManager());
-  });
+  ExprTimeTraceScope TimeScope(this, Ctx, "isIntegerConstantExpr");
 
   if (Ctx.getLangOpts().CPlusPlus11)
     return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, nullptr, Loc);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136549.469974.patch
Type: text/x-patch
Size: 2955 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221023/fcb4b99d/attachment-0001.bin>