Re: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Nathaniel Shead <nathanieloshead@gmail.com>]

On Mon, Jan 15, 2024 at 06:10:55PM -0500, Jason Merrill wrote:
> On 1/8/24 10:27, Patrick Palka wrote:
> > On Mon, 8 Jan 2024, Nathaniel Shead wrote:
> > > On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote:
> > > > On Sun, 3 Dec 2023, Nathaniel Shead wrote:
> > > > > 
> > > > > The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same
> > > > > underlying bit. This is causing confusion when attempting to determine
> > > > > the interface for a streamed-in class type, since the modules code
> > > > > currently assumes that all DECL_EXTERNAL types are extern templates.
> > > > > However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence
> > > > > DECL_EXTERNAL) is marked on various other kinds of declarations, such as
> > > > > vtables, which causes them to never be emitted.
> 
> But a vtable isn't a TYPE_DECL?
> 
> I suspect what you mean is that maybe_suppress_debug_info is setting
> TYPE_DECL_SUPPRESS_DEBUG to try to avoid duplication of debug info for
> classes with vtables, and then the modules code is wrongly assuming that you
> can check DECL_EXTERNAL for TYPE_DECL, and that it's set only if
> CLASSTYPE_INTERFACE_ONLY is also set, which is wrong in this case, so we
> avoid emitting the vtable or anything else for that class.
> 

Ah, right; I definitely misread what was going on.

> It seems unnecessary to start setting DECL_EXTERNAL on the TYPE_DECL to mean
> the exact same thing as CLASSTYPE_INTERFACE_ONLY.  Rather, the modules code
> should stop trying to check DECL_EXTERNAL on a TYPE_DECL.
> 
> Under what circumstances does it make sense for CLASSTYPE_INTERFACE_ONLY to
> be set in the context of modules, anyway?  We probably want to propagate it
> for things in the global module so that various libstdc++ explicit
> instantiations work the same with import std.
> 
> For an class imported from a named module, this ties into the earlier
> discussion about vtables and inlines that hasn't resolved yet in the ABI
> committee.  But it's certainly significantly interface-like.  And I would
> expect maybe_suppress_debug_info to suppress the debug info for such a class
> on the assumption that the module unit has the needed debug info.
> 
> Jason
> 

I didn't consider messing with 'CLASSTYPE_INTERFACE_ONLY' because the
modules code currently always sets 'lang->interface_unknown = true' on
read with the comment "Redetermine interface". But I see now as well
this comment:

  // Interfaceness is recalculated upon reading.  May have to revisit?
  // How do dllexport and dllimport interact across a module?
  // lang->interface_only
  // lang->interface_unknown

So it seems reasonable to reconsider this now. I hadn't considered
declarations in the GMF but I think you're right that we'll want to
propagate it; I guess in general things in the GMF should try to behave
as close to how they would have if they were #included directly in the
importing TU.

I suspect this fix will be more involved but I'll try to wrap my head
around it and see what I can come up with.