On 11/4/23 02:40, waffl3x wrote:
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.
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. ~decl.cc:10331 grokfndecl ``` int staticp = ctype && TREE_CODE (type) == FUNCTION_TYPE; ``` ~decl.cc:10506 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; ~decl.cc:10529 ``` /* 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 <nat...@acm.org> Date: Tue Jul 28 08:57:36 2020 -0700 c++: Set more DECL_CONTEXTsI discovered we were not setting DECL_CONTEXT in a few cases, andgrokfndecl'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.
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
