Re: Unreviewed cygwin/mingw dllimport patch for PRs 5287. 7910, 11021

[Danny Smith <danny_r_smith_2001 at yahoo dot co dot nz>]

 --- Jason Merrill <jason@redhat.com> wrote: > On Wed, 11 Jun 2003 08:02:40 +1000 (EST), Danny
Smith <danny_r_smith_2001@yahoo.co.nz> wrote:
> 
> >  --- Jason Merrill <jason@redhat.com> wrote: > + 	/* `extern' needn't be specified with
> dllimport.
> > + 	/* `extern' needn't be specified with dllimport. Specify
> > + 	   `extern' now and hope for the best. Be careful that we don't
> > + 	   set TREE_PUBLIC for variables with static storage duration
> > +	   (class static data is already public),  so we can catch syntax
> > +	   ambiguity  below.  */
> 
> I'm saying that the case you want to complain about is !TREE_PUBLIC.
> TREE_STATIC is set for both
>   int i;
> and
>   static int j;
> 
> You just don't want to set TREE_PUBLIC if it isn't already set.


My reason for the test for TREE_STATIC of dllimport'd variables in this:

if (is_attribute_p ("dllimport", name)) 
    &&TREE_CODE (node) == VAR_DECL)
  {
    if (!TREE_STATIC (node))
     /* This is needed for declarations of variables within functions */
      TREE_PUBLIC (node) = 1; 
    DECL_EXTERNAL (node) = 1;
  }

/* This is for all dllimport/export'd symbols */
if (! TREE_PUBLIC (node))
   error ("external linkage required")


is to allow this, by setting TREE_PUBLIC as well DECL_EXTERNAL:

void foo()
{
  __attribute__((dllimport))
  int foovar;     // OK,  implicit "extern" 
  foovar++;
}

but fail this  by refusing to set TREE_PUBLIC because of explicit
static 

void bar()
{
  __attribute__((dllimport))
  static int imp_var;   // { dg-error "external linkage required" }
  imp_var++;
}

and also fail all other cases (dllexported vars and functions, dllimported
functions) where TREE_PUBLIC is not already set.

How else can I achieve that? 

> 
> >> !       /* Turn off importing C++ virtual methods to able to create 
> >> ! 	 vtables using thunks. */
> >> !       else if (TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE
> >> ! 	       && DECL_VIRTUAL_P (decl))
> >> !         {
> >> !           return 0;
> >> !         }
> >> 
> >> I don't understand the rationale for this; please hold this bit back until
> >> your next patch to handle multiple inheritance.
> >
> > OK, My empirical rationale is PRs c++/5287 and c++/11021 which fail without 
> > this hunk and succeed with it.  I've gone back and looked through all the dialogue
> > between Jason and Mumit Khan ca. 1998/1999, and I confess I do not understand it all. 
> > But -- given the minimal cost of ignoring the attibute for virtual methods -- 
> > this seemed like the simplest solution and one that is "local" to windows ports. 
> 
> IMO, ABI decisions shouldn't be made based on what's the simplest solution.
> Incidentally, the DLL model isn't unique to Windows, though I suppose BeOS
> doesn't count for much these days...:)
> 
> Why does this patch fix those bugs?
> 

In this testcase:
struct __attribute__((dllimport)) A
{
  virtual void vfunc(void);
};

struct B : public A
{
};

B aB;

The abort occurs  for the vtable entry for B::vfunc.
Beacuse of the dllimport, we cannot use a pointer to the function as
constant address, so  

 __attribute__((dllimport)) foo();
 void* pfn =  (void*) foo;

causes problems. 

That is my limited understanding of this.  I will need considerable more time
to figure out exactly what is going on. 

Danny

> Jason 

http://mobile.yahoo.com.au - Yahoo! Mobile
- Check & compose your email via SMS on your Telstra or Vodafone mobile.