On 9/1/26 4:34 AM, Jakub Jelinek wrote:
On Thu, Aug 27, 2026 at 11:51:05AM -0700, Jason Merrill wrote:
Shall we remove the hash_map record on deallocation instead or store
something magic other than void_node?

Removing it sounds elegant, if there isn't a diagnostic quality reason to go
the other way.

As I wrote yesterday, remove instead of values.put (t, void_node); when
going out of storage caused too many diagnostic regressions, so this patch
goes for two magic values instead of one.

So do you want instead 2 magic values, one for zero initialization
implicit stuff and one for uninitialized implicit stuff?
Or it can be a separate flag or flags from the val variable.
Say bool is_zero_init = false, is_unitialized = false;

Either sounds fine, but I lean toward 2 magic values.

Used 2 magic values, but as everything here is contained in the same
function, named constants with those magic values and used comparisons
against those constants.

+       if (TREE_CODE (val) != CONSTRUCTOR)
+         return boolean_false_node;

This seems like an assertion rather than a return false?

Wonder about error_mark_node or something similar, but perhaps I could make
it into assert, yes (not done in the following patch yet).

Used gcc_assert now.

It seems a bit confusing to have overloads with subtly different meanings
(modifying or not).  We might call the new one get_raw_value_ptr and not
have special handling of void_node, so it's simpler to distinguish between
out-of-lifetime and no object?

Added get_raw_value_ptr.

+         else if (DECL_P (obj) && ctx->global->is_outside_lifetime (obj))
+           {
+             error_at (loc, "%qs on %qE outside its lifetime is not a "
+                       "constant expression",
+                       "__builtin_is_within_lifetime", obj);
+             inform (DECL_SOURCE_LOCATION (obj), "declared here");
+           }

I'd expect this to return false once we resolve the distinction between end
of lifetime and end of storage (no object).

It doesn't, the patch now uses this for both
   if (!valp || *valp == void_node)
and for the !valp case ctx->global->is_outside_lifetime (obj) will be
false, but for *valp == void_node it will be true.
While the void_list_node case (used for CLOBBER_OBJECT_END rather than
CLOBBER_STORAGE_END or going out of scope in other ways) doesn't make it
to this and instead returns boolean_false_node (it shouldn't appear in
DECL_INITIALIZER and so check_mutable doesn't need to be handled for that
case either).

+  void destroy_value (tree t, bool past_storage_end = true)
    {
      if (TREE_CODE (t) == VAR_DECL
        || TREE_CODE (t) == PARM_DECL
        || TREE_CODE (t) == RESULT_DECL)
-      values.put (t, void_node);
+      values.put (t, past_storage_end ? void_node : void_list_node);

This seems backwards to me, since void_node is also used for out-of-lifetime subobjects (which are still within storage duration).

Jason

Reply via email to