> > I'm unfortunately going down a rabbit hole again.
> > 
> > --function.h:608
> > `/* If pointers to member functions use the least significant bit to indicate whether a function is virtual, ensure a pointer to this function will have that bit clear. */ #define MINIMUM_METHOD_BOUNDARY \\ ((TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) \\ ? MAX (FUNCTION_BOUNDARY, 2 * BITS_PER_UNIT) : FUNCTION_BOUNDARY)`
> 
> 
> So yes, it was for PMFs using the low bit of the pointer to indicate a
> virtual member function. Since an xob memfn can't be virtual, it's
> correct for them to have the same alignment as a static memfn.

Is it worth considering whether we want to support virtual xobj member
functions in the future? If that were the case would it be better if we
aligned things a little differently here? Or might it be better if we
wanted to support it as an extension to just effectively translate the
declaration back to one that is a METHOD_TYPE? I imagine this would be
the best solution for non-standard support of the syntax. We would
simply have to forbid by-value and conversion semantics and on the
user's side they would get consistent syntax.

However, this flies in the face of the defective/contradictory spec for
virtual function overrides. So I'm not really sure whether we would
want to do this. I just want to raise the question before we lock in
the alignment, if pushing the patch locks it in that is, I'm not really
sure if it needs to be stable or not.

> > I stumbled upon this while cleaning up the patch, grokfndecl is just so
> > full of cruft it's crazy hard to reason about. There's more than one
> > block that I am near certain is completely dead code. I would like to
> > just ignore them for now but some of them unfortunately pertain to xobj
> > functions. I just don't feel good about putting in any hacks, but to
> > really get any modifications in here correct it would need to be
> > refactored much more than I should be doing in this patch.
> > 
> > Here's another example that I'm not sure how I want to address it.
> > 
> > :10331~decl.cc grokfndecl
> > `int staticp = ctype && TREE_CODE (type) == FUNCTION_TYPE;`
> > :10506~decl.cc grokfndecl
> > `/* If this decl has namespace scope, set that up. */ if (in_namespace) set_decl_namespace (decl, in_namespace, friendp); else if (ctype) DECL_CONTEXT (decl) = ctype; else DECL_CONTEXT (decl) = FROB_CONTEXT (current_decl_namespace ());`
> > And just a few lines down;
> > :10529~decl.cc
> > `/* Should probably propagate const out from type to decl I bet (mrs). */ if (staticp) { DECL_STATIC_FUNCTION_P (decl) = 1; DECL_CONTEXT (decl) = ctype; }`
> > 
> > If staticp is true, ctype must have been non-null, and if ctype is
> > non-null, the context for decl should have been set in the second
> > block. So why was the code in the second block added?
> > 
> > commit f3665bd1111c1799c0421490b5e655f977570354
> > Author: Nathan Sidwell nathan@acm.org
> > Date: Tue Jul 28 08:57:36 2020 -0700
> > 
> > c++: Set more DECL_CONTEXTs
> > 
> > I discovered we were not setting DECL_CONTEXT in a few cases, and
> > grokfndecl's control flow wasn't making it clear that we were doing it
> > in all cases.
> > 
> > gcc/cp/
> > * cp-gimplify.c (cp_genericize_r): Set IMPORTED_DECL's context.
> > * cp-objcp-common.c (cp_pushdecl): Set decl's context.
> > * decl.c (grokfndecl): Make DECL_CONTEXT setting clearer.
> > 
> > According to the commit, it was because it was not clear, which quite
> > frankly I can agree to, it just wasn't determined that the code below
> > is redundantly setting the context so it wasn't removed.
> > 
> > This puts me in a dilemma though, do I put another condition in that
> > code block for the xobj case even though the code is nearly dead? Or do
> > I give it a minor refactor for it to make a little more sense? If I add
> > to the code I feel like it's just going to add to the problem, while if
> > I give it a minor refactor it still won't look great and has a greater
> > chance of breaking something.
> > 
> > In this case I'm going to risk refactoring it, staticp is only used in
> > that 1 place so I will just rip it out. I am not concerned with decl's
> > type spontaneously changing to something that is not FUNCTION_TYPE, and
> > if it did I think there are bigger problems afoot.
> > 
> > I guess I'll know if I went too far with the refactoring when the patch
> > reaches you, do let me know about this one specifically though because
> > it took up a lot of my time trying to decide how to address it.
> 
> 
> Removing the redundant DECL_CONTEXT setting seems appropriate, and
> changing how staticp is handled to reflect that xobfns can also have
> FUNCTION_TYPE.

I removed static_p as it was only used in that one case, I'm pretty
happy with the resulting code but I saw you replied on the patch as
well so I'll see if you commented on it in the review and address your
thoughts there.

> > All tests seemed to pass when applied to GCC14, but the results did
> > something funny where it said tests disappeared and new tests appeared
> > and passed. The ones that disappeared and the new ones that appeared
> > looked like they were identical so I'm not worrying about it. Just
> > mentioning it in case this is something I do need to look into.
> 
> 
> That doesn't sound like a problem, but I'm curious about the specific
> output you're seeing.
> 
> Jason

I've attached a few test result comparisons so you can take a look.

Alex