[PATCH] AmigaOS 4 port contribution

Joseph S. Myers jsm@polyomino.org.uk
Sun Oct 5 18:58:00 GMT 2003


On Sun, 5 Oct 2003, Marcus Comstedt wrote:

> The thing has to be done in the front end though, since the extra
> argument has to be included when doing check_function_arguments().
> Of course, I can move the actual code to the backend and make a call
> from the front end, but that somehow seems like a layering violation.

Start by giving a proper specification of the language extension - I mean
in terms of textual edits to the C99 standard, not the vague description
given under the attribute documentation.  You don't, for example, say how
the Interface object is determined from the pointer to function involved
in the function call; there appear to be some restrictions on the syntax
used to create that Interface object, but various things are unclear, e.g.  
can an Interface value that isn't an lvalue be used (e.g. (foo ?  
interface1 : interface2).bar()), and in that case is the pointer a pointer
to a copy?

Then it can be considered whether there should be a generic target
function by which the back end can make such modifications to function
calls.  Of course any such target function must be documented (your patch
doesn't document the TARGET_AMIGAOS_DECL_ATTRIBUTES macro).

Other points:

* I take it the changes to libtool scripts are already in libtool CVS (as 
required in codingconventions.html)?

> +/* Subroutines used for code generation on Amiga OS 4
> +   Copyright (C) 1991, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001
> +   Free Software Foundation, Inc.

I doubt your copyright dates, in particular that there have been no
significant changes since 2001.  Please check them all carefully.

> +This file is part of GNU CC.

We've been getting rid of references to "GNU CC", don't reintroduce them.

This source file (amigaos.c) is also missing comments before all the 
functions.

> +/* AmigaOS4 specific macros for use with __atribute__((linearvarargs)) */
> +#if defined(__amigaos__) && defined (__PPC__)
> +
> +#define va_startlinear(AP, x)                  \

You can't unconditionally add macros to <stdarg.h> or other standard
headers like this, va_* is *not* a reserved namespace.  Please put all
this in an AmigaOS-specific header rather than touching stdarg.h at all.

You don't actually document what these macros are meant to do, but why 
can't va_start act appropriately according to the attributes on the 
calling function - why do you need va_startlinear at all?

-- 
Joseph S. Myers
jsm@polyomino.org.uk



More information about the Gcc-patches mailing list