Re: [PATCH] Remove poly_int_pod

[Jason Merrill]

On 9/28/23 05:55, Richard Sandiford wrote:

poly_int was written before the switch to C++11 and so couldn't
use explicit default constructors.  This led to an awkward split
between poly_int_pod and poly_int.  poly_int simply inherited from
poly_int_pod and added constructors, with the argumentless constructor
having an empty body.  But inheritance meant that poly_int had to
repeat the assignment operators from poly_int_pod (again, no C++11,
so no "using" to inherit base-class implementations).

All that goes away if we switch to using default constructors.

The main complication is ensuring that braced initialisation still
gives a constexpr, so that static variables can be initialised without
runtime code.  The two problems here are:

(1) When initialising a poly_int with fewer than N
 coefficients, the other coefficients need to be a zero of
 the same precision as the explicit coefficients.  This was
 previously done in a for loop using wi::ints_for<...>::zero,
 but C++11 constexpr constructors can't have function bodies.
 The patch instead uses a series of delegated initialisers to
 fill in the implicit coefficients.


Perhaps it's time to update the bootstrap requirement to C++14 (i.e. GCC 
5, from eight years ago).  Not that this would affect this particular patch.


Jason

[PATCH v2 RFA] diagnostic: add permerror variants with opt

[Jason Merrill]

This revision changes from using DK_PEDWARN for permerror-with-option to using
DK_PERMERROR.

Tested x86_64-pc-linux-gnu.  OK for trunk?

-- 8< --

In the discussion of promoting some pedwarns to be errors by default, rather
than move them all into -fpermissive it seems to me to make sense to support
DK_PERMERROR with an option flag.  This way will also work with
-fpermissive, but users can also still use -Wno-error=narrowing to downgrade
that specific diagnostic rather than everything affected by -fpermissive.

So, for diagnostics that we want to make errors by default we can just
change the pedwarn call to permerror.

The tests check desired behavior for such a permerror in a system header
with various flags.  The patch preserves the existing permerror behavior of
ignoring -w and system headers by default, but respecting them when
downgraded to a warning by -fpermissive.

This seems similar to but a bit better than the approach of forcing
-pedantic-errors that I previously used for -Wnarrowing: specifically, in
that now -w by itself is not enough to silence the -Wnarrowing
error (integer-pack2.C).

gcc/ChangeLog:

* doc/invoke.texi: Move -fpermissive to Warning Options.
* diagnostic.cc (update_effective_level_from_pragmas): Remove
redundant system header check.
(diagnostic_report_diagnostic): Move down syshdr/-w check.
(diagnostic_impl): Handle DK_PERMERROR with an option number.
(permerror): Add new overloads.
* diagnostic-core.h (permerror): Declare them.

gcc/cp/ChangeLog:

* typeck2.cc (check_narrowing): Use permerror.

gcc/testsuite/ChangeLog:

* g++.dg/ext/integer-pack2.C: Add -fpermissive.
* g++.dg/diagnostic/sys-narrow.h: New test.
* g++.dg/diagnostic/sys-narrow1.C: New test.
* g++.dg/diagnostic/sys-narrow1a.C: New test.
* g++.dg/diagnostic/sys-narrow1b.C: New test.
* g++.dg/diagnostic/sys-narrow1c.C: New test.
* g++.dg/diagnostic/sys-narrow1d.C: New test.
* g++.dg/diagnostic/sys-narrow1e.C: New test.
* g++.dg/diagnostic/sys-narrow1f.C: New test.
* g++.dg/diagnostic/sys-narrow1g.C: New test.
* g++.dg/diagnostic/sys-narrow1h.C: New test.
* g++.dg/diagnostic/sys-narrow1i.C: New test.
---
 gcc/doc/invoke.texi   | 22 +++---
 gcc/diagnostic-core.h |  3 +
 gcc/testsuite/g++.dg/diagnostic/sys-narrow.h  |  2 +
 gcc/cp/typeck2.cc | 10 +--
 gcc/diagnostic.cc | 67 ---
 gcc/testsuite/g++.dg/diagnostic/sys-narrow1.C |  4 ++
 .../g++.dg/diagnostic/sys-narrow1a.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1b.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1c.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1d.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1e.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1f.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1g.C  |  5 ++
 .../g++.dg/diagnostic/sys-narrow1h.C  |  6 ++
 .../g++.dg/diagnostic/sys-narrow1i.C  |  6 ++
 gcc/testsuite/g++.dg/ext/integer-pack2.C  |  2 +-
 16 files changed, 117 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow.h
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1a.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1b.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1c.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1d.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1e.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1f.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1g.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1h.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1i.C

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4085fc90907..6b6506a75b2 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -231,7 +231,7 @@ in the following sections.
 -fnew-inheriting-ctors
 -fnew-ttp-matching
 -fno-nonansi-builtins  -fnothrow-opt  -fno-operator-names
--fno-optional-diags  -fpermissive
+-fno-optional-diags
 -fno-pretty-templates
 -fno-rtti  -fsized-deallocation
 -ftemplate-backtrace-limit=@var{n}
@@ -323,7 +323,7 @@ Objective-C and Objective-C++ Dialects}.
 @item Warning Options
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
 @gccoptlist{-fsyntax-only  -fmax-errors=@var{n}  -Wpedantic
--pedantic-errors
+-pedantic-errors -fpermissive
 -w  -Wextra  -Wall  -Wabi=@var{n}
 -Waddress  -Wno-address-of-packed-member  -Waggregate-return
 -Walloc-size-larger-than=@var{byte-size}  -Walloc-zero
@@ -3494,12 +3494,6 @@ Disable diagnostics that the standard says a compiler 
does not need to
 issue.  Currently, the only such diagnostic issued by G++ is the 

Re: [PATCH] c++: print source code in print_instantiation_partial_context_line

[Jason Merrill]

On 10/3/23 12:48, David Malcolm wrote:

As mentioned in my Cauldron talk, this patch adds a call to
diagnostic_show_locus to the "required from here" messages
in print_instantiation_partial_context_line, so that e.g., rather
than the rather mystifying:

In file included from ../x86_64-pc-linux-gnu/libstdc++-v3/include/memory:78,
  from ../../src/demo-1.C:1:
../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h: In instantiation of 
‘std::__detail::__unique_ptr_t<_Tp> std::make_unique(_Args&& ...) [with _Tp = bar; _Args = 
{}; __detail::__unique_ptr_t<_Tp> = __detail::__unique_ptr_t]’:
../../src/demo-1.C:15:32:   required from here
../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h:1066:30: error: 
no matching function for call to ‘bar::bar()’
  1066 | { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); 
}
   |  ^~~
../../src/demo-1.C:10:3: note: candidate: ‘bar::bar(int)’
10 |   bar (int);
   |   ^~~
../../src/demo-1.C:10:3: note:   candidate expects 1 argument, 0 provided
../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(const bar&)’
 7 | class bar : public foo
   |   ^~~
../../src/demo-1.C:7:7: note:   candidate expects 1 argument, 0 provided
../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(bar&&)’
../../src/demo-1.C:7:7: note:   candidate expects 1 argument, 0 provided

we emit:

In file included from ../x86_64-pc-linux-gnu/libstdc++-v3/include/memory:78,
  from ../../src/demo-1.C:1:
../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h: In instantiation of 
‘std::__detail::__unique_ptr_t<_Tp> std::make_unique(_Args&& ...) [with _Tp = bar; _Args = 
{}; __detail::__unique_ptr_t<_Tp> = __detail::__unique_ptr_t]’:
../../src/demo-1.C:15:32:   required from here
15 |   return std::make_unique ();
   |  ~~^~
../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h:1066:30: error: 
no matching function for call to ‘bar::bar()’
  1066 | { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); 
}
   |  ^~~
../../src/demo-1.C:10:3: note: candidate: ‘bar::bar(int)’
10 |   bar (int);
   |   ^~~
../../src/demo-1.C:10:3: note:   candidate expects 1 argument, 0 provided
../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(const bar&)’
 7 | class bar : public foo
   |   ^~~
../../src/demo-1.C:7:7: note:   candidate expects 1 argument, 0 provided
../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(bar&&)’
../../src/demo-1.C:7:7: note:   candidate expects 1 argument, 0 provided

which shows the code that's leading to the error (the bad call to
std::make_unique).


Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?


OK, thanks.  Now that you mention it, that's long been a small annoyance 
that never quite reached the point that it occurred to me to fix it.


Jason



gcc/cp/ChangeLog:
* error.cc (print_instantiation_partial_context_line): Call
diagnostic_show_locus.

gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/static_assert3.C: Add directives for
additional source printing.
* g++.dg/template/error60.C: New test.

Signed-off-by: David Malcolm 
---
  gcc/cp/error.cc   |  2 +
  .../g++.dg/diagnostic/static_assert3.C|  7 +++-
  gcc/testsuite/g++.dg/template/error60.C   | 37 +++
  3 files changed, 45 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/template/error60.C

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index ef96e140f24..767478cf5fd 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -3774,6 +3774,8 @@ print_instantiation_partial_context_line 
(diagnostic_context *context,
   ? _("recursively required from here\n")
   : _("required from here\n"));
  }
+  gcc_rich_location rich_loc (loc);
+  diagnostic_show_locus (context, &rich_loc, DK_NOTE);
  }
  
  /* Same as print_instantiation_full_context but less verbose.  */

diff --git a/gcc/testsuite/g++.dg/diagnostic/static_assert3.C 
b/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
index 5d363884508..4ec53f17120 100644
--- a/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
+++ b/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
@@ -5,6 +5,11 @@
  template  struct is_same { static constexpr bool 
value = false; };
  template  struct is_same { static constexpr bool value = 
true; };
  
+/* { dg-begin-multiline-output "" }

+  f(0, 1.3);
+  ~^~~~
+   { dg-end-multiline-output "" } */
+
  template 
  void f(T, U)
  {
@@ -32,5 +37,5 @@ void f(T, U)
  
  void g()

  {
- f(0, 1.3);
+ f(0, 1.3); // { dg-message " required from here" }
  }
diff --git a/gcc/testsuite/g++.dg/template/error60.C 
b/gcc/testsuite/g++.dg/template/error60.C
new file mode 1

Re: [PATCH] c++: merge tsubst_copy into tsubst_copy_and_build

[Jason Merrill]

On 10/3/23 08:41, Patrick Palka wrote:

On Mon, 2 Oct 2023, Patrick Palka wrote:


Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

The relationship between tsubst_copy_and_build and tsubst_copy (two of
the main template argument substitution routines for expression trees)
is rather hazy.  The former is mostly a superset of the latter, with
some differences.

The main difference is that they handle many tree codes differently, but
much of the tree code handling in tsubst_copy appears to be dead code[1].
This is because tsubst_copy only gets directly called in a few places
and mostly on id-expressions.  The interesting exceptions are PARM_DECL,
VAR_DECL, BIT_NOT_EXPR, SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE:

  * for PARM_DECL and VAR_DECL, tsubst_copy_and_build calls tsubst_copy
followed by doing some extra handling of its own
  * for BIT_NOT_EXPR tsubst_copy implicitly handles unresolved destructor
calls (i.e. the first operand is an identifier or a type)
  * for SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE tsubst_copy
refrains from doing name lookup of the terminal name

Other more minor differences are that tsubst_copy exits early when
'args' is null, and it calls maybe_dependent_member_ref


That's curious, since what that function does seems like name lookup; I 
wouldn't think we would want to call it when tf_no_name_lookup.



and finally it dispatches to tsubst for type trees.


And it looks like you fix the callers to avoid that?


Thus tsubst_copy is (at this point) similar enough to tsubst_copy_and_build
that it makes sense to merge the two functions, with the main difference
being the name lookup behavior[2].  So this patch merges tsubst_copy into
tsubst_copy_and_build via a new tsubst tf_no_name_lookup which controls
name lookup and resolution of a (top-level) id-expression.

[1]: http://thrifty.mooo.com:8008/gcc-lcov/gcc/cp/pt.cc.gcov.html#17231
[2]: I don't know the history of tsubst_copy but I would guess it was
added before we settled on using processing_template_decl to control
whether our AST building routines perform semantic checking and return
non-templated trees, and so we needed a separate tsubst routine that
avoids semantic checking and always returns a templated tree for e.g.
partial substitution.


Oops, this is wrong -- tsubst_copy_and_build came after tsubst_copy,
and was introduced as an optimization with the intent of getting rid
of tsubst_copy eventually:
https://gcc.gnu.org/pipermail/gcc-patches/2003-January/093659.html


I wonder if we want to add a small tsubst_name wrapper to call 
tsubst_copy_and_build with tf_no_name_lookup?


Can we also merge in tsubst_expr and use that name instead of the 
unwieldy tsubst_copy_and_build?


Jason

Re: [PATCH v2 RFA] diagnostic: add permerror variants with opt

[Jason Merrill]

On 10/4/23 03:07, Florian Weimer wrote:

* Jason Merrill:


@@ -6159,6 +6153,18 @@ errors by @option{-pedantic-errors}.  For instance:
  -Wwrite-strings @r{(C++11 or later)}
  }
  
+@opindex fpermissive

+@item -fpermissive
+Downgrade some required diagnostics about nonconformant code from
+errors to warnings.  Thus, using @option{-fpermissive} allows some
+nonconforming code to compile.  Some C++ diagnostics are controlled
+only by this flag, but it also downgrades some diagnostics that have
+their own flag:
+
+@gccoptlist{
+-Wnarrowing @r{(C++)}
+}
+
  @opindex Wall
  @opindex Wno-all
  @item -Wall


Does compiling with -Wno-narrowing also accept the obsolete constructs?
The documentation isn't clear about it.  The existing test
gcc/testsuite/g++.dg/cpp0x/initlist55.C suggests it's possible.  Maybe
add an explicit example to the documentation?


The documentation for -Wnarrowing already says that -Wno-narrowing 
suppresses the diagnostic.  It seems clear to me that if there's no 
error, the construct is accepted?



What about the impact of -w?


As I said in the patch comment, -w by itself has no effect. 
-fpermissive -w silences the diagnostic.



As far as the internal API is concerned, will there be a way to query
whether -Wno-narrowing etc. have been specified?


There's warning_enabled_at, but that doesn't account for the different 
rules for permerrors, such as the above -w behavior.  We could add a 
similar function for permerrors, that could also indicate whether the 
diagnostic is a warning or error?


More generally there's OPTION_SET_P.

Jason

Re: [PATCH] wwwdocs: Add ADL to C++ non-bugs

[Jason Merrill]

On 10/3/23 10:45, Jonathan Wakely wrote:

We have a long history of INVALID bugs about std functions being
available in the global namespace (PRs 27846, 67566, 82619, 99865,
110602, 111553, probably others). Let's document it.

Also de-prioritize the C++98-only bugs, which are unlikely to affect
anybody nowadays.

OK for wwwdocs?


OK, thanks.

Jason

Re: [PATCH] c++: Improve diagnostics for constexpr cast from void*

[Jason Merrill]

On 10/9/23 06:03, Nathaniel Shead wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu with
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.

-- >8 --

This patch improves the errors given when casting from void* in C++26 to
include the expected type if the type of the pointed-to object was
not similar to the casted-to type.

It also ensures (for all standard modes) that void* casts are checked
even for DECL_ARTIFICIAL declarations, such as lifetime-extended
temporaries, and is only ignored for cases where we know it's OK (heap
identifiers and source_location::current). This provides more accurate
diagnostics when using the pointer and ensures that some other casts
from void* are now correctly rejected.

gcc/cp/ChangeLog:

* constexpr.cc (is_std_source_location_current): New.
(cxx_eval_constant_expression): Only ignore cast from void* for
specific cases and improve other diagnostics.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-cast4.C: New test.

Signed-off-by: Nathaniel Shead 
---
  gcc/cp/constexpr.cc  | 83 +---
  gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
  2 files changed, 78 insertions(+), 12 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..f38d541a662 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
  && is_std_allocator_allocate (call->fundef->decl));
  }
  
+/* Return true if FNDECL is std::source_location::current.  */

+
+static inline bool
+is_std_source_location_current (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+return false;
+
+  tree name = DECL_NAME (fndecl);
+  if (name == NULL_TREE || !id_equal (name, "current"))
+return false;
+
+  tree ctx = DECL_CONTEXT (fndecl);
+  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
+return false;
+
+  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
+  return name && id_equal (name, "source_location");
+}
+
+/* Overload for the above taking constexpr_call*.  */
+
+static inline bool
+is_std_source_location_current (const constexpr_call *call)
+{
+  return (call
+ && call->fundef
+ && is_std_source_location_current (call->fundef->decl));
+}
+
  /* Return true if FNDECL is __dynamic_cast.  */
  
  static inline bool

@@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
if (TYPE_PTROB_P (type)
&& TYPE_PTR_P (TREE_TYPE (op))
&& VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
-   /* Inside a call to std::construct_at or to
-  std::allocator::{,de}allocate, we permit casting from void*
+   /* Inside a call to std::construct_at,
+  std::allocator::{,de}allocate, or
+  std::source_location::current, we permit casting from void*
   because that is compiler-generated code.  */
&& !is_std_construct_at (ctx->call)
-   && !is_std_allocator_allocate (ctx->call))
+   && !is_std_allocator_allocate (ctx->call)
+   && !is_std_source_location_current (ctx->call))
  {
/* Likewise, don't error when casting from void* when OP is
   &heap uninit and similar.  */
tree sop = tree_strip_nop_conversions (op);
-   if (TREE_CODE (sop) == ADDR_EXPR
-   && VAR_P (TREE_OPERAND (sop, 0))
-   && DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
+   tree decl = NULL_TREE;
+   if (TREE_CODE (sop) == ADDR_EXPR)
+ decl = TREE_OPERAND (sop, 0);
+   if (decl
+   && VAR_P (decl)
+   && DECL_ARTIFICIAL (decl)
+   && (DECL_NAME (decl) == heap_identifier
+   || DECL_NAME (decl) == heap_uninit_identifier
+   || DECL_NAME (decl) == heap_vec_identifier
+   || DECL_NAME (decl) == heap_vec_uninit_identifier))
  /* OK */;
/* P2738 (C++26): a conversion from a prvalue P of type "pointer to
   cv void" to a pointer-to-object type T unless P points to an
   object whose type is similar to T.  */
-   else if (cxx_dialect > cxx23
-&& (sop = cxx_fold_indirect_ref (ctx, loc,
- TREE_TYPE (type), sop)))
+   else if (cxx_dialect > cxx23)
  {
-   r = build1 (ADDR_EXPR, type, sop);
-   break;
+   r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
+   if (r)
+ {
+   r = build1 (ADDR_EXPR, type, r);
+   break;
+ }
+   if (!ctx->quiet)
+ {
+   if (TREE_CODE (sop) == ADDR_EXPR)
+ {
+ 

Re: [PATCH v4] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286]

[Jason Merrill]

On 10/8/23 21:03, Nathaniel Shead wrote:

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html

+ && (TREE_CODE (t) == MODIFY_EXPR
+ /* Also check if initializations have implicit change of active
+member earlier up the access chain.  */
+ || !refs->is_empty())


I'm not sure what the cumulative point of these two tests is.  TREE_CODE 
(t) will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.


As I understand it, the problematic case is something like 
constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what 
is this check doing?


Incidentally, I think constexpr-union6.C could use a test where we pass 
&u.s to a function other than construct_at, and then try (and fail) to 
assign to the b member from that function.


Jason

[pushed] c++: mangle multiple levels of template parms [PR109422]

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

This becomes be more important with concepts, but can also be seen with
generic lambdas.

PR c++/109422

gcc/cp/ChangeLog:

* mangle.cc (write_template_param): Also mangle level.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/lambda-generic-mangle1.C: New test.
* g++.dg/cpp2a/lambda-generic-mangle1a.C: New test.
---
 gcc/cp/mangle.cc| 13 +
 gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1.C |  9 +
 .../g++.dg/cpp2a/lambda-generic-mangle1a.C  | 10 ++
 3 files changed, 32 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1a.C

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index d88c779bfa2..d079f724910 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3921,6 +3921,7 @@ static void
 write_template_param (const tree parm)
 {
   int parm_index;
+  int level;
 
   MANGLE_TRACE_TREE ("template-parm", parm);
 
@@ -3930,10 +3931,12 @@ write_template_param (const tree parm)
 case TEMPLATE_TEMPLATE_PARM:
 case BOUND_TEMPLATE_TEMPLATE_PARM:
   parm_index = TEMPLATE_TYPE_IDX (parm);
+  level = TEMPLATE_TYPE_LEVEL (parm);
   break;
 
 case TEMPLATE_PARM_INDEX:
   parm_index = TEMPLATE_PARM_IDX (parm);
+  level = TEMPLATE_PARM_LEVEL (parm);
   break;
 
 default:
@@ -3941,6 +3944,16 @@ write_template_param (const tree parm)
 }
 
   write_char ('T');
+  if (level > 1)
+{
+  if (abi_warn_or_compat_version_crosses (19))
+   G.need_abi_warning = 1;
+  if (abi_version_at_least (19))
+   {
+ write_char ('L');
+ write_compact_number (level - 1);
+   }
+}
   /* NUMBER as it appears in the mangling is (-1)-indexed, with the
  earliest template param denoted by `_'.  */
   write_compact_number (parm_index);
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1.C 
b/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1.C
new file mode 100644
index 000..0051307f53d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1.C
@@ -0,0 +1,9 @@
+// PR c++/109422
+// { dg-do compile { target c++20 } }
+
+struct C {
+  template
+  void f(decltype([](T, auto) { return 0; })) {}
+};
+void g() { C().f({}); }
+// { dg-final { scan-assembler "_ZN1C1fIiEEvDTtlNS_UlT_TL0__E_EEE" } }
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1a.C 
b/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1a.C
new file mode 100644
index 000..dc7b0125631
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-generic-mangle1a.C
@@ -0,0 +1,10 @@
+// PR c++/109422
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fabi-version=18" }
+
+struct C {
+  template
+  void f(decltype([](T, auto) { return 0; })) {}
+};
+void g() { C().f({}); }
+// { dg-final { scan-assembler "_ZN1C1fIiEEvDTtlNS_UlT_T_E_EEE" } }

base-commit: aaa5a5318adbefe87c1b781b8a3e5fc332e661ec
-- 
2.39.3

Re: [PATCH v6] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286]

[Jason Merrill]

On 10/12/23 04:53, Nathaniel Shead wrote:

On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:

On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:

On 10/8/23 21:03, Nathaniel Shead wrote:

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html

+ && (TREE_CODE (t) == MODIFY_EXPR
+ /* Also check if initializations have implicit change of active
+member earlier up the access chain.  */
+ || !refs->is_empty())


I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.

As I understand it, the problematic case is something like
constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
this check doing?


The reasoning was to correctly handle cases like the the following (in
constexpr-union6.C):

   constexpr int test1() {
 U u {};
 std::construct_at(&u.s, S{ 1, 2 });
 return u.s.b;
   }
   static_assert(test1() == 2);

The initialisation of &u.s here is not a member access expression within
the call to std::construct_at, since it's just a pointer, but this code
is still legal; in general, an INIT_EXPR to initialise a union member
should always be OK (I believe?), hence constraining to just
MODIFY_EXPR.

However, just that would then (incorrectly) allow all the following
cases in that test to compile, such as

   constexpr int test2() {
 U u {};
 int* p = &u.s.b;
 std::construct_at(p, 5);
 return u.s.b;
   }
   constexpr int x2 = test2();

since the INIT_EXPR is really only initialising 'b', but the implicit
"modification" of active member to 'u.s' is illegal.

Maybe a better way of expressing this condition would be something like
this?

   /* An INIT_EXPR of the last member in an access chain is always OK,
  but still check implicit change of members earlier on; see
  cpp2a/constexpr-union6.C.  */
   && !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())

Otherwise I'll see if I can rework some of the other conditions instead.


Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
to a function other than construct_at, and then try (and fail) to assign to
the b member from that function.

Jason



Sounds good; I've added the following test:

   constexpr void foo(S* s) {
 s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized 
.U::k." }
   }
   constexpr int test3() {
 U u {};
 foo(&u.s);  // { dg-message "in .constexpr. expansion" }
 return u.s.b;
   }
   constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }

Incidentally I found this particular example caused a very unhelpful
error + ICE due to reporting that S could not be value-initialized in
the current version of the patch. The updated version below fixes that
by using 'build_zero_init' instead -- is this an appropriate choice
here?

A similar (but unrelated) issue is with e.g.
   
   struct S { const int a; int b; };

   union U { int k; S s; };

   constexpr int test() {
 U u {};
 return u.s.b;
   }
   constexpr int x = test();

giving me this pretty unhelpful error message:

/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
 6 |   return u.s.b;
   |  ~~^
/home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default 
definition would be ill-formed:
 1 | struct S { const int a; int b; };
   |^
/home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
/home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
 1 | struct S { const int a; int b; };
   |  ^
/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
 6 |   return u.s.b;
   |  ~~^
/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’

but I'll try and fix this separately (it exists on current trunk without
this patch as well).


While attempting to fix this I found a much better way of handling
value-initialised unions. Here's a new version of the patch which also
includes the fix for accessing the wrong member of a value-initialised
union as well.

Additionally includes an `auto_diagnostic_group` for the `inform`
diagnostics as Marek helpfully informed me about in my other patch.

Bootstrapped and regtested on x86_64-pc-linux-gnu.


@@ -4496,21 +4491,36 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, 
tree t,

break;
}
  }
-  if (TREE_CODE (TREE_TYPE (whole)) == UNION_TYPE
-  && CONSTRUCTOR_NELTS (whole) > 0)
+  if (TREE_COD

Re: [PATCH] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

 PR c++/111660

gcc/cp/ChangeLog:

 * cp-gimplify.cc (cp_fold_immediate_r) : Return
 integer_zero_node instead of break;.
 (cp_fold_immediate): Return true if cp_fold_immediate_r returned
 error_mark_node.

gcc/testsuite/ChangeLog:

 * g++.dg/cpp0x/hog1.C: New test.
---
  gcc/cp/cp-gimplify.cc |  9 ++--
  gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
  2 files changed, 82 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
/* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from 
looking any farther.


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?

Jason

Re: [PATCH v2] c++: implement P2564, consteval needs to propagate up [PR107687]

[Jason Merrill]

On 10/10/23 13:20, Marek Polacek wrote:

Thanks for looking into this.  It's kept me occupied for quite a while.


Thanks, nice work.


On Tue, Aug 29, 2023 at 03:26:46PM -0400, Jason Merrill wrote:

On 8/23/23 15:49, Marek Polacek wrote:

+struct A {
+  int x;
+  int y = id(x);
+};
+
+template
+constexpr int k(int) {  // k is not an immediate function because 
A(42) is a
+  return A(42).y;   // constant expression and thus not 
immediate-escalating
+}


Needs use(s) of k to test the comment.


True, and that revealed what I think is a bug in the standard.
In the test I'm saying:

// ??? [expr.const]#example-9 says:
//   k is not an immediate function because A(42) is a
//   constant expression and thus not immediate-escalating
// But I think the call to id(x) is *not* a constant expression and thus
// it is an immediate-escalating expression.  Therefore k *is*
// an immediate function.  So we get the error below.  clang++ agrees.
  
id(x) is not a constant expression because x isn't constant.


Not when considering id(x) by itself, but in the evaluation of A(42), 
the member x has just been initialized to constant 42.  And A(42) is 
constant-evaluated because "An aggregate initialization is an immediate 
invocation if it evaluates a default member initializer that has a 
subexpression that is an immediate-escalating expression."


I assume clang doesn't handle this passage properly yet because it was 
added during core review of the paper, for parity between aggregate 
initialization and constructor escalation.


This can be a follow-up patch.


So.  I think we want to refrain from instantiating things early
given how many problems that caused.  On the other hand, stashing
all the immediate-escalating decls into immediate_escalating_decls
and walking their bodies isn't going to be cheap.  I've checked
how big the vectors can get, but our testsuite isn't the best litmus
test because it's mostly smallish testcases without many #includes.
The worst offender is uninit-pr105562.C with

(gdb) p immediate_escalating_decls->length()
$2 = 2204
(gdb) p deferred_escalating_exprs->length()
$3 = 501

Compiling uninit-pr105562.C with g++13 and g++14 with this patch:
real 7.51 real 7.67
user 7.32 user 7.49
sys 0.15  sys 0.14

I've made sure not to walk the same bodies twice.  But there's room
for further optimization; I suppose we could escalate instantiated
functions right away rather than putting them into
immediate_escalating_decls and waiting till later.


Absolutely; if we see a call to a known consteval function, we should 
escalate...immediately.  As the patch seems to do already?



I'm not certain
if I can just look at DECL_TEMPLATE_INSTANTIATED.


I'm not sure what you mean, but a constexpr function being instantiated 
doesn't necessarily imply that everything it calls has been 
instantiated, so we might not know yet if it needs to escalate.



I suppose some
functions cannot possibly be promoted because they don't contain
any CALL_EXPRs.  So we may be able to rule them out while doing
cp_fold_r early.


Yes.  Or, the only immediate-escalating functions referenced have 
already been checked.


We can also do some escalation during constexpr evaluation: all the 
functions involved need to be instantiated for the evaluation, and if we 
encounter an immediate-escalating expression while evaluating a call to 
an immediate-escalating function, we can promote it then.  Though we 
can't necessarily mark it as not needing promotion, as there might be 
i-e exprs in branches that the particular evaluation doesn't take.



If a function is trivial, can it ever be promoted?


This might be a question for CWG.  clang thinks so:

struct A {
  consteval A() = default;
  consteval A(const A&) = default;
  int i;
};
struct B : A { }; // B constructors promote to consteval
B b; // ok, constant-initialization
B b2(b); // error, immediate invocation isn't constant


And so on.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
This patch implements P2564, described at , whereby
certain functions are promoted to consteval.  For example:

   consteval int id(int i) { return i; }

   template 
   constexpr int f(T t)
   {
 return t + id(t); // id causes f to be promoted to consteval
   }

   void g(int i)
   {
 f (3);
   }

now compiles.  Previously the code was ill-formed: we would complain
that 't' in 'f' is not a constant expression.  Since 'f' is now
consteval, it means that the call to id(t) is in an immediate context,
so doesn't have to produce a constant -- this is how we allow consteval
functions composition.  But making 'f' consteval also means that
the call to 'f' in 'g' must yield a constant; failure to do so results
in an error.  I made the effort to

Re: [PATCH v2] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/13/23 14:53, Marek Polacek wrote:

On Thu, Oct 12, 2023 at 09:41:43PM -0400, Jason Merrill wrote:

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

  PR c++/111660

gcc/cp/ChangeLog:

  * cp-gimplify.cc (cp_fold_immediate_r) : Return
  integer_zero_node instead of break;.
  (cp_fold_immediate): Return true if cp_fold_immediate_r returned
  error_mark_node.

gcc/testsuite/ChangeLog:

  * g++.dg/cpp0x/hog1.C: New test.
---
   gcc/cp/cp-gimplify.cc |  9 ++--
   gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
   2 files changed, 82 insertions(+), 4 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
 if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
 if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
 /* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from
looking any farther.


You are right.  The line above works as expected, but

   1 ? 1 : ((1 ? 1 : id (42)), id (i));

shows the problem (when the expression isn't used as an initializer).


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?


I hope this version is better.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.


Is this number still accurate for this version?

This change seems algorithmically better than the current code, but 
still problematic: if we have nested COND_EXPR A/B/C/D/E, it looks like 
we will end up cp_fold_immediate_r walking the arms of E five times, 
once for each COND_EXPR.


What I was thinking by handling COND_EXPR in cp_fold_r was to cp_fold_r 
walk its subtrees (or cp_fold_immediate_r if it's clear from op0 that 
the branch isn't taken) so we can clear *walk_subtrees and we don't 
fold_imm walk a node more than once.



The ff_fold_immediate flag is unused after this patch but since I'm
using it in the P2564 patch, I'm not removing it now.

 PR c++/111660

gcc/cp/ChangeLog:

 * cp-gimplify.cc (cp_fold_immediate_r) : Don't
handle it here.
 (cp_fold_r): Handle COND_EXPR here.

gcc/testsuite/ChangeLog:

 * g++.dg/cpp0x/hog1.C: New test.
* g++.dg/cpp2a/consteval36.C: New test.
---
  gcc/cp/cp-gimplify.cc| 38 +---
  gcc/testsuite/g++.dg/cpp0x/hog1.C| 77 
  gcc/testsuite/g++.dg/cpp2a/consteval36.C | 18 ++
  3 files changed, 111 insertions(+), 22 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval36.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..801cba141cb 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1052,27 +1052,6 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
  
switch (TREE_CODE (stmt))

  {
-/* Unfortunately we must handle code like

Re: [PATCH] c++: fix truncated diagnostic in C++23 [PR111272]

[Jason Merrill]

On 10/13/23 18:15, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?


OK.


-- >8 --
In C++23, since P2448, a constexpr function F that calls a non-constexpr
function N is OK as long as we don't actually call F in a constexpr
context.  So instead of giving an error in maybe_save_constexpr_fundef,
we only give an error when evaluating the call.  Unfortunately, as shown
in this PR, the diagnostic can be truncated:

z.C:10:13: note: 'constexpr Jam::Jam()' is not usable as a 'constexpr' function 
because:
10 |   constexpr Jam() { ft(); }
   | ^~~

...because what?  With this patch, we say:

z.C:10:13: note: 'constexpr Jam::Jam()' is not usable as a 'constexpr' function 
because:
10 |   constexpr Jam() { ft(); }
   | ^~~
z.C:10:23: error: call to non-'constexpr' function 'int Jam::ft()'
10 |   constexpr Jam() { ft(); }
   | ~~^~
z.C:8:7: note: 'int Jam::ft()' declared here
 8 |   int ft() { return 42; }
   |   ^~

Like maybe_save_constexpr_fundef, explain_invalid_constexpr_fn should
also check the body of a constructor, not just the mem-initializer.

PR c++/111272

gcc/cp/ChangeLog:

* constexpr.cc (explain_invalid_constexpr_fn): Also check the body of
a constructor in C++14 and up.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1y/constexpr-diag1.C: New test.
---
  gcc/cp/constexpr.cc  | 10 +-
  gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C | 21 
  2 files changed, 30 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..dde4fec4a44 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1098,7 +1098,15 @@ explain_invalid_constexpr_fn (tree fun)
  body = massage_constexpr_body (fun, body);
  require_potential_rvalue_constant_expression (body);
  if (DECL_CONSTRUCTOR_P (fun))
-   cx_check_missing_mem_inits (DECL_CONTEXT (fun), body, true);
+   {
+ cx_check_missing_mem_inits (DECL_CONTEXT (fun), body, true);
+ if (cxx_dialect > cxx11)
+   {
+ /* Also check the body, not just the ctor-initializer.  */
+ body = DECL_SAVED_TREE (fun);
+ require_potential_rvalue_constant_expression (body);
+   }
+   }
}
  }
  }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C
