On Fri, 13 Jun 2025, Jason Merrill wrote:

> Tested x86_64-pc-linux-gnu, any comments?  Bikeshedding?
> 
> -- 8< --
> 
> We already error about a type definition causing a concept check to change
> value, but it would be useful to diagnose this for other SFINAE contexts as
> well; the memoization problem also affects templates.  So
> -Wsfinae-incomplete remembers if we've failed a requirement for a complete
> type in a non-tf_error context, and later warns if the type becomes
> complete.

It seems useful to also warn for function return type deduction failure
followed by defining the function, to have parity with the satisfaction
value tracking.

> 
> This warning is enabled by default; I think the signal-to-noise ratio is
> high enough to warrant that, and it catches things that are likely to make
> the program "ill-formed, no diagnostic required".
> 
> The data for this warning uses GTY((cache)) to persist through GC, but allow
> entries to be discarded if the key is not otherwise marked.
> 
> I don't think it's desirable to export/import this information in modules,
> it makes sense for it to be local to a single TU.
> 
> -Wsfinae-incomplete=2 adds a warning at the point of failure, which is
> primarily intended to help with debugging warnings from the default mode.
> 
> gcc/ChangeLog:
> 
>       * doc/invoke.texi: Document -Wsfinae-incomplete.
> 
> gcc/c-family/ChangeLog:
> 
>       * c.opt: Add -Wsfinae-incomplete.
>       * c.opt.urls: Regenerate.
> 
> gcc/cp/ChangeLog:
> 
>       * constraint.cc (failed_completions_map): New.
>       (note_failed_type_completion): Rename from
>       note_failed_type_completion_for_satisfaction.  Add
>       -Wsfinae-incomplete handling.
>       (failed_completion_location): New.
>       * class.cc (finish_struct_1): Add -Wsfinae-incomplete warning.
>       * decl.cc (require_deduced_type): Adjust.
>       * typeck.cc (complete_type_or_maybe_complain): Adjust.
>       (cxx_sizeof_or_alignof_type): Call note_failed_type_completion.
>       * cp-tree.h: Adjust.
> 
> libstdc++-v3/ChangeLog:
> 
>       * testsuite/20_util/is_complete_or_unbounded/memoization.cc
>       * testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc:
>       Expect -Wsfinae-incomplete.
> 
> gcc/testsuite/ChangeLog:
> 
>       * g++.dg/cpp2a/concepts-complete1.C
>       * g++.dg/cpp2a/concepts-complete2.C
>       * g++.dg/cpp2a/concepts-complete4.C: Expect -Wsfinae-incomplete.
> ---
>  gcc/doc/invoke.texi                           | 13 ++++++++
>  gcc/c-family/c.opt                            |  8 +++++
>  gcc/cp/cp-tree.h                              |  3 +-
>  gcc/cp/class.cc                               | 10 ++++++
>  gcc/cp/constraint.cc                          | 32 +++++++++++++++++--
>  gcc/cp/decl.cc                                |  2 +-
>  gcc/cp/typeck.cc                              | 13 ++++++--
>  .../g++.dg/cpp2a/concepts-complete1.C         |  2 +-
>  .../g++.dg/cpp2a/concepts-complete2.C         |  2 +-
>  .../g++.dg/cpp2a/concepts-complete4.C         |  2 +-
>  .../is_complete_or_unbounded/memoization.cc   |  2 +-
>  .../memoization_neg.cc                        |  2 +-
>  gcc/c-family/c.opt.urls                       |  6 ++++
>  13 files changed, 85 insertions(+), 12 deletions(-)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 382cc9fa7a8..e3a6d2aced3 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -4448,6 +4448,19 @@ to filter out those warnings.
>  Disable the warning about the case when an exception handler is shadowed by
>  another handler, which can point out a wrong ordering of exception handlers.
>  
> +@opindex Wsfinae-incomplete
> +@opindex Wno-sfinae-incomplete
> +Warn about a class that is found to be incomplete in a context where
> +that causes substitution failure rather than an error, and then is
> +defined later in the translation unit.  This is problematic because
> +template instantiations or concept checks could have different results
> +if they first occur either before or after the definition.  To fix
> +this warning, avoid depending on its completeness until after it.
> +
> +This warning is enabled by default.  @option{-Wsfinae-incomplete=2}
> +adds a warning at the point of substitution failure, to make it easier
> +to track down problems flagged by the default mode.
> +
>  @opindex Wstrict-null-sentinel
>  @opindex Wno-strict-null-sentinel
>  @item -Wstrict-null-sentinel @r{(C++ and Objective-C++ only)}
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 50ba856fedb..8af466d1ed1 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1319,6 +1319,14 @@ Wsequence-point
>  C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ 
> ObjC++,Wall)
>  Warn about possible violations of sequence point rules.
>  
> +Wsfinae-incomplete=
> +C++ ObjC++ Var(warn_sfinae_incomplete) Warning Init(1) Joined RejectNegative 
> UInteger IntegerRange(0, 2)
> +Warn about an incomplete type affecting semantics in a non-error context.
> +
> +Wsfinae-incomplete
> +C++ ObjC++ Warning Alias(Wsfinae-incomplete=, 1, 0)
> +Warn about an incomplete type affecting semantics in a non-error context.
> +
>  Wshadow-ivar
>  ObjC ObjC++ Var(warn_shadow_ivar) EnabledBy(Wshadow) Init(1) Warning
>  Warn if a local declaration hides an instance variable.
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index d663d6ec225..ebfc1bb4a96 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8842,7 +8842,8 @@ extern hashval_t iterative_hash_constraint      (tree, 
> hashval_t);
>  extern hashval_t hash_atomic_constraint         (tree);
>  extern void diagnose_constraints                (location_t, tree, tree);
>  
> -extern void note_failed_type_completion_for_satisfaction (tree);
> +extern void note_failed_type_completion              (tree, tsubst_flags_t);
> +extern location_t failed_completion_location (tree);
>  
>  /* in logic.cc */
>  extern bool subsumes                            (tree, tree);
> diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
> index db39e579870..cdfc3c3e596 100644
> --- a/gcc/cp/class.cc
> +++ b/gcc/cp/class.cc
> @@ -7920,6 +7920,16 @@ finish_struct_1 (tree t)
>        return;
>      }
>  
> +  if (location_t fcloc = failed_completion_location (t))
> +    {
> +      auto_diagnostic_group adg;
> +      if (warning (OPT_Wsfinae_incomplete_,
> +                "defining %qT, which previously failed to be complete "
> +                "in a SFINAE context", t))
> +     inform (fcloc, "here.  Use %qs for a diagnostic at that point",
> +             "-Wsfinae-incomplete=2");
> +    }
> +
>    /* If this type was previously laid out as a forward reference,
>       make sure we lay it out again.  */
>    TYPE_SIZE (t) = NULL_TREE;
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 90625707043..e46df11ca24 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -1836,7 +1836,7 @@ tsubst_parameter_mapping (tree map, tree args, 
> tsubst_flags_t complain, tree in_
>  static bool satisfying_constraint;
>  
>  /* A vector of incomplete types (and of declarations with undeduced return 
> type),
> -   appended to by note_failed_type_completion_for_satisfaction.  The
> +   appended to by note_failed_type_completion.  The
>     satisfaction caches use this in order to keep track of "potentially 
> unstable"
>     satisfaction results.
>  
> @@ -1845,12 +1845,17 @@ static bool satisfying_constraint;
>  
>  static GTY((deletable)) vec<tree, va_gc> *failed_type_completions;
>  
> +/* A map of where types were found to be incomplete in SFINAE context, for
> +   warning if they are later completed.  */
> +
> +static GTY((cache)) hash_map<tree, location_t> *failed_completions_map;

For sake of PCH I think this needs to be a decl_location_traits (or a
new type_location_traits) so that the hash function uses DECL_UID /
TYPE_UID instead of unstable pointer identity.

> +
>  /* Called whenever a type completion (or return type deduction) failure 
> occurs
>     that definitely affects the meaning of the program, by e.g. inducing
>     substitution failure.  */
>  
>  void
> -note_failed_type_completion_for_satisfaction (tree t)
> +note_failed_type_completion (tree t, tsubst_flags_t complain)
>  {
>    if (satisfying_constraint)
>      {
> @@ -1858,6 +1863,29 @@ note_failed_type_completion_for_satisfaction (tree t)
>                          || (DECL_P (t) && undeduced_auto_decl (t)));
>        vec_safe_push (failed_type_completions, t);
>      }
> +  if (!(complain & tf_error) && CLASS_TYPE_P (t) && !dependent_type_p (t)
> +      && warning_enabled_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (t)),
> +                          OPT_Wsfinae_incomplete_))
> +    {
> +      if (warn_sfinae_incomplete > 1)
> +     warning (OPT_Wsfinae_incomplete_,
> +              "failed to complete %qT in SFINAE context", t);
> +      if (!failed_completions_map)
> +     failed_completions_map = hash_map<tree, location_t>::create_ggc ();
> +      failed_completions_map->put (TYPE_MAIN_VARIANT (t), input_location);
> +    }
> +}
> +
> +/* If T was previously found to be incomplete in SFINAE context, return the
> +   location where that happened, otherwise UNKNOWN_LOCATION.  */
> +
> +location_t
> +failed_completion_location (tree t)
> +{
> +  if (failed_completions_map)
> +    if (location_t *p = failed_completions_map->get (TYPE_MAIN_VARIANT (t)))
> +      return *p;
> +  return UNKNOWN_LOCATION;
>  }
>  
>  /* Returns true if the range [BEGIN, END) of elements within the
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 4c8a2052aee..718d0e5ede2 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -19989,7 +19989,7 @@ require_deduced_type (tree decl, tsubst_flags_t 
> complain)
>       /* We probably already complained about deduction failure.  */;
>        else if (complain & tf_error)
>       error ("use of %qD before deduction of %<auto%>", decl);
> -      note_failed_type_completion_for_satisfaction (decl);
> +      note_failed_type_completion (decl, complain);
>        return false;
>      }
>    return true;
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index ac1eb397f01..f4b49b792e3 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -156,7 +156,7 @@ complete_type_or_maybe_complain (tree type, tree value, 
> tsubst_flags_t complain)
>      {
>        if (complain & tf_error)
>       cxx_incomplete_type_diagnostic (value, type, DK_ERROR);
> -      note_failed_type_completion_for_satisfaction (type);
> +      note_failed_type_completion (type, complain);
>        return NULL_TREE;
>      }
>    else
> @@ -2084,7 +2084,14 @@ cxx_sizeof_or_alignof_type (location_t loc, tree type, 
> enum tree_code op,
>  
>    bool dependent_p = dependent_type_p (type);
>    if (!dependent_p)
> -    complete_type (type);
> +    {
> +      complete_type (type);
> +      if (!COMPLETE_TYPE_P (type))
> +     /* Call this here because the incompleteness diagnostic comes from
> +        c_sizeof_or_alignof_type instead of
> +        complete_type_or_maybe_complain.  */
> +     note_failed_type_completion (type, complain);
> +    }
>    if (dependent_p
>        /* VLA types will have a non-constant size.  In the body of an
>        uninstantiated template, we don't need to try to compute the
> @@ -2106,7 +2113,7 @@ cxx_sizeof_or_alignof_type (location_t loc, tree type, 
> enum tree_code op,
>  
>    return c_sizeof_or_alignof_type (loc, complete_type (type),
>                                  op == SIZEOF_EXPR, std_alignof,
> -                                complain);
> +                                complain & (tf_warning_or_error));
>  }
>  
>  /* Return the size of the type, without producing any warnings for
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C 
> b/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> index e8487bf9c09..9f2e9259f7f 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> @@ -12,7 +12,7 @@ template <class T> char f() { return 0; }
>  
>  struct A;
>  static_assert (sizeof (f<A>()) == 1); // { dg-message "first evaluated to 
> 'false' from here" }
> -struct A { typedef int type; };
> +struct A { typedef int type; };            // { dg-warning 
> Wsfinae-incomplete }
>  static_assert (sizeof (f<A>()) > 1); // { dg-error "assert" }
>  // { dg-message "required from here" "" { target *-*-* } .-1 }
>  static_assert (sizeof (f<A>()) > 1);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C 
> b/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> index b2c11606737..46952a4e59c 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> @@ -18,6 +18,6 @@ template <class T> char f() { return 0; }
>  
>  struct A;
>  static_assert (sizeof (f<A>()) == 1); // { dg-message "first evaluated to 
> 'false' from here" }
> -struct A { typedef int type; };
> +struct A { typedef int type; };            // { dg-warning 
> Wsfinae-incomplete }
>  static_assert (sizeof (f<A>()) > 1); // { dg-error "assert" }
>  static_assert (sizeof (f<A>()) > 1);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete4.C 
> b/gcc/testsuite/g++.dg/cpp2a/concepts-complete4.C
> index 988b0ddcfdd..7be9f50af6b 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-complete4.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete4.C
> @@ -8,6 +8,6 @@ struct A;
>  
>  static_assert(!C<A>);
>  
> -struct A { static constexpr bool value = false; };
> +struct A { static constexpr bool value = false; }; // { dg-warning 
> Wsfinae-incomplete }
>  
>  static_assert(C<A>); // { dg-error "assert" }
> diff --git 
> a/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization.cc 
> b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization.cc
> index 256b84df60f..59af024969f 100644
> --- a/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization.cc
> +++ b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization.cc
> @@ -23,7 +23,7 @@ struct X;
>  static_assert(
>    !std::__is_complete_or_unbounded(std::__type_identity<X>{}), "error");
>  
> -struct X{};
> +struct X{};                  // { dg-warning Wsfinae-incomplete }
>  static_assert(
>    std::__is_complete_or_unbounded(std::__type_identity<X>{}),
>    "Result memoized. This leads to worse diagnostics");
> diff --git 
> a/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc 
> b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc
> index 8e207b584dc..264efa77996 100644
> --- 
> a/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc
> +++ 
> b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc
> @@ -25,5 +25,5 @@
>  struct X;
>  constexpr bool res_incomplete = std::is_move_constructible<X>::value; // { 
> dg-error "required from here" }
>  
> -struct X{};
> +struct X{};                                                         // { 
> dg-warning Wsfinae-incomplete }
>  constexpr bool res_complete = std::is_default_constructible<X>::value; // { 
> dg-bogus "required from here" }
> diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
> index ad6d8a0b387..65d1221c4ad 100644
> --- a/gcc/c-family/c.opt.urls
> +++ b/gcc/c-family/c.opt.urls
> @@ -756,6 +756,12 @@ UrlSuffix(gcc/Warning-Options.html#index-Wno-self-move)
>  Wsequence-point
>  UrlSuffix(gcc/Warning-Options.html#index-Wno-sequence-point)
>  
> +Wsfinae-incomplete=
> +UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wno-sfinae-incomplete)
> +
> +Wsfinae-incomplete
> +UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wno-sfinae-incomplete)
> +
>  Wshadow-ivar
>  UrlSuffix(gcc/Warning-Options.html#index-Wno-shadow-ivar)
>  
> 
> base-commit: ad56e4632b05e66da35d6c23e45312b3cfbb646c
> -- 
> 2.49.0
> 
> 

Reply via email to