Re: [PATCH] c++: Don't mix timevar_start and auto_cond_timevar for TV_NAME_LOOKUP [PR116681]

[Jason Merrill]

On 9/13/24 1:31 PM, Simon Martin wrote:

We currently ICE upon the following testcase when using -ftime-report

=== cut here ===
template < int> using __conditional_t = int;
template < typename _Iter >
concept random_access_iterator = requires { new _Iter; };
template < typename _Iterator >
struct reverse_iterator {
   using iterator_concept =
     __conditional_t< random_access_iterator< _Iterator>>;
};
void RemoveBottom() {
   int iter;
   for (reverse_iterator< int > iter;;)
       ;
}
=== cut here ===

The problem is that qualified_namespace_lookup does a plain start() of
the TV_NAME_LOOKUP timer (that asserts that the timer is not already
started). However this timer has already been cond_start()'d in the call
stack - by pushdecl - so the assert fails.

This patch simply ensures that we always conditionally start this timer
(which is done in all other places that use it).

OK, thanks.

Successfully tested on x86_64-pc-linux-gnu.

        PR c++/116681

gcc/cp/ChangeLog:

        * name-lookup.cc (qualified_namespace_lookup): Use an
        auto_cond_timer instead of using timevar_start and timevar_stop.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/concepts-pr116681.C: New test.

---
  gcc/cp/name-lookup.cc                         |  3 +--
  .../g++.dg/cpp2a/concepts-pr116681.C          | 20 +++++++++++++++++++
  2 files changed, 21 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index bfe17b7cb2f..c7a693e02d5 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -7421,10 +7421,9 @@ tree lookup_qualified_name (tree t, const char *p, 
LOOK_want w, bool c)
  static bool
  qualified_namespace_lookup (tree scope, name_lookup *lookup)
  {
-  timevar_start (TV_NAME_LOOKUP);
+  auto_cond_timevar tv (TV_NAME_LOOKUP);
    query_oracle (lookup->name);
    bool found = lookup->search_qualified (ORIGINAL_NAMESPACE (scope));
-  timevar_stop (TV_NAME_LOOKUP);
    return found;
  }

diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C

new file mode 100644
index 00000000000..f1b47797f1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C
@@ -0,0 +1,20 @@
+// PR c++/116681
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-ftime-report" }
+// { dg-allow-blank-lines-in-output 1 }
+// { dg-prune-output "Time variable" }
+// { dg-prune-output "k" }
+// { dg-prune-output "\[0-9\]+%" }
+
+template < int> using __conditional_t = int;
+template < typename _Iter >
+concept random_access_iterator = requires { new _Iter; };
+template < typename _Iterator > struct reverse_iterator {
+  using iterator_concept = __conditional_t< random_access_iterator< _Iterator 
>>;
+};
+void RemoveBottom()
+{
+  int iter;
+  for (reverse_iterator< int > iter;;)
+      ;
+}