new file mode 100644
index 000..0e2909e83ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C
@@ -0,0 +1,21 @@
+// PR c++/111272
+// { dg-do compile { target c++14 } }
+// { dg-options "-Werror=invalid-constexpr" }
+// { dg-prune-output "some warnings being treated as errors" }
+
+struct Jam
+{
+  // constexpr  // n.b.
+  int ft() { return 42; } // { dg-message "declared here" }
+
+  constexpr Jam() { ft(); } // { dg-error "call to non-.constexpr. function" }
+// { dg-message "declared here" "" { target c++20_down } .-1 }
+};
+
+constexpr bool test()
+{
+  Jam j; // { dg-error "called in a constant expression" }
+  return true;
+}
+
+static_assert(test(), ""); // { dg-error "non-constant condition" }

base-commit: d78fef5371759849944966dec65d9e987efba509

[pushed] c++: improve fold-expr location

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

I want to distinguish between constraint && and fold-expressions there of
written by the user and those implied by template parameter
type-constraints; to that end, let's improve our EXPR_LOCATION for an
explicit fold-expression.

The fold3.C change is needed because this moves the caret from the end of
the expression to the operator, which means the location of the error refers
to the macro invocation rather than the macro definition; both locations are
still printed, but which one is an error and which a note changes.

gcc/cp/ChangeLog:

* parser.cc (cp_parser_fold_expression): Track location range.
* semantics.cc (finish_unary_fold_expr)
(finish_left_unary_fold_expr, finish_right_unary_fold_expr)
(finish_binary_fold_expr): Add location parm.
* constraint.cc (finish_shorthand_constraint): Pass it.
* pt.cc (convert_generic_types_to_packs): Likewise.
* cp-tree.h: Adjust.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/diagnostic3.C: Add expected column.
* g++.dg/cpp1z/fold3.C: Adjust diagnostic lines.
---
 gcc/cp/cp-tree.h|  6 +-
 gcc/cp/constraint.cc|  3 +-
 gcc/cp/parser.cc| 16 --
 gcc/cp/pt.cc|  4 +-
 gcc/cp/semantics.cc | 25 +
 gcc/testsuite/g++.dg/concepts/diagnostic3.C |  4 +-
 gcc/testsuite/g++.dg/cpp1z/fold3.C  | 62 ++---
 7 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6e34952da99..efcd2de54e5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8209,9 +8209,9 @@ extern void maybe_warn_about_useless_cast   
(location_t, tree, tree,
 tsubst_flags_t);
 extern tree cp_perform_integral_promotions  (tree, tsubst_flags_t);
 
-extern tree finish_left_unary_fold_expr  (tree, int);
-extern tree finish_right_unary_fold_expr (tree, int);
-extern tree finish_binary_fold_expr  (tree, tree, int);
+extern tree finish_left_unary_fold_expr  (location_t, tree, int);
+extern tree finish_right_unary_fold_expr (location_t, tree, int);
+extern tree finish_binary_fold_expr  (location_t, tree, tree, int);
 extern tree treat_lvalue_as_rvalue_p(tree, bool);
 extern bool decl_in_std_namespace_p (tree);
 extern void maybe_warn_pessimizing_move (tree, tree, bool);
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index c9e4e7043cd..64b64e17857 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1607,7 +1607,8 @@ finish_shorthand_constraint (tree decl, tree constr)
 
   /* Make the check a fold-expression if needed.  */
   if (apply_to_each_p && declared_pack_p)
-check = finish_left_unary_fold_expr (check, TRUTH_ANDIF_EXPR);
+check = finish_left_unary_fold_expr (DECL_SOURCE_LOCATION (decl),
+check, TRUTH_ANDIF_EXPR);
 
   return check;
 }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f3abae716fe..59b9852895e 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -5533,6 +5533,8 @@ static cp_expr
 cp_parser_fold_expression (cp_parser *parser, tree expr1)
 {
   cp_id_kind pidk;
+  location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+  const cp_token *token = nullptr;
 
   // Left fold.
   if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
@@ -5540,6 +5542,7 @@ cp_parser_fold_expression (cp_parser *parser, tree expr1)
   if (expr1)
return error_mark_node;
   cp_lexer_consume_token (parser->lexer);
+  token = cp_lexer_peek_token (parser->lexer);
   int op = cp_parser_fold_operator (parser);
   if (op == ERROR_MARK)
 {
@@ -5551,10 +5554,11 @@ cp_parser_fold_expression (cp_parser *parser, tree 
expr1)
 false, &pidk);
   if (expr == error_mark_node)
 return error_mark_node;
-  return finish_left_unary_fold_expr (expr, op);
+  loc = make_location (token->location, loc, parser->lexer);
+  return finish_left_unary_fold_expr (loc, expr, op);
 }
 
-  const cp_token* token = cp_lexer_peek_token (parser->lexer);
+  token = cp_lexer_peek_token (parser->lexer);
   int op = cp_parser_fold_operator (parser);
   if (op == ERROR_MARK)
 {
@@ -5585,7 +5589,10 @@ cp_parser_fold_expression (cp_parser *parser, tree expr1)
 
   // Right fold.
   if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_PAREN))
-return finish_right_unary_fold_expr (expr1, op);
+{
+  loc = make_location (token->location, loc, parser->lexer);
+  return finish_right_unary_fold_expr (loc, expr1, op);
+}
 
   if (cp_lexer_next_token_is_not (parser->lexer, token->type))
 {
@@ -5598,7 +5605,8 @@ cp_parser_fold_expression (cp_parser *parser, tree expr1)
   tree expr2 = cp_parser_cast_e

PING Re: [PATCH v2 RFA] diagnostic: add permerror variants with opt

[Jason Merrill]

Ping?

On 10/3/23 17:09, Jason Merrill wrote:

This revision changes from using DK_PEDWARN for permerror-with-option to using
DK_PERMERROR.

Tested x86_64-pc-linux-gnu.  OK for trunk?

-- 8< --

In the discussion of promoting some pedwarns to be errors by default, rather
than move them all into -fpermissive it seems to me to make sense to support
DK_PERMERROR with an option flag.  This way will also work with
-fpermissive, but users can also still use -Wno-error=narrowing to downgrade
that specific diagnostic rather than everything affected by -fpermissive.

So, for diagnostics that we want to make errors by default we can just
change the pedwarn call to permerror.

The tests check desired behavior for such a permerror in a system header
with various flags.  The patch preserves the existing permerror behavior of
ignoring -w and system headers by default, but respecting them when
downgraded to a warning by -fpermissive.

This seems similar to but a bit better than the approach of forcing
-pedantic-errors that I previously used for -Wnarrowing: specifically, in
that now -w by itself is not enough to silence the -Wnarrowing
error (integer-pack2.C).

gcc/ChangeLog:

* doc/invoke.texi: Move -fpermissive to Warning Options.
* diagnostic.cc (update_effective_level_from_pragmas): Remove
redundant system header check.
(diagnostic_report_diagnostic): Move down syshdr/-w check.
(diagnostic_impl): Handle DK_PERMERROR with an option number.
(permerror): Add new overloads.
* diagnostic-core.h (permerror): Declare them.

gcc/cp/ChangeLog:

* typeck2.cc (check_narrowing): Use permerror.

gcc/testsuite/ChangeLog:

* g++.dg/ext/integer-pack2.C: Add -fpermissive.
* g++.dg/diagnostic/sys-narrow.h: New test.
* g++.dg/diagnostic/sys-narrow1.C: New test.
* g++.dg/diagnostic/sys-narrow1a.C: New test.
* g++.dg/diagnostic/sys-narrow1b.C: New test.
* g++.dg/diagnostic/sys-narrow1c.C: New test.
* g++.dg/diagnostic/sys-narrow1d.C: New test.
* g++.dg/diagnostic/sys-narrow1e.C: New test.
* g++.dg/diagnostic/sys-narrow1f.C: New test.
* g++.dg/diagnostic/sys-narrow1g.C: New test.
* g++.dg/diagnostic/sys-narrow1h.C: New test.
* g++.dg/diagnostic/sys-narrow1i.C: New test.
---
  gcc/doc/invoke.texi   | 22 +++---
  gcc/diagnostic-core.h |  3 +
  gcc/testsuite/g++.dg/diagnostic/sys-narrow.h  |  2 +
  gcc/cp/typeck2.cc | 10 +--
  gcc/diagnostic.cc | 67 ---
  gcc/testsuite/g++.dg/diagnostic/sys-narrow1.C |  4 ++
  .../g++.dg/diagnostic/sys-narrow1a.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1b.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1c.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1d.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1e.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1f.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1g.C  |  5 ++
  .../g++.dg/diagnostic/sys-narrow1h.C  |  6 ++
  .../g++.dg/diagnostic/sys-narrow1i.C  |  6 ++
  gcc/testsuite/g++.dg/ext/integer-pack2.C  |  2 +-
  16 files changed, 117 insertions(+), 40 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow.h
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1a.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1b.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1c.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1d.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1e.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1f.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1g.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1h.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/sys-narrow1i.C

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4085fc90907..6b6506a75b2 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -231,7 +231,7 @@ in the following sections.
  -fnew-inheriting-ctors
  -fnew-ttp-matching
  -fno-nonansi-builtins  -fnothrow-opt  -fno-operator-names
--fno-optional-diags  -fpermissive
+-fno-optional-diags
  -fno-pretty-templates
  -fno-rtti  -fsized-deallocation
  -ftemplate-backtrace-limit=@var{n}
@@ -323,7 +323,7 @@ Objective-C and Objective-C++ Dialects}.
  @item Warning Options
  @xref{Warning Options,,Options to Request or Suppress Warnings}.
  @gccoptlist{-fsyntax-only  -fmax-errors=@var{n}  -Wpedantic
--pedantic-errors
+-pedantic-errors -fpermissive
  -w  -Wextra  -Wall  -Wabi=@var{n}
  -Waddress  -Wno-address-of-packed-member  -Waggregate-return
  -Walloc-size-larger-than=@var{byte-size}  -Walloc-zero
@@ -3494,12 +3494,6 @@ Disable diagnostics that the standard say

Re: [PATCH] c++: Add missing auto_diagnostic_groups to constexpr.cc

[Jason Merrill]

On 10/17/23 12:34, Marek Polacek wrote:

On Tue, Oct 17, 2023 at 09:35:21PM +1100, Nathaniel Shead wrote:

Marek pointed out in another patch of mine [1] that I was missing an
auto_diagnostic_group to correctly associate informative notes with
their errors in structured error outputs. This patch goes through
constexpr.cc to correct this in other locations which seem to have the
same issue.


Thanks for the patch.  I went through all of them and they all seem correct.

So, LGTM, but can't approve.


Thanks, I pushed it with a Reviewed-By of Marek.

Jason

Re: [PATCH v3] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/16/23 20:39, Marek Polacek wrote:

On Sat, Oct 14, 2023 at 01:13:22AM -0400, Jason Merrill wrote:

On 10/13/23 14:53, Marek Polacek wrote:

On Thu, Oct 12, 2023 at 09:41:43PM -0400, Jason Merrill wrote:

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

   PR c++/111660

gcc/cp/ChangeLog:

   * cp-gimplify.cc (cp_fold_immediate_r) : Return
   integer_zero_node instead of break;.
   (cp_fold_immediate): Return true if cp_fold_immediate_r returned
   error_mark_node.

gcc/testsuite/ChangeLog:

   * g++.dg/cpp0x/hog1.C: New test.
---
gcc/cp/cp-gimplify.cc |  9 ++--
gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
2 files changed, 82 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
  if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
  if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
  /* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from
looking any farther.


You are right.  The line above works as expected, but

1 ? 1 : ((1 ? 1 : id (42)), id (i));

shows the problem (when the expression isn't used as an initializer).


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?


I hope this version is better.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.


Is this number still accurate for this version?


It is.  I ran time(1) a few more times and the results were 0m0.033s - 0m0.035s.
That said, ...


This change seems algorithmically better than the current code, but still
problematic: if we have nested COND_EXPR A/B/C/D/E, it looks like we will
end up cp_fold_immediate_r walking the arms of E five times, once for each
COND_EXPR.


...this is accurate.  I should have addressed the redundant folding in v2
even though the compilation is pretty much immediate.
  

What I was thinking by handling COND_EXPR in cp_fold_r was to cp_fold_r walk
its subtrees (or cp_fold_immediate_r if it's clear from op0 that the branch
isn't taken) so we can clear *walk_subtrees and we don't fold_imm walk a
node more than once.


I agree I should do better here.  How's this, then?  I've added
debug_generic_expr to cp_fold_immediate_r to see if it gets the same
expr multiple times and it doesn't seem to.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.030s.

The ff_fold_immediate flag is unused after this patch but since

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 9/25/23 21:56, waffl3x wrote:


Also, just a quick update on my copyright assignment, I have sent an
e-mail to the FSF and haven't gotten a response yet. From what I was
reading, I am confident that it's my preferred option going forward
though. Hopefully they get back to me soon.


Any progress on this, or do I need to coax the process along?  :)

Jason

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 9/25/23 21:56, waffl3x wrote:


On the plus side, I took my time to figure out how to best to pass down
information about whether a param is an xobj param. My initial
impression on what you were suggesting was to push another node on the
front of the list, but I stared at it for a few hours and didn't think
it would work out.


I was thinking to set a TREE_LANG_FLAG on the TREE_LIST node.


However, eventually I realized that the purpose
member if free for xobj params as it is illegal for them to have
default arguments.


Hmm, is it?  I see that clang thinks so, but I don't know where they get 
that idea from.  The grammar certainly allows it:



attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause


and I don't see anything else that prohibits it.

Jason

[pushed] c++: mangling tweaks

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

Most of this is introducing the abi_check function to reduce the verbosity
of most places that check -fabi-version.

The start_mangling change is to avoid needing to zero-initialize additional
members of the mangling globals, though I'm not actually adding any.

The comment documents existing semantics.

gcc/cp/ChangeLog:

* mangle.cc (abi_check): New.
(write_prefix, write_unqualified_name, write_discriminator)
(write_type, write_member_name, write_expression)
(write_template_arg, write_template_param): Use it.
(start_mangling): Assign from {}.
* cp-tree.h: Update comment.
---
 gcc/cp/cp-tree.h |  6 ++--
 gcc/cp/mangle.cc | 85 +---
 2 files changed, 34 insertions(+), 57 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index efcd2de54e5..1d7df62961e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4969,12 +4969,14 @@ get_vec_init_expr (tree t)
 /* The DECL_TEMPLATE_PARMS are a list.  The TREE_PURPOSE of each node
is a INT_CST whose TREE_INT_CST_LOW indicates the level of the
template parameters, with 1 being the outermost set of template
-   parameters.  The TREE_VALUE is a vector, whose elements are the
+   parameters.  The TREE_TYPE is TEMPLATE_PARMS_CONSTRAINTS.
+   The TREE_VALUE is a vector, whose elements are the
template parameters at each level.  Each element in the vector is a
TREE_LIST, whose TREE_VALUE is a PARM_DECL (if the parameter is a
non-type parameter), or a TYPE_DECL (if the parameter is a type
parameter) or a TEMPLATE_DECL (if the parameter is a template
-   parameter).  The TREE_PURPOSE is the default value, if any.  The
+   parameter).  The TREE_PURPOSE is the default value, if any.
+   The TREE_TYPE is TEMPLATE_PARM_CONSTRAINTS.  The
TEMPLATE_PARM_INDEX for the parameter is available as the
DECL_INITIAL (for a PARM_DECL) or as the TREE_TYPE (for a
TYPE_DECL).
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index d079f724910..afa68da871c 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -275,6 +275,17 @@ static tree mangle_special_for_type (const tree, const 
char *);
 #define write_unsigned_number(NUMBER)  \
   write_number ((NUMBER), /*unsigned_p=*/1, 10)
 
+/* Check for -fabi-version dependent mangling and also set the need_abi_warning
+   flag as appropriate.  */
+
+static bool
+abi_check (int ver)
+{
+  if (abi_warn_or_compat_version_crosses (ver))
+G.need_abi_warning = true;
+  return abi_version_at_least (ver);
+}
+
 /* If DECL is a template instance (including the uninstantiated template
itself), return its TEMPLATE_INFO.  Otherwise return NULL.  */
 
@@ -1267,9 +1278,7 @@ write_prefix (const tree node)
  /* Before ABI 18, we did not count these as substitution
 candidates.  This leads to incorrect demanglings (and
 ABI divergence to other compilers).  */
- if (abi_warn_or_compat_version_crosses (18))
-   G.need_abi_warning = true;
- if (!abi_version_at_least (18))
+ if (!abi_check (18))
return;
}
 }
@@ -1542,9 +1551,7 @@ write_unqualified_name (tree decl)
   && any_abi_below (11))
 if (tree mtags = missing_abi_tags (decl))
   {
-   if (abi_warn_or_compat_version_crosses (11))
- G.need_abi_warning = true;
-   if (!abi_version_at_least (11))
+   if (!abi_check (11))
  tags = chainon (mtags, tags);
   }
   write_abi_tags (tags);
@@ -2094,9 +2101,7 @@ write_discriminator (const int discriminator)
   write_char ('_');
   if (discriminator - 1 >= 10)
{
- if (abi_warn_or_compat_version_crosses (11))
-   G.need_abi_warning = 1;
- if (abi_version_at_least (11))
+ if (abi_check (11))
write_char ('_');
}
   write_unsigned_number (discriminator - 1);
@@ -2425,9 +2430,7 @@ write_type (tree type)
 
  if (etype && !type_uses_auto (etype))
{
- if (abi_warn_or_compat_version_crosses (5))
-   G.need_abi_warning = 1;
- if (!abi_version_at_least (5))
+ if (!abi_check (5))
{
  write_type (etype);
  return;
@@ -2448,10 +2451,8 @@ write_type (tree type)
 
case NULLPTR_TYPE:
  write_string ("Dn");
- if (abi_version_at_least (7))
+ if (abi_check (7))
++is_builtin_type;
- if (abi_warn_or_compat_version_crosses (7))
-   G.need_abi_warning = 1;
  break;
 
case TYPEOF_TYPE:
@@ -2935,10 +2936,8 @@ write_member_name (tree member)
 {
   if (IDENTIFIER_ANY_OP_P (member))
{
- if (abi_version_at_least (11))
+ if (abi_check (11))
  

Re: [PATCH] c++: accepts-invalid with =delete("") [PR111840]

[Jason Merrill]

On 10/17/23 17:38, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?


OK.


-- >8 --
r6-2367 added a DECL_INITIAL check to cp_parser_simple_declaration
so that we don't emit multiple errors in g++.dg/parse/error57.C.
But that means we don't diagnose

   int f1() = delete("george_crumb");

anymore, because fn decls often have error_mark_node in their
DECL_INITIAL.  (The code may be allowed one day via https://wg21.link/P2573R0.)

I was hoping I could use cp_parser_error_occurred but that would
regress error57.C.

PR c++/111840

gcc/cp/ChangeLog:

* parser.cc (cp_parser_simple_declaration): Do cp_parser_error
for FUNCTION_DECLs.

gcc/testsuite/ChangeLog:

* g++.dg/parse/error65.C: New test.
---
  gcc/cp/parser.cc | 14 +++---
  gcc/testsuite/g++.dg/parse/error65.C | 10 ++
  2 files changed, 17 insertions(+), 7 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/parse/error65.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 59b9852895e..57b62fb7363 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -15669,6 +15669,7 @@ cp_parser_simple_declaration (cp_parser* parser,
maybe_range_for_decl,
&init_loc,
&auto_result);
+  const bool fndecl_p = TREE_CODE (decl) == FUNCTION_DECL;
/* If an error occurred while parsing tentatively, exit quickly.
 (That usually happens when in the body of a function; each
 statement is treated as a declaration-statement until proven
@@ -15682,16 +15683,13 @@ cp_parser_simple_declaration (cp_parser* parser,
 init-declarator, they shall all form declarations of
 variables.  */
  if (auto_function_declaration == NULL_TREE)
-   auto_function_declaration
- = TREE_CODE (decl) == FUNCTION_DECL ? decl : error_mark_node;
- else if (TREE_CODE (decl) == FUNCTION_DECL
-  || auto_function_declaration != error_mark_node)
+   auto_function_declaration = fndecl_p ? decl : error_mark_node;
+ else if (fndecl_p || auto_function_declaration != error_mark_node)
{
  error_at (decl_specifiers.locations[ds_type_spec],
"non-variable %qD in declaration with more than one "
"declarator with placeholder type",
-   TREE_CODE (decl) == FUNCTION_DECL
-   ? decl : auto_function_declaration);
+   fndecl_p ? decl : auto_function_declaration);
  auto_function_declaration = error_mark_node;
}
}
@@ -15763,7 +15761,9 @@ cp_parser_simple_declaration (cp_parser* parser,
  /* If we have already issued an error message we don't need
 to issue another one.  */
  if ((decl != error_mark_node
-  && DECL_INITIAL (decl) != error_mark_node)
+  /* grokfndecl sets DECL_INITIAL to error_mark_node for
+ functions.  */
+  && (fndecl_p || DECL_INITIAL (decl) != error_mark_node))
  || cp_parser_uncommitted_to_tentative_parse_p (parser))
cp_parser_error (parser, "expected %<,%> or %<;%>");
  /* Skip tokens until we reach the end of the statement.  */
diff --git a/gcc/testsuite/g++.dg/parse/error65.C 
b/gcc/testsuite/g++.dg/parse/error65.C
new file mode 100644
index 000..d9e0a4bfbcb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/error65.C
@@ -0,0 +1,10 @@
+// PR c++/111840
+// { dg-do compile { target c++11 } }
+
+// NB: =delete("reason") may be allowed via P2573.
+int f1() = delete("should have a reason"); // { dg-error "expected" }
+int f2() = delete[""]; // { dg-error "expected" }
+int f3() = delete{""}; // { dg-error "expected" }
+int f4() = delete""; // { dg-error "expected" }
+int f5() = delete[{'a'""; // { dg-error "expected" }
+int i = f5();

base-commit: bac21b7ea62bd3a7911e01cf803d6bf6516fbf7b

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 10/18/23 07:46, waffl3x wrote:

Any progress on this, or do I need to coax the process along?  :)


Yeah, I've been working on it since the copyright assignment process
has finished, originally I was going to note that on my next update
which I had hoped to finish today or tomorrow. Well, in truth I was
hoping to send one the same day that copyright assignment finished, but
I found a nasty bug so I spent all day adding test cases for all the
relevant overloadable operators. Currently, it crashes when calling a
subscript operator declared with an explicit object parameter in a
dependent context. I haven't looked into the fix yet, but I plan to.

Also, before I forget, what is the process for confirming my copyright
assignment on your end? Do you just need to check in with the FSF to
see if it went through? Let me know if there's anything you need from
me regarding that.

Aside from the bug that's currently present in the first patch, I only
have like 1 or 2 little things I want to change about it. I will make
those few changes to patch 1, finish patch 2 (diagnostics) which will
also include test cases for the new bug I found. After I am done that I
plan on adding the things that are missing, because I suspect that
looking into that will get me close to finding the fix for the crash.


Hmm, is it? I see that clang thinks so, but I don't know where they get
that idea from. The grammar certainly allows it:


attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause


and I don't see anything else that prohibits it.


You would be right for P0847R7, but CWG DR 2653 changed that. You can
find the updated grammar in dcl.fct section 3 (subsection? I'm not
really sure to be honest.)

I've also included a copy of the grammar here for your convenience.

https://eel.is/c++draft/dcl.fct#nt:parameter-declaration
parameter-declaration:
   attribute-specifier-seqopt thisopt decl-specifier-seq declarator
   attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause
   attribute-specifier-seqopt thisopt decl-specifier-seq abstract-declaratoropt
   attribute-specifier-seqopt decl-specifier-seq abstract-declaratoropt = 
initializer-clause


Ah, yes, thanks.


I was thinking to set a TREE_LANG_FLAG on the TREE_LIST node.


I did figure this is originally what you meant, and I can still change
it to go this route since I'm sure it's likely just as good. But I do
recall something I didn't like in the implementation that nudged me
towards using the purpose member instead. Either way, not a big deal. I
think I just liked not having to mess with a linked list as I am not
used to them as a data structure, it might have been that simple. :^)


I wouldn't expect to need any actual dealing with the linked list, just 
setting a flag in cp_parameter_declaration_list at the same point as the 
existing PARENTHESIZED_LIST_P flag.


But given CWG2653 as you pointed out, your current approach is fine.


I will try to get something done today, but I was struggling with
writing some of the tests, there's also a lot more of them now. I also
wrote a bunch of musings in comments that I would like feedback on.

My most concrete question is, how exactly should I be testing a
pedwarn, I want to test that I get the correct warning and error with
the separate flags, do I have to create two separate tests for each one?


Yes.  I tend to use letter suffixes for tests that vary only in flags 
(and expected results), e.g. feature1a.C, feature1b.C.



I'm just going to include the little wall I wrote in decl.cc regarding
pedwarn, just in case I can't get this done tonight so I can get some
feedback regarding it. On the other hand, it might just not be very
relevant to this patch in particular as I kind of noted, but maybe
there's some way to do what I was musing about that I've overlooked. It
does end up a bit ranty I guess, hopefully that doesn't make it
confusing.

```
/* I believe we should make a switch for this feature specifically,
I recall seeing discussion regarding enabling newer language
features when set to older standards. I would advocate for a
specific flag for each specific feature. Maybe they should all
be under an override flag? -foverride-dialect=xobj,ifconstexpr (?)
I dont think it makes sense to give each feature override it's own
flag. I don't recall what the consensus was around this discussion
either though.

For the time being it's controlled by pedantic. I am concerned that
tying this to pedantic going forward that one might want to enable
-pedantic-errors while also enabling select features from newer
dialects. It didn't look like this use case is supported to me.

I suppose this will require redesign work to support, so for
the purposes of this patch, emitting a pedwarn seems correct.
I just don't like that it can't be suppressed on an individual
basis.  */
if (xobj_parm && cxx_dialect < cxx23)
   pedwarn(DECL_SOURCE_LOCATION (xo

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 10/18/23 13:28, waffl3x wrote:

I will try to get something done today, but I was struggling with
writing some of the tests, there's also a lot more of them now. I also
wrote a bunch of musings in comments that I would like feedback on.

My most concrete question is, how exactly should I be testing a
pedwarn, I want to test that I get the correct warning and error with
the separate flags, do I have to create two separate tests for each one?



Yes. I tend to use letter suffixes for tests that vary only in flags
(and expected results), e.g. feature1a.C, feature1b.C.


Will do.


Instead of OPT_Wpedantic, this should be controlled by
-Wc++23-extensions (OPT_Wc__23_extensions)


Yeah, I'll do this.


If you wanted, you could add a more specific warning option for this
(e.g. -Wc++23-explicit-this) which is also affected by
-Wc++23-extensions, but I would lean toward just using the existing
flag. Up to you.


I brought it up in irc and there was some pushback to my point of view
on it, so I'll just stick with OPT_Wc__23_extensions for now. I do
think a more sophisticated interface would be beneficial but I will
bring discussion around that up again in the future.

I've seen plenty of these G_ or _ macros on strings around like in
grokfndecl for these errors.

G_("static member function %qD cannot have cv-qualifier")
G_("non-member function %qD cannot have cv-qualifier")

G_("static member function %qD cannot have ref-qualifier")
G_("non-member function %qD cannot have ref-qualifier")

I have been able to figure out it relates to translation, but not
exactly what the protocol around them is.


The protocol is described in gcc/ABOUT-GCC-NLS.  In general, "strings" 
passed directly to a diagnostic function don't need any decoration, but 
if they're assigned to a variable first, they need G_() so they're 
recognized as diagnostic strings to be added to the translation table.


The _() macro is used for strings that are going to be passed to a %s, 
but better to avoid doing that for strings that need translation.  N_() 
is (rarely) used for strings that aren't diagnostic format strings, but 
get passed to another function that passes them to _().


Jason

Re: [PATCH v3] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/19/23 09:39, Patrick Palka wrote:

On Tue, 17 Oct 2023, Marek Polacek wrote:


On Tue, Oct 17, 2023 at 04:49:52PM -0400, Jason Merrill wrote:

On 10/16/23 20:39, Marek Polacek wrote:

On Sat, Oct 14, 2023 at 01:13:22AM -0400, Jason Merrill wrote:

On 10/13/23 14:53, Marek Polacek wrote:

On Thu, Oct 12, 2023 at 09:41:43PM -0400, Jason Merrill wrote:

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

PR c++/111660

gcc/cp/ChangeLog:

* cp-gimplify.cc (cp_fold_immediate_r) : Return
integer_zero_node instead of break;.
(cp_fold_immediate): Return true if cp_fold_immediate_r returned
error_mark_node.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/hog1.C: New test.
---
 gcc/cp/cp-gimplify.cc |  9 ++--
 gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
 2 files changed, 82 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
   if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
   if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
   /* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from
looking any farther.


You are right.  The line above works as expected, but

 1 ? 1 : ((1 ? 1 : id (42)), id (i));

shows the problem (when the expression isn't used as an initializer).


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?


I hope this version is better.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.


Is this number still accurate for this version?


It is.  I ran time(1) a few more times and the results were 0m0.033s - 0m0.035s.
That said, ...


This change seems algorithmically better than the current code, but still
problematic: if we have nested COND_EXPR A/B/C/D/E, it looks like we will
end up cp_fold_immediate_r walking the arms of E five times, once for each
COND_EXPR.


...this is accurate.  I should have addressed the redundant folding in v2
even though the compilation is pretty much immediate.

What I was thinking by handling COND_EXPR in cp_fold_r was to cp_fold_r walk
its subtrees (or cp_fold_immediate_r if it's clear from op0 that the branch
isn't taken) so we can clear *walk_subtrees and we don't fold_imm walk a
node more than once.


I agree I should do better here.  How's this, then?  I've added
debug_generic_expr to cp_fold_immediate_r to see if it gets the same
expr multiple times and it doesn't seem to.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
t

Re: [PATCH 2/1] c++: more non-static memfn call dependence cleanup [PR106086]

[Jason Merrill]

On 10/12/23 14:49, Patrick Palka wrote:

On Tue, 26 Sep 2023, Patrick Palka wrote:


Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

-- >8 --

This follow-up patch removes some more repetition of the type-dependent


On second thought there's no good reason to split these patches into a two
part series, so here's a single squashed patch:


OK.


-- >8 --

Subject: [PATCH] c++: non-static memfn call dependence cleanup [PR106086]

In cp_parser_postfix_expression and in the CALL_EXPR case of
tsubst_copy_and_build, we essentially repeat the type-dependent and
COMPONENT_REF callee cases of finish_call_expr.  This patch deduplicates
this logic by making both spots consistently go through finish_call_expr.

This allows us to easily fix PR106086 -- which is about us neglecting to
capture 'this' when we resolve a use of a non-static member function of
the current instantiation only at lambda regeneration time -- by moving
the call to maybe_generic_this_capture from the parser to finish_call_expr
so that we consider capturing 'this' at regeneration time as well.

PR c++/106086

gcc/cp/ChangeLog:

* parser.cc (cp_parser_postfix_expression): Consolidate three
calls to finish_call_expr, one to build_new_method_call and
one to build_min_nt_call_vec into one call to finish_call_expr.
Don't call maybe_generic_this_capture here.
* pt.cc (tsubst_copy_and_build) : Remove
COMPONENT_REF callee handling.
(type_dependent_expression_p): Use t_d_object_e_p instead of
t_d_e_p for COMPONENT_REF and OFFSET_REF.
* semantics.cc (finish_call_expr): In the type-dependent case,
call maybe_generic_this_capture here instead.

gcc/testsuite/ChangeLog:

* g++.dg/template/crash127.C: Expect additional error due to
being able to check the member access expression ahead of time.
Strengthen the test by not instantiating the class template.
* g++.dg/cpp1y/lambda-generic-this5.C: New test.
---
  gcc/cp/parser.cc  | 52 +++
  gcc/cp/pt.cc  | 27 +-
  gcc/cp/semantics.cc   | 12 +++--
  .../g++.dg/cpp1y/lambda-generic-this5.C   | 22 
  gcc/testsuite/g++.dg/template/crash127.C  |  3 +-
  5 files changed, 38 insertions(+), 78 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-this5.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f3abae716fe..b00ef36b831 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -8047,54 +8047,12 @@ cp_parser_postfix_expression (cp_parser *parser, bool 
address_p, bool cast_p,
close_paren_loc);
iloc_sentinel ils (combined_loc);
  
-	if (TREE_CODE (postfix_expression) == COMPONENT_REF)

- {
-   tree instance = TREE_OPERAND (postfix_expression, 0);
-   tree fn = TREE_OPERAND (postfix_expression, 1);
-
-   if (processing_template_decl
-   && (type_dependent_object_expression_p (instance)
-   || (!BASELINK_P (fn)
-   && TREE_CODE (fn) != FIELD_DECL)
-   || type_dependent_expression_p (fn)
-   || any_type_dependent_arguments_p (args)))
- {
-   maybe_generic_this_capture (instance, fn);
-   postfix_expression
- = build_min_nt_call_vec (postfix_expression, args);
- }
-   else if (BASELINK_P (fn))
- {
- postfix_expression
-   = (build_new_method_call
-  (instance, fn, &args, NULL_TREE,
-   (idk == CP_ID_KIND_QUALIFIED
-? LOOKUP_NORMAL|LOOKUP_NONVIRTUAL
-: LOOKUP_NORMAL),
-   /*fn_p=*/NULL,
-   complain));
- }
-   else
- postfix_expression
-   = finish_call_expr (postfix_expression, &args,
-   /*disallow_virtual=*/false,
-   /*koenig_p=*/false,
-   complain);
- }
-   else if (TREE_CODE (postfix_expression) == OFFSET_REF
-|| TREE_CODE (postfix_expression) == MEMBER_REF
-|| TREE_CODE (postfix_expression) == DOTSTAR_EXPR)
+   if (TREE_CODE (postfix_expression) == OFFSET_REF
+   || TREE_CODE (postfix_expression) == MEMBER_REF
+   || TREE_CODE (postfix_expression) == DOTSTAR_EXPR)
  postfix_expression = (build_offset_ref_call_from_tree
(postfix_expression, &args,
 complain));
-   else if (idk == CP_ID_KIND_QUALIFIED)

[PATCH] ABOUT-GCC-NLS: add usage guidance

[Jason Merrill]

A recent question led me to look at this file again, and it occurred to me that
it could use to offer more guidance.  OK for trunk?

-- 8< --

gcc/ChangeLog:

* ABOUT-GCC-NLS: Add usage guidance.
---
 gcc/ABOUT-GCC-NLS | 13 +
 1 file changed, 13 insertions(+)

diff --git a/gcc/ABOUT-GCC-NLS b/gcc/ABOUT-GCC-NLS
index e90a67144e3..4c8b94d0811 100644
--- a/gcc/ABOUT-GCC-NLS
+++ b/gcc/ABOUT-GCC-NLS
@@ -23,6 +23,19 @@ For example, GCC source code should not contain calls like 
`error
 ("unterminated comment")' instead, as it is the `error' function's
 responsibility to translate the message before the user sees it.
 
+In general, use no markup for strings that are the immediate format string
+argument of a diagnostic function.  Use G_("str") for strings that will be
+used as the format string for a diagnostic but are e.g. assigned to a
+variable first.  Use N_("str") for other strings, particularly in a
+statically allocated array, that will be translated later by
+e.g. _(msgs[idx]).  Use _("str") for strings that will not be translated
+elsewhere.
+
+Avoid using %s to compose a diagnostic message from multiple translateable
+strings; instead, write out the full diagnostic message for each variant.
+Only use %s for message components that do not need translation, such as
+keywords.
+
 By convention, any function parameter in the GCC sources whose name
 ends in `msgid' is expected to be a message requiring translation.
 If the parameter name ends with `gmsgid', it is assumed to be a GCC

base-commit: faa0e82b409362ba022f6872cea9677e9dd42f0c
-- 
2.39.3

[pushed] c++: use G_ instead of _

[Jason Merrill]

Tested with make po/gcc.pot to see that the strings are still there (though in
a different place, now with the gcc-internal-format tag).

Applying to trunk.

-- 8< --

Since these strings are passed to error_at, they should be marked for
translation with G_, like other diagnostic messages, rather than _, which
forces immediate (redundant) translation.  The use of N_ is less
problematic, but also imprecise.

gcc/cp/ChangeLog:

* parser.cc (cp_parser_primary_expression): Use G_.
(cp_parser_using_enum): Likewise.
* decl.cc (identify_goto): Likewise.
---
 gcc/cp/decl.cc   |  4 ++--
 gcc/cp/parser.cc | 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 255c4026bdb..ce4c89dea70 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -3607,8 +3607,8 @@ identify_goto (tree decl, location_t loc, const 
location_t *locus,
 {
   bool complained
 = emit_diagnostic (diag_kind, loc, 0,
-  decl ? N_("jump to label %qD")
-  : N_("jump to case label"), decl);
+  decl ? G_("jump to label %qD")
+  : G_("jump to case label"), decl);
   if (complained && locus)
 inform (*locus, "  from here");
   return complained;
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 57b62fb7363..c77e93ef104 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -6206,8 +6206,8 @@ cp_parser_primary_expression (cp_parser *parser,
  {
const char *msg
  = (TREE_CODE (decl) == PARM_DECL
-? _("parameter %qD may not appear in this context")
-: _("local variable %qD may not appear in this context"));
+? G_("parameter %qD may not appear in this context")
+: G_("local variable %qD may not appear in this context"));
error_at (id_expression.get_location (), msg,
  decl.get_value ());
return error_mark_node;
@@ -22145,16 +22145,16 @@ cp_parser_using_enum (cp_parser *parser)
  shall have a reachable enum-specifier.  */
   const char *msg = nullptr;
   if (cxx_dialect < cxx20)
-msg = _("% "
-   "only available with %<-std=c++20%> or %<-std=gnu++20%>");
+msg = G_("% "
+"only available with %<-std=c++20%> or %<-std=gnu++20%>");
   else if (dependent_type_p (type))
-msg = _("% of dependent type %qT");
+msg = G_("% of dependent type %qT");
   else if (TREE_CODE (type) != ENUMERAL_TYPE)
-msg = _("% of non-enumeration type %q#T");
+msg = G_("% of non-enumeration type %q#T");
   else if (!COMPLETE_TYPE_P (type))
-msg = _("% of incomplete type %qT");
+msg = G_("% of incomplete type %qT");
   else if (OPAQUE_ENUM_P (type))
-msg = _("% of %qT before its enum-specifier");
+msg = G_("% of %qT before its enum-specifier");
   if (msg)
 {
   location_t loc = make_location (start, start, end);

base-commit: 04d6c74564b7eb51660a00b35353aeab706b5a50
-- 
2.39.3

[PATCH RFA] diagnostic: rename new permerror overloads

[Jason Merrill]

OK for trunk?

-- 8< --

While checking another change, I noticed that the new permerror overloads
break gettext with "permerror used incompatibly as both
 --keyword=permerror:2 --flag=permerror:2:gcc-internal-format and
 --keyword=permerror:3 --flag=permerror:3:gcc-internal-format".  So let's
change the name.

gcc/ChangeLog:

* diagnostic-core.h (permerror): Rename new overloads...
(permerror_opt): To this.
* diagnostic.cc: Likewise.

gcc/cp/ChangeLog:

* typeck2.cc (check_narrowing): Adjust.
---
 gcc/diagnostic-core.h | 7 ---
 gcc/cp/typeck2.cc | 6 +++---
 gcc/diagnostic.cc | 4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 2d9909f18bd..04eba3d140e 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -105,9 +105,10 @@ extern bool pedwarn (rich_location *, int, const char *, 
...)
 extern bool permerror (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern bool permerror (rich_location *, const char *,
   ...) ATTRIBUTE_GCC_DIAG(2,3);
-extern bool permerror (location_t, int, const char *, ...) 
ATTRIBUTE_GCC_DIAG(3,4);
-extern bool permerror (rich_location *, int, const char *,
-  ...) ATTRIBUTE_GCC_DIAG(3,4);
+extern bool permerror_opt (location_t, int, const char *, ...)
+  ATTRIBUTE_GCC_DIAG(3,4);
+extern bool permerror_opt (rich_location *, int, const char *,
+  ...) ATTRIBUTE_GCC_DIAG(3,4);
 extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void sorry_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
index ab819d4e49d..8e63fb1dc0e 100644
--- a/gcc/cp/typeck2.cc
+++ b/gcc/cp/typeck2.cc
@@ -1109,9 +1109,9 @@ check_narrowing (tree type, tree init, tsubst_flags_t 
complain,
   else if (complain & tf_error)
{
  int savederrorcount = errorcount;
- permerror (loc, OPT_Wnarrowing,
-"narrowing conversion of %qE from %qH to %qI",
-init, ftype, type);
+ permerror_opt (loc, OPT_Wnarrowing,
+"narrowing conversion of %qE from %qH to %qI",
+init, ftype, type);
  if (errorcount == savederrorcount)
ok = true;
}
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index ecf1097bd2c..0f392358aef 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -2027,7 +2027,7 @@ permerror (rich_location *richloc, const char *gmsgid, 
...)
diagnostic can also be downgraded by -Wno-error=opt.  */
 
 bool
-permerror (location_t location, int opt, const char *gmsgid, ...)
+permerror_opt (location_t location, int opt, const char *gmsgid, ...)
 {
   auto_diagnostic_group d;
   va_list ap;
@@ -2041,7 +2041,7 @@ permerror (location_t location, int opt, const char 
*gmsgid, ...)
 /* Same as "permerror" above, but at RICHLOC.  */
 
 bool
-permerror (rich_location *richloc, int opt, const char *gmsgid, ...)
+permerror_opt (rich_location *richloc, int opt, const char *gmsgid, ...)
 {
   gcc_assert (richloc);
 

base-commit: f53de2baae5a6992d93d58951c4c0a25ee678091
-- 
2.39.3

Re: [C++ Patch] Adjust one more error message to use rich_location::add_range

[Jason Merrill]

OK.

On Mon, Jul 2, 2018 at 6:58 AM, Paolo Carlini  wrote:
> Hi,
>
> I was double checking my pending patch and going through the errors we emit
> in decl.c and elsewhere about thread_local and __thread and noticed another
> place, in parser.c, where using rich_location::add_range seems natural.
> Note, we could in principle swap location and
> decl_specs->locations[ds_thread] in the error basing on the gnu bool and
> ensure that the caret always points to __thread. All in all, I don't think
> it's worth it...
>
> Thanks, Paolo.
>
> ///
>

Re: C++ PATCH to add a test for c++/84306

[Jason Merrill]

OK.

On Tue, Jul 3, 2018 at 10:14 AM, Marek Polacek  wrote:
> This patch merely adds a test for an already fixed issue.
>
> Tested on x86_64-linux, ok for trunk?
>
> 2018-07-03  Marek Polacek  
>
> PR c++/84306
> * g++.dg/overload/conv-op3.C: New test.
>
> diff --git gcc/testsuite/g++.dg/overload/conv-op3.C 
> gcc/testsuite/g++.dg/overload/conv-op3.C
> index e69de29bb2d..9d04a37fe5e 100644
> --- gcc/testsuite/g++.dg/overload/conv-op3.C
> +++ gcc/testsuite/g++.dg/overload/conv-op3.C
> @@ -0,0 +1,18 @@
> +// c++/84306
> +// { dg-do link { target c++11 } }
> +
> +struct foo {
> +  foo() = default;
> +
> +  foo(foo const&);
> +
> +  template
> +  explicit foo(T&&) { }
> +};
> +
> +int
> +main()
> +{
> +  foo f1;
> +  foo f2{f1};
> +}

C++ PATCH for c++/86378, functional cast in template noexcept-specifier

[Jason Merrill]

This was a simple typo in strip_typedefs_expr, where we were iterating
but then building a new list consisting entirely of the first element.

Tested x86_64-pc-linux-gnu, applying to trunk, 8, 7.
commit 1cab1ce37320aac67d8fbf88d10930f5c769cfb1
Author: Jason Merrill 
Date:   Mon Jul 2 15:09:58 2018 -0400

PR c++/86378 - functional cast in noexcept-specifier.

* tree.c (strip_typedefs_expr) [TREE_LIST]: Fix iteration.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 361248d4b52..b1333f55e39 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1735,9 +1735,9 @@ strip_typedefs_expr (tree t, bool *remove_attributes)
 	tree it;
 	for (it = t; it; it = TREE_CHAIN (it))
 	  {
-	tree val = strip_typedefs_expr (TREE_VALUE (t), remove_attributes);
+	tree val = strip_typedefs_expr (TREE_VALUE (it), remove_attributes);
 	vec_safe_push (vec, val);
-	if (val != TREE_VALUE (t))
+	if (val != TREE_VALUE (it))
 	  changed = true;
 	gcc_assert (TREE_PURPOSE (it) == NULL_TREE);
 	  }
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept33.C b/gcc/testsuite/g++.dg/cpp0x/noexcept33.C
new file mode 100644
index 000..c5a03de38dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept33.C
@@ -0,0 +1,28 @@
+// PR c++/86378
+// { dg-do compile { target c++11 } }
+
+struct Pepper {};
+struct Apple { Apple(int) {} };
+
+struct Combination : Apple, Pepper
+{
+  Combination(Pepper p, Apple a)
+: Apple(a), Pepper(p)
+  {}
+};
+
+struct MyCombination
+{
+  using Spice = Pepper;
+  using Fruit = Apple;
+
+  Combination combination;
+
+  template
+  constexpr MyCombination(T&& t)
+  noexcept(noexcept(Combination(Spice(), Fruit(t
+: combination(Spice(), Fruit(t))
+  {}
+};
+
+MyCombination obj(Apple(4));

Re: C++ PATCH for c++/86201, diagnostic routines re-entered crash

[Jason Merrill]

OK.

On Tue, Jul 3, 2018 at 12:08 PM, Marek Polacek  wrote:
> This is an ICE of kind "Error reporting routines re-entered".  From the PR:
>
> The problem here is that we report the missing return value:
>  9224 permerror (input_location, "return-statement with no value, in "
>  9225"function returning %qT", valtype);
> but permerror will end up calling print_instantiation_full_context, which ends
> up calling dump_template_bindings and then tsubst -> tsubst_copy_and_build ->
> build_functional_cast -> ... -> ocp_convert which has (complain is tf_none)
>  829   if (complain & tf_warning)
>  830 return cp_truthvalue_conversion (e);
>  831   else
>  832 {
>  833   /* Prevent bogus -Wint-in-bool-context warnings coming
>  834  from c_common_truthvalue_conversion down the line.  */
>  835   warning_sentinel w (warn_int_in_bool_context);
>  836   return cp_truthvalue_conversion (e);
>  837 }
> So we call cp_truthvalue_conversion -> c_common_truthvalue_conversion ->
> build_binary_op which only calls cp_build_binary_op but with
> tf_warning_or_error.  So even though the warning
>  4736   if ((complain & tf_warning)
>  4737   && (FLOAT_TYPE_P (type0) || FLOAT_TYPE_P (type1)))
>  4738 warning (OPT_Wfloat_equal,
>  4739  "comparing floating point with == or != is unsafe");
> is properly guarded, we still re-enter the diagnostic routines.
>
> But since we're parsing decltype we can check c_inhibit_evaluation_warnings
> which means we won't call warning().
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-07-03  Marek Polacek  
>
> PR c++/86201
> * typeck.c (cp_build_binary_op): Check c_inhibit_evaluation_warnings.
>
> * g++.dg/diagnostic/pr86201.C: New test.
>
> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> index 3a4f1cdf479..ea4ce9649cd 100644
> --- gcc/cp/typeck.c
> +++ gcc/cp/typeck.c
> @@ -4734,6 +4734,7 @@ cp_build_binary_op (location_t location,
>if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
> goto vector_compare;
>if ((complain & tf_warning)
> + && c_inhibit_evaluation_warnings == 0
>   && (FLOAT_TYPE_P (type0) || FLOAT_TYPE_P (type1)))
> warning (OPT_Wfloat_equal,
>  "comparing floating point with == or != is unsafe");
> diff --git gcc/testsuite/g++.dg/diagnostic/pr86201.C 
> gcc/testsuite/g++.dg/diagnostic/pr86201.C
> index e69de29bb2d..e7019c22d95 100644
> --- gcc/testsuite/g++.dg/diagnostic/pr86201.C
> +++ gcc/testsuite/g++.dg/diagnostic/pr86201.C
> @@ -0,0 +1,12 @@
> +// PR c++/86201
> +// { dg-do compile { target c++11 } }
> +
> +template 
> +auto fn1 (V&& v) -> decltype(U(v))
> +{
> +  return; // { dg-error "return-statement with no value" }
> +}
> +void fn2 ()
> +{
> +  fn1(1.0);
> +}

Re: [C++ Patch] More location fixes to grokdeclarator

[Jason Merrill]

On Thu, Jun 28, 2018 at 5:39 AM, Paolo Carlini  wrote:
> Hi,
>
> On 28/06/2018 03:22, David Malcolm wrote:
>>
>> [snip]
>>>
>>> If I'm following you right, the idea is that gcc should complain
>>> because two different things in the user's source code contradict
>>> each
>>> other.
>>>
>>> In such circumstances, I think we ought to try to print *both*
>>> locations, so that we're showing, rather than just telling.
>>
>> Or to put in another way, if two things in the user's source contradict
>> each other, we should show the user both.  The user is going to have to
>> decide to delete one (or both) of them, and we don't know which one,
>> but at least by showing both it helps him/her take their next action.
>
> Sure, makes sense. Thus the below uses rich_location the way you explained.
> I also added 2 specific testcases and extended a bit another one to exercise
> a bit more min_location..Of course the patch doesn't add max_location
> anymore, I suspect we are not going to find uses for a max anytime soon,
> because we really want rich_location with multiple ranges in all those
> cases...

>if ((type_quals & TYPE_QUAL_VOLATILE)
> -  && (loc == UNKNOWN_LOCATION || locations[ds_volatile] < loc))
> +  && (loc == UNKNOWN_LOCATION
> +  || linemap_location_before_p (line_table,
> +locations[ds_volatile], loc)))
>  loc = locations[ds_volatile];

>if ((type_quals & TYPE_QUAL_RESTRICT)
> -  && (loc == UNKNOWN_LOCATION || locations[ds_restrict] < loc))
> +  && (loc == UNKNOWN_LOCATION
> +  || linemap_location_before_p (line_table,
> +locations[ds_restrict], loc)))
>  loc = locations[ds_restrict];

Why not use min_location here?

Jason

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill]

On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek  wrote:
> On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek  wrote:
>> > This PR complains about us accepting invalid code like
>> >
>> >   template struct A {};
>> >   A<-1> a;
>> >
>> > Where we should detect the narrowing: [temp.arg.nontype] says
>> > "A template-argument for a non-type template-parameter shall be a converted
>> > constant expression ([expr.const]) of the type of the template-parameter."
>> > and a converted constant expression can contain only
>> > - integral conversions other than narrowing conversions,
>> > - [...]."
>> > It spurred e.g.
>> > <https://stackoverflow.com/questions/28184888/how-implicit-conversion-works-for-non-type-template-parameters>
>> > and has >=3 dups so it has some visibility.
>> >
>> > I think build_converted_constant_expr needs to set check_narrowing.
>> > check_narrowing also always mentions that it's in { } but that is no longer
>> > true; in the future it will also apply to <=>.  We'd probably have to add 
>> > a new
>> > flag to struct conversion if wanted to distinguish between these.
>> >
>> > This does not yet fix detecting narrowing in function templates (78244).
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >
>> > 2018-06-27  Marek Polacek  
>> >
>> > PR c++/57891
>> > * call.c (build_converted_constant_expr): Set check_narrowing.
>> > * decl.c (compute_array_index_type): Add warning sentinel.  Use
>> > input_location.
>> > * pt.c (convert_nontype_argument): Return NULL_TREE if any errors
>> > were reported.
>> > * typeck2.c (check_narrowing): Don't mention { } in diagnostic.
>> >
>> > * g++.dg/cpp0x/Wnarrowing6.C: New test.
>> > * g++.dg/cpp0x/Wnarrowing7.C: New test.
>> > * g++.dg/cpp0x/Wnarrowing8.C: New test.
>> > * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>> > * g++.dg/init/new43.C: Adjust dg-error.
>> > * g++.dg/other/fold1.C: Likewise.
>> > * g++.dg/parse/array-size2.C: Likewise.
>> > * g++.dg/other/vrp1.C: Add dg-error.
>> > * g++.dg/template/char1.C: Likewise.
>> > * g++.dg/ext/builtin12.C: Likewise.
>> > * g++.dg/template/dependent-name3.C: Adjust dg-error.
>> >
>> > diff --git gcc/cp/call.c gcc/cp/call.c
>> > index 209c1fd2f0e..956c7b149dc 100644
>> > --- gcc/cp/call.c
>> > +++ gcc/cp/call.c
>> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, tree 
>> > expr, tsubst_flags_t complain)
>> >  }
>> >
>> >if (conv)
>> > -expr = convert_like (conv, expr, complain);
>> > +{
>> > +  conv->check_narrowing = !processing_template_decl;
>>
>> Why !processing_template_decl?  This needs a comment.
>
> Otherwise we'd warn for e.g.
>
> template struct S { char a[N]; };
> S<1> s;
>
> where compute_array_index_type will try to convert the size of the array 
> (which
> is a template_parm_index of type int when parsing the template) to size_type.
> So I guess I can say that we need to wait for instantiation?

We certainly shouldn't give a narrowing diagnostic about a
value-dependent expression.  It probably makes sense to check that at
the top of check_narrowing, with all the other early exit conditions.
But if we do know the constant value in the template, it's good to
complain then rather than wait for instantiation.

Jason

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill]

On Tue, Jul 3, 2018 at 2:58 PM, Marek Polacek  wrote:
> On Tue, Jul 03, 2018 at 12:40:51PM -0400, Jason Merrill wrote:
>> On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek  wrote:
>> > On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>> >> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek  
>> >> wrote:
>> >> > This PR complains about us accepting invalid code like
>> >> >
>> >> >   template struct A {};
>> >> >   A<-1> a;
>> >> >
>> >> > Where we should detect the narrowing: [temp.arg.nontype] says
>> >> > "A template-argument for a non-type template-parameter shall be a 
>> >> > converted
>> >> > constant expression ([expr.const]) of the type of the 
>> >> > template-parameter."
>> >> > and a converted constant expression can contain only
>> >> > - integral conversions other than narrowing conversions,
>> >> > - [...]."
>> >> > It spurred e.g.
>> >> > <https://stackoverflow.com/questions/28184888/how-implicit-conversion-works-for-non-type-template-parameters>
>> >> > and has >=3 dups so it has some visibility.
>> >> >
>> >> > I think build_converted_constant_expr needs to set check_narrowing.
>> >> > check_narrowing also always mentions that it's in { } but that is no 
>> >> > longer
>> >> > true; in the future it will also apply to <=>.  We'd probably have to 
>> >> > add a new
>> >> > flag to struct conversion if wanted to distinguish between these.
>> >> >
>> >> > This does not yet fix detecting narrowing in function templates (78244).
>> >> >
>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >> >
>> >> > 2018-06-27  Marek Polacek  
>> >> >
>> >> > PR c++/57891
>> >> > * call.c (build_converted_constant_expr): Set check_narrowing.
>> >> > * decl.c (compute_array_index_type): Add warning sentinel.  Use
>> >> > input_location.
>> >> > * pt.c (convert_nontype_argument): Return NULL_TREE if any 
>> >> > errors
>> >> > were reported.
>> >> > * typeck2.c (check_narrowing): Don't mention { } in diagnostic.
>> >> >
>> >> > * g++.dg/cpp0x/Wnarrowing6.C: New test.
>> >> > * g++.dg/cpp0x/Wnarrowing7.C: New test.
>> >> > * g++.dg/cpp0x/Wnarrowing8.C: New test.
>> >> > * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>> >> > * g++.dg/init/new43.C: Adjust dg-error.
>> >> > * g++.dg/other/fold1.C: Likewise.
>> >> > * g++.dg/parse/array-size2.C: Likewise.
>> >> > * g++.dg/other/vrp1.C: Add dg-error.
>> >> > * g++.dg/template/char1.C: Likewise.
>> >> > * g++.dg/ext/builtin12.C: Likewise.
>> >> > * g++.dg/template/dependent-name3.C: Adjust dg-error.
>> >> >
>> >> > diff --git gcc/cp/call.c gcc/cp/call.c
>> >> > index 209c1fd2f0e..956c7b149dc 100644
>> >> > --- gcc/cp/call.c
>> >> > +++ gcc/cp/call.c
>> >> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, tree 
>> >> > expr, tsubst_flags_t complain)
>> >> >  }
>> >> >
>> >> >if (conv)
>> >> > -expr = convert_like (conv, expr, complain);
>> >> > +{
>> >> > +  conv->check_narrowing = !processing_template_decl;
>> >>
>> >> Why !processing_template_decl?  This needs a comment.
>> >
>> > Otherwise we'd warn for e.g.
>> >
>> > template struct S { char a[N]; };
>> > S<1> s;
>> >
>> > where compute_array_index_type will try to convert the size of the array 
>> > (which
>> > is a template_parm_index of type int when parsing the template) to 
>> > size_type.
>> > So I guess I can say that we need to wait for instantiation?
>>
>> We certainly shouldn't give a narrowing diagnostic about a
>> value-dependent expression.  It probably makes sense to check that at
>> the top of check_narrowing, with all the other early exit conditions.
>> But if we do know the constant value in the template, it&#

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill]

On Tue, Jul 3, 2018 at 3:41 PM, Jason Merrill  wrote:
> On Tue, Jul 3, 2018 at 2:58 PM, Marek Polacek  wrote:
>> On Tue, Jul 03, 2018 at 12:40:51PM -0400, Jason Merrill wrote:
>>> On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek  wrote:
>>> > On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>>> >> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek  
>>> >> wrote:
>>> >> > This PR complains about us accepting invalid code like
>>> >> >
>>> >> >   template struct A {};
>>> >> >   A<-1> a;
>>> >> >
>>> >> > Where we should detect the narrowing: [temp.arg.nontype] says
>>> >> > "A template-argument for a non-type template-parameter shall be a 
>>> >> > converted
>>> >> > constant expression ([expr.const]) of the type of the 
>>> >> > template-parameter."
>>> >> > and a converted constant expression can contain only
>>> >> > - integral conversions other than narrowing conversions,
>>> >> > - [...]."
>>> >> > It spurred e.g.
>>> >> > <https://stackoverflow.com/questions/28184888/how-implicit-conversion-works-for-non-type-template-parameters>
>>> >> > and has >=3 dups so it has some visibility.
>>> >> >
>>> >> > I think build_converted_constant_expr needs to set check_narrowing.
>>> >> > check_narrowing also always mentions that it's in { } but that is no 
>>> >> > longer
>>> >> > true; in the future it will also apply to <=>.  We'd probably have to 
>>> >> > add a new
>>> >> > flag to struct conversion if wanted to distinguish between these.
>>> >> >
>>> >> > This does not yet fix detecting narrowing in function templates 
>>> >> > (78244).
>>> >> >
>>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>> >> >
>>> >> > 2018-06-27  Marek Polacek  
>>> >> >
>>> >> > PR c++/57891
>>> >> > * call.c (build_converted_constant_expr): Set check_narrowing.
>>> >> > * decl.c (compute_array_index_type): Add warning sentinel.  Use
>>> >> > input_location.
>>> >> > * pt.c (convert_nontype_argument): Return NULL_TREE if any 
>>> >> > errors
>>> >> > were reported.
>>> >> > * typeck2.c (check_narrowing): Don't mention { } in diagnostic.
>>> >> >
>>> >> > * g++.dg/cpp0x/Wnarrowing6.C: New test.
>>> >> > * g++.dg/cpp0x/Wnarrowing7.C: New test.
>>> >> > * g++.dg/cpp0x/Wnarrowing8.C: New test.
>>> >> > * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>>> >> > * g++.dg/init/new43.C: Adjust dg-error.
>>> >> > * g++.dg/other/fold1.C: Likewise.
>>> >> > * g++.dg/parse/array-size2.C: Likewise.
>>> >> > * g++.dg/other/vrp1.C: Add dg-error.
>>> >> > * g++.dg/template/char1.C: Likewise.
>>> >> > * g++.dg/ext/builtin12.C: Likewise.
>>> >> > * g++.dg/template/dependent-name3.C: Adjust dg-error.
>>> >> >
>>> >> > diff --git gcc/cp/call.c gcc/cp/call.c
>>> >> > index 209c1fd2f0e..956c7b149dc 100644
>>> >> > --- gcc/cp/call.c
>>> >> > +++ gcc/cp/call.c
>>> >> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, tree 
>>> >> > expr, tsubst_flags_t complain)
>>> >> >  }
>>> >> >
>>> >> >if (conv)
>>> >> > -expr = convert_like (conv, expr, complain);
>>> >> > +{
>>> >> > +  conv->check_narrowing = !processing_template_decl;
>>> >>
>>> >> Why !processing_template_decl?  This needs a comment.
>>> >
>>> > Otherwise we'd warn for e.g.
>>> >
>>> > template struct S { char a[N]; };
>>> > S<1> s;
>>> >
>>> > where compute_array_index_type will try to convert the size of the array 
>>> > (which
>>> > is a template_parm_index of type int when parsing the template) to 
&g

Re: [C++ Patch] More location fixes to grokdeclarator

[Jason Merrill]

OK.

On Tue, Jul 3, 2018 at 2:37 PM, Paolo Carlini  wrote:
> Hi,
>
> On 03/07/2018 18:36, Jason Merrill wrote:
>>
>>
>>> if ((type_quals & TYPE_QUAL_VOLATILE)
>>> -  && (loc == UNKNOWN_LOCATION || locations[ds_volatile] < loc))
>>> +  && (loc == UNKNOWN_LOCATION
>>> +  || linemap_location_before_p (line_table,
>>> +locations[ds_volatile], loc)))
>>>   loc = locations[ds_volatile];
>>> if ((type_quals & TYPE_QUAL_RESTRICT)
>>> -  && (loc == UNKNOWN_LOCATION || locations[ds_restrict] < loc))
>>> +  && (loc == UNKNOWN_LOCATION
>>> +  || linemap_location_before_p (line_table,
>>> +locations[ds_restrict], loc)))
>>>   loc = locations[ds_restrict];
>>
>> Why not use min_location here?
>
> Indeed. Thus I successfully tested the below.
>
> Thanks,
> Paolo.
>
> //

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill]

On Tue, Jul 3, 2018 at 4:35 PM, Marek Polacek  wrote:
> On Tue, Jul 03, 2018 at 03:41:43PM -0400, Jason Merrill wrote:
>> On Tue, Jul 3, 2018 at 2:58 PM, Marek Polacek  wrote:
>> > On Tue, Jul 03, 2018 at 12:40:51PM -0400, Jason Merrill wrote:
>> >> On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek  wrote:
>> >> > On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>> >> >> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek  
>> >> >> wrote:
>> >> >> > This PR complains about us accepting invalid code like
>> >> >> >
>> >> >> >   template struct A {};
>> >> >> >   A<-1> a;
>> >> >> >
>> >> >> > Where we should detect the narrowing: [temp.arg.nontype] says
>> >> >> > "A template-argument for a non-type template-parameter shall be a 
>> >> >> > converted
>> >> >> > constant expression ([expr.const]) of the type of the 
>> >> >> > template-parameter."
>> >> >> > and a converted constant expression can contain only
>> >> >> > - integral conversions other than narrowing conversions,
>> >> >> > - [...]."
>> >> >> > It spurred e.g.
>> >> >> > <https://stackoverflow.com/questions/28184888/how-implicit-conversion-works-for-non-type-template-parameters>
>> >> >> > and has >=3 dups so it has some visibility.
>> >> >> >
>> >> >> > I think build_converted_constant_expr needs to set check_narrowing.
>> >> >> > check_narrowing also always mentions that it's in { } but that is no 
>> >> >> > longer
>> >> >> > true; in the future it will also apply to <=>.  We'd probably have 
>> >> >> > to add a new
>> >> >> > flag to struct conversion if wanted to distinguish between these.
>> >> >> >
>> >> >> > This does not yet fix detecting narrowing in function templates 
>> >> >> > (78244).
>> >> >> >
>> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >> >> >
>> >> >> > 2018-06-27  Marek Polacek  
>> >> >> >
>> >> >> > PR c++/57891
>> >> >> > * call.c (build_converted_constant_expr): Set 
>> >> >> > check_narrowing.
>> >> >> > * decl.c (compute_array_index_type): Add warning sentinel.  
>> >> >> > Use
>> >> >> > input_location.
>> >> >> > * pt.c (convert_nontype_argument): Return NULL_TREE if any 
>> >> >> > errors
>> >> >> > were reported.
>> >> >> > * typeck2.c (check_narrowing): Don't mention { } in 
>> >> >> > diagnostic.
>> >> >> >
>> >> >> > * g++.dg/cpp0x/Wnarrowing6.C: New test.
>> >> >> > * g++.dg/cpp0x/Wnarrowing7.C: New test.
>> >> >> > * g++.dg/cpp0x/Wnarrowing8.C: New test.
>> >> >> > * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>> >> >> > * g++.dg/init/new43.C: Adjust dg-error.
>> >> >> > * g++.dg/other/fold1.C: Likewise.
>> >> >> > * g++.dg/parse/array-size2.C: Likewise.
>> >> >> > * g++.dg/other/vrp1.C: Add dg-error.
>> >> >> > * g++.dg/template/char1.C: Likewise.
>> >> >> > * g++.dg/ext/builtin12.C: Likewise.
>> >> >> > * g++.dg/template/dependent-name3.C: Adjust dg-error.
>> >> >> >
>> >> >> > diff --git gcc/cp/call.c gcc/cp/call.c
>> >> >> > index 209c1fd2f0e..956c7b149dc 100644
>> >> >> > --- gcc/cp/call.c
>> >> >> > +++ gcc/cp/call.c
>> >> >> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, 
>> >> >> > tree expr, tsubst_flags_t complain)
>> >> >> >  }
>> >> >> >
>> >> >> >if (conv)
>> >> >> > -expr = convert_like (conv, expr, complain);
>> >> >> > +{
>> >> >> > +  conv->check_narrowing =

Re: [C++ PATCH] PR c++/86398

[Jason Merrill]

Ok.

On Wed, Jul 4, 2018, 11:52 AM Ville Voutilainen 
wrote:

> This has been buggy for a while, but since we were implementing
> the library triviality traits in terms of just the intrinsic since GCC 8,
> not too many users ran into it. The bug has been worked around
> in the library, but I'd rather have the intrinsic give the right
> answer and not require the library work-around.
>
> Tested on Linux-PPC64, ok for trunk and the gcc-8 branch?
>
> 2018-07-04  Ville Voutilainen  
>
> gcc/cp/
>
> PR c++/86398
> * method.c (is_trivially_xible): Return false
> if is_xible_helper returns a NULL_TREE.
>
> testsuite/
>
> PR c++/86398
> * g++.dg/ext/is_trivially_constructible1.C: Add new tests.
>

Re: [C++ PATCH] PR c++/79133

[Jason Merrill]

Did you consider handling this in check_local_shadow?

On Fri, Jul 6, 2018 at 7:50 PM, Ville Voutilainen
 wrote:
> Tested on Linux-PPC64. Ok for trunk, perhaps with the change
> that I move the test under cpp1y, since it's a c++14 test anyway?
>
> I considered pushing the captures into the parameter scope. I don't
> know how to do that; changing the pushdecl_outermost_localscope
> to a pushdecl doesn't seem to cut it; I guess that I should add
> a new function into name-lookup.[ch], but I wonder whether
> that makes sense, considering that this is lambda-only functionality.
> I also wonder whether it makes more sense than the solution
> in this patch, considering that we handle packs here as well
> and capturepack/parampack, capturepack/param, capture/parampack
> and capture/param clashes.
>
> Guidance welcome. This approach has the benefit that it, well,
> seems to work. :)
>
> 2018-07-07  Ville Voutilainen  
>
> gcc/cp/
>
> PR c++/79133
> * lambda.c (start_lambda_function): Reject captures and parameters
> with the same name.
>
> testsuite/
>
> PR c++/79133
> * g++.dg/cpp0x/lambda/lambda-shadow3.C: New.

Re: [PATCH] Fix PR86413

[Jason Merrill]

OK.

On Fri, Jul 6, 2018 at 8:14 PM, Richard Biener  wrote:
>
> The following fixes
>
> FAIL: gcc.dg/guality/pr48437.c   -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  line 14 i == 0
>
> because we now prune non-local/VAR_DECLs from BLOCK trees during
> free-lang-data (after we emitted early dwarf).  gen_block_die
> isn't prepared for that and now refuses to add low/high-PC
> attributes to blocks that got all BLOCK_VARS stripped that way.
>
> The fix is to simply always emit them for early generated DIEs
> (so we only ever elide the DIE creation during early dwarf).
>
> Note this would allow us to prune BLOCK_VARS completely after
> early dwarf generation (but we need to keep the BLOCK tree itself
> for scoping obviously).
>
> Bootstrap / regtest running on x86_64-unknown-linux-gnu, ok?
>
> Thanks,
> Richard.
>
> 2018-07-06  Richard Biener  
>
> PR debug/86413
> * dwarf2out.c (gen_block_die): For an early generated DIE
> always output high/low PC attributes.
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index a7c4620cfc3..95232177d83 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -25622,6 +25622,11 @@ gen_block_die (tree stmt, dw_die_ref context_die)
>  /* The outer scopes for inlinings *must* always be represented.  We
> generate DW_TAG_inlined_subroutine DIEs for them.  (See below.) */
>  must_output_die = 1;
> +  else if (BLOCK_DIE (stmt))
> +/* If we already have a DIE then it was filled early.  Meanwhile
> +   we might have pruned all BLOCK_VARS as optimized out but we
> +   still want to generate high/low PC attributes so output it.  */
> +must_output_die = 1;
>else
>  {
>/* Determine if this block directly contains any "significant"

Re: [PATCH] C++: Fix PR86083

[Jason Merrill]

On Fri, Jul 6, 2018 at 5:20 PM, Andreas Krebbel  wrote:
> On 06/20/2018 01:41 PM, Andreas Krebbel wrote:
>> When turning a user-defined numerical literal into an operator
>> invocation the literal needs to be translated to the execution
>> character set.
>>
>> Bootstrapped and regtested on s390x. x86_64 still running.
>> Ok to apply if x86_64 is clean?
>>
>> Bye,
>>
>> -Andreas-
>>
>> gcc/cp/ChangeLog:
>>
>> 2018-06-20  Andreas Krebbel  
>>
>>   PR C++/86082
>>   * parser.c (make_char_string_pack):
>>   (cp_parser_userdef_numeric_literal):
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-06-20  Andreas Krebbel  
>>
>>   PR C++/86082
>>   * g++.dg/pr86082.C: New test.
>
> I've tested the patch also on GCC 7 and 8 branch. Ok to apply there as well?

Hmm, it seems safe enough, but also seems like a change with a limited
audience.  Would it work for you to include the patch just in your
builds of GCC 7 and 8?

Jason

Re: [C++ Patch] Use rich_location::add_range in three more places

[Jason Merrill]

Ok.

On Mon, Jul 9, 2018, 6:42 AM Paolo Carlini  wrote:

> Hi,
>
> noticed three additional error messages where an additional range seems
> appropriate. Tested x86_64-linux.
>
> Thanks, Paolo.
>
> //
>
>

C++ PATCH for c++/86480, nested variadic lambda and constexpr if

[Jason Merrill]

The problem here was that when looking for unexpanded packs, we looked
into IF_STMT_EXTRA_ARGS of a constexpr if, which includes the bindings
for function parameter packs, but not in a way that uses them.  So we
shouldn't look there for unexpanded packs.

Trying to write a smaller testcase revealed another bug, whereby
collecting the bindings for captured variables wasn't respecting
unevaluated context.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit c63bbedf903c28225d9cc273c14d3023e6074fa5
Author: Jason Merrill 
Date:   Fri Jul 13 23:22:26 2018 +1000

PR c++/86480 - nested variadic lambda and constexpr if.

* pt.c (find_parameter_packs_r) [IF_STMT]: Don't walk into
IF_STMT_EXTRA_ARGS.
* tree.c (cp_walk_subtrees) [DECLTYPE_TYPE]: Set
cp_unevaluated_operand.
[ALIGNOF_EXPR] [SIZEOF_EXPR] [NOEXCEPT_EXPR]: Likewise.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7e858a19d76..2b885793d3f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3865,6 +3865,17 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees, void* data)
 	return NULL_TREE;
   }
 
+case IF_STMT:
+  cp_walk_tree (&IF_COND (t), &find_parameter_packs_r,
+		ppd, ppd->visited);
+  cp_walk_tree (&THEN_CLAUSE (t), &find_parameter_packs_r,
+		ppd, ppd->visited);
+  cp_walk_tree (&ELSE_CLAUSE (t), &find_parameter_packs_r,
+		ppd, ppd->visited);
+  /* Don't walk into IF_STMT_EXTRA_ARGS.  */
+  *walk_subtrees = 0;
+  return NULL_TREE;
+
 default:
   return NULL_TREE;
 }
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index b1333f55e39..7e2a77bf67b 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4865,7 +4865,19 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
   break;
 
 case DECLTYPE_TYPE:
-  WALK_SUBTREE (DECLTYPE_TYPE_EXPR (*tp));
+  ++cp_unevaluated_operand;
+  /* We can't use WALK_SUBTREE here because of the goto.  */
+  result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (*tp), func, data, pset);
+  --cp_unevaluated_operand;
+  *walk_subtrees_p = 0;
+  break;
+
+case ALIGNOF_EXPR:
+case SIZEOF_EXPR:
+case NOEXCEPT_EXPR:
+  ++cp_unevaluated_operand;
+  result = cp_walk_tree (&TREE_OPERAND (*tp, 0), func, data, pset);
+  --cp_unevaluated_operand;
   *walk_subtrees_p = 0;
   break;
  
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C
new file mode 100644
index 000..cbdb38d95c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if24.C
@@ -0,0 +1,21 @@
+// PR c++/86480
+// { dg-additional-options -std=c++17 }
+
+template  constexpr bool val = true;
+
+template 
+void f()
+{
+  [](auto... p)
+{
+  []{
+	if constexpr (val) { return true; }
+	return false;
+  }();
+}(42);
+}
+
+int main()
+{
+  f();
+}

Re: [C++ PATCH] Disallow type specifiers among lambda-declarator decl-specifier-seq (PR c++/86550)

[Jason Merrill]

On Wed, Jul 18, 2018 at 4:57 AM, Jakub Jelinek  wrote:
> The standard says:
> "In the decl-specifier-seq of the lambda-declarator, each decl-specifier shall
> either be mutable or constexpr."
> and the C++ FE has CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR flag for that.
> But as implemented, it is actually
> CP_PARSER_FLAGS_ONLY_TYPE_OR_MUTABLE_OR_CONSTEXPR
> as it allows mutable, constexpr and type specifiers.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2018-07-17  Jakub Jelinek  
>
> PR c++/86550
> * parser.c (cp_parser_decl_specifier_seq): Don't parse a type 
> specifier
> if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.

I think the diagnostic would be better if we parse the type-specifier
and then give an error about it being invalid in this context, rather
than not parse it and therefore give a syntax error; the constraint is
semantic rather than syntactic.

Jason

Re: [C++ Patch] Another permerror related tweak

[Jason Merrill]

OK.  And I think this qualifies as obvious; you don't need to wait for
approval on further patches like this.

Jason

On Tue, Jul 17, 2018 at 10:17 PM, Paolo Carlini
 wrote:
> Hi again,
>
> here I noticed a pair of consecutive permerrors (instead of permerror +
> inform) and also that the first one isn't exploiting DECL_SOURCE_LOCATION
> (which makes a real difference at least in a couple of cases which I'm
> explicitly testing below). Tested x86_64-linux.
>
> Thanks, Paolo.
>
> /
>

Re: C++ PATCH for c++/86190, bogus -Wsign-conversion warning

[Jason Merrill]

OK.

On Fri, Jul 13, 2018 at 11:43 PM, Marek Polacek  wrote:
> Ping.
>
> On Tue, Jul 03, 2018 at 09:35:24AM -0400, Marek Polacek wrote:
>> This PR complains about bogus -Wsign-conversion warning even with an
>> explicit static_cast.  It started with this hunk from the delayed folding
>> merge:
>>
>> @@ -5028,20 +5022,12 @@ cp_build_binary_op (location_t location,
>>
>>if (short_compare)
>> {
>> - /* Don't write &op0, etc., because that would prevent op0
>> -from being kept in a register.
>> -Instead, make copies of the our local variables and
>> -pass the copies by reference, then copy them back afterward.  */
>> - tree xop0 = op0, xop1 = op1, xresult_type = result_type;
>> + /* We call shorten_compare only for diagnostic-reason.  */
>> + tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
>> +  xresult_type = result_type;
>>   enum tree_code xresultcode = resultcode;
>> - tree val
>> -   = shorten_compare (location, &xop0, &xop1, &xresult_type,
>> + shorten_compare (location, &xop0, &xop1, &xresult_type,
>>&xresultcode);
>> - if (val != 0)
>> -   return cp_convert (boolean_type_node, val, complain);
>> - op0 = xop0, op1 = xop1;
>> - converted = 1;
>> - resultcode = xresultcode;
>> }
>>
>>if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
>>
>> which means that converted is now unset so we go to
>>
>>  5350   if (! converted)
>>  5351 {
>>  5352   if (TREE_TYPE (op0) != result_type)
>>  5353 op0 = cp_convert_and_check (result_type, op0, complain);
>>  5354   if (TREE_TYPE (op1) != result_type)
>>  5355 op1 = cp_convert_and_check (result_type, op1, complain);
>>
>> and cp_convert_and_check gives those warning.  The direct comparison
>> of types instead of same_type_p means we can try to convert same types,
>> but it still wouldn't fix this PR.  What we should probably do is to
>> simply disable -Wsign-conversion conversion for comparison, because
>> -Wsign-compare will warn for those.  With this patch, the C++ FE will
>> follow what the C FE and clang++ do.
>>
>> Also fix some formatting that's been bothering me, while at it.
>>
>> Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
>>
>> 2018-07-03  Marek Polacek  
>>
>>   PR c++/86190 - bogus -Wsign-conversion warning
>>   * typeck.c (cp_build_binary_op): Fix formatting.  Add a warning
>>   sentinel.
>>
>>   * g++.dg/warn/Wsign-conversion-3.C: New test.
>>   * g++.dg/warn/Wsign-conversion-4.C: New test.
>>
>> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
>> index 3a4f1cdf479..cfd1dd8b150 100644
>> --- gcc/cp/typeck.c
>> +++ gcc/cp/typeck.c
>> @@ -5311,12 +5311,13 @@ cp_build_binary_op (location_t location,
>>
>>if (short_compare)
>>   {
>> -   /* We call shorten_compare only for diagnostic-reason.  */
>> -   tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
>> -xresult_type = result_type;
>> +   /* We call shorten_compare only for diagnostics.  */
>> +   tree xop0 = fold_simple (op0);
>> +   tree xop1 = fold_simple (op1);
>> +   tree xresult_type = result_type;
>> enum tree_code xresultcode = resultcode;
>> shorten_compare (location, &xop0, &xop1, &xresult_type,
>> -&xresultcode);
>> +&xresultcode);
>>   }
>>
>>if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
>> @@ -5349,6 +5350,7 @@ cp_build_binary_op (location_t location,
>>   otherwise, it will be given type RESULT_TYPE.  */
>>if (! converted)
>>  {
>> +  warning_sentinel w (warn_sign_conversion, short_compare);
>>if (TREE_TYPE (op0) != result_type)
>>   op0 = cp_convert_and_check (result_type, op0, complain);
>>if (TREE_TYPE (op1) != result_type)
>> diff --git gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C 
>> gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> index e69de29bb2d..2c3fef31475 100644
>> --- gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> +++ gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
>> @@ -0,0 +1,13 @@
>> +// PR c++/86190
>> +// { dg-options "-Wsign-conversion -Wsign-compare" }
>> +
>> +typedef unsigned long sz_t;
>> +sz_t s();
>> +bool f(int i) { return s() < (unsigned long) i; }
>> +bool f2(int i) { return s() < static_cast(i); }
>> +bool f3(int i) { return s() < i; } // { dg-warning "comparison of integer 
>> expressions of different signedness" }
>> +bool f4(int i) { return s() < (long) i; } // { dg-warning "comparison of 
>> integer expressions of different signedness" }
>> +bool f5(short int i) { return s() < (int) i; } // { dg-warning "comparison 
>> of integer expressions of different signedness" }
>> +bool f6(signed char i) { return s() < (int) i; } // { dg-warning 
>> "comparison of integer expressions of different signedness" }
>> +bool f7(unsigned char i) { return s() < i; }
>> +bool f8(signed char i) { 

Re: [C++ Patch] PR 59480 ("Missing error diagnostic: friend declaration specifying a default argument must be a definition")

[Jason Merrill]

OK.

On Thu, Jul 12, 2018 at 7:52 PM, Paolo Carlini  wrote:
> Hi,
>
> the below resolves the bug report and its duplicates by implementing - in a
> rather straightforward way, I believe - the resolution of DR 136, which also
> made into C++17. Note that in the patch I used permerror instead of a plain
> error for consistency with the other check
> (check_redeclaration_no_default_args) which I added (rather) recently, and
> I'm exploiting that to allow two existing testcases to compile as they are.
> Tested x86_64-linux.
>
> Thanks, Paolo.
>
> /
>

Re: [C++ PATCH] Disallow type specifiers among lambda-declarator decl-specifier-seq (PR c++/86550)

[Jason Merrill]

OK.

On Wed, Jul 18, 2018 at 6:20 PM, Jakub Jelinek  wrote:
> On Wed, Jul 18, 2018 at 11:34:30AM +1000, Jason Merrill wrote:
>> On Wed, Jul 18, 2018 at 4:57 AM, Jakub Jelinek  wrote:
>> > The standard says:
>> > "In the decl-specifier-seq of the lambda-declarator, each decl-specifier 
>> > shall
>> > either be mutable or constexpr."
>> > and the C++ FE has CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR flag for that.
>> > But as implemented, it is actually
>> > CP_PARSER_FLAGS_ONLY_TYPE_OR_MUTABLE_OR_CONSTEXPR
>> > as it allows mutable, constexpr and type specifiers.
>> >
>> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>> > trunk?
>> >
>> > 2018-07-17  Jakub Jelinek  
>> >
>> > PR c++/86550
>> > * parser.c (cp_parser_decl_specifier_seq): Don't parse a type 
>> > specifier
>> > if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.
>>
>> I think the diagnostic would be better if we parse the type-specifier
>> and then give an error about it being invalid in this context, rather
>> than not parse it and therefore give a syntax error; the constraint is
>> semantic rather than syntactic.
>
> So like this?
> It will diagnose each bool and int separately, but that is similar to how it
> will diagnose
> [] () static extern thread_local inline virtual virtual explicit {}
> too.
>
> 2018-07-18  Jakub Jelinek  
>
> PR c++/86550
> * parser.c (cp_parser_decl_specifier_seq): Diagnose invalid type
> specifier if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.
>
> * g++.dg/cpp0x/lambda/lambda-86550.C: New test.
>
> --- gcc/cp/parser.c.jj  2018-07-17 20:08:07.630224343 +0200
> +++ gcc/cp/parser.c 2018-07-18 10:09:10.655030931 +0200
> @@ -13797,6 +13797,9 @@ cp_parser_decl_specifier_seq (cp_parser*
>   found_decl_spec = true;
>   if (!is_cv_qualifier)
> decl_specs->any_type_specifiers_p = true;
> +
> + if ((flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR) != 0)
> +   error_at (token->location, "type-specifier invalid in 
> lambda");
> }
> }
>
> --- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C.jj 2018-07-18 
> 10:05:02.894767883 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-86550.C2018-07-18 
> 10:13:41.373318350 +0200
> @@ -0,0 +1,9 @@
> +// PR c++/86550
> +// { dg-do compile { target c++11 } }
> +
> +void
> +foo ()
> +{
> +  auto a = []() bool {};   // { dg-error "type-specifier 
> invalid in lambda" }
> +  auto b = []() bool bool bool bool int {};// { dg-error "type-specifier 
> invalid in lambda" }
> +}
>
>
> Jakub

Re: [PATCH 3/5] C++: clean up cp_printer

[Jason Merrill]

OK.

On Sat, Jul 28, 2018 at 8:02 AM, David Malcolm  wrote:
> This makes it easier to compare cp_printer with gcc_cxxdiag_char_table
> in c-format.c.
>
> No functional change intended.
>
> gcc/cp/ChangeLog:
> * error.c (cp_printer): In the leading comment, move "%H" and "I"
> into alphabetical order, and add missing "%G" and "K".  Within the
> switch statement, move cases 'G', 'H', 'I' and 'K" so that the
> cases are in alphabetical order.
> ---
>  gcc/cp/error.c | 46 --
>  1 file changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/gcc/cp/error.c b/gcc/cp/error.c
> index b0d8e32..7a644fd 100644
> --- a/gcc/cp/error.c
> +++ b/gcc/cp/error.c
> @@ -4025,6 +4025,10 @@ defer_phase_2_of_type_diff (deferred_printed_type 
> *deferred,
> %D   declaration.
> %E   expression.
> %F   function declaration.
> +   %G   gcall *
> +   %H   type difference (from).
> +   %I   type difference (to).
> +   %K   tree
> %L  language as used in extern "lang".
> %O  binary operator.
> %P   function parameter whose position is indicated by an integer.
> @@ -4032,9 +4036,7 @@ defer_phase_2_of_type_diff (deferred_printed_type 
> *deferred,
> %S   substitution (template + args)
> %T   type.
> %V   cv-qualifier.
> -   %X   exception-specification.
> -   %H   type difference (from)
> -   %I   type difference (to).  */
> +   %X   exception-specification.  */
>  static bool
>  cp_printer (pretty_printer *pp, text_info *text, const char *spec,
> int precision, bool wide, bool set_locus, bool verbose,
> @@ -4076,6 +4078,21 @@ cp_printer (pretty_printer *pp, text_info *text, const 
> char *spec,
>break;
>  case 'E': result = expr_to_string (next_tree); break;
>  case 'F': result = fndecl_to_string (next_tree, verbose);  break;
> +case 'G':
> +  percent_G_format (text);
> +  return true;
> +case 'H':
> +  defer_phase_2_of_type_diff (&postprocessor->m_type_a, next_tree,
> + buffer_ptr, verbose, *quoted);
> +  return true;
> +case 'I':
> +  defer_phase_2_of_type_diff (&postprocessor->m_type_b, next_tree,
> + buffer_ptr, verbose, *quoted);
> +  return true;
> +case 'K':
> +  t = va_arg (*text->args_ptr, tree);
> +  percent_K_format (text, t);
> +  return true;
>  case 'L': result = language_to_string (next_lang); break;
>  case 'O': result = op_to_string (false, next_tcode);   break;
>  case 'P': result = parm_to_string (next_int);  break;
> @@ -4090,29 +4107,6 @@ cp_printer (pretty_printer *pp, text_info *text, const 
> char *spec,
>  case 'V': result = cv_to_string (next_tree, verbose);  break;
>  case 'X': result = eh_spec_to_string (next_tree, verbose);  break;
>
> -case 'G':
> -  percent_G_format (text);
> -  return true;
> -
> -case 'K':
> -  t = va_arg (*text->args_ptr, tree);
> -  percent_K_format (text, t);
> -  return true;
> -
> -case 'H':
> -  {
> -   defer_phase_2_of_type_diff (&postprocessor->m_type_a, next_tree,
> -   buffer_ptr, verbose, *quoted);
> -   return true;
> -  }
> -
> -case 'I':
> -  {
> -   defer_phase_2_of_type_diff (&postprocessor->m_type_b, next_tree,
> -   buffer_ptr, verbose, *quoted);
> -   return true;
> -  }
> -
>  default:
>return false;
>  }
> --
> 1.8.5.3
>

Re: [PATCH][c++] Fix DECL_BY_REFERENCE of clone parms

[Jason Merrill]

OK.

On Tue, Jul 31, 2018 at 7:22 PM, Richard Biener  wrote:
> On Mon, 30 Jul 2018, Tom de Vries wrote:
>
>> Hi,
>>
>> Consider test.C compiled at -O0 -g:
>> ...
>> class string {
>> public:
>>   string (const char *p) { this->p = p ; }
>>   string (const string &s) { this->p = s.p; }
>>
>> private:
>>   const char *p;
>> };
>>
>> class foo {
>> public:
>>   foo (string dir_hint) {}
>> };
>>
>> int
>> main (void)
>> {
>>   std::string s = "This is just a string";
>>   foo bar(s);
>>   return 0;
>> }
>> ...
>>
>> When parsing foo::foo, the dir_hint parameter gets a DECL_ARG_TYPE of
>> 'struct string & restrict'.  Then during finish_struct, we call
>> clone_constructors_and_destructors and create clones for foo::foo, and
>> set the DECL_ARG_TYPE in the same way.
>>
>> Later on, during finish_function, cp_genericize is called for the original
>> foo::foo, which sets the type of parm dir_hint to DECL_ARG_TYPE, and sets
>> DECL_BY_REFERENCE of dir_hint to 1.
>>
>> After that, during maybe_clone_body update_cloned_parm is called with:
>> ...
>> (gdb) call debug_generic_expr (parm.typed.type)
>> struct string & restrict
>> (gdb) call debug_generic_expr (cloned_parm.typed.type)
>> struct string
>> ...
>> The type of the cloned_parm is then set to the type of parm, but
>> DECL_BY_REFERENCE is not set.
>>
>> When doing cp_genericize for the clone later on,
>> TREE_ADDRESSABLE (TREE_TYPE ()) is no longer true for the updated type of
>> the parm, so DECL_BY_REFERENCE is not set there either.
>>
>> This patch fixes the problem by copying DECL_BY_REFERENCE in 
>> update_cloned_parm.
>>
>> Build and reg-tested on x86_64.
>>
>> OK for trunk?
>
> Thanks for tracking this down.  It looks OK to me but please leave
> Jason and Nathan a day to comment.
>
> Otherwise OK for trunk and also for branches after a while.
>
> Thanks,
> Richard.
>
>> Thanks,
>> - Tom
>>
>> [c++] Fix DECL_BY_REFERENCE of clone parms
>>
>> 2018-07-30  Tom de Vries  
>>
>>   PR debug/86687
>>   * optimize.c (update_cloned_parm): Copy DECL_BY_REFERENCE.
>>
>>   * g++.dg/guality/pr86687.C: New test.
>>
>> ---
>>  gcc/cp/optimize.c  |  2 ++
>>  gcc/testsuite/g++.dg/guality/pr86687.C | 28 
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c
>> index 0e9b84ed8a4..3923a5fc6c4 100644
>> --- a/gcc/cp/optimize.c
>> +++ b/gcc/cp/optimize.c
>> @@ -46,6 +46,8 @@ update_cloned_parm (tree parm, tree cloned_parm, bool 
>> first)
>>/* We may have taken its address.  */
>>TREE_ADDRESSABLE (cloned_parm) = TREE_ADDRESSABLE (parm);
>>
>> +  DECL_BY_REFERENCE (cloned_parm) = DECL_BY_REFERENCE (parm);
>> +
>>/* The definition might have different constness.  */
>>TREE_READONLY (cloned_parm) = TREE_READONLY (parm);
>>
>> diff --git a/gcc/testsuite/g++.dg/guality/pr86687.C 
>> b/gcc/testsuite/g++.dg/guality/pr86687.C
>> new file mode 100644
>> index 000..140a6fce596
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/guality/pr86687.C
>> @@ -0,0 +1,28 @@
>> +// PR debug/86687
>> +// { dg-do run }
>> +// { dg-options "-g" }
>> +
>> +class string {
>> +public:
>> +  string (int p) { this->p = p ; }
>> +  string (const string &s) { this->p = s.p; }
>> +
>> +  int p;
>> +};
>> +
>> +class foo {
>> +public:
>> +  foo (string dir_hint) {
>> +p = dir_hint.p; // { dg-final { gdb-test . "dir_hint.p" 3 } }
>> +  }
>> +
>> +  int p;
>> +};
>> +
>> +int
>> +main (void)
>> +{
>> +  string s = 3;
>> +  foo bar(s);
>> +  return !(bar.p == 3);
>> +}
>>
>>
>
> --
> Richard Biener 
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
> 21284 (AG Nuernberg)

Re: [PATCH] convert braced initializers to strings (PR 71625)

[Jason Merrill]

On Tue, Jul 31, 2018 at 9:51 AM, Martin Sebor  wrote:
> The middle-end contains code to determine the lengths of constant
> character arrays initialized by string literals.  The code is used
> in a number of optimizations and warnings.
>
> However, the code is unable to deal with constant arrays initialized
> using the braced initializer syntax, as in
>
>   const char a[] = { '1', '2', '\0' };
>
> The attached patch extends the C and C++ front-ends to convert such
> initializers into a STRING_CST form.
>
> The goal of this work is to both enable existing optimizations for
> such arrays, and to help detect bugs due to using non-nul terminated
> arrays where nul-terminated strings are expected.  The latter is
> an extension of the GCC 8 _Wstringop-overflow and
> -Wstringop-truncation warnings that help detect or prevent reading
> past the end of dynamically created character arrays.  Future work
> includes detecting potential past-the-end reads from uninitialized
> local character arrays.

>   && TYPE_MAIN_VARIANT (TREE_TYPE (valtype)) == char_type_node)

Why? Don't we want this for other character types as well?

Jason

Re: [C++2A] Implement P1008R1 - prohibit aggregates with user-declared constructors

[Jason Merrill]

On Mon, Jul 30, 2018 at 9:01 PM, Jakub Jelinek  wrote:
> Seems what is considered an aggregate type keeps changing in every single
> C++ version.

Indeed :/

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

This is OK.  I think we could also use a -Wc++2a-compat warning, and
an inform in 2a mode about why your initializer doesn't work any more,
but they don't need to go in right now.

Jason

Re: [C++ PATCH] Implement P0595R1 - so far as __builtin_is_constant_evaluated rather than std::is_constant_evaluated magic builtin

[Jason Merrill]

On Mon, Jul 23, 2018 at 8:50 PM, Richard Biener
 wrote:
> On Mon, Jul 23, 2018 at 12:28 PM Jakub Jelinek  wrote:
>>
>> On Mon, Jul 23, 2018 at 12:17:42PM +0200, Richard Biener wrote:
>> > > Bootstrapped/regtested on x86_64-linux.
>> >
>> > Thanks for working on this.  I wonder if we can completely hide this
>> > from the middle-end, without requiring defining of c_dialect_cxx.
>> > There is the BUILT_IN_FRONTEND class so you could somewhere
>> > manually inject a decl in that class for C++?
>>
>> But then I couldn't handle folding of the builtin in the middle-end to false,
>> which is what I need (because in the FE it needs to be either folded to
>> true, or its folding deferred until later).
>> Or maybe in the C++ gimplification langhook?
>
> Yes, I was thinking the C++ langhook or its fully_fold routine.

fully_fold is too soon (until constexpr evaluation uses the
pre-genericize form), but the gimplification hook should work.

>> Seems we have a single BUILT_IN_FRONTEND builtin in the whole compiler,
>> __integer_pack, but it doesn't act as a normal builtin, given it is a
>> templatish magic.
>
> Yeah, I think at some point we considered removing BUILT_IN_FRONTEND ...
>
> Nowadays internal-use builtins can easily be internal-functions but of couse
> this one will eventually be used from libstdc++.

Immediately, I'd think.

Jason

Re: [PATCH] ABOUT-GCC-NLS: add usage guidance

[Jason Merrill]

On 10/19/23 11:21, Jakub Jelinek wrote:

On Thu, Oct 19, 2023 at 11:11:30AM -0400, Jason Merrill wrote:

A recent question led me to look at this file again, and it occurred to me that
it could use to offer more guidance.  OK for trunk?

-- 8< --

gcc/ChangeLog:

* ABOUT-GCC-NLS: Add usage guidance.
---
  gcc/ABOUT-GCC-NLS | 13 +
  1 file changed, 13 insertions(+)

diff --git a/gcc/ABOUT-GCC-NLS b/gcc/ABOUT-GCC-NLS
index e90a67144e3..4c8b94d0811 100644
--- a/gcc/ABOUT-GCC-NLS
+++ b/gcc/ABOUT-GCC-NLS
@@ -23,6 +23,19 @@ For example, GCC source code should not contain calls like 
`error
  ("unterminated comment")' instead, as it is the `error' function's
  responsibility to translate the message before the user sees it.
  
+In general, use no markup for strings that are the immediate format string

+argument of a diagnostic function.  Use G_("str") for strings that will be
+used as the format string for a diagnostic but are e.g. assigned to a
+variable first.  Use N_("str") for other strings, particularly in a
+statically allocated array, that will be translated later by
+e.g. _(msgs[idx]).  Use _("str") for strings that will not be translated


The difference between G_ and N_ is whether they are GCC internal diagnostic
format strings or printf family format strings (gcc-internal-format vs.
c-format in po/gcc.pot), not anything else, so G_ can be used in statically
allocated array as well and both for diagnostic routines and printf* family
when translation is desirable it is eventually translated through _() or
other gettext invocations, either inside of diagnostics.cc or elsewhere.
So, the note about statically allocated array should move to G_ as well, and
N_ should be described as similarly, but for strings which after translation
are passed to printf family functions or so.  ANd the translated later by
e.g. note should be again for both and said that it is done automatically
for GCC diagnostic routines.


How about this?

 In general, use no markup for strings that are the immediate format string
 argument of a diagnostic function.  Use G_("str") for strings that will be
 used as the format string for a diagnostic but are e.g. assigned to a
 variable first.  Use N_("str") for strings that are not diagnostic format
 strings, but will still be translated later.  Use _("str") for strings 
that

 will not be translated elsewhere.  It's important not to use _("str") in
 the initializer of a statically allocated variable; use one of the others
 instead and make sure that uses of that variable translate the string,
 whether directly with _(msg) or by passing it to a diagnostic or other
 function that performs the translation.

Jason

Re: [PATCH v3] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/19/23 10:14, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 10:06:01AM -0400, Jason Merrill wrote:

On 10/19/23 09:39, Patrick Palka wrote:

On Tue, 17 Oct 2023, Marek Polacek wrote:


On Tue, Oct 17, 2023 at 04:49:52PM -0400, Jason Merrill wrote:

On 10/16/23 20:39, Marek Polacek wrote:

On Sat, Oct 14, 2023 at 01:13:22AM -0400, Jason Merrill wrote:

On 10/13/23 14:53, Marek Polacek wrote:

On Thu, Oct 12, 2023 at 09:41:43PM -0400, Jason Merrill wrote:

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

 PR c++/111660

gcc/cp/ChangeLog:

 * cp-gimplify.cc (cp_fold_immediate_r) : Return
 integer_zero_node instead of break;.
 (cp_fold_immediate): Return true if cp_fold_immediate_r returned
 error_mark_node.

gcc/testsuite/ChangeLog:

 * g++.dg/cpp0x/hog1.C: New test.
---
  gcc/cp/cp-gimplify.cc |  9 ++--
  gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
  2 files changed, 82 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
/* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from
looking any farther.


You are right.  The line above works as expected, but

  1 ? 1 : ((1 ? 1 : id (42)), id (i));

shows the problem (when the expression isn't used as an initializer).


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?


I hope this version is better.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.


Is this number still accurate for this version?


It is.  I ran time(1) a few more times and the results were 0m0.033s - 0m0.035s.
That said, ...


This change seems algorithmically better than the current code, but still
problematic: if we have nested COND_EXPR A/B/C/D/E, it looks like we will
end up cp_fold_immediate_r walking the arms of E five times, once for each
COND_EXPR.


...this is accurate.  I should have addressed the redundant folding in v2
even though the compilation is pretty much immediate.

What I was thinking by handling COND_EXPR in cp_fold_r was to cp_fold_r walk
its subtrees (or cp_fold_immediate_r if it's clear from op0 that the branch
isn't taken) so we can clear *walk_subtrees and we don't fold_imm walk a
node more than once.


I agree I should do better here.  How's this, then?  I've added
debug_generic_expr to cp_fold_immediate_r to see if it gets the same
expr multiple times and it doesn't seem to.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn'

Re: [PATCH v3] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/19/23 12:55, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 12:32:49PM -0400, Jason Merrill wrote:

On 10/19/23 10:14, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 10:06:01AM -0400, Jason Merrill wrote:

On 10/19/23 09:39, Patrick Palka wrote:

On Tue, 17 Oct 2023, Marek Polacek wrote:


On Tue, Oct 17, 2023 at 04:49:52PM -0400, Jason Merrill wrote:

On 10/16/23 20:39, Marek Polacek wrote:

On Sat, Oct 14, 2023 at 01:13:22AM -0400, Jason Merrill wrote:

On 10/13/23 14:53, Marek Polacek wrote:

On Thu, Oct 12, 2023 at 09:41:43PM -0400, Jason Merrill wrote:

On 10/12/23 17:04, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.

I've added some debug prints to make sure that the rest of cp_fold_r
is still performed as before.

  PR c++/111660

gcc/cp/ChangeLog:

  * cp-gimplify.cc (cp_fold_immediate_r) : Return
  integer_zero_node instead of break;.
  (cp_fold_immediate): Return true if cp_fold_immediate_r returned
  error_mark_node.

gcc/testsuite/ChangeLog:

  * g++.dg/cpp0x/hog1.C: New test.
---
   gcc/cp/cp-gimplify.cc |  9 ++--
   gcc/testsuite/g++.dg/cpp0x/hog1.C | 77 +++
   2 files changed, 82 insertions(+), 4 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/hog1.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index bdf6e5f98ff..ca622ca169a 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1063,16 +1063,16 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, 
void *data_)
break;
 if (TREE_OPERAND (stmt, 1)
  && cp_walk_tree (&TREE_OPERAND (stmt, 1), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
 if (TREE_OPERAND (stmt, 2)
  && cp_walk_tree (&TREE_OPERAND (stmt, 2), cp_fold_immediate_r, data,
-  nullptr))
+  nullptr) == error_mark_node)
return error_mark_node;
 /* We're done here.  Don't clear *walk_subtrees here though: we're 
called
 from cp_fold_r and we must let it recurse on the expression with
 cp_fold.  */
-  break;
+  return integer_zero_node;


I'm concerned this will end up missing something like

1 ? 1 : ((1 ? 1 : 1), immediate())

as the integer_zero_node from the inner ?: will prevent walk_tree from
looking any farther.


You are right.  The line above works as expected, but

   1 ? 1 : ((1 ? 1 : id (42)), id (i));

shows the problem (when the expression isn't used as an initializer).


Maybe we want to handle COND_EXPR in cp_fold_r instead of here?


I hope this version is better.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The problem is that the COND_EXPR
case recursively walks the arms of a COND_EXPR, but after processing
both arms it doesn't end the walk; it proceeds to walk the
sub-expressions of the outermost COND_EXPR, triggering again walking
the arms of the nested COND_EXPR, and so on.  This patch brings the
compile time down to about 0m0.033s.


Is this number still accurate for this version?


It is.  I ran time(1) a few more times and the results were 0m0.033s - 0m0.035s.
That said, ...


This change seems algorithmically better than the current code, but still
problematic: if we have nested COND_EXPR A/B/C/D/E, it looks like we will
end up cp_fold_immediate_r walking the arms of E five times, once for each
COND_EXPR.


...this is accurate.  I should have addressed the redundant folding in v2
even though the compilation is pretty much immediate.

What I was thinking by handling COND_EXPR in cp_fold_r was to cp_fold_r walk
its subtrees (or cp_fold_immediate_r if it's clear from op0 that the branch
isn't taken) so we can clear *walk_subtrees and we don't fold_imm walk a
node more than once.


I agree I should do better here.  How's this, then?  I've added
debug_generic_expr to cp_fold_immediate_r to see if it gets the same
expr multiple times and it doesn't seem to.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
My recent patch introducing cp_fold_immediate_r caused exponential
compile time with nested COND_EXPRs.  The 

Re: [PATCH v3] c++: Fix compile-time-hog in cp_fold_immediate_r [PR111660]

[Jason Merrill]

On 10/19/23 14:45, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 01:02:33PM -0400, Jason Merrill wrote:

On 10/19/23 12:55, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 12:32:49PM -0400, Jason Merrill wrote:

On 10/19/23 10:14, Marek Polacek wrote:

On Thu, Oct 19, 2023 at 10:06:01AM -0400, Jason Merrill wrote:

On 10/19/23 09:39, Patrick Palka wrote:

On Tue, 17 Oct 2023, Marek Polacek wrote:

[...]

  case PTRMEM_CST:
if (TREE_CODE (PTRMEM_CST_MEMBER (stmt)) == FUNCTION_DECL
  && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (stmt)))
@@ -1162,8 +1141,35 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
tree stmt = *stmt_p;
enum tree_code code = TREE_CODE (stmt);
-  if (cxx_dialect > cxx17)
-cp_fold_immediate_r (stmt_p, walk_subtrees, data);
+  if (cxx_dialect >= cxx20)
+{
+  /* Unfortunately we must handle code like
+  false ? bar () : 42
+where we have to check bar too.  The cp_fold call below could
+fold the ?: into a constant before we've checked it.  */
+  if (code == COND_EXPR)
+   {
+ auto then_fn = cp_fold_r, else_fn = cp_fold_r;
+ /* See if we can figure out if either of the branches is dead.  If it
+is, we don't need to do everything that cp_fold_r does.  */
+ tree cond = maybe_constant_value (TREE_OPERAND (stmt, 0));
+ if (integer_zerop (cond))
+   then_fn = cp_fold_immediate_r;
+ else if (TREE_CODE (cond) == INTEGER_CST)
+   else_fn = cp_fold_immediate_r;
+
+ cp_walk_tree (&TREE_OPERAND (stmt, 0), cp_fold_r, data, nullptr);


I wonder about doing this before maybe_constant_value, to hopefully reduce
the redundant calculations?  OK either way.


Yeah, I was toying with that, I had

  foo() ? x : y

where foo was a constexpr function but the cp_fold_r on op0 didn't eval it
to a constant :(.


IIUC that's because cp_fold evaluates constexpr calls only with -O, so
doing cp_fold_r before maybe_constant_value on the condition should
still be desirable in that case?


Hmm, and if the cp_fold_r doesn't reduce the test to a constant, I would
think that folding the COND_EXPR also won't discard the other branch, so we
shouldn't need to work harder to get a constant here, so we don't need to
call maybe_constant_value at all?


Sorry, I hadn't seen this message when I posted the tweak.  But the
maybe_constant_value call on TREE_OPERAND (stmt, 0) should still make
sense because it can render a branch dead even on -O0.  Right?


But if op0 isn't constant after cp_fold, folding the COND_EXPR won't discard
the branch, so we don't need to do the extra work to find out that it's
actually dead.


Hmm, so how about this?  Thus far dg.exp passed.

-- >8 --
This patch is an optimization tweak for cp_fold_r.  If we cp_fold_r the
COND_EXPR's op0 first, we may be able to evaluate it to a constant if -O.
cp_fold has:

3143 if (callee && DECL_DECLARED_CONSTEXPR_P (callee)
3144 && !flag_no_inline)
...
3151 r = maybe_constant_value (x, /*decl=*/NULL_TREE,

flag_no_inline is 1 for -O0:

1124   if (opts->x_optimize == 0)
1125 {
1126   /* Inlining does not work if not optimizing,
1127  so force it not to be done.  */
1128   opts->x_warn_inline = 0;
1129   opts->x_flag_no_inline = 1;
1130 }

but otherwise it's 0 and cp_fold will maybe_constant_value calls to
constexpr functions.  And if it doesn't, then folding the COND_EXPR
will keep both arms, and we can avoid calling maybe_constant_value.

gcc/cp/ChangeLog:

* cp-gimplify.cc (cp_fold_r): Don't call maybe_constant_value.
---
   gcc/cp/cp-gimplify.cc | 7 +++
   1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index a282c3930a3..385042aeef2 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1152,13 +1152,12 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void 
*data_)
  auto then_fn = cp_fold_r, else_fn = cp_fold_r;
  /* See if we can figure out if either of the branches is dead.  If it
 is, we don't need to do everything that cp_fold_r does.  */
- tree cond = maybe_constant_value (TREE_OPERAND (stmt, 0));
- if (integer_zerop (cond))
+ cp_walk_tree (&TREE_OPERAND (stmt, 0), cp_fold_r, data, nullptr);
+ if (integer_zerop (TREE_OPERAND (stmt, 0)))
then_fn = cp_fold_immediate_r;
- else if (TREE_CODE (cond) == INTEGER_CST)
+ else if (TREE_CODE (TREE_OPERAND (stmt, 0)) == INTEGER_CST)


You probably want to STRIP_NOPS before checking the TREE_CODE, like
fold_ternary_loc and integer_zerop do.


Ok, or use integer_nonzerop like Patrick suggested:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?


OK.


-- >8 --
This patch

Re: [PATCH] c++: merge tsubst_copy into tsubst_copy_and_build

[Jason Merrill]

On 10/4/23 12:08, Patrick Palka wrote:

On Tue, 3 Oct 2023, Jason Merrill wrote:


On 10/3/23 08:41, Patrick Palka wrote:

On Mon, 2 Oct 2023, Patrick Palka wrote:


Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

The relationship between tsubst_copy_and_build and tsubst_copy (two of
the main template argument substitution routines for expression trees)
is rather hazy.  The former is mostly a superset of the latter, with
some differences.

The main difference is that they handle many tree codes differently, but
much of the tree code handling in tsubst_copy appears to be dead code[1].
This is because tsubst_copy only gets directly called in a few places
and mostly on id-expressions.  The interesting exceptions are PARM_DECL,
VAR_DECL, BIT_NOT_EXPR, SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE:

   * for PARM_DECL and VAR_DECL, tsubst_copy_and_build calls tsubst_copy
 followed by doing some extra handling of its own
   * for BIT_NOT_EXPR tsubst_copy implicitly handles unresolved destructor
 calls (i.e. the first operand is an identifier or a type)
   * for SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE tsubst_copy
 refrains from doing name lookup of the terminal name

Other more minor differences are that tsubst_copy exits early when
'args' is null, and it calls maybe_dependent_member_ref


That's curious, since what that function does seems like name lookup; I
wouldn't think we would want to call it when tf_no_name_lookup.


Ah, that makes sense I think.




and finally it dispatches to tsubst for type trees.


And it looks like you fix the callers to avoid that?


Yes, I'll make a note of that in the commit message.




Thus tsubst_copy is (at this point) similar enough to
tsubst_copy_and_build
that it makes sense to merge the two functions, with the main difference
being the name lookup behavior[2].  So this patch merges tsubst_copy into
tsubst_copy_and_build via a new tsubst tf_no_name_lookup which controls
name lookup and resolution of a (top-level) id-expression.

[1]: http://thrifty.mooo.com:8008/gcc-lcov/gcc/cp/pt.cc.gcov.html#17231
[2]: I don't know the history of tsubst_copy but I would guess it was
added before we settled on using processing_template_decl to control
whether our AST building routines perform semantic checking and return
non-templated trees, and so we needed a separate tsubst routine that
avoids semantic checking and always returns a templated tree for e.g.
partial substitution.


Oops, this is wrong -- tsubst_copy_and_build came after tsubst_copy,
and was introduced as an optimization with the intent of getting rid
of tsubst_copy eventually:
https://gcc.gnu.org/pipermail/gcc-patches/2003-January/093659.html


I wonder if we want to add a small tsubst_name wrapper to call
tsubst_copy_and_build with tf_no_name_lookup?


Good idea, that'll complement tsubst_scope nicely.



Can we also merge in tsubst_expr and use that name instead of the unwieldy
tsubst_copy_and_build?


That'd be nice.  Another idea would be to rename tsubst_expr to
tsubst_stmt and make it disjoint from tsubst_copy_and_build, and then
rename tsubst_copy_and_build to tsubst_expr, to draw a distinction
between statement-like trees (the substitution of which typically has
side effects like calling add_stmt) and expression-like trees (which
don't usually have such side effects).  I can work on that as a
follow-up patch.

Here's v2 which guards the call to maybe_dependent_member_ref and adds
tsubst_name, bootstrapped and regtested on x86_64-pc-linux-gnu:


OK.

Re: [PATCH 2/1] c++: rename tsubst_copy_and_build and tsubst_expr

[Jason Merrill]

On 10/4/23 15:23, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?


OK.


-- >8 --

After the previous patch, we currently have two tsubst entry points
for expression trees: tsubst_copy_and_build and tsubst_expr.  But the
latter is just a superset of the former that also handles statement
trees.  We could merge the two entry points so that we just have
tsubst_expr, but it seems natural to distinguish statement trees from
expression trees and to maintain a separate entry point for them.

To that end, this this patch renames tsubst_copy_and_build to
tsubst_expr, and renames the current tsubst_expr to tsubst_stmt, which
continues to be a superset of the former (since sometimes expression
trees appear in statement contexts, e.g. a branch of an IF_STMT could be
NOP_EXPR).  Making tsubst_stmt disjoint from tsubst_expr is left as
future work (if deemed desirable).

This patch in turn renames suitable existing uses of tsubst_expr (that
expect to take statement trees) to use tsubst_stmt.  Thus untouched
tsubst_expr calls are implicitly strengthened to expect only expression
trees after this patch.  And since I'm not familiar with the
tsubst_omp_* routines this patch renames all tsubst_expr uses there to
tsubst_stmt to ensure no unintended functional change in that area.
This patch also moves the handling of CO_YIELD_EXPR and CO_AWAIT_EXPR
from tsubst_stmt to tsubst_expr since they're expression trees.

gcc/cp/ChangeLog:

* cp-lang.cc (objcp_tsubst_copy_and_build): Rename to ...
(objcp_tsubst_expr): ... this.
* cp-objcp-common.h (objcp_tsubst_copy_and_build): Rename to ...
(objcp_tsubst_expr): ... this.
* cp-tree.h (tsubst_copy_and_build): Remove declaration.
* init.cc (maybe_instantiate_nsdmi_init): Use tsubst_expr
instead of tsubst_copy_and_build.
* pt.cc (expand_integer_pack): Likewise.
(instantiate_non_dependent_expr_internal): Likewise.
(instantiate_class_template): Use tsubst_stmt instead of
tsubst_expr for STATIC_ASSERT.
(tsubst_function_decl): Adjust tsubst_copy_and_build uses.
(tsubst_arg_types): Likewise.
(tsubst_exception_specification): Likewise.
(tsubst_tree_list): Likewise.
(tsubst): Likewise.
(tsubst_name): Likewise.
(tsubst_omp_clause_decl): Use tsubst_stmt instead of tsubst_expr.
(tsubst_omp_clauses): Likewise.
(tsubst_copy_asm_operands): Adjust tsubst_copy_and_build use.
(tsubst_omp_for_iterator): Use tsubst_stmt instead of tsubst_expr.
(tsubst_expr): Rename to ...
(tsubst_stmt): ... this.
: Move to tsubst_expr.
(tsubst_omp_udr): Use tsubst_stmt instead of tsubst_expr.
(tsubst_non_call_postfix_expression): Adjust tsubst_copy_and_build
use.
(tsubst_lambda_expr): Likewise.  Use tsubst_stmt instead of
tsubst_expr for the body of a lambda.
(tsubst_copy_and_build_call_args): Rename to ...
(tsubst_call_args): ... this.  Adjust tsubst_copy_and_build use.
(tsubst_copy_and_build): Rename to tsubst_expr.  Adjust
tsubst_copy_and_build and tsubst_copy_and_build_call_args use.
: Use tsubst_stmt instead of tsubst_expr.
(maybe_instantiate_noexcept): Adjust tsubst_copy_and_build use.
(instantiate_body): Use tsubst_stmt instead of tsubst_expr for
substituting the function body.
(tsubst_initializer_list): Adjust tsubst_copy_and_build use.

gcc/objcp/ChangeLog:

* objcp-lang.cc (objcp_tsubst_copy_and_build): Rename to ...
(objcp_tsubst_expr): ... this.  Adjust tsubst_copy_and_build
uses.
---
  gcc/cp/cp-lang.cc|   6 +-
  gcc/cp/cp-objcp-common.h |   2 +-
  gcc/cp/cp-tree.h |   1 -
  gcc/cp/init.cc   |   3 +-
  gcc/cp/pt.cc | 177 +--
  gcc/objcp/objcp-lang.cc  |   5 +-
  6 files changed, 85 insertions(+), 109 deletions(-)

diff --git a/gcc/cp/cp-lang.cc b/gcc/cp/cp-lang.cc
index 2f541460c4a..f2ed83de4fb 100644
--- a/gcc/cp/cp-lang.cc
+++ b/gcc/cp/cp-lang.cc
@@ -113,10 +113,8 @@ struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
  /* The following function does something real, but only in Objective-C++.  */
  
  tree

-objcp_tsubst_copy_and_build (tree /*t*/,
-tree /*args*/,
-tsubst_flags_t /*complain*/,
-tree /*in_decl*/)
+objcp_tsubst_expr (tree /*t*/, tree /*args*/, tsubst_flags_t /*complain*/,
+  tree /*in_decl*/)
  {
return NULL_TREE;
  }
diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h
index 80893aa1752..1408301a300 100644
--- a/gcc/cp/cp-objcp-common.h
+++ b/gcc/cp/cp-objcp-common.h
@@ -24,7 +24,7 @@ along with GCC; see the file COPYING3.  If not see
  /* In cp/objcp-common.c, cp/cp-lang.cc and objcp/objcp-lang.cc.  */
  
  extern tree cp_get_d

Re: [PATCH 2/2] c++: remove NON_DEPENDENT_EXPR, part 2

[Jason Merrill]

On 9/25/23 16:43, Patrick Palka wrote:

This much more mechanical patch removes build_non_dependent_expr
(and make_args_non_dependent) and adjusts callers accordingly,
no functional change.


These two patches are OK either separately or squashed, whichever you 
prefer.



gcc/cp/ChangeLog:

* call.cc (build_new_method_call): Remove calls to
build_non_dependent_expr and/or make_args_non_dependent.
* coroutines.cc (finish_co_return_stmt): Likewise.
* cp-tree.h (build_non_dependent_expr): Remove.
(make_args_non_dependent): Remove.
* decl2.cc (grok_array_decl): Remove calls to
build_non_dependent_expr and/or make_args_non_dependent.
(build_offset_ref_call_from_tree): Likewise.
* init.cc (build_new): Likewise.
* pt.cc (make_args_non_dependent): Remove.
(test_build_non_dependent_expr): Remove.
(cp_pt_cc_tests): Adjust.
* semantics.cc (finish_expr_stmt): Remove calls to
build_non_dependent_expr and/or make_args_non_dependent.
(finish_for_expr): Likewise.
(finish_call_expr): Likewise.
(finish_omp_atomic): Likewise.
* typeck.cc (finish_class_member_access_expr): Likewise.
(build_x_indirect_ref): Likewise.
(build_x_binary_op): Likewise.
(build_x_array_ref): Likewise.
(build_x_vec_perm_expr): Likewise.
(build_x_shufflevector): Likewise.
(build_x_unary_op): Likewise.
(cp_build_addressof): Likewise.
(build_x_conditional_expr):
(build_x_compound_expr): Likewise.
(build_static_cast): Likewise.
(build_x_modify_expr): Likewise.
(check_return_expr): Likewise.
* typeck2.cc (build_x_arrow): Likewise.
---
  gcc/cp/call.cc   |  7 +--
  gcc/cp/coroutines.cc |  3 ---
  gcc/cp/cp-tree.h |  2 --
  gcc/cp/decl2.cc  | 17 +++-
  gcc/cp/init.cc   |  5 -
  gcc/cp/pt.cc | 46 
  gcc/cp/semantics.cc  | 25 ++--
  gcc/cp/typeck.cc | 31 -
  gcc/cp/typeck2.cc|  1 -
  9 files changed, 6 insertions(+), 131 deletions(-)

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index e8dafbd8ba6..15079ddf6dc 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -11430,12 +11430,7 @@ build_new_method_call (tree instance, tree fns, vec **args,
  }
  
if (processing_template_decl)

-{
-  orig_args = args == NULL ? NULL : make_tree_vector_copy (*args);
-  instance = build_non_dependent_expr (instance);
-  if (args != NULL)
-   make_args_non_dependent (*args);
-}
+orig_args = args == NULL ? NULL : make_tree_vector_copy (*args);
  
/* Process the argument list.  */

if (args != NULL && *args != NULL)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index df3cc820797..a5464becf7f 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1351,9 +1351,6 @@ finish_co_return_stmt (location_t kw, tree expr)
 to undo it so we can try to treat it as an rvalue below.  */
expr = maybe_undo_parenthesized_ref (expr);
  
-  if (processing_template_decl)

-   expr = build_non_dependent_expr (expr);
-
if (error_operand_p (expr))
return error_mark_node;
  }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 66b9a9c4b9a..8b9a7d58462 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7488,8 +7488,6 @@ extern bool any_value_dependent_elements_p  
(const_tree);
  extern bool dependent_omp_for_p   (tree, tree, tree, 
tree);
  extern tree resolve_typename_type (tree, bool);
  extern tree template_for_substitution (tree);
-inline tree build_non_dependent_expr   (tree t) { return t; } // XXX 
remove
-extern void make_args_non_dependent(vec *);
  extern bool reregister_specialization (tree, tree, tree);
  extern tree instantiate_non_dependent_expr(tree, tsubst_flags_t = 
tf_error);
  extern tree instantiate_non_dependent_expr_internal (tree, tsubst_flags_t);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 344e19ec98b..0aa1e355972 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -427,14 +427,8 @@ grok_array_decl (location_t loc, tree array_expr, tree 
index_exp,
  return build_min_nt_loc (loc, ARRAY_REF, array_expr, index_exp,
   NULL_TREE, NULL_TREE);
}
-  array_expr = build_non_dependent_expr (array_expr);
-  if (index_exp)
-   index_exp = build_non_dependent_expr (index_exp);
-  else
-   {
- orig_index_exp_list = make_tree_vector_copy (*index_exp_list);
- make_args_non_dependent (*index_exp_list);
-   }
+  if (!index_exp)
+   orig_index_exp_list = make_tree_vector_copy (*index_exp_list);
  }
  
type = TREE_TYPE (array_expr);

@@ -5435,18 +5429,13 @@ build_offset_ref_call_from_tree (tree fn, vec **args,
   

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 10/19/23 17:05, waffl3x wrote:

Also, I'm not sure what %qs is, should I be using that instead of %s
for strings?


The q prefix means quoted, with ' or other quotation marks, depending on 
the locale.



On another topic, I have been trying to fix taking pointers to explicit
object member functions today, as I ended up breaking that when I
started setting static_function to false for them. Originally it just
worked so I haven't touched any code related to this, but now that they
aren't being treating like static member functions as much so a few
things just broke. What I'm asking myself now is whether it would be
appropriate to just opt-in to static member function behavior for this
one, and I'm not sure that would be correct.

So I started by checking what it did before I turned off the
static_function flag. It's was being passed into cp_build_addr_expr_1
as a baselink node, while regular member functions are passed in as an
offset_ref node. I then checked what the case is for static member
function, and unsurprisingly those are also handled wrapped in a
baselink node, but this raised some questions for me.

I am now trying to figure out what exactly a baselink is, and why it's
used for static member functions. My current best guess is that
method_type nodes already hold the information that a baselink does,
and that information is needed in general. If that is the case, it
might just be correct to just do the same thing for explicit object
member functions, but I wonder if there is more to it, but maybe there
isn't. It worked just fine before when the static_function was still
being set after all.

Any insight on this is appreciated.


A BASELINK expresses the result of name lookup for a member function, 
since we need to pass information about the name lookup context along to 
after overload resolution.


An OFFSET_REF (with PTRMEM_OK_P) is used to express that we saw the 
&A::f syntax, so we could build a pointer to member if it resolves to an 
implicit-object member function.


For an overload set containing only a single static member function, 
build_offset_ref doesn't bother to build an OFFSET_REF, but returns the 
BASELINK itself.


I think we need the OFFSET_REF for an explicit-object member function 
because it expresses that the code satisfies the requirement "If the 
operand names an explicit object member function, the operand shall be a 
qualified-id."


It might simplify things to remove the optimization in build_offset_ref 
so we get an OFFSET_REF even for a single static member function, and 
add support for that to cp_build_addr_expr_1.


Jason

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 10/19/23 19:35, waffl3x wrote:

(waffl3x (me))
At a glance it seems like all I need to do then is disable the
PTRMEM_OK_P flag then.


I'm just now realizing that I'm almost certainly wrong about this. It
still needs PTRMEM_OK_P set if there are any implicit-object member
functions in the overload set. That is, if OFFSET_REF includes that
information... but it doesn't seem like it does? Reading the
information on OFFSET_REF, particularly build_offset_ref, seems to
indicate that OFFSET_REF (at least historically) was only for things
with a pointer to member type.


Or things that might end up with pointer-to-member type after overload 
resolution.



An OFFSET_REF (with PTRMEM_OK_P) is used to express that we saw the
&A::f syntax, so we could build a pointer to member if it resolves to an
implicit-object member function.

For an overload set containing only a single static member function,
build_offset_ref doesn't bother to build an OFFSET_REF, but returns the
BASELINK itself.


Based on what you've said, I assume that OFFSET_REF handles static
member functions that are overloaded. But as I've said this seems to
contradict the comments I'm reading, so I'm not sure that I'm
understanding you correctly.


That's right.  For instance,

struct A {
  static void g();
  static void g(int);
};

void (*p)(int) = &A::g; // cp_build_addr_expr_1 gets an OFFSET_REF


I think we need the OFFSET_REF for an explicit-object member function
because it expresses that the code satisfies the requirement "If the
operand names an explicit object member function, the operand shall be a
qualified-id."


I do agree here, but it does reinforce that OFFSET_REF is no longer
just for members represented by pointer to member type. So that might
be something to take into consideration.


An OFFSET_REF that isn't type_unknown_p, agreed.


It might simplify things to remove the optimization in build_offset_ref
so we get an OFFSET_REF even for a single static member function, and
add support for that to cp_build_addr_expr_1.


I don't think this should be necessary, the "right thing" should just
be done for explicit-object member functions. With all the stuff going
on here that I missed I'm starting to wonder how function overloads
ever worked at all in my patch. On the other hand though, this
optimization probably could be documented better, but I very well might
have missed it even if it were.

Hell, maybe it needs a greater redesign altogether, it seems strange to
me to bundle overload information in with a construct for a specific
expression. (Assuming that's whats happening of course, I still don't
fully understand it.) It's not like this has rules unique to it for how
overload resolution is decided, right? Initializing a param/variable of
pointer to function type with an overloaded function resolves that with
similar rules, I think? Maybe it is a little different now that I write
it out loud.

I wasn't going to finish my musings about that, but it made me realize
that it might not actually be correct for address of explicit-object
member functions to be wrapped by OFFSET_REF. I mean surely it's fine
because based on what you've said static member functions are also
wrapped by OFFSET_REF, so it's likely fully implemented, especially
considering things worked before. But now that there are 2 different
varieties of class members that the address of them can be taken, it
might make sense to split things up a bit? Then again, why were static
member functions ever handled the same way? Taking the address of other
static members isn't handled in the same way here is it?


Functions are different because of overloading; in general we can't 
decide what an expression that names a function actually means until we 
have enough context to decide which function, exactly.  So we represent 
the id-expression largely as lookup+syntax until overload resolution 
turns it into a specific function.  The type_unknown_p check earlier in 
cp_build_addr_expr_1 is for that case.


An id-expression that names a single non-template function 
(!really_overloaded_fn) is handled somewhat differently, as we don't 
need to defer everything.  But that means various special-case code.


Currently build_offset_ref special-cases &A::f for a single static 
member function, but we can't use the same special case for single 
explicit object member functions because we need to distinguish between 
&A::f and &f somehow to check the requirement I quoted above.  So it 
seems to me we'll need to add support for single explicit object member 
functions in the OFFSET_REF handling in cp_build_addr_expr_1.  And I 
thought if we're doing that, perhaps we want to move the single static 
handling over there as well, but that's not necessary.


Jason

Re: [PATCH v6] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286]

[Jason Merrill]

On 10/12/23 18:05, Nathaniel Shead wrote:

On Thu, Oct 12, 2023 at 04:24:00PM -0400, Jason Merrill wrote:

On 10/12/23 04:53, Nathaniel Shead wrote:

On Wed, Oct 11, 2023 at 12:48:12AM +1100, Nathaniel Shead wrote:

On Mon, Oct 09, 2023 at 04:46:46PM -0400, Jason Merrill wrote:

On 10/8/23 21:03, Nathaniel Shead wrote:

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631203.html

+ && (TREE_CODE (t) == MODIFY_EXPR
+ /* Also check if initializations have implicit change of active
+member earlier up the access chain.  */
+ || !refs->is_empty())


I'm not sure what the cumulative point of these two tests is.  TREE_CODE (t)
will be either MODIFY_EXPR or INIT_EXPR, and either should be OK.

As I understand it, the problematic case is something like
constexpr-union2.C, where we're also looking at a MODIFY_EXPR.  So what is
this check doing?


The reasoning was to correctly handle cases like the the following (in
constexpr-union6.C):

constexpr int test1() {
  U u {};
  std::construct_at(&u.s, S{ 1, 2 });
  return u.s.b;
}
static_assert(test1() == 2);

The initialisation of &u.s here is not a member access expression within
the call to std::construct_at, since it's just a pointer, but this code
is still legal; in general, an INIT_EXPR to initialise a union member
should always be OK (I believe?), hence constraining to just
MODIFY_EXPR.

However, just that would then (incorrectly) allow all the following
cases in that test to compile, such as

constexpr int test2() {
  U u {};
  int* p = &u.s.b;
  std::construct_at(p, 5);
  return u.s.b;
}
constexpr int x2 = test2();

since the INIT_EXPR is really only initialising 'b', but the implicit
"modification" of active member to 'u.s' is illegal.

Maybe a better way of expressing this condition would be something like
this?

/* An INIT_EXPR of the last member in an access chain is always OK,
   but still check implicit change of members earlier on; see
   cpp2a/constexpr-union6.C.  */
&& !(TREE_CODE (t) == INIT_EXPR && refs->is_empty ())

Otherwise I'll see if I can rework some of the other conditions instead.


Incidentally, I think constexpr-union6.C could use a test where we pass &u.s
to a function other than construct_at, and then try (and fail) to assign to
the b member from that function.

Jason



Sounds good; I've added the following test:

constexpr void foo(S* s) {
  s->b = 10;  // { dg-error "accessing .U::s. member instead of initialized 
.U::k." }
}
constexpr int test3() {
  U u {};
  foo(&u.s);  // { dg-message "in .constexpr. expansion" }
  return u.s.b;
}
constexpr int x3 = test3();  // { dg-message "in .constexpr. expansion" }

Incidentally I found this particular example caused a very unhelpful
error + ICE due to reporting that S could not be value-initialized in
the current version of the patch. The updated version below fixes that
by using 'build_zero_init' instead -- is this an appropriate choice
here?

A similar (but unrelated) issue is with e.g.
struct S { const int a; int b; };
union U { int k; S s; };

constexpr int test() {
  U u {};
  return u.s.b;
}
constexpr int x = test();

giving me this pretty unhelpful error message:

/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
  6 |   return u.s.b;
|  ~~^
/home/ns/main.cpp:1:8: note: ‘S::S()’ is implicitly deleted because the default 
definition would be ill-formed:
  1 | struct S { const int a; int b; };
|^
/home/ns/main.cpp:1:8: error: uninitialised const member in ‘struct S’
/home/ns/main.cpp:1:22: note: ‘const int S::a’ should be initialised
  1 | struct S { const int a; int b; };
|  ^
/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’
  6 |   return u.s.b;
|  ~~^
/home/ns/main.cpp:8:23:   in ‘constexpr’ expansion of ‘test()’
/home/ns/main.cpp:6:12: error: use of deleted function ‘S::S()’

but I'll try and fix this separately (it exists on current trunk without
this patch as well).


While attempting to fix this I found a much better way of handling
value-initialised unions. Here's a new version of the patch which also
includes the fix for accessing the wrong member of a value-initialised
union as well.

Additionally includes an `auto_diagnostic_group` for the `inform`
diagnostics as Marek helpfully informed me about in my other patch.

Bootstrapped and regtested on x86_64-pc-linux-gnu.


@@ -4496,21 +4491,36 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, 
tree t,

break;
   

Re: [PATCH v2] c++: Improve diagnostics for constexpr cast from void*

[Jason Merrill]

On 10/11/23 11:41, Marek Polacek wrote:

On Wed, Oct 11, 2023 at 10:57:06AM +1100, Nathaniel Shead wrote:

On Mon, Oct 09, 2023 at 04:10:20PM -0400, Jason Merrill wrote:

On 10/9/23 06:03, Nathaniel Shead wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu with
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.

-- >8 --

This patch improves the errors given when casting from void* in C++26 to
include the expected type if the type of the pointed-to object was
not similar to the casted-to type.

It also ensures (for all standard modes) that void* casts are checked
even for DECL_ARTIFICIAL declarations, such as lifetime-extended
temporaries, and is only ignored for cases where we know it's OK (heap
identifiers and source_location::current). This provides more accurate
diagnostics when using the pointer and ensures that some other casts
from void* are now correctly rejected.

gcc/cp/ChangeLog:

* constexpr.cc (is_std_source_location_current): New.
(cxx_eval_constant_expression): Only ignore cast from void* for
specific cases and improve other diagnostics.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-cast4.C: New test.

Signed-off-by: Nathaniel Shead 
---
   gcc/cp/constexpr.cc  | 83 +---
   gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
   2 files changed, 78 insertions(+), 12 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..f38d541a662 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
  && is_std_allocator_allocate (call->fundef->decl));
   }
+/* Return true if FNDECL is std::source_location::current.  */
+
+static inline bool
+is_std_source_location_current (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+return false;
+
+  tree name = DECL_NAME (fndecl);
+  if (name == NULL_TREE || !id_equal (name, "current"))
+return false;
+
+  tree ctx = DECL_CONTEXT (fndecl);
+  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
+return false;
+
+  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
+  return name && id_equal (name, "source_location");
+}
+
+/* Overload for the above taking constexpr_call*.  */
+
+static inline bool
+is_std_source_location_current (const constexpr_call *call)
+{
+  return (call
+ && call->fundef
+ && is_std_source_location_current (call->fundef->decl));
+}
+
   /* Return true if FNDECL is __dynamic_cast.  */
   static inline bool
@@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
if (TYPE_PTROB_P (type)
&& TYPE_PTR_P (TREE_TYPE (op))
&& VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
-   /* Inside a call to std::construct_at or to
-  std::allocator::{,de}allocate, we permit casting from void*
+   /* Inside a call to std::construct_at,
+  std::allocator::{,de}allocate, or
+  std::source_location::current, we permit casting from void*
   because that is compiler-generated code.  */
&& !is_std_construct_at (ctx->call)
-   && !is_std_allocator_allocate (ctx->call))
+   && !is_std_allocator_allocate (ctx->call)
+   && !is_std_source_location_current (ctx->call))
  {
/* Likewise, don't error when casting from void* when OP is
   &heap uninit and similar.  */
tree sop = tree_strip_nop_conversions (op);
-   if (TREE_CODE (sop) == ADDR_EXPR
-   && VAR_P (TREE_OPERAND (sop, 0))
-   && DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
+   tree decl = NULL_TREE;
+   if (TREE_CODE (sop) == ADDR_EXPR)
+ decl = TREE_OPERAND (sop, 0);
+   if (decl
+   && VAR_P (decl)
+   && DECL_ARTIFICIAL (decl)
+   && (DECL_NAME (decl) == heap_identifier
+   || DECL_NAME (decl) == heap_uninit_identifier
+   || DECL_NAME (decl) == heap_vec_identifier
+   || DECL_NAME (decl) == heap_vec_uninit_identifier))
  /* OK */;
/* P2738 (C++26): a conversion from a prvalue P of type "pointer to
   cv void" to a pointer-to-object type T unless P points to an
   object whose type is similar to T.  */
-   else if (cxx_dialect > cxx23
-&& (sop = cxx_fold_indirect_ref (ctx, loc,
- TREE_TYPE (type), sop)))
+   else if (cxx_dialect > cxx23)
  {
-   r = build1 (ADDR_EXPR, type, sop);
-

Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill]

On 10/20/23 00:34, waffl3x wrote:


Based on what you've said, I assume that OFFSET_REF handles static
member functions that are overloaded. But as I've said this seems to
contradict the comments I'm reading, so I'm not sure that I'm
understanding you correctly.



That's right. For instance,

struct A {
static void g();
static void g(int);
};

void (*p)(int) = &A::g; // cp_build_addr_expr_1 gets an OFFSET_REF


I think we need the OFFSET_REF for an explicit-object member function
because it expresses that the code satisfies the requirement "If the
operand names an explicit object member function, the operand shall be a
qualified-id."


I do agree here, but it does reinforce that OFFSET_REF is no longer
just for members represented by pointer to member type. So that might
be something to take into consideration.



An OFFSET_REF that isn't type_unknown_p, agreed.


It might simplify things to remove the optimization in build_offset_ref
so we get an OFFSET_REF even for a single static member function, and
add support for that to cp_build_addr_expr_1.


I don't think this should be necessary, the "right thing" should just
be done for explicit-object member functions. With all the stuff going
on here that I missed I'm starting to wonder how function overloads
ever worked at all in my patch. On the other hand though, this
optimization probably could be documented better, but I very well might
have missed it even if it were.

Hell, maybe it needs a greater redesign altogether, it seems strange to
me to bundle overload information in with a construct for a specific
expression. (Assuming that's whats happening of course, I still don't
fully understand it.) It's not like this has rules unique to it for how
overload resolution is decided, right? Initializing a param/variable of
pointer to function type with an overloaded function resolves that with
similar rules, I think? Maybe it is a little different now that I write
it out loud.

I wasn't going to finish my musings about that, but it made me realize
that it might not actually be correct for address of explicit-object
member functions to be wrapped by OFFSET_REF. I mean surely it's fine
because based on what you've said static member functions are also
wrapped by OFFSET_REF, so it's likely fully implemented, especially
considering things worked before. But now that there are 2 different
varieties of class members that the address of them can be taken, it
might make sense to split things up a bit? Then again, why were static
member functions ever handled the same way? Taking the address of other
static members isn't handled in the same way here is it?



Functions are different because of overloading; in general we can't
decide what an expression that names a function actually means until we
have enough context to decide which function, exactly. So we represent
the id-expression largely as lookup+syntax until overload resolution
turns it into a specific function. The type_unknown_p check earlier in
cp_build_addr_expr_1 is for that case.


Yeah this all makes sense, but that's why I was confused by the
following documentation from cp-tree.def.

```
/* An OFFSET_REF is used in two situations:

1. An expression of the form `A::m' where `A' is a class and `m' is
   a non-static member.  In this case, operand 0 will be a TYPE
   (corresponding to `A') and operand 1 will be a FIELD_DECL,
   BASELINK, or TEMPLATE_ID_EXPR (corresponding to `m').

   The expression is a pointer-to-member if its address is taken,
   but simply denotes a member of the object if its address is not
   taken.
```


Yeah, I'll adjust that statement to be more conditional.  Is this clearer?

1. An expression of the form `A::m' where `A' is a class and `m' is 

   a non-static member or an overload set.  In this case, operand 0 

   will be a TYPE (corresponding to `A') and operand 1 will be a 

   FIELD_DECL, BASELINK, or TEMPLATE_ID_EXPR (corresponding to 
`m').



   The expression is a pointer-to-member if its address is taken 
(and
   if, after any overload resolution, 'm' does not designate a 

   static or explicit object member function).  It simply denotes a 

   member of the object if its address is not taken. 




An OFFSET_REF that isn't type_unknown_p, agreed.

But I suppose that's what this might have been referring to.

So is that the case then? OFFSET_REF might be for a regular address of
member expression unless it's type_unknown_p?


If it's type_unknown_p we don't know which member it is, so we don't 
know whether taking its address gives a PMF or a regular pointer.


If it's not type_unknown_p, we know which member it is, and we currently 
skip building it at all for static members, so taking its address always 
gives a pointer to member.  explicit object member functions will make 
that assumption no longer valid.



An id-expression that names a single non-template function
(!really_overloaded_fn) is handled somewh

Re: [PATCH] c-family: char8_t and aliasing in C vs C++ [PR111884]

[Jason Merrill]

On 10/20/23 12:31, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?


OK.


-- >8 --
In the PR, Joseph says that in C char8_t is not a distinct type.  So
we should behave as if it can alias anything, like ordinary char.
In C, unsigned_char_type_node == char8_type_node, so with this patch
we return 0 instead of -1.  And the following comment says:

   /* The C standard guarantees that any object may be accessed via an
  lvalue that has narrow character type (except char8_t).  */
   if (t == char_type_node
   || t == signed_char_type_node
   || t == unsigned_char_type_node)
 return 0;

Which appears to be wrong, so I'm adjusting that as well.

PR c/111884

gcc/c-family/ChangeLog:

* c-common.cc (c_common_get_alias_set): Return -1 for char8_t only
in C++.

gcc/testsuite/ChangeLog:

* c-c++-common/alias-1.c: New test.
---
  gcc/c-family/c-common.cc |  7 ---
  gcc/testsuite/c-c++-common/alias-1.c | 23 +++
  2 files changed, 27 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/c-c++-common/alias-1.c

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index f044db5b797..0efdc677217 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -3828,12 +3828,13 @@ c_common_get_alias_set (tree t)
if (!TYPE_P (t))
  return -1;
  
-  /* Unlike char, char8_t doesn't alias. */

-  if (flag_char8_t && t == char8_type_node)
+  /* Unlike char, char8_t doesn't alias in C++.  (In C, char8_t is not
+ a distinct type.)  */
+  if (flag_char8_t && t == char8_type_node && c_dialect_cxx ())
  return -1;
  
/* The C standard guarantees that any object may be accessed via an

- lvalue that has narrow character type (except char8_t).  */
+ lvalue that has narrow character type.  */
if (t == char_type_node
|| t == signed_char_type_node
|| t == unsigned_char_type_node)
diff --git a/gcc/testsuite/c-c++-common/alias-1.c 
b/gcc/testsuite/c-c++-common/alias-1.c
new file mode 100644
index 000..d72fec47f76
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/alias-1.c
@@ -0,0 +1,23 @@
+/* PR c/111884 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+/* { dg-additional-options "-std=c++20" { target c++ } } */
+/* { dg-additional-options "-std=c2x" { target c } } */
+
+int f(int i)
+{
+int f = 1;
+return i[(unsigned char *)&f];
+}
+
+int g(int i)
+{
+int f = 1;
+return i[(signed char *)&f];
+}
+
+int h(int i)
+{
+int f = 1;
+return i[(char *)&f];
+}

base-commit: eb15fad3190a8b33e3e451b964ff1ecf08bbb113

Re: [PATCH] c++: Implement C++26 P2361R6 - Unevaluated strings [PR110342]

[Jason Merrill]

On 8/24/23 09:58, Jakub Jelinek wrote:

Hi!

The following patch implements C++26 unevaluated-string.
As it seems to me just extra pedanticity, it is implemented only for
-std=c++26 or -std=gnu++26 and later and only if -pedantic/-pedantic-errors.


Hmm, I assumed it was accepted as a DR, but apparently not.  In addition 
to making things ill-formed, it clarifies that these strings are never 
converted to the execution character set.  Do we support 
cross-compilation to EBCDIC, which was the motivating context?


Jason

[pushed] c++: fix tourney logic

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.  Patrick, sorry I didn't apply
this sooner.

-- 8< --

In r13-3766 I changed the logic at the end of tourney to avoid redundant
comparisons, but the change also meant skipping any less-good matches
between the champ_compared_to_predecessor candidate and champ itself.

This should not be a correctness issue, since we believe that joust is a
partial order.  But it can lead to missed warnings, as in this testcase.

gcc/cp/ChangeLog:

* call.cc (tourney): Only skip champ_compared_to_predecessor.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wsign-promo1.C: New test.
---
 gcc/cp/call.cc   |  5 +++--
 gcc/testsuite/g++.dg/warn/Wsign-promo1.C | 15 +++
 2 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsign-promo1.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 657eca93d23..a49fde949d5 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -13227,10 +13227,11 @@ tourney (struct z_candidate *candidates, 
tsubst_flags_t complain)
  been compared to.  */
 
   for (challenger = candidates;
-   challenger != champ
-&& challenger != champ_compared_to_predecessor;
+   challenger != champ;
challenger = challenger->next)
 {
+  if (challenger == champ_compared_to_predecessor)
+   continue;
   fate = joust (champ, challenger, 0, complain);
   if (fate != 1)
return NULL;
diff --git a/gcc/testsuite/g++.dg/warn/Wsign-promo1.C 
b/gcc/testsuite/g++.dg/warn/Wsign-promo1.C
new file mode 100644
index 000..51b76eee735
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsign-promo1.C
@@ -0,0 +1,15 @@
+// Check that we get joust warnings from comparing the final champ to a
+// candidate between it and the previous champ.
+
+// { dg-additional-options -Wsign-promo }
+
+struct A { A(int); };
+
+enum E { e };
+
+int f(int, A);
+int f(unsigned, A);
+int f(int, int);
+
+int i = f(e, 42);  // { dg-warning "passing 'E'" }
+// { dg-warning "in call to 'int f" "" { target *-*-* } .-1 }

base-commit: 084addf8a700fab9222d4127ab8524920d0ca481
-- 
2.39.3

[pushed] testsuite: constexpr-diag1.C and implicit constexpr

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

This test doesn't break as expected with implicit constexpr.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1y/constexpr-diag1.C: Add -fno-implicit-constexpr.
---
 gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C
index 0e2909e83ef..e12633ca085 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-diag1.C
@@ -1,6 +1,6 @@
 // PR c++/111272
 // { dg-do compile { target c++14 } }
-// { dg-options "-Werror=invalid-constexpr" }
+// { dg-options "-Werror=invalid-constexpr -fno-implicit-constexpr" }
 // { dg-prune-output "some warnings being treated as errors" }
 
 struct Jam

base-commit: 084addf8a700fab9222d4127ab8524920d0ca481
-- 
2.39.3

[pushed] c++: abstract class and overload resolution

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

In my implementation of P0929 I treated a conversion to an rvalue of
abstract class type as a bad conversion, but that's still too soon to check
it; we need to wait until we're done with overload resolution.

gcc/cp/ChangeLog:

* call.cc (implicit_conversion_1): Rename...
(implicit_conversion): ...to this.  Remove the old wrapper.

gcc/testsuite/ChangeLog:

* g++.dg/template/sfinae-dr657.C: Adjust.
---
 gcc/cp/call.cc   | 30 
 gcc/testsuite/g++.dg/template/sfinae-dr657.C |  7 ++---
 2 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index a49fde949d5..2eb54b5b6ed 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -2032,12 +2032,14 @@ reference_binding (tree rto, tree rfrom, tree expr, 
bool c_cast_p, int flags,
   return conv;
 }
 
-/* Most of the implementation of implicit_conversion, with the same
-   parameters.  */
+/* Returns the implicit conversion sequence (see [over.ics]) from type
+   FROM to type TO.  The optional expression EXPR may affect the
+   conversion.  FLAGS are the usual overloading flags.  If C_CAST_P is
+   true, this conversion is coming from a C-style cast.  */
 
 static conversion *
-implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p,
-  int flags, tsubst_flags_t complain)
+implicit_conversion (tree to, tree from, tree expr, bool c_cast_p,
+int flags, tsubst_flags_t complain)
 {
   conversion *conv;
 
@@ -2167,26 +2169,6 @@ implicit_conversion_1 (tree to, tree from, tree expr, 
bool c_cast_p,
   return NULL;
 }
 
-/* Returns the implicit conversion sequence (see [over.ics]) from type
-   FROM to type TO.  The optional expression EXPR may affect the
-   conversion.  FLAGS are the usual overloading flags.  If C_CAST_P is
-   true, this conversion is coming from a C-style cast.  */
-
-static conversion *
-implicit_conversion (tree to, tree from, tree expr, bool c_cast_p,
-int flags, tsubst_flags_t complain)
-{
-  conversion *conv = implicit_conversion_1 (to, from, expr, c_cast_p,
-   flags, complain);
-  if (!conv || conv->bad_p)
-return conv;
-  if (conv_is_prvalue (conv)
-  && CLASS_TYPE_P (conv->type)
-  && CLASSTYPE_PURE_VIRTUALS (conv->type))
-conv->bad_p = true;
-  return conv;
-}
-
 /* Like implicit_conversion, but return NULL if the conversion is bad.
 
This is not static so that check_non_deducible_conversion can call it within
diff --git a/gcc/testsuite/g++.dg/template/sfinae-dr657.C 
b/gcc/testsuite/g++.dg/template/sfinae-dr657.C
index 36c11e65918..bb19108c5d8 100644
--- a/gcc/testsuite/g++.dg/template/sfinae-dr657.C
+++ b/gcc/testsuite/g++.dg/template/sfinae-dr657.C
@@ -1,7 +1,6 @@
-// DR 657 SUPERSEDED BY DR 1646
+// DR 657 SUPERSEDED BY P0929
 // Test that a return or parameter type with abstract class type DOES NOT cause
-// a deduction failure, but there is no implicit conversion sequence for
-// a parameter of abstract class type.
+// a deduction failure or conversion failure.
 
 struct A
 {
@@ -19,5 +18,5 @@ template int arg(...);
 int main()
 {
   int i = declval();// { dg-error "ambiguous" }
-  i = arg(1);
+  i = arg(1);   // { dg-error "abstract" }
 }

base-commit: 7d4e99131606badcdc343cce62691245dac11bb4
-- 
2.39.3

Re: [PATCH] c++: Implement C++26 P2361R6 - Unevaluated strings [PR110342]

[Jason Merrill]

On 10/20/23 17:59, Jakub Jelinek wrote:

On Fri, Oct 20, 2023 at 04:12:48PM -0400, Jason Merrill wrote:

On 8/24/23 09:58, Jakub Jelinek wrote:

The following patch implements C++26 unevaluated-string.
As it seems to me just extra pedanticity, it is implemented only for
-std=c++26 or -std=gnu++26 and later and only if -pedantic/-pedantic-errors.


Hmm, I assumed it was accepted as a DR, but apparently not.  In addition to
making things ill-formed, it clarifies that these strings are never
converted to the execution character set.


I believe we implement it that way.  cp_parser_unevaluated_string_literal
(but several other spots as well) pass false to translate argument.


Ah, right.  The patch is OK, thanks.

Jason

Re: [PATCH v5 2/5] libcpp: add a function to determine UTF-8 validity of a C string

[Jason Merrill]

On 10/23/23 11:16, David Malcolm wrote:

On Wed, Jan 25, 2023 at 4:09 PM Ben Boeckel via Gcc  wrote:


This simplifies the interface for other UTF-8 validity detections when a
simple "yes" or "no" answer is sufficient.

libcpp/

 * charset.cc: Add `_cpp_valid_utf8_str` which determines whether
 a C string is valid UTF-8 or not.
 * internal.h: Add prototype for `_cpp_valid_utf8_str`.

Signed-off-by: Ben Boeckel 


[going through patches in patchwork]

What's the status of this patch; did this ever get committed?


It was superseded.

Jason

Re: [PATCH] rust: build failure after NON_DEPENDENT_EXPR removal [PR111899]

[Jason Merrill]

On 10/23/23 05:10, Thomas Schwinge wrote:

Hi Patrick!

On 2023-10-20T13:36:30-0400, Patrick Palka  wrote:

Built on x86_64-pc-linux-gnu, pushed to trunk as obvious (hopefully).

-- >8 --

This patch removes stray NON_DEPENDENT_EXPR checks following the removal
of this tree code from the C++ FE.  (Since this restores the build I
supppose it means the Rust FE never creates NON_DEPENDENT_EXPR trees in
the first place, so no further analysis is needed.)


ACK, thanks!


For context: indeed, a non-trivial amount of C++ front end 'constexpr'
code was copied into the Rust front end, for implementing related Rust
functionality, mostly as part of the 2022 GSoC project
"Support for Constant Folding in Rust Frontend" (Faisal Abbas),
.

Yes, this should eventually be cleaned up (and merged with the original
C++ front end code, as much as feasible -- which I don't know whether or
to which extent it is).


It would be nice to move a lot of the constexpr code into the 
middle-end, but I expect that would be a significant project.


Jason

Re: [PATCH v23 02/33] c-family, c++: Look up built-in traits via identifier node

[Jason Merrill]

On 10/20/23 09:53, Ken Matsui wrote:

Since RID_MAX soon reaches 255 and all built-in traits are used approximately
once in a C++ translation unit, this patch removes all RID values for built-in


These two lines are too long; please wrap at 75 columns so they don't go 
over 80 when git log adds 4 spaces at the beginning.



traits and uses the identifier node to look up the specific trait.  Rather
than holding traits as keywords, we set all trait identifiers as cik_trait,
which is a new cp_identifier_kind.  As cik_reserved_for_udlit was unused and
cp_identifier_kind is 3 bits, we replaced the unused field with the new
cik_trait.  Also, the later patch handles a subsequent token to the built-in
identifier so that we accept the use of non-function-like built-in trait
identifiers.

  /* True if this identifier is for any operator name (including
-   conversions).  Value 4, 5, 6 or 7.  */
+   conversions).  Value 4, 5, or 6.  */
  #define IDENTIFIER_ANY_OP_P(NODE) \
-  (IDENTIFIER_KIND_BIT_2 (NODE))
+  (IDENTIFIER_KIND_BIT_2 (NODE) && !IDENTIFIER_TRAIT_P (NODE))

...

+/* True if this identifier is the name of a built-in trait.  */
+#define IDENTIFIER_TRAIT_P(NODE)   \
+  (IDENTIFIER_KIND_BIT_0 (NODE)\
+   && IDENTIFIER_KIND_BIT_1 (NODE) \
+   && IDENTIFIER_KIND_BIT_2 (NODE))


The other macros use &, not &&; we might as well stay consistent with 
that pattern.


Jason

Re: [PATCH v23 03/33] c++: Accept the use of built-in trait identifiers

[Jason Merrill]

On 10/20/23 09:53, Ken Matsui wrote:

This patch accepts the use of built-in trait identifiers when they are
actually not used as traits.  Specifically, we check if the subsequent token
is '(' for ordinary built-in traits or is '<' only for the special
__type_pack_element built-in trait.  If those identifiers are used
differently, the parser treats them as normal identifiers.  This allows
us to accept code like: struct __is_pointer {};.

+/* Peeks the corresponding built-in trait if a given token is
 a built-in trait.  Otherwise, returns nullptr.  */
  
  static const cp_trait *

+cp_lexer_peek_trait (cp_lexer *lexer, const cp_token *token1)


Passing in both the lexer and the peeked token seems awkward, let's just 
pass in the lexer.  Looking up the peeked token again is fast.



  {
+  if (token1->type == CPP_NAME && IDENTIFIER_TRAIT_P (token1->u.value))
+{
+  const cp_trait &trait = cp_traits[IDENTIFIER_CP_INDEX (token1->u.value)];
+  const bool is_pack_element = (trait.kind == CPTK_TYPE_PACK_ELEMENT);
+
+  /* Check if the subsequent token is a `<' token to
+ __type_pack_element or is a `(' token to everything else.  */


git complains about indentation with spaces instead of a tab on this line.

Jason

Re: [PATCH v23 32/33] c++: Implement __is_invocable built-in trait

[Jason Merrill]

On 10/20/23 17:37, Patrick Palka wrote:

On Fri, 20 Oct 2023, Patrick Palka wrote:


On Fri, 20 Oct 2023, Patrick Palka wrote:


On Fri, 20 Oct 2023, Ken Matsui wrote:


This patch implements built-in trait for std::is_invocable.


Nice!  My email client unfortunately ate my first review attempt, so
apologies for my brevity this time around.


gcc/cp/ChangeLog:

* cp-trait.def: Define __is_invocable.
* constraint.cc (diagnose_trait_expr): Handle CPTK_IS_INVOCABLE.
* semantics.cc (trait_expr_value): Likewise.
(finish_trait_expr): Likewise.
(is_invocable_p): New function.
* method.h: New file to export build_trait_object in method.cc.


Given how much larger semantics.cc is than method.cc, maybe let's put 
is_invocable_p in method.cc instead?  And in general declarations can go 
in cp-tree.h.



diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 7cccbae5287..cc2e400531a 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -45,6 +45,10 @@ along with GCC; see the file COPYING3.  If not see
  #include "gomp-constants.h"
  #include "predict.h"
  #include "memmodel.h"
+#include "method.h"
+
+#include "print-tree.h"
+#include "tree-pretty-print.h"
  
  /* There routines provide a modular interface to perform many parsing

 operations.  They may therefore be used during actual parsing, or
@@ -11714,6 +11718,133 @@ classtype_has_nothrow_assign_or_copy_p (tree type, 
bool assign_p)
return saw_copy;
  }
  
+/* Return true if FN_TYPE is invocable with the given ARG_TYPES.  */

+
+static bool
+is_invocable_p (tree fn_type, tree arg_types)


(Sorry for the spam)  We'll eventually want to implement a built-in for
invoke_result, so perhaps we should preemptively factor out the bulk
of this function into a 'build_INVOKE' helper function that returns the
built tree?


+{
+  /* ARG_TYPES must be a TREE_VEC.  */
+  gcc_assert (TREE_CODE (arg_types) == TREE_VEC);
+
+  /* Access check is required to determine if the given is invocable.  */
+  deferring_access_check_sentinel acs (dk_no_deferred);
+
+  /* std::is_invocable is an unevaluated context.  */
+  cp_unevaluated cp_uneval_guard;
+
+  bool is_ptrdatamem;
+  bool is_ptrmemfunc;
+  if (TREE_CODE (fn_type) == REFERENCE_TYPE)
+{
+  tree deref_fn_type = TREE_TYPE (fn_type);
+  is_ptrdatamem = TYPE_PTRDATAMEM_P (deref_fn_type);
+  is_ptrmemfunc = TYPE_PTRMEMFUNC_P (deref_fn_type);
+
+  /* Dereference fn_type if it is a pointer to member.  */
+  if (is_ptrdatamem || is_ptrmemfunc)
+   fn_type = deref_fn_type;
+}
+  else
+{
+  is_ptrdatamem = TYPE_PTRDATAMEM_P (fn_type);
+  is_ptrmemfunc = TYPE_PTRMEMFUNC_P (fn_type);
+}
+
+  if (is_ptrdatamem && TREE_VEC_LENGTH (arg_types) != 1)
+/* A pointer to data member with non-one argument is not invocable.  */
+return false;
+
+  if (is_ptrmemfunc && TREE_VEC_LENGTH (arg_types) == 0)
+/* A pointer to member function with no arguments is not invocable.  */
+return false;
+
+  /* Construct an expression of a pointer to member.  */
+  tree datum;
+  if (is_ptrdatamem || is_ptrmemfunc)
+{
+  tree datum_type = TREE_VEC_ELT (arg_types, 0);
+
+  /* Dereference datum.  */
+  if (CLASS_TYPE_P (datum_type))
+   {
+ bool is_refwrap = false;
+
+ tree datum_decl = TYPE_NAME (TYPE_MAIN_VARIANT (datum_type));
+ if (decl_in_std_namespace_p (datum_decl))
+   {
+ tree name = DECL_NAME (datum_decl);
+ if (name && (id_equal (name, "reference_wrapper")))
+   {
+ /* Handle std::reference_wrapper.  */
+ is_refwrap = true;
+ datum_type = cp_build_reference_type (datum_type, false);


Why do you change datum_type from std::reference_wrapper<...> to 
std::reference_wrapper<...>&?



+   }
+   }
+
+ datum = build_trait_object (datum_type);
+
+ /* If datum_type was not std::reference_wrapper, check if it has
+operator*() overload.  If datum_type was std::reference_wrapper,
+avoid dereferencing the datum twice.  */
+ if (!is_refwrap)
+   if (get_class_binding (datum_type, get_identifier ("operator*")))


We probably should use lookup_member instead of get_class_binding since
IIUC the latter doesn't look into bases:

   struct A { int m; };
   struct B { A& operator*(): };
   struct C : B { };
   static_assert(std::is_invocable_v);

However, I notice that the specification of INVOKE
(https://eel.is/c++draft/func.require#lib:INVOKE) doesn't mention name
lookup at all so it strikes me as suspicious that we'd perform name
lookup here.


Agreed.  It seems that whether or not to build_x_indirect_ref should 
depend instead on whether f is a pointer to a member of decltype(t1) (as 
well as is_refwrap).



 I think this would misbehave for:

   struct A { };
   struct B : A { A& operator*() = delete; };
   static_assert(std::is_inv

Re: [PATCH] c++: cp_stabilize_reference and non-dep exprs [PR111919]

[Jason Merrill]

On 10/23/23 19:49, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

-- >8 --

After the removal of NON_DEPENDENT_EXPR, cp_stabilize_reference which
used to just exit early for NON_DEPENDENT_EXPR is now more prone to
passing a weird templated tree to middle-end routines, which leads to a
crash from contains_placeholder_p in the testcase below.  It seems the
best fix is to just disable cp_stabilize_reference when in a template
context like we already do for cp_save_expr; it seems SAVE_EXPR should
never appear in a templated tree (since e.g. tsubst doesn't handle it).


Hmm.  We don't want the result of cp_stabilize_reference (or 
cp_save_expr) to end up in the resulting trees in template context. 
Having a SAVE_EXPR in the result would actually be helpful for catching 
such a bug.


That said, the patch is OK.


PR c++/111919

gcc/cp/ChangeLog:

* tree.cc (cp_stabilize_reference): Do nothing when
processing_template_decl.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent27.C: New test.
---
  gcc/cp/tree.cc  | 4 
  gcc/testsuite/g++.dg/template/non-dependent27.C | 8 
  2 files changed, 12 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent27.C

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index a3d61d3e7c9..417c92ba76f 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -408,6 +408,10 @@ bitfield_p (const_tree ref)
  tree
  cp_stabilize_reference (tree ref)
  {
+  if (processing_template_decl)
+/* As in cp_save_expr.  */
+return ref;
+
STRIP_ANY_LOCATION_WRAPPER (ref);
switch (TREE_CODE (ref))
  {
diff --git a/gcc/testsuite/g++.dg/template/non-dependent27.C 
b/gcc/testsuite/g++.dg/template/non-dependent27.C
new file mode 100644
index 000..cf7af6e6425
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent27.C
@@ -0,0 +1,8 @@
+// PR c++/111919
+
+int i[3];
+
+template
+void f() {
+  i[42 / (int) sizeof (T)] |= 0;
+}

Re: [PATCH] c++: error with bit-fields and scoped enums [PR111895]

[Jason Merrill]

On 10/24/23 12:18, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Here we issue a bogus error: invalid operands of types 'unsigned char:2'
and 'int' to binary 'operator!=' when casting a bit-field of scoped enum
type to bool.

In build_static_cast_1, perform_direct_initialization_if_possible returns
NULL_TREE, because the invented declaration T t(e) fails, which is
correct.  So we go down to ocp_convert, which has code to deal with this
case:
   /* We can't implicitly convert a scoped enum to bool, so convert
  to the underlying type first.  */
   if (SCOPED_ENUM_P (intype) && (convtype & CONV_STATIC))
 e = build_nop (ENUM_UNDERLYING_TYPE (intype), e);
but the SCOPED_ENUM_P is false since intype is .
This could be fixed by using unlowered_expr_type.  But then
c_common_truthvalue_conversion/CASE_CONVERT has a similar problem, and
unlowered_expr_type is a C++-only function.

Rather than adding a dummy unlowered_expr_type to C, I think we should
follow [expr.static.cast]p3: "the lvalue-to-rvalue conversion is applied
to the bit-field and the resulting prvalue is used as the operand of the
static_cast."  There are no prvalue bit-fields, so the l-to-r conversion
will get us an expression whose type is the enum.  (I thought we didn't
need decay_conversion because that does a whole lot more but using it
would make sense to me too.)


It's possible that we might want some of that more, particularly 
mark_rvalue_use; decay_conversion seems like the right answer.  OK with 
that change.


rvalue() would also make sense, though that seems to be missing a call 
to unlowered_expr_type at the moment.  In fact, after "otherwise, it's 
the lvalue-to-rvalue conversion" in decay_conv should probably just be a 
call to rvalue, with missing bits added to the latter function.


Jason

Re: [PATCH] c++: build_new_1 and non-dep array size [PR111929]

[Jason Merrill]

On 10/24/23 13:03, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
like the right approach?

-- >8 --

This PR is another instance of NON_DEPENDENT_EXPR having acted as an
"analysis barrier" for middle-end routines, and now that it's gone we
may end up passing weird templated trees (that have a generic tree code)
to the middle-end which leads to an ICE.  In the testcase below the
non-dependent array size 'var + 42' is expressed as an ordinary
PLUS_EXPR, but whose operand types have different precisions -- long and
int respectively -- naturally because templated trees encode only the
syntactic form of an expression devoid of e.g. implicit conversions
(typically).  This type incoherency triggers a wide_int assert during
the call to size_binop in build_new_1 which requires the operand types
have the same precision.

This patch fixes this by replacing our incremental folding of 'size'
within build_new_1 with a single call to cp_fully_fold (which is a no-op
in template context) once 'size' is fully built.


This is OK, but we could probably also entirely skip a lot of the 
calculation in a template, since we don't care about any values.  Can we 
skip the entire if (array_p) block?



PR c++/111929

gcc/cp/ChangeLog:

* init.cc (build_new_1): Use convert, build2, build3 instead of
fold_convert, size_binop and fold_build3 when building 'size'.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent28.C: New test.
---
  gcc/cp/init.cc  | 9 +
  gcc/testsuite/g++.dg/template/non-dependent28.C | 6 ++
  2 files changed, 11 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent28.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index d48bb16c7c5..56c1b5e9f5e 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -3261,7 +3261,7 @@ build_new_1 (vec **placement, tree type, 
tree nelts,
max_outer_nelts = wi::udiv_trunc (max_size, inner_size);
max_outer_nelts_tree = wide_int_to_tree (sizetype, max_outer_nelts);
  
-  size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));

+  size = build2 (MULT_EXPR, sizetype, size, convert (sizetype, nelts));
  
if (TREE_CODE (cst_outer_nelts) == INTEGER_CST)

{
@@ -3344,7 +3344,7 @@ build_new_1 (vec **placement, tree type, 
tree nelts,
/* Use a class-specific operator new.  */
/* If a cookie is required, add some extra space.  */
if (array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type))
-   size = size_binop (PLUS_EXPR, size, cookie_size);
+   size = build2 (PLUS_EXPR, sizetype, size, cookie_size);
else
{
  cookie_size = NULL_TREE;
@@ -3358,8 +3358,8 @@ build_new_1 (vec **placement, tree type, 
tree nelts,
if (cxx_dialect >= cxx11 && flag_exceptions)
errval = throw_bad_array_new_length ();
if (outer_nelts_check != NULL_TREE)
-   size = fold_build3 (COND_EXPR, sizetype, outer_nelts_check,
-   size, errval);
+   size = build3 (COND_EXPR, sizetype, outer_nelts_check, size, errval);
+  size = cp_fully_fold (size);
/* Create the argument list.  */
vec_safe_insert (*placement, 0, size);
/* Do name-lookup to find the appropriate operator.  */
@@ -3418,6 +3418,7 @@ build_new_1 (vec **placement, tree type, 
tree nelts,
/* If size is zero e.g. due to type having zero size, try to
 preserve outer_nelts for constant expression evaluation
 purposes.  */
+  size = cp_fully_fold (size);
if (integer_zerop (size) && outer_nelts)
size = build2 (MULT_EXPR, TREE_TYPE (size), size, outer_nelts);
  
diff --git a/gcc/testsuite/g++.dg/template/non-dependent28.C b/gcc/testsuite/g++.dg/template/non-dependent28.C

new file mode 100644
index 000..3e45154f61d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent28.C
@@ -0,0 +1,6 @@
+// PR c++/111929
+
+template
+void f(long var) {
+  new int[var + 42];
+}

Re: [PATCH v2 1/3] c++: sort candidates according to viability

[Jason Merrill]

On 10/23/23 19:51, Patrick Palka wrote:

The second patch in this series is new and ensures that the candidates
list isn't mysteriously missing some candidates when noting other
candidates due to deletedness.

-- >8 --

This patch:

   * changes splice_viable to move the non-viable candidates to the end
 of the list instead of removing them outright


Please consistently spell this "non-viable" rather than "unviable".


+tourney (struct z_candidate *&candidates, tsubst_flags_t complain)
  {
struct z_candidate *champ = candidates, *challenger;
int fate;
struct z_candidate *champ_compared_to_predecessor = nullptr;
+  struct z_candidate *champ_predecessor = nullptr;
+  struct z_candidate *challenger_predecessor = champ;


Rather than adding two more variables to keep synchronized, maybe we 
want champ and challenger to be **, like the tail variables in 
splice_viable?


Jason

Re: [PATCH v2 2/3] c++: remember candidates that we ignored

[Jason Merrill]

On 10/23/23 19:51, Patrick Palka wrote:

During overload resolution, we sometimes outright ignore a function from
the overload set and leave no trace of it in the candidates list, for
example when we find a perfect non-template candidate we discard all
function templates, or when the callee is a template-id we discard all
non-template functions.  We should still however make note of these
unviable functions when diagnosing overload resolution failure, but
that's not possible if they're not present in the returned candidates
list.

To that end, this patch reworks add_candidates to add such ignored
functions to the list.  The new rr_ignored rejection reason is somewhat
of a catch-all; we could perhaps split it up into more specific rejection
reasons, but I leave that as future work.


OK with the same unviable -> non-viable change as the 1/3 patch.

Jason

Re: [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness

[Jason Merrill]

On 10/23/23 19:51, Patrick Palka wrote:

With the previous two patches in place, we can now extend our
deletedness diagnostic to note the other considered candidates, e.g.:

   deleted16.C: In function 'int main()':
   deleted16.C:10:4: error: use of deleted function 'void f(int)'
  10 |   f(0);
 |   ~^~~
   deleted16.C:5:6: note: declared here
   5 | void f(int) = delete;
 |  ^
   deleted16.C:5:6: note: candidate: 'void f(int)' (deleted)
   deleted16.C:6:6: note: candidate: 'void f(...)'
   6 | void f(...);
 |  ^
   deleted16.C:7:6: note: candidate: 'void f(int, int)'
   7 | void f(int, int);
 |  ^
   deleted16.C:7:6: note:   candidate expects 2 arguments, 1 provided

For now, these these notes are disabled when a deleted special member
function is selected because it introduces a lot of new "cannot bind
reference" errors in the testsuite when noting non-viable candidates,
e.g. in cpp0x/initlist-opt1.C we would need to expect an error when
noting unviability of A(A&&).  (It'd be nice if we could downgrade such
errors into notes when noting candidates...)


What about my suggestion to make printing the other candidates an 
opt-in, with the normal output just suggesting the use of that option 
for more information, like -ftemplate-backtrace-limit?


Jason

[pushed] tree: update address_space comment

[Jason Merrill]

Pushing as obvious.

-- 8< --

Mention front-end uses of the address_space bit-field, and remove the
inaccurate "only".

gcc/ChangeLog:

* tree-core.h (struct tree_base): Update address_space comment.
---
 gcc/tree-core.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 4dc36827d32..13435344401 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1082,10 +1082,11 @@ struct GTY(()) tree_base {
 
   unsigned spare1 : 8;
 
-  /* This field is only used with TREE_TYPE nodes; the only reason it is
+  /* For _TYPE nodes, this is TYPE_ADDR_SPACE; the reason it is
 present in tree_base instead of tree_type is to save space.  The size
 of the field must be large enough to hold addr_space_t values.
-For CONSTRUCTOR nodes this holds the clobber_kind enum.  */
+For CONSTRUCTOR nodes this holds the clobber_kind enum.
+The C++ front-end uses this in IDENTIFIER_NODE and NAMESPACE_DECL.  */
   unsigned address_space : 8;
 } bits;
 

base-commit: 406709b1c7b134a7a05445837f406e98c04f76f0
-- 
2.39.3

Re: [PATCH] c++: another build_new_1 folding fix [PR111929]

[Jason Merrill]

On 10/25/23 12:03, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

We also need to avoid folding 'outer_nelts_check' when in a template
context to prevent an ICE on the below testcase.  This patch achieves
this by replacing the fold_build2 call with build2 (cp_fully_fold will
later fold the overall expression if appropriate).

In passing, this patch removes an unnecessary call to convert on 'nelts'
since it should always already be a size_t (and 'convert' isn't the best
conversion entry point to use anyway since it doesn't take a complain
parameter.)

PR c++/111929

gcc/cp/ChangeLog:

* init.cc (build_new_1): Remove unnecessary call to convert
on 'nelts'.


OK.


 Use build2 instead of fold_build2 for 'outer_nelts_checks'.


Also OK, but can we skip that whole block when processing_template_decl? 
 Seems no point to build runtime checks.


Jason

Re: [PATCH] c++/modules: fix up recent testcases

[Jason Merrill]

On 10/25/23 14:32, Patrick Palka wrote:

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

Declaring get() inline seems necessary to avoid link failure:

   /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()':
   
decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18):
 undefined reference to `f@pr105322.Decltype()::A::get()'

Not sure if that's expected?


That seems like a bug.  My guess is that the bug is treating f()::A::get 
as internal linkage, but Nathan should have a better understanding of 
how it's supposed to work.



-- >8 --

This fixes some minor issues with the testcases from
r14-4806-g084addf8a700fa.

gcc/testsuite/ChangeLog:

* g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do
directive.  Declare f()::A::get() inline.
* g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do
directive.
---
  gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++--
  gcc/testsuite/g++.dg/modules/lambda-5_a.C   | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C 
b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
index ca66e8b598a..6512f151aae 100644
--- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C
+++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
@@ -1,5 +1,5 @@
  // PR c++/105322
-// { dg-module-do link
+// { dg-module-do link }
  // { dg-additional-options -fmodules-ts }
  // { dg-module-cmi pr105322.Decltype }
  
@@ -7,7 +7,7 @@ export module pr105322.Decltype;
  
  auto f() {

struct A { int m;
-int get () { return m; }
+inline int get () { return m; }
};
return A{};
  }
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C 
b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
index 6b589d4965c..37d0e77b1e1 100644
--- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
@@ -1,5 +1,5 @@
  // PR c++/105322
-// { dg-module-do link
+// { dg-module-do link }
  // { dg-additional-options -fmodules-ts }
  // { dg-module-cmi pr105322.Lambda }
  

Re: [PATCH] c++: simplify build_new_1 when in a template context

[Jason Merrill]

On 10/26/23 14:01, Patrick Palka wrote:

Since when in a template context we end up just discarding the result
of build_new_1, we don't have to bother with much of the code generation
it performs.  This patch makes the function exit early, returning a dummy
non-erroneous result, once we've done pretty much all ahead of time checks
that we could have.  In passing avoid building up 'outer_nelts_check' in
a template context too.


It seems like this stops checking the calls to the constructor and 
operator delete?


Jason

Re: [PATCH] c++: more ahead-of-time -Wparentheses warnings

[Jason Merrill]

On 10/25/23 14:55, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR,
we can easily extend the -Wparentheses warning in convert_for_assignment
to consider (non-dependent) templated assignment operator expressions as
well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond.

gcc/cp/ChangeLog:

* cp-tree.h (is_assignment_op_expr_p): Declare.
* semantics.cc (is_assignment_op_expr_p): Generalize to return
true for assignment operator expression, not just one that
have been resolved to an operator overload.
(maybe_convert_cond): Remove now-redundant checks around
is_assignment_op_expr_p.
* typeck.cc (convert_for_assignment): Look through implicit
INDIRECT_REF in -Wparentheses warning logic, and generalize
to use is_assignment_op_expr_p.


Do we want to factor out the whole warning logic rather than adjust it 
in both places?


Jason

Re: [PATCH v4] c-family: Implement __has_feature and __has_extension [PR60512]

[Jason Merrill]

On 10/25/23 06:28, Alex Coplan wrote:

On 11/10/2023 14:31, Alex Coplan wrote:

On 27/09/2023 15:27, Alex Coplan wrote:

Hi,

This is a v4 patch to address Jason's feedback here:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630911.html

w.r.t. v3 it just removes a comment now that some uncertainty around
cxx_binary_literals has been resolved, and updates the documentation as
suggested to point to the Clang docs.

--


Incidentally, putting a 8< or >8 in the line of dashes lets git am 
--scissors prune the text above the line.



This patch implements clang's __has_feature and __has_extension in GCC.
Currently the patch aims to implement all documented features (and some
undocumented ones) following the documentation at
https://clang.llvm.org/docs/LanguageExtensions.html with the exception
of the legacy features for C++ type traits.  These are omitted, since as
the clang documentation notes, __has_builtin is the correct "modern" way
to query for these (which GCC already implements).


Gentle ping on this:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631525.html


Ping^2



+static const hf_feature_info has_feature_table[] =


You might use constexpr for these tables?

OK either way, thanks!

Jason

Re: ping: [PATCH] preprocessor: c++: Support `#pragma GCC target' macros [PR87299]

[Jason Merrill]

On 10/13/23 17:28, Lewis Hyatt wrote:

On Tue, Sep 12, 2023 at 04:09:21PM -0400, Lewis Hyatt wrote:

On Tue, Aug 8, 2023 at 5:53 PM Jason Merrill  wrote:


On 7/31/23 22:22, Lewis Hyatt via Gcc-patches wrote:

`#pragma GCC target' is not currently handled in preprocess-only mode (e.g.,
when running gcc -E or gcc -save-temps). As noted in the PR, this means that
if the target pragma defines any macros, those macros are not effective in
preprocess-only mode. Similarly, such macros are not effective when
compiling with C++ (even when compiling without -save-temps), because C++
does not process the pragma until after all tokens have been obtained from
libcpp, at which point it is too late for macro expansion to take place.

Since r13-1544 and r14-2893, there is a general mechanism to handle pragmas
under these conditions as well, so resolve the PR by using the new "early
pragma" support.

toplev.cc required some changes because the target-specific handlers for
`#pragma GCC target' may call target_reinit(), and toplev.cc was not expecting
that function to be called in preprocess-only mode.

I added some additional testcases from the PR for x86. The other targets
that support `#pragma GCC target' (aarch64, arm, nios2, powerpc, s390)
already had tests verifying that the pragma sets macros as expected; here I
have added -save-temps to some of them, to test that it now works in
preprocess-only mode as well.

gcc/c-family/ChangeLog:

   PR preprocessor/87299
   * c-pragma.cc (init_pragma): Register `#pragma GCC target' and
   related pragmas in preprocess-only mode, and enable early handling.
   (c_reset_target_pragmas): New function refactoring code from...
   (handle_pragma_reset_options): ...here.
   * c-pragma.h (c_reset_target_pragmas): Declare.

gcc/cp/ChangeLog:

   PR preprocessor/87299
   * parser.cc (cp_lexer_new_main): Call c_reset_target_pragmas ()
   after preprocessing is complete, before starting compilation.

gcc/ChangeLog:

   PR preprocessor/87299
   * toplev.cc (no_backend): New static global.
   (finalize): Remove argument no_backend, which is now a
   static global.
   (process_options): Likewise.
   (do_compile): Likewise.
   (target_reinit): Don't do anything in preprocess-only mode.
   (toplev::main): Adapt to no_backend change.
   (toplev::finalize): Likewise.

gcc/testsuite/ChangeLog:

   PR preprocessor/87299
   * c-c++-common/pragma-target-1.c: New test.
   * c-c++-common/pragma-target-2.c: New test.
   * g++.target/i386/pr87299-1.C: New test.
   * g++.target/i386/pr87299-2.C: New test.
   * gcc.target/i386/pr87299-1.c: New test.
   * gcc.target/i386/pr87299-2.c: New test.
   * gcc.target/s390/target-attribute/tattr-2.c: Add -save-temps to the
   options, to test preprocess-only mode as well.
   * gcc.target/aarch64/pragma_cpp_predefs_1.c: Likewise.
   * gcc.target/arm/pragma_arch_attribute.c: Likewise.
   * gcc.target/nios2/custom-fp-2.c: Likewise.
   * gcc.target/powerpc/float128-3.c: Likewise.
---

Notes:
  Hello-

  This patch fixes the PR by enabling early pragma handling for `#pragma GCC
  target' and related pragmas such as `#pragma GCC push_options'. I did not
  need to touch any target-specific code, however I did need to make a 
change
  to toplev.cc, affecting all targets, to make it safe to call 
target_reinit()
  in preprocess-only mode. (Otherwise, it would be necessary to modify the
  implementation of target pragmas in every target, to avoid this code 
path.)
  That was the only complication I ran into.

  Regarding testing, I did: (thanks to GCC compile farm for the non-x86
  targets)

  bootstrap + regtest all languages - x86_64-pc-linux-gnu
  bootstrap + regtest c/c++ - powerpc64le-unknown-linux-gnu,
  aarch64-unknown-linux-gnu

  The following backends also implement this pragma so ought to be tested:
  arm
  nios2
  s390

  I am not able to test those directly. I did add coverage to their 
testsuites
  (basically, adding -save-temps to any existing test, causes it to test the
  pragma in preprocess-only mode.) Then, I verified on x86_64 with a cross
  compiler, that the modified testcases fail before the patch and pass
  afterwards. nios2 is an exception, it does not set any libcpp macros when
  handling the pragma, so there is nothing to test, but I did verify that
  processing the pragma in preprocess-only mode does not cause any problems.
  The cross compilers tested were targets arm-unknown-linux-gnueabi,
  nios2-unknown-linux, and s390-ibm-linux.

  Please let me know if it looks OK? Thanks!


The c-family and C++ changes look good.  David, any comments on the
toplev changes?

Jason



Thanks for the review! To be clear, I should continue to wait fo

Re: [PATCH] c++: Implement C++ DR 2406 - [[fallthrough]] attribute and iteration statements

[Jason Merrill]

On 8/28/23 06:34, Richard Biener wrote:

On Fri, 25 Aug 2023, Jakub Jelinek wrote:


Hi!

The following patch implements
CWG 2406 - [[fallthrough]] attribute and iteration statements
The genericization of some loops leaves nothing at all or just a label
after a body of a loop, so if the loop is later followed by
case or default label in a switch, the fallthrough statement isn't
diagnosed.

The following patch implements it by marking the IFN_FALLTHROUGH call
in such a case, such that during gimplification it can be pedantically
diagnosed even if it is followed by case or default label or some normal
labels followed by case/default labels.

While looking into this, I've discovered other problems.
expand_FALLTHROUGH_r is removing the IFN_FALLTHROUGH calls from the IL,
but wasn't telling that to walk_gimple_stmt/walk_gimple_seq_mod, so
the callers would then skip the next statement after it, and it would
return non-NULL if the removed stmt was last in the sequence.  This could
lead to wi->callback_result being set even if it didn't appear at the very
end of switch sequence.
The patch makes use of wi->removed_stmt such that the callers properly
know what happened, and use different way to handle the end of switch
sequence case.

That change discovered a bug in the gimple-walk handling of
wi->removed_stmt.  If that flag is set, the callback is telling the callers
that the current statement has been removed and so the innermost
walk_gimple_seq_mod shouldn't gsi_next.  The problem is that
wi->removed_stmt is only reset at the start of a walk_gimple_stmt, but that
can be too late for some cases.  If we have two nested gimple sequences,
say GIMPLE_BIND as the last stmt of some gimple seq, we remove the last
statement inside of that GIMPLE_BIND, set wi->removed_stmt there, don't
do gsi_next correctly because already gsi_remove moved us to the next stmt,
there is no next stmt, so we return back to the caller, but wi->removed_stmt
is still set and so we don't do gsi_next even in the outer sequence, despite
the GIMPLE_BIND (etc.) not being removed.  That means we walk the
GIMPLE_BIND with its whole sequence again.
The patch fixes that by resetting wi->removed_stmt after we've used that
flag in walk_gimple_seq_mod.  Nothing really uses that flag after the
outermost walk_gimple_seq_mod, it is just a private notification that
the stmt callback has removed a stmt.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?


The gimple-walk.cc/gimplify.cc changes are OK, I don't understand
the c-gimplify.cc one.


Seems like it would be good to document this non-obvious meaning of 
*_NOTHROW for ICF_FALLTHROUGH.  Maybe at its entry in internal-fn.def?


OK with that change.

Jason

Re: [PATCH] c++, v2: Implement C++26 P2741R3 - user-generated static_assert messages [PR110348]

[Jason Merrill]

On 9/18/23 13:21, Jakub Jelinek wrote:

Here is an updated version of the patch.
Compared to the last version, based on the discussion in the PR, the patch
1) warns (but only that) if size()/data() methods aren't declared
constexpr/consteval (or implicitly constexpr)


The language requirements also seem to be satisfied by

constexpr const char msg[] = "foo";
struct A { constexpr int operator () () { return sizeof(msg); } };
struct B { constexpr const char * operator()() { return msg; } };
struct C {
  A size;
  B data;
};
constexpr int i = C().size();
constexpr const char *p = C().data();
static_assert (false, C());

constexpr int size() { return sizeof(msg); }
constexpr const char *data() { return msg; }
struct D {
  int (*size)() = ::size;
  const char *(*data)() = ::data;
};
constexpr int di = D().size();
constexpr const char *dp = D().data();
static_assert (false, D());

so we shouldn't assume that size/data are methods.


2) as I don't see a function which would determine if some expression
is core constant expression (for the data() case), the patch just as an
optimization tries to fold_nondependent_expr msg.data() expression
quietly, if it is a constant expression, passes it to c_getstr if len > 0
and if successful, only tries to constant expression evaluate
msg.data()[0] and msg.data()[len - 1], otherwise it will constant
expression evaluate the characters one by one;
for the len == 0 case, it will fold_nondependent_expr + check result is
integer_zero_node for (msg.data(), 0) which I think should fail if
msg.data() is not a core constant expression, but succeed if it is
even if it is not constant expression


Sounds good.


3) already the earlier version of the patch was passing
manifestly_const_eval=true argument, you said in the PR you've raised
it in CWG, I've newly added testsuite coverage for that


CWG agreed with this direction.


--- gcc/cp/semantics.cc.jj  2023-09-05 17:26:51.849921954 +0200
+++ gcc/cp/semantics.cc 2023-09-18 14:31:55.269431759 +0200
@@ -11388,11 +11389,77 @@ finish_static_assert (tree condition, tr
  
if (check_for_bare_parameter_packs (condition))

  condition = error_mark_node;
+  if (check_for_bare_parameter_packs (message))
+return;
+
+  if (TREE_CODE (message) != STRING_CST
+  && !type_dependent_expression_p (message))
+{
+  message_sz
+   = finish_class_member_access_expr (message,
+  get_identifier ("size"),
+  false, tf_none);
+  if (TREE_CODE (message_sz) != COMPONENT_REF)
+   message_sz = error_mark_node;
+  if (message_sz != error_mark_node)
+   message_sz = build_new_method_call (message,
+   TREE_OPERAND (message_sz, 1),
+   NULL, NULL_TREE, LOOKUP_NORMAL,
+   NULL, tf_none);


This should probably use finish_call_expr instead of 
build_new_method_call because of my example above, and also so you don't 
need to pull out the TREE_OPERAND or check the result of name lookup.



+  message_data
+   = finish_class_member_access_expr (message,
+  get_identifier ("data"),
+  false, tf_none);
+  if (TREE_CODE (message_data) != COMPONENT_REF)
+   message_data = error_mark_node;
+  if (message_data != error_mark_node)
+   message_data = build_new_method_call (message,
+ TREE_OPERAND (message_data, 1),
+ NULL, NULL_TREE, LOOKUP_NORMAL,
+ NULL, tf_none);


Likewise.


+  if (message_sz == error_mark_node
+ || message_data == error_mark_node)
+   {
+ error_at (location, "% message must be a string "
+ "literal or object with % and "
+ "% members");


This diagnostic should be just if the calls to 
finish_class_member_access_expr fail; better to get the normal 
diagnostics from finish_call_expr if the calls fail for whatever reason.



+ return;
+   }
+  if (tree s
+ = cp_get_callee_fndecl_nofold (extract_call_expr (message_sz)))
+   if (!DECL_DECLARED_CONSTEXPR_P (s))
+ warning_at (location, 0, "% message %qs "
+  "member not %", "size()");
+  message_sz = perform_implicit_conversion (size_type_node, message_sz,
+   tf_none);


This should probably use build_converted_constant_expr?


+  if (message_sz == error_mark_node)
+   {
+ error_at (location, "% message % member "
+ "function must be implicitly convertible to "
+ "%");
+ return;
+   }
+  if (tree d
+ 

Re: [PATCH] c++: more ahead-of-time -Wparentheses warnings

[Jason Merrill]

On 10/26/23 17:37, Patrick Palka wrote:

On Thu, 26 Oct 2023, Patrick Palka wrote:


On Thu, 26 Oct 2023, Jason Merrill wrote:


On 10/25/23 14:55, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR,
we can easily extend the -Wparentheses warning in convert_for_assignment
to consider (non-dependent) templated assignment operator expressions as
well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond.

gcc/cp/ChangeLog:

* cp-tree.h (is_assignment_op_expr_p): Declare.
* semantics.cc (is_assignment_op_expr_p): Generalize to return
true for assignment operator expression, not just one that
have been resolved to an operator overload.
(maybe_convert_cond): Remove now-redundant checks around
is_assignment_op_expr_p.
* typeck.cc (convert_for_assignment): Look through implicit
INDIRECT_REF in -Wparentheses warning logic, and generalize
to use is_assignment_op_expr_p.


Do we want to factor out the whole warning logic rather than adjust it in both
places?


Sounds good, like so?  Bootstrap / regtest in progress.

-- >8 --

Subject: [PATCH] c++: more ahead-of-time -Wparentheses warnings

Now that we don't have to worry about looking through NON_DEPENDENT_EXPR,
we can easily extend the -Wparentheses warning in convert_for_assignment
to consider (non-dependent) templated assignment operator expressions as
well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond.

gcc/cp/ChangeLog:

* cp-tree.h (maybe_warn_unparenthesized_assignment): Declare.
* semantics.cc (is_assignment_op_expr_p): Generalize to return
true for assignment operator expression, not just one that
have been resolved to an operator overload.
(maybe_warn_unparenthesized_assignment): Factored out from ...
(maybe_convert_cond): ... here.
(finish_parenthesized_expr): Also mention
maybe_warn_unparenthesized_assignment.
* typeck.cc (convert_for_assignment): Replace -Wparentheses
warning logic with maybe_warn_unparenthesized_assignment.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wparentheses-13.C: Strengthen by expecting that
the -Wparentheses warning are issued ahead of time.
* g++.dg/warn/Wparentheses-23.C: Likewise.
* g++.dg/warn/Wparentheses-32.C: Remove xfails.
---
  gcc/cp/cp-tree.h|  1 +
  gcc/cp/semantics.cc | 55 ++---
  gcc/cp/typeck.cc| 13 ++---
  gcc/testsuite/g++.dg/warn/Wparentheses-13.C |  2 -
  gcc/testsuite/g++.dg/warn/Wparentheses-23.C |  3 --
  gcc/testsuite/g++.dg/warn/Wparentheses-32.C |  8 +--
  6 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 30fe716b109..98b29e9cf81 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7875,6 +7875,7 @@ extern tree lambda_regenerating_args  (tree);
  extern tree most_general_lambda   (tree);
  extern tree finish_omp_target (location_t, tree, tree, bool);
  extern void finish_omp_target_clauses (location_t, tree, tree *);
+extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t);
  
  /* in tree.cc */

  extern int cp_tree_operand_length (const_tree);
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 72ec72de690..5664da9f4f2 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -840,15 +840,20 @@ finish_goto_stmt (tree destination)
return add_stmt (build_stmt (input_location, GOTO_EXPR, destination));
  }
  
-/* Returns true if CALL is a (possibly wrapped) CALL_EXPR or AGGR_INIT_EXPR

-   to operator= () that is written as an operator expression. */
+/* Returns true if T corresponds to an assignment operator expression.  */
+
  static bool
-is_assignment_op_expr_p (tree call)
+is_assignment_op_expr_p (tree t)
  {
-  if (call == NULL_TREE)
+  if (t == NULL_TREE)
  return false;
  
-  call = extract_call_expr (call);

+  if (TREE_CODE (t) == MODIFY_EXPR
+  || (TREE_CODE (t) == MODOP_EXPR
+ && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR))
+return true;
+
+  tree call = extract_call_expr (t);
if (call == NULL_TREE
|| call == error_mark_node
|| !CALL_EXPR_OPERATOR_SYNTAX (call))
@@ -860,6 +865,28 @@ is_assignment_op_expr_p (tree call)
  && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR);
  }
  
+/* Maybe warn about an unparenthesized 'a = b' (appearing in a boolean

+   context).  */
+
+void
+maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain)
+{
+  t = REFERENCE_REF_P (t) ? TREE_OPERAND (t, 0) : t;


Consider this to instead be written more idiomatically as


OK.

Jason

Re: [PATCH v3 1/3] c++: sort candidates according to viability

[Jason Merrill]

On 10/27/23 15:55, Patrick Palka wrote:

New in patch 1/3:
   * consistently use "non-viable" instead of "unviable"
 throughout
   * make 'champ' and 'challenger' in 'tourney' be z_candidate**
 to simplify moving 'champ' to the front of the list.  drive-by
 cleanups in tourney, including renaming 'champ_compared_to_predecessor'
 to 'previous_worse_champ' for clarity.
New in patch 2/3:
   * consistently use "non-viable" instead of "unviable" throughout
New in patch 3/3:
   * introduce new -fnote-all-cands flag that controls noting other
 candidates when diagnosing deletedness, and also controls
 noting "ignored" candidates in general.

-- >8 --

This patch:

   * changes splice_viable to move the non-viable candidates to the end
 of the list instead of removing them outright
   * makes tourney move the best candidate to the front of the candidate
 list
   * adjusts print_z_candidates to preserve our behavior of printing only
 viable candidates when diagnosing ambiguity
   * adds a parameter to print_z_candidates to control this default behavior
 (the follow-up patch will want to print all candidates when diagnosing
 deletedness)

Thus after this patch we have access to the entire candidate list through
the best viable candidate.

This change also happens to fix diagnostics for the below testcase where
we currently neglect to note the third candidate, since the presence of
the two unordered non-strictly viable candidates causes splice_viable to
prematurely get rid of the non-viable third candidate.

gcc/cp/ChangeLog:

* call.cc: Include "tristate.h".
(splice_viable): Sort the candidate list according to viability.
Don't remove non-viable candidates from the list.
(print_z_candidates): Add defaulted only_viable_p parameter.
By default only print non-viable candidates if there is no
viable candidate.
(tourney): Make 'candidates' parameter a reference.


Why, when all the callers use the return value?

OK without that change.

Jason

Re: [PATCH v3 3/3] c++: note other candidates when diagnosing deletedness

[Jason Merrill]

On 10/27/23 15:55, Patrick Palka wrote:

With the previous two patches in place, we can now extend our
deletedness diagnostic to note the other considered candidates, e.g.:

   deleted16.C: In function 'int main()':
   deleted16.C:10:4: error: use of deleted function 'void f(int)'
  10 |   f(0);
 |   ~^~~
   deleted16.C:5:6: note: declared here
   5 | void f(int) = delete;
 |  ^
   deleted16.C:5:6: note: candidate: 'void f(int)' (deleted)
   deleted16.C:6:6: note: candidate: 'void f(...)'
   6 | void f(...);
 |  ^
   deleted16.C:7:6: note: candidate: 'void f(int, int)'
   7 | void f(int, int);
 |  ^
   deleted16.C:7:6: note:   candidate expects 2 arguments, 1 provided

These notes are controlled by a new command line flag -fnote-all-cands,
which also controls whether we note ignored candidates more generally.

gcc/ChangeLog:

* doc/invoke.texi (C++ Dialect Options): Document -fnote-all-cands.

gcc/c-family/ChangeLog:

* c.opt: Add -fnote-all-cands.

gcc/cp/ChangeLog:

* call.cc (print_z_candidates): Only print ignored candidates
when -fnote-all-cands is set.
(build_over_call): When diagnosing deletedness, call
print_z_candidates if -fnote-all-cands is set.


My suggestion was also to suggest using the flag in cases where it would 
make a difference, e.g.


note: some candidates omitted, use '-fnote-all-cands' to display them

Maybe "-fdiagnostics-all-candidates"?

Jason

Re: [PATCH] c++: Implement C++26 P1854R4 - Making non-encodable string literals ill-formed [PR110341]

[Jason Merrill]

On 8/25/23 16:49, Jakub Jelinek wrote:

Hi!

This paper voted in as DR makes some multi-character literals ill-formed.
'abcd' stays valid, but e.g. 'á' is newly invalid in UTF-8 exec charset
while valid e.g. in ISO-8859-1, because it is a single character which needs
2 bytes to be encoded.

The following patch does that by checking (only pedantically, especially
because it is a DR) if we'd emit a -Wmultichar warning because character
constant has more than one byte in it whether the number of bytes in the
narrow string matches number of bytes in CPP_STRING32 divided by char32_t
size in bytes.  If it is, it is normal multi-character literal constant
and is diagnosed normally with -Wmultichar, if the number of bytes is
larger, at least one of the c-chars in the sequence was encoded as 2+
bytes.

Now, doing this way has 2 drawbacks, some of the diagnostics which doesn't
result in cpp_interpret_string_1 failures can be printed twice, once
when calling cpp_interpret_string_1 for CPP_CHAR, once for CPP_STRING32.
And, functionally I think it must work 100% correctly if host source
character set is UTF-8 (because all valid UTF-8 chars are encodable in
UTF-32), but might not work for some control codes in UTF-EBCDIC if
that is the source character set (though I don't know if we really actually
support it, e.g. Linux iconv certainly doesn't).
All we actually need is count the number of c-chars in the literal,
alternative would be to write custom character counter which would quietly
interpret/skip over + count escape sequences and decode UTF-8 characters
in between those escape sequences.  But we'd need to have something similar
also for UTF-EBCDIC if it works at all, and from what I've looked, we don't
have anyything like that implemented in libcpp nor anywhere else in GCC.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or ok with some tweaks to avoid the second round of diagnostics from
cpp_interpret_string_1/convert_escape?  Or reimplement that second time and
count manually?

2023-08-25  Jakub Jelinek  

PR c++/110341
libcpp/
* charset.cc: Implement C++ 26 P1854R4 - Making non-encodable string
literals ill-formed.
(narrow_str_to_charconst): Change last type from cpp_ttype to
const cpp_token *.  For C++ if pedantic and i > 1 in CPP_CHAR
interpret token also as CPP_STRING32 and if number of characters
in the CPP_STRING32 is larger than number of bytes in CPP_CHAR,
pedwarn on it.
(cpp_interpret_charconst): Adjust narrow_str_to_charconst caller.
gcc/testsuite/
* g++.dg/cpp26/literals1.C: New test.
* g++.dg/cpp26/literals2.C: New test.
* g++.dg/cpp23/wchar-multi1.C (c, d): Expect an error rather than
warning.

--- gcc/testsuite/g++.dg/cpp26/literals1.C.jj   2023-08-25 17:23:06.662878355 
+0200
+++ gcc/testsuite/g++.dg/cpp26/literals1.C  2023-08-25 17:37:03.085132304 
+0200
@@ -0,0 +1,65 @@
+// C++26 P1854R4 - Making non-encodable string literals ill-formed
+// { dg-do compile { target c++11 } }
+// { dg-require-effective-target int32 }
+// { dg-options "-pedantic-errors -finput-charset=UTF-8 -fexec-charset=UTF-8" }
+
+int d = '😁';   // { dg-error "character too 
large for character literal type" }

...

+char16_t m = u'😁'; // { dg-error "character 
constant too long for its type" }


Why are these different diagnostics?  Why doesn't the first line already 
hit the existing diagnostic that the second gets?


Both could be clearer that the problem is that the single source 
character can't be encoded as a single execution character.


Jason

[pushed] c++: retval dtor on rethrow [PR112301]

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

In r12-6333 for PR33799, I fixed the example in [except.ctor]/2.  In that
testcase, the exception is caught and the function returns again,
successfully.

In this testcase, however, the exception is rethrown, and hits two separate
cleanups: one in the try block and the other in the function body.  So we
destroy twice an object that was only constructed once.

Fortunately, the fix for the normal case is easy: we just need to clear the
"return value constructed by return" flag when we do it the first time.

This gets more complicated with the named return value optimization, since
we don't want to destroy the return value while the NRV variable is still in
scope.

PR c++/112301
PR c++/102191
PR c++/33799

gcc/cp/ChangeLog:

* except.cc (maybe_splice_retval_cleanup): Clear
current_retval_sentinel when destroying retval.
* semantics.cc (nrv_data): Add in_nrv_cleanup.
(finalize_nrv): Set it.
(finalize_nrv_r): Fix handling of throwing cleanups.

gcc/testsuite/ChangeLog:

* g++.dg/eh/return1.C: Add more cases.
---
 gcc/cp/except.cc  | 18 ++-
 gcc/cp/semantics.cc   | 47 +-
 gcc/testsuite/g++.dg/eh/return1.C | 81 ++-
 3 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index e32efb30457..d966725db9b 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -1284,7 +1284,15 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
current_retval_sentinel so that we know that the return value needs to be
destroyed on throw.  Do the same if the current function might use the
named return value optimization, so we don't destroy it on return.
-   Otherwise, returns NULL_TREE.  */
+   Otherwise, returns NULL_TREE.
+
+   The sentinel is set to indicate that we're in the process of returning, and
+   therefore should destroy a normal return value on throw, and shouldn't
+   destroy a named return value variable on normal scope exit.  It is set on
+   return, and cleared either by maybe_splice_retval_cleanup, or when an
+   exception reaches the NRV scope (finalize_nrv_r).  Note that once return
+   passes the NRV scope, it's effectively a normal return value, so cleanup
+   past that point is handled by maybe_splice_retval_cleanup. */
 
 tree
 maybe_set_retval_sentinel ()
@@ -1361,6 +1369,14 @@ maybe_splice_retval_cleanup (tree compound_stmt, bool 
is_try)
  tsi_delink (&iter);
}
   tree dtor = build_cleanup (retval);
+  if (!function_body)
+   {
+ /* Clear the sentinel so we don't try to destroy the retval again on
+rethrow (c++/112301).  */
+ tree clear = build2 (MODIFY_EXPR, boolean_type_node,
+  current_retval_sentinel, boolean_false_node);
+ dtor = build2 (COMPOUND_EXPR, void_type_node, clear, dtor);
+   }
   tree cond = build3 (COND_EXPR, void_type_node, current_retval_sentinel,
  dtor, void_node);
   tree cleanup = build_stmt (loc, CLEANUP_STMT,
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 52044be7af8..a0f2edcf117 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4982,6 +4982,7 @@ public:
   tree result;
   hash_table > visited;
   bool simple;
+  bool in_nrv_cleanup;
 };
 
 /* Helper function for walk_tree, used by finalize_nrv below.  */
@@ -4997,7 +4998,7 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
   if (TYPE_P (*tp))
 *walk_subtrees = 0;
   /* If there's a label, we might need to destroy the NRV on goto (92407).  */
-  else if (TREE_CODE (*tp) == LABEL_EXPR)
+  else if (TREE_CODE (*tp) == LABEL_EXPR && !dp->in_nrv_cleanup)
 dp->simple = false;
   /* Change NRV returns to just refer to the RESULT_DECL; this is a nop,
  but differs from using NULL_TREE in that it indicates that we care
@@ -5016,16 +5017,59 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* 
data)
   else if (TREE_CODE (*tp) == CLEANUP_STMT
   && CLEANUP_DECL (*tp) == dp->var)
 {
+  dp->in_nrv_cleanup = true;
+  cp_walk_tree (&CLEANUP_BODY (*tp), finalize_nrv_r, data, 0);
+  dp->in_nrv_cleanup = false;
+  cp_walk_tree (&CLEANUP_EXPR (*tp), finalize_nrv_r, data, 0);
+  *walk_subtrees = 0;
+
   if (dp->simple)
+   /* For a simple NRV, just run it on the EH path.  */
CLEANUP_EH_ONLY (*tp) = true;
   else
{
+ /* Not simple, we need to check current_retval_sentinel to decide
+whether to run it.  If it's set, we're returning normally and
+don't want to destroy the NRV.  If the sentinel is not set, we're
+leaving scope some other way, either by flowing off the end of its
+scope or throwing an exception.  */
  tree cond = build3 (COND_EXPR, void_type_node,
 

[pushed] c++: use hash_set in nrv_data

[Jason Merrill]

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

I noticed we were using a hash_table directly here instead of the simpler
hash_set interface.  Also, let's check for the variable itself and repeats
earlier, since they should happen more often than any of the other cases.

gcc/cp/ChangeLog:

* semantics.cc (nrv_data): Change visited to hash_set.
(finalize_nrv_r): Reorganize.
---
 gcc/cp/semantics.cc | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index a0f2edcf117..37bffca8e55 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4980,7 +4980,7 @@ public:
 
   tree var;
   tree result;
-  hash_table > visited;
+  hash_set visited;
   bool simple;
   bool in_nrv_cleanup;
 };
@@ -4991,12 +4991,22 @@ static tree
 finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
 {
   class nrv_data *dp = (class nrv_data *)data;
-  tree_node **slot;
 
   /* No need to walk into types.  There wouldn't be any need to walk into
  non-statements, except that we have to consider STMT_EXPRs.  */
   if (TYPE_P (*tp))
 *walk_subtrees = 0;
+
+  /* Replace all uses of the NRV with the RESULT_DECL.  */
+  else if (*tp == dp->var)
+*tp = dp->result;
+
+  /* Avoid walking into the same tree more than once.  Unfortunately, we
+ can't just use walk_tree_without duplicates because it would only call
+ us for the first occurrence of dp->var in the function body.  */
+  else if (dp->visited.add (*tp))
+*walk_subtrees = 0;
+
   /* If there's a label, we might need to destroy the NRV on goto (92407).  */
   else if (TREE_CODE (*tp) == LABEL_EXPR && !dp->in_nrv_cleanup)
 dp->simple = false;
@@ -5086,18 +5096,6 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
   SET_EXPR_LOCATION (init, EXPR_LOCATION (*tp));
   *tp = init;
 }
-  /* And replace all uses of the NRV with the RESULT_DECL.  */
-  else if (*tp == dp->var)
-*tp = dp->result;
-
-  /* Avoid walking into the same tree more than once.  Unfortunately, we
- can't just use walk_tree_without duplicates because it would only call
- us for the first occurrence of dp->var in the function body.  */
-  slot = dp->visited.find_slot (*tp, INSERT);
-  if (*slot)
-*walk_subtrees = 0;
-  else
-*slot = *tp;
 
   /* Keep iterating.  */
   return NULL_TREE;

base-commit: 36a26298ec7dfca615d4ba411a3508d1287d6ce5
-- 
2.39.3

Re: [PATCH] c++: Implement C++26 P1854R4 - Making non-encodable string literals ill-formed [PR110341]

[Jason Merrill]

On 11/2/23 03:53, Jakub Jelinek wrote:

On Fri, Oct 27, 2023 at 07:05:34PM -0400, Jason Merrill wrote:

--- gcc/testsuite/g++.dg/cpp26/literals1.C.jj   2023-08-25 17:23:06.662878355 
+0200
+++ gcc/testsuite/g++.dg/cpp26/literals1.C  2023-08-25 17:37:03.085132304 
+0200
@@ -0,0 +1,65 @@
+// C++26 P1854R4 - Making non-encodable string literals ill-formed
+// { dg-do compile { target c++11 } }
+// { dg-require-effective-target int32 }
+// { dg-options "-pedantic-errors -finput-charset=UTF-8 -fexec-charset=UTF-8" }
+
+int d = '😁';   // { dg-error "character too 
large for character literal type" }

...

+char16_t m = u'😁'; // { dg-error "character 
constant too long for its type" }


Why are these different diagnostics?  Why doesn't the first line already hit
the existing diagnostic that the second gets?

Both could be clearer that the problem is that the single source character
can't be encoded as a single execution character.


The first diagnostics is the newly added in the patch which takes precedence
over the existing diagnostics (and wouldn't actually trigger without the
patch).  Sure, I could make that new diagnostics more specific, but all
I generally know is that (str2.len / nbwc) c-chars are encodable in str.len
execution character set code units.
So, would you like 2 different messages, one for str2.len / nbwb == 1
"single character not encodable in a single execution character code unit"


Sounds good, but let's drop "single".


and otherwise
"%d characters need %d execution character code units"
or
"at least one character not encodable in a single execution character code unit"
or something different?


The latter sounds good.  Maybe adding "in multicharacter literal"?


Everything else (i.e. u8 case in narrow_str_to_charconst and L, u and U
cases in wide_str_to_charconst) is already covered by existing diagnostics
which has the "character constant too long for its type"
wording and covers for both C and C++ both the cases where there are more
than one c-chars in the literal (allowed in the L case for < C++23) and
when one c-char encodes in more than one code units (but this time
it isn't execution character set, but UTF-8 character set for u8,
wide execution character set for L, UTF-16 character set for u and
UTF-32 for U).
Plus the same "character constant too long for its type" diagnostics
is emitted if normal narrow literal has several c-chars encodable all as
single execution character code units, but more than can fit into int.

So, do you want to change just the new diagnostics (and what is your
preferred wording), or use the old diagnostics wording also for the
new one, or do you want to change the preexisting diagnostics as well
and e.g. differentiate there between the single c-char cases which need
more than one code unit and different wording for more than one c-char?
Note, if we differentiate between those, we'd need to count how many
c-chars we have even for the u8, L, u and U cases if we see more than
one code unit, similarly how the patch does that (and also the follow-up
patch tweaks).


Under the existing diagnostic I'd like to distinguish the different 
cases more, e.g.


"multicharacter literal with %d characters exceeds 'int' size of %d bytes"
"multicharacter literal cannot have an encoding prefix"

Jason