Re: [PATCH] c++: modules and std::source_location::current() def arg [PR100881]

2022-12-08 Thread Nathan Sidwell via Gcc-patches

On 12/7/22 16:50, Patrick Palka wrote:

We currently declare __builtin_source_location with a const void* return
type instead of the true type (const std::source_location::__impl*), and
later when folding this builtin we just obtain the true type via name
lookup.

But the below testcase demonstrates this name lookup approach seems to
interact poorly with modules, since we may import an entity that uses
std::source_location::current() in a default argument (or DMI) without
also importing , and thus the name lookup will fail
when folding the builtin at the call site unless we also import
.

This patch fixes by instead initially declaring __builtin_source_location
with an auto return type and updating it appropriately upon its first use.
Thus when folding calls to this builtin we can fish out the true return
type through the type of the CALL_EXPR and avoid needing to do name
lookup.


That's a clever approach!  LGTM

nathan

--
Nathan Sidwell



[PATCH] c++: modules and std::source_location::current() def arg [PR100881]

2022-12-07 Thread Patrick Palka via Gcc-patches
We currently declare __builtin_source_location with a const void* return
type instead of the true type (const std::source_location::__impl*), and
later when folding this builtin we just obtain the true type via name
lookup.

But the below testcase demonstrates this name lookup approach seems to
interact poorly with modules, since we may import an entity that uses
std::source_location::current() in a default argument (or DMI) without
also importing , and thus the name lookup will fail
when folding the builtin at the call site unless we also import
.

This patch fixes by instead initially declaring __builtin_source_location
with an auto return type and updating it appropriately upon its first use.
Thus when folding calls to this builtin we can fish out the true return
type through the type of the CALL_EXPR and avoid needing to do name
lookup.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
reasonable?

PR c++/100881

gcc/cp/ChangeLog:

* constexpr.cc (cxx_eval_builtin_function_call): Adjust calls
to fold_builtin_source_location.
* cp-gimplify.cc (cp_gimplify_expr): Likewise.
(cp_fold): Likewise.
(get_source_location_impl_type): Remove location_t parameter and
adjust accordingly.  No longer static.
(fold_builtin_source_location): Take a CALL_EXPR tree instead of a
location and obtain the impl type from its return type.
* cp-tree.h (enum cp_tree_index): Remove CPTI_SOURCE_LOCATION_IMPL
enumerator.
(source_location_impl): Remove.
(fold_builtin_source_location): Adjust parameter type.
(get_source_location_impl_type): Declare.
* decl.cc (cxx_init_decl_processing): Declare
__builtin_source_location with auto return type instead of
const void*.
(require_deduced_type): Update the return type of
__builtin_source_location.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/srcloc3.C: Adjust expected note s/evaluating/using.
* g++.dg/cpp2a/srcloc4.C: Likewise.
* g++.dg/cpp2a/srcloc5.C: Likewise.
* g++.dg/cpp2a/srcloc6.C: Likewise.
* g++.dg/cpp2a/srcloc7.C: Likewise.
* g++.dg/cpp2a/srcloc8.C: Likewise.
* g++.dg/cpp2a/srcloc9.C: Likewise.
* g++.dg/cpp2a/srcloc10.C: Likewise.
* g++.dg/cpp2a/srcloc11.C: Likewise.
* g++.dg/cpp2a/srcloc12.C: Likewise.
* g++.dg/cpp2a/srcloc13.C: Likewise.
* g++.dg/modules/pr100881_a.C: New test.
* g++.dg/modules/pr100881_b.C: New test.
---
 gcc/cp/constexpr.cc   |  2 +-
 gcc/cp/cp-gimplify.cc | 58 +++
 gcc/cp/cp-tree.h  |  8 +---
 gcc/cp/decl.cc| 25 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc10.C |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc11.C |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc12.C |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc13.C |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc3.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc4.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc5.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc6.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc7.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc8.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/srcloc9.C  |  2 +-
 gcc/testsuite/g++.dg/modules/pr100881_a.C | 28 +++
 gcc/testsuite/g++.dg/modules/pr100881_b.C |  8 
 17 files changed, 101 insertions(+), 50 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr100881_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/pr100881_b.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0b43ae4ece3..6ff994fd599 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1492,7 +1492,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, 
tree t, tree fun,
   temp_override ovr (current_function_decl);
   if (ctx->call && ctx->call->fundef)
current_function_decl = ctx->call->fundef->decl;
-  return fold_builtin_source_location (EXPR_LOCATION (t));
+  return fold_builtin_source_location (t);
 }
 
   int strops = 0;
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 983f2a566a6..6ad8458ab28 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -722,7 +722,7 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, 
gimple_seq *post_p)
break;
  case CP_BUILT_IN_SOURCE_LOCATION:
*expr_p
- = fold_builtin_source_location (EXPR_LOCATION (*expr_p));
+ = fold_builtin_source_location (*expr_p);
break;
  case CP_BUILT_IN_IS_CORRESPONDING_MEMBER:
*expr_p
@@ -2850,7 +2850,7 @@ cp_fold (tree x)
  case CP_BUILT_IN_IS_CONSTANT_EVALUATED:
break;
  case CP_BUILT_IN_SOURCE_LOCATION:
-   x = fold_builtin_source_location (EXPR_LOCATION (x));
+