This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


+ 	/* `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 declared static (class static
+ 	   data is already public)  so we can catch syntax ambiguity
+ 	   below.  */
+ 	  if (!TREE_STATIC (node))
+ 	    TREE_PUBLIC (node) = 1;

This is wrong; TREE_STATIC means static storage duration, not C 'static'.
The way to test for internal vs. external linkage is to look at
TREE_PUBLIC.

!       /* Don't mark defined functions as dllimport.  If the definition
! 	 itself was marked with dllimport, then an error is raised. This
! 	 handles the case when the definition overrides an earlier
! 	 declaration.  */
!       if (TREE_CODE (decl) ==  FUNCTION_DECL && DECL_INITIAL (decl)
! 	  && !DECL_INLINE (decl))
! 	{
! 	  warning ("%Hdefinition of '%D' previously marked as dllimport: attribute ignored",
! 		   &DECL_SOURCE_LOCATION (decl), decl);
! 	  return 0;
! 	}

I think the warning should say "declaration", not "definition".

!           /* Should in-class initialization of static consts raise an
! 	     error too, or just ignore the attribute, as here?  */
! 	  else if (DECL_INITIAL (decl)
! 		   && !TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (decl)))
! 	    {
! 	      warning ("%Hin-class initialization of '%D': dllimport attribute ignored.", 
! 		       &DECL_SOURCE_LOCATION (decl), decl);	
! 	      return 0;
! 	    }

In-class initialization of static consts should not cause them to be
treated differently from variables without in-class initializers.

!       /* 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.

Incidentally, talking about "throw"ing an error is misleading, since we
don't use exception handling for error reporting in C++.  I'd probably say
"report an error" or "complain".

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]