bebbo / amiga-gcc

The GNU C-Compiler with Binutils and other useful tools for cross development for Amiga
GNU General Public License v2.0
312 stars 66 forks source link

Download aside, what’s needed for NDK3.2 support #222

Closed acf closed 3 years ago

acf commented 3 years ago

I was wondering if there’s been any investigation into what would required to install the new NDK3.2 alongside NDK13 and NDK39 ?

I realise there isn’t anywhere to directly downlad it from so it’s not going to part of the Makefile for a while, even if the rest is easy.

I had thought I might start playing and running the same FD tools over the new NDK as you do in the Makefile but I’ve been reading stuff from gulliver and boemann that seems to suggest it should work out of the box and the protos are ready to go.

Anyway, figured I’d ask here rather than just another EAB thread.

(unlike an apple M1 this is an issue where I can buy you what you need if you don’t have it already) :)

bebbo commented 3 years ago

you need a better sed it's working fine here

pushed a possible fix - untested as usual^^

acf commented 3 years ago

So it was working fine for you ? Thats interesting that the builtin sed on the BigSur is not so good. Maybe need to add a homebrew version to the instructions 🤔

I'll try the fix. :-). Thanks!

bebbo commented 3 years ago

So it was working fine for you ? Thats interesting that the builtin sed on the BigSur is not so good. Maybe need to add a homebrew version to the instructions 🤔

I'll try the fix. :-). Thanks!

rm -rf projects/NDK* is sufficient to retest the NDK stuff

acf commented 3 years ago

All good. this was the only odd messages left, but for the first time the whole process ran to completion first time on 3.2 and I built a couple of test projects with no issue :-)

Screenshot 2021-08-12 at 16 35 37

Thanks for your patience with my slow pace of digging.

acf commented 3 years ago

Oh, and can probably take the "wont fix" label off :-)

acf commented 3 years ago

I've chatted with Olaf on the 3.2 team and he says that firstly the [...] seems to have somehow slipped through. It was used to indicate an optional argument by commodore.

My pet theory is that this funky ellipsis is an artefact of the process which produced the original sfd files. Commodore produced the sfd tool so that it could generate the same function prototype and pragma header files from the sfd file templates. These templates had to be produced by "augmenting" the existing fd files (I presume "fd" stands for "function description" or something like that) with 'C' syntax to produce the function prototypes. At that time, this information (register usage, result and function parameter types) was most easily available through the AutoDocs. You could have analyzed the source code which the AutoDocs describe and gotten the same information, too. But it would have taken longer... My guess is that Commodore used Unix tools and scripts to extract the function names and register usage from the each fd file and matched these against what could be found in the corresponding AutoDocs. Perl 4 was already available by 1990 and Perl was used a lot in making things such as the bug/enhancement request database work which Commodore maintained. Why the AutoDocs angle? The funky ellipsis (, [struct BitMap *bm2]) can only be found in the layers.library and graphics.library AutoDocs, e.g. the BltBitMap() function in graphics.library has an optional final parameter (, [PLANEPTR tempA]). Since the "markup" for the optional parameter has no good purpose at all (you can't omit it if you're using pragmas or inline header files, and there is only one single valid function prototype generated) I guess it was dropped and survived in the layers_lib.sfd file because nobody brought it in line with what was done for graphicsl_lib.sfd, for example.

As regards the whole process, he's suggesting that they are aiming to ship 3.2 with working inlines out of the box so perhaps you dont even need to generate from SFD? I wasn't sure if there was a reason they were no use to you or if you just stuck to the same process as 3.9 for "simplicity". If there's something lacking in the shipped inlines they'd be happy to correct it and save you having to regen.

bebbo commented 3 years ago

... if they would really care, they could look into the stuff which needs a patch here^^

Your words in God's ear...

acf commented 3 years ago

So the stuff here: https://github.com/bebbo/amiga-gcc/tree/master/patches/NDK3.2 ?

acf commented 3 years ago

And thats still separate from generating inlines, right ?

bebbo commented 3 years ago

So the stuff here: https://github.com/bebbo/amiga-gcc/tree/master/patches/NDK3.2 ?

yes - and that's not complete yet. clib/compiler-specific.h does not cover gcc and the patches for NDK 3.9 cover a tad more... plus the __stdargs which might change to __STDARGS if clib/compiler-specific.h is complete and gets included...

acf commented 3 years ago

cc @obarthel. I feel a bit like the man in the middle relaying chinese whispers when you are both much smarter than me. :-)

I figured I'd try and bring you all together and get out of the way. http://eab.abime.net/showthread.php?t=107903

chris-y commented 3 years ago

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

In graphics/gfxmacros.h, GetOutlinePen() is defined twice.

The SFD and FD directories are missing all the files for the ReAction classes.

chris-y commented 3 years ago

Also there's something wrong with timer_lib.sfd, as the second line is * "timer.device"

If I use the NDK's inlines and try to build NetSurf I get lots of undefined references. I've just rebuilt the inlines myself and (aside from a couple of errors I've not fixed yet) it seems to build OK - so there's something different/wrong with the included inlines.

obarthel commented 3 years ago

... if they would really care, they could look into the stuff which needs a patch here^^

Your words in God's ear...

We do care, but we need to know what we've got to aim for.

The goal is to ship an NDK 3.2 with header files that cover all the historic compilers which are still relevant for native development and production code (i.e. building the operating system itself) as well as cross-compilers. The latter are historic (e.g. 2.95.3) and much more modern versions of GCC (e.g. 6.5.0b) as well as vbcc which can be used both for cross-development and native development.

We had to start all over again with the state of the Amiga OS 3.1 header files as they were in 1994, which is why the sfd files and the operating system header files don't match the state of the NDK 3.9. In places they are close, in other places they are far from close. We adopted some of the AmigaOS 4 header file changes, but picked different solutions where we learned from what didn't work so well on AmigaOS 4. You may not know it, but I'm the guy to blame most for the state of the NDK 3.5, 3.9, 3.2 as well as the AmigaOS4 NDKs. I've had my grubby fingers in all of them ;-)

The thing is, what we have now still needs a lot of work. For example, a large chunk of the 'C' and assembly language header file changes which were prepared for the NDK 3.2 R3 didn't even make it into the NDK 3.2 R3 due to an oversight. The vbcc inline header files, as they are, are incomplete.

I know from experience that the NDK 3.9 has its own share of problems, but having been around for almost 20 years now it's an acquired taste and might be mistaken for the gold standard in the right light. But they are frozen in time and not going to be mended. The same is true about the NDK 3.1 header files which shipped some 25+ years ago with the Geek Gadgets, etc. CD-ROMs.

So the plan still is to get the "canonical" NDK header files right instead of asking everybody to ship a fixed-up version of these "canonical" NDK header files.

What do you need fixing, and how should it work?

obarthel commented 3 years ago

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

In graphics/gfxmacros.h, GetOutlinePen() is defined twice.

That's because the cleaned-up version was not released with the NDK 3.2 R3 release ☹️

It's defined twice because the header file can define one or the other version of the macro if a specific preprocessor symbol is defined.

Getting rid of GetOutlinePen() was one of the goals of the NDK 3.2 R3 release. Part of the job was figuring out why it even was part of "graphics/gfxmacros.h". Turns out that Commodore introduced a graphics.library function by that name in V39 and nobody noticed that the macro was still there...

The SFD and FD directories are missing all the files for the ReAction classes.

Which was at the time deliberate because we were not certain about whether they were needed in this form. It's no longer in doubt that they are needed now, though.

So the stuff here: https://github.com/bebbo/amiga-gcc/tree/master/patches/NDK3.2 ?

yes - and that's not complete yet. clib/compiler-specific.h does not cover gcc and the patches for NDK 3.9 cover a tad more...

Sorry about that ☹️ The changes were in place, but they didn't get copied to the release archive. Most notably, the vbcc-specific header files were missing and all of the "legacy" Aztec 'C', DICE, Maxon 'C' and StormC interface definitions were omitted.

plus the __stdargs which might change to __STDARGS if clib/compiler-specific.h is complete and gets included...

The current "clib/compiler-specific.h" file covers it. In line with the other peculiar preprocessor symbols it's called __STDARGS__ (which bumps against the rules for naming preprocessor symbols with two leading underscore characters, but now we're stuck with them).

obarthel commented 3 years ago

Also there's something wrong with timer_lib.sfd, as the second line is * "timer.device"

That's how the 3.1 SFD files rolled back in 1994, I'm afraid. The SFD files were built from the already existing FD files and augmented by fancy control instructions such as "==include". And in an FD file the "*" at the beginning of a line introduces a comment. This is still valid, although it arguably serves no good purpose. It's just a left-over. As I mentioned, we had to start all over again for the NDK 3.2 from the state of the 3.1 header and SFD files, and some of these files are still closer to 3.1 than to 3.9 (or 4.x).

If I use the NDK's inlines and try to build NetSurf I get lots of undefined references. I've just rebuilt the inlines myself and (aside from a couple of errors I've not fixed yet) it seems to build OK - so there's something different/wrong with the included inlines.

Which warnings are you seeing? Could you provide a sample?

chris-y commented 3 years ago

Also there's something wrong with timer_lib.sfd, as the second line is * "timer.device"

That's how the 3.1 SFD files rolled back in 1994, I'm afraid. The SFD files were built from the already existing FD files and augmented by fancy control instructions such as "==include". And in an FD file the "*" at the beginning of a line introduces a comment. This is still valid, although it arguably serves no good purpose. It's just a left-over. As I mentioned, we had to start all over again for the NDK 3.2 from the state of the 3.1 header and SFD files, and some of these files are still closer to 3.1 than to 3.9 (or 4.x).

OK, that's interesting. If I run fd2pragma on this file it doesn't like AddTime/CmpTime/SubTime - complaining about brackets. I thought the * line might have been upsetting it but clearly there's something else wrong there (or a bug in fd2pragma).

If I use the NDK's inlines and try to build NetSurf I get lots of undefined references. I've just rebuilt the inlines myself and (aside from a couple of errors I've not fixed yet) it seems to build OK - so there's something different/wrong with the included inlines.

Which warnings are you seeing? Could you provide a sample?

Sure. Loads of undefined references at the linker stage. The compiler works fine otherwise - I tested a very minimal DisplayBeep() program and it built that without complaint. It could be the makefile is doing something weird, but as it works with regenerated inlines that seems unlikely :confused:

build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o(.text+0x1a):build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o: undefined reference to `_FreeClass'
build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o(.text+0x28):build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o: undefined reference to `_CloseLibrary'
build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o(.text+0x66):build/Linux-amigaos3/frontends_amiga_agclass_amigaguide_class.o: undefined reference to `_OpenLibrary'
...
/opt/netsurf/m68k-unknown-amigaos/env/lib/libnsutils.a(src_time.o)(.text+0x12):src_time.o: undefined reference to `_ReadEClock'
/opt/netsurf/m68k-unknown-amigaos/cross/lib/gcc/m68k-unknown-amigaos/3.4.6/../../../../m68k-unknown-amigaos/bin/ld: link errors found, deleting executable `NetSurf'
bebbo commented 3 years ago

Thanks for dropping by and your comments

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

I can't judge if the timeval related changes are good or bad or whatever. But adding this define with check won't hurt, but would help a lot. Also using a union for modern "C" plus staying backwards compatible...:

#ifndef _TIMEVAL_DEFINED
#define _TIMEVAL_DEFINED
#if __STDC_VERSION__ >= 199901L
struct timeval {
    union {
        time_t          tv_sec;         /* seconds */
        time_t          tv_secs;
    };
    union {
        suseconds_t     tv_usec;        /* and microseconds */
        suseconds_t     tv_micro;
    };
};
#else
struct timeval {
        unsigned int     tv_sec;         /* seconds */
        unsigned int     tv_usec;        /* and microseconds */
};
#define tv_secs tv_sec
#define tv_micro tv_usec
#endif 
#endif

In graphics/gfxmacros.h, GetOutlinePen() is defined twice.

That's because the cleaned-up version was not released with the NDK 3.2 R3 release ☹️

It's defined twice because the header file can define one or the other version of the macro if a specific preprocessor symbol is defined.

Getting rid of GetOutlinePen() was one of the goals of the NDK 3.2 R3 release. Part of the job was figuring out why it even was part of "graphics/gfxmacros.h". Turns out that Commodore introduced a graphics.library function by that name in V39 and nobody noticed that the macro was still there...

Then I'm nobody, since I patched this already with NDK 3.9. The solution should be symmetric to SetOutlinePen:

 /* synonym for GetOPen for consistency with SetOutlinePen */
-#define GetOutlinePen(rp) GetOPen(rp)
+#define GetOPen(rp) GetOutlinePen(rp) 

The SFD and FD directories are missing all the files for the ReAction classes.

Which was at the time deliberate because we were not certain about whether they were needed in this form. It's no longer in doubt that they are needed now, though.

good.

So the stuff here: https://github.com/bebbo/amiga-gcc/tree/master/patches/NDK3.2 ?

yes - and that's not complete yet. clib/compiler-specific.h does not cover gcc and the patches for NDK 3.9 cover a tad more...

Sorry about that ☹️ The changes were in place, but they didn't get copied to the release archive. Most notably, the vbcc-specific header files were missing and all of the "legacy" Aztec 'C', DICE, Maxon 'C' and StormC interface definitions were omitted.

plus the __stdargs which might change to __STDARGS if clib/compiler-specific.h is complete and gets included...

The current "clib/compiler-specific.h" file covers it. In line with the other peculiar preprocessor symbols it's called "STDARGS" (which bumps against the rules for naming preprocessor symbols with two leading underscore characters, but now we're stuck with them).

it contains __STDARGS__ which is reasonable. But it's missing support for gcc. I'll provide a patch soon. But more important: if there is __STDARGS__ then it must be applied to all clib/*_protos.h. I'm adding this via sed.

LC_CTYPE=C sed -i.bak -E 's/([a-zA-Z0-9 _]*)([[:blank:]]+|\*)([a-zA-Z0-9_]+)\(/\1\2 __stdargs \3(/g' $$i
rm $$i.bak

I'll use __STDARGS__ once it's working.

back to the sfd files: at least dos, exec and locale should set the basetype, e.g.:

+==basetype struct DosLibrary * 

since the members of these bases are used frequently.

Also supporting objective-C would be nice - this needs a patch to intuition/classes.h:

+#ifdef __OBJC__
+  IntuitionClass;
+#else
+  Class;
+#endif 

Since Class is already used in objective-C.

but anyway: I can live with patching the hell out of the ndk^^

obarthel commented 3 years ago

Thanks for dropping by and your comments

Happy to be around, it's the least I can do to clean up a mess I've been involved in making 🙄

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

I can't judge if the timeval related changes are good or bad or whatever. But adding this define with check won't hurt, but would help a lot. Also using a union for modern "C" plus staying backwards compatible...:

#ifndef _TIMEVAL_DEFINED
#define _TIMEVAL_DEFINED
#if __STDC_VERSION__ >= 199901L
struct timeval {
    union {
        time_t          tv_sec;         /* seconds */
        time_t          tv_secs;
    };
    union {
        suseconds_t     tv_usec;        /* and microseconds */
        suseconds_t     tv_micro;
    };
};
#else
struct timeval {
        unsigned int     tv_sec;         /* seconds */
        unsigned int     tv_usec;        /* and microseconds */
};
#define tv_secs tv_sec
#define tv_micro tv_usec
#endif 
#endif

I would suggest not to change the layout and meaning of the timeval even if the unions will eventually compile to the same thing. The AmigaOS header files are surprisingly brittle and there are side-effects in dealing with the clash between the POSIX timeval and its distant cousin in "devices/timer.h". Put another way: this is already a three-way mess and one more variation will lead to what few new Amiga developers might adopt and then be terribly disappointed either way.

Part of the work which involves the NDK is in figuring out what the heck was going on when the header files were put together, how we deal with changes now (Commodore had its share of problems there) and how we can make the data types, data structures and how they are defined consistent and reliable. And part of this is to explain what's being done, and why, and what you're going to do if your code no longer builds.

That's a lot of talk, for sure. Until we get to an agreement of what makes sense for the dreaded Amiga timeval, I suggest not deploying your changes without explaining what you are attempting to enable and how the original layout looked like. I'd hope that Amiga developers were clever enough to figure this out by themselves, but it all goes much easier if you put in a few words and references by yourself. You'd be amazed by what previous generations of Amiga developers ended up speculating about Commodore's unexplained changes. We're not supposed to grow Amiga software development folklore 😄

Incidentally, I spent some time researching where the Amiga "timeval" may have come from. Due to its name and history (the final stretch of Amiga operating system development too place between 1984/1985 on Sun 2 and then later Sun 3 workstations at Commodore-Amiga, Inc.) SunOS 2 or SunOS 3 might have been influences. But (I checked old SunOS 2 and SunOS 3 installation tape copies) this doesn't seem to be it. Both SunOS 2 and SunOS 3 use the same definition as our contemporary POSIX timeval. The Amiga timeval is unique and does not share the member names of the BSD Unix which SunOS 2 and SunOS 3 are based upon.

In graphics/gfxmacros.h, GetOutlinePen() is defined twice.

That's because the cleaned-up version was not released with the NDK 3.2 R3 release ☹️ It's defined twice because the header file can define one or the other version of the macro if a specific preprocessor symbol is defined. Getting rid of GetOutlinePen() was one of the goals of the NDK 3.2 R3 release. Part of the job was figuring out why it even was part of "graphics/gfxmacros.h". Turns out that Commodore introduced a graphics.library function by that name in V39 and nobody noticed that the macro was still there...

Then I'm nobody, since I patched this already with NDK 3.9. The solution should be symmetric to SetOutlinePen:

 /* synonym for GetOPen for consistency with SetOutlinePen */
-#define GetOutlinePen(rp) GetOPen(rp)
+#define GetOPen(rp) GetOutlinePen(rp) 

The macro is obsolete, has been obsolete since at least 1991, it's just that nobody at Commodore noticed that it was obsolete.

It makes sense to retain it for the sake of building code which can be run on Kickstart 1.1-3.x without binary changes to account for 3.x. But even then it would have to be documented and, if possible, made safe to use. Without those decorations I question its usefulness because its side-effects (e.g. conflicts with the inline header files) are unpredictable.

The SFD and FD directories are missing all the files for the ReAction classes.

Which was at the time deliberate because we were not certain about whether they were needed in this form. It's no longer in doubt that they are needed now, though.

good.

So the stuff here: https://github.com/bebbo/amiga-gcc/tree/master/patches/NDK3.2 ?

yes - and that's not complete yet. clib/compiler-specific.h does not cover gcc and the patches for NDK 3.9 cover a tad more...

Sorry about that ☹️ The changes were in place, but they didn't get copied to the release archive. Most notably, the vbcc-specific header files were missing and all of the "legacy" Aztec 'C', DICE, Maxon 'C' and StormC interface definitions were omitted.

plus the __stdargs which might change to __STDARGS if clib/compiler-specific.h is complete and gets included...

The current "clib/compiler-specific.h" file covers it. In line with the other peculiar preprocessor symbols it's called "STDARGS" (which bumps against the rules for naming preprocessor symbols with two leading underscore characters, but now we're stuck with them).

it contains __STDARGS__ which is reasonable. But it's missing support for gcc. I'll provide a patch soon.

I can share the unpleasantly unpublished NDK 3.2 R3 version of the "compiler-specific.h" file with you, so that you might put it to good use and wouldn't need to patch individual parts. As is common with all the recent published (and accidentally unpublished) changes, they are documented in context.

But more important: if there is __STDARGS__ then it must be applied to all clib/*_protos.h.

Hold on! Why? Only a small subset of functions in <clib/alib_protos.h>, <clib/debug_protos.h> and <clib/ddebug_protos.h> require parameter passing on the stack. As for the rest, the pragmas/pragma/inline header files for legacy and contemporary 'C' compilers function safely without them.

If you must change the NDK header files, please document this fact in the files your setup produces, as otherwise developers will assume that this is exactly how they should look like, running into thorny problems with other compilers.

I'm adding this via sed.

LC_CTYPE=C sed -i.bak -E 's/([a-zA-Z0-9 _]*)([[:blank:]]+|\*)([a-zA-Z0-9_]+)\(/\1\2 __stdargs \3(/g' $$i
rm $$i.bak

I'll use __STDARGS__ once it's working.

back to the sfd files: at least dos, exec and locale should set the basetype, e.g.:

+==basetype struct DosLibrary * 

since the members of these bases are used frequently.

Yes, the original reason for the basetype keyword in the sfd files was to get rid of the many warnings compilation produced for GCC-compiled code, in particular. Unfortunately, because we had to start over from the 3.1 files (including the SFD files) the "basetype" and "libname" information was not added back. Which still has to happen, and this time has to be correct for the ReAction classes.

Also supporting objective-C would be nice - this needs a patch to intuition/classes.h:

+#ifdef __OBJC__
+  IntuitionClass;
+#else
+  Class;
+#endif 

Since Class is already used in objective-C.

I'd say that Objective 'C' had its time in the sun (twice) and does not represent a significant portion of software developed for the Amiga. As such there's a greater risk of adding more entropy to the Amiga header files than they deserve without a tangible benefit. Now synchronizing the contents of the 68k assembly language header files with the 'C' language header files (which always used to be done by hand) would have a much more beneficial impact 😄

but anyway: I can live with patching the hell out of the ndk^^

You're not supposed to take responsibility for patching the thing. I say it again: we funny people, who try to do their best in the absence of adult supervision as AmigaOS 68k developers, should be shipping consistent, correct and reliable operating system header files and take responsibility for doing so. You may see the current NDK 3.2 troubles as something which can be fixed gently and with little technical effort. I see the need to get the NDK right and to fix the long-standing issues in the documentation that is both part of the AutoDocs and the header files. Unless somebody stops us (such as by dropping a few anvils or injuctions from the troposphere), we'll keep chipping away at the NDK. Which would likely bump against what your work currently entails. You should not need to adapt your Makefile and scripts when that happens. We'd like to get this covered before it becomes extra work for you.

In preparation for the NDK 3.2 work I went on an expedition to collect the original and unmodified 1.1, 1.2 and 1.3 header files as they were when the first and second edition Amiga ROM kernel reference manuals went to print. The header files which Commodore shipped in the day (1985, 1986, 1987) were "stripped" by the compiler vendors for performance and disk space reasons. This made some sense, but it also deprived developers of the documentation that is part of the header files. For these reasons I strongly suggest caution when changing the header files, even if it is for the better. I had to wrestle with these stripped and truncated header files myself when I was starting out as a developer... And you might accidentally learn something from these modified files which turned out to be wrong, because you didn't know the original contents.

bebbo commented 3 years ago

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

#ifndef _TIMEVAL_DEFINED
#define _TIMEVAL_DEFINED
#endif

maybe at least that #ifdef part? Then it doesn't matter which one is included first...

It makes sense to retain it for the sake of building code which can be run on Kickstart 1.1-3.x without binary changes to account for 3.x. But even then it would have to be documented and, if possible, made safe to use. Without those decorations I question its usefulness because its side-effects (e.g. conflicts with the inline header files) are unpredictable.

well - better support for older kickstarts would require adding something like

#if (__KICKSTART_VERSION >= 46)
...
#endif

right now the comment is evaluated and the files get truncated to provide only old stuff to 1.3

it contains __STDARGS__ which is reasonable. But it's missing support for gcc. I'll provide a patch soon.

I can share the unpleasantly unpublished NDK 3.2 R3 version of the "compiler-specific.h" file with you, so that you might put it to good use and wouldn't need to patch individual parts. As is common with all the recent published (and accidentally unpublished) changes, they are documented in context.

I gcc-ified that compiler-specific.h file now - feel free to look.

But more important: if there is __STDARGS__ then it must be applied to all clib/*_protos.h.

Hold on! Why? Only a small subset of functions in <clib/alib_protos.h>, <clib/debug_protos.h> and <clib/ddebug_protos.h> require parameter passing on the stack. As for the rest, the pragmas/pragma/inline header files for legacy and contemporary 'C' compilers function safely without them.

why?

  1. if inlines are used then there is no need for the prototypes
  2. if no inlines are used the functions are provided in link library with __stdargs

=> it does not hurt to __stdarg all prototypes - and it's easier to implement for me^^

Also supporting objective-C would be nice - this needs a patch to intuition/classes.h:

+#ifdef __OBJC__
+  IntuitionClass;
+#else
+  Class;
+#endif 

Since Class is already used in objective-C.

I'd say that Objective 'C' had its time in the sun (twice) and does not represent a significant portion of software developed for the Amiga. As such there's a greater risk of adding more entropy to the Amiga header files than they deserve without a tangible benefit. Now synchronizing the contents of the 68k assembly language header files with the 'C' language header files (which always used to be done by hand) would have a much more beneficial impact 😄

there is at least one objective-c user alive: @Midar

but anyway: I can live with patching the hell out of the ndk^^

... For these reasons I strongly suggest caution when changing the header files, even if it is for the better...

hehe - the community sometimes tells me that I did something insane again^^ no fun no risk

obarthel commented 3 years ago

Also there's something wrong with timer_lib.sfd, as the second line is * "timer.device"

That's how the 3.1 SFD files rolled back in 1994, I'm afraid. The SFD files were built from the already existing FD files and augmented by fancy control instructions such as "==include". And in an FD file the "*" at the beginning of a line introduces a comment. This is still valid, although it arguably serves no good purpose. It's just a left-over. As I mentioned, we had to start all over again for the NDK 3.2 from the state of the 3.1 header and SFD files, and some of these files are still closer to 3.1 than to 3.9 (or 4.x).

OK, that's interesting. If I run fd2pragma on this file it doesn't like AddTime/CmpTime/SubTime - complaining about brackets. I thought the * line might have been upsetting it but clearly there's something else wrong there (or a bug in fd2pragma).

I have to ask: which of the many fd2pragma tools are you using?

If I use the NDK's inlines and try to build NetSurf I get lots of undefined references. I've just rebuilt the inlines myself and (aside from a couple of errors I've not fixed yet) it seems to build OK - so there's something different/wrong with the included inlines.

Which warnings are you seeing? Could you provide a sample?

Sure. Loads of undefined references at the linker stage. The compiler works fine otherwise - I tested a very minimal DisplayBeep() program and it built that without complaint. It could be the makefile is doing something weird, but as it works with regenerated inlines that seems unlikely 😕

[..]

Whoah! That's 895 error messages involving 253 operating system functions (exec, dos, intuition, graphics, timer, iffparse, locale, diskfont, amigaguide, icon, datatypes, ReAction classes). Your code seems to have been built without working inline header files, and maybe with just the proto/clib header files providing the function prototypes (assuming that you didn't get an überabzählbar number of warnings during compilation).

chris-y commented 3 years ago

Also there's something wrong with timer_lib.sfd, as the second line is * "timer.device"

That's how the 3.1 SFD files rolled back in 1994, I'm afraid. The SFD files were built from the already existing FD files and augmented by fancy control instructions such as "==include". And in an FD file the "*" at the beginning of a line introduces a comment. This is still valid, although it arguably serves no good purpose. It's just a left-over. As I mentioned, we had to start all over again for the NDK 3.2 from the state of the 3.1 header and SFD files, and some of these files are still closer to 3.1 than to 3.9 (or 4.x).

OK, that's interesting. If I run fd2pragma on this file it doesn't like AddTime/CmpTime/SubTime - complaining about brackets. I thought the * line might have been upsetting it but clearly there's something else wrong there (or a bug in fd2pragma).

I have to ask: which of the many fd2pragma tools are you using?

Good question. Appears to be fd2pragma 2.197a (10/02/2017).

If I use the NDK's inlines and try to build NetSurf I get lots of undefined references. I've just rebuilt the inlines myself and (aside from a couple of errors I've not fixed yet) it seems to build OK - so there's something different/wrong with the included inlines.

Which warnings are you seeing? Could you provide a sample?

Sure. Loads of undefined references at the linker stage. The compiler works fine otherwise - I tested a very minimal DisplayBeep() program and it built that without complaint. It could be the makefile is doing something weird, but as it works with regenerated inlines that seems unlikely confused [..]

Whoah! That's 895 error messages involving 253 operating system functions (exec, dos, intuition, graphics, timer, iffparse, locale, diskfont, amigaguide, icon, datatypes, ReAction classes). Your code seems to have been built without working inline header files, and maybe with just the proto/clib header files providing the function prototypes (assuming that you didn't get an überabzählbar number of warnings during compilation).

No unusual warnings - looks as it should until it tries to link. I guess it must be taking the wrong route through the proto header? Not sure if this helps, but these are the preprocessor defines:

#define __DBL_MIN_EXP__ (-1021)
#define __FLT_MIN__ 1.17549435e-38F
#define __CHAR_BIT__ 8
#define __mc68020__ 1
#define __WCHAR_MAX__ 4294967295U
#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
#define __FLT_EVAL_METHOD__ 0
#define __chip __attribute__((__chip__))
#define __DBL_MIN_10_EXP__ (-307)
#define __FINITE_MATH_ONLY__ 0
#define __GNUC_PATCHLEVEL__ 6
#define __ixemul__ 1
#define __SHRT_MAX__ 32767
#define __LDBL_MAX__ 1.7976931348623157e+308L
#define __saveds __attribute__((__saveds__))
#define __LDBL_MAX_EXP__ 1024
#define __SCHAR_MAX__ 127
#define __USER_LABEL_PREFIX__ _
#define __STDC_HOSTED__ 1
#define __LDBL_HAS_INFINITY__ 1
#define __DBL_DIG__ 15
#define __FLT_EPSILON__ 1.19209290e-7F
#define __amiga__ 1
#define __LDBL_MIN__ 2.2250738585072014e-308L
#define __DECIMAL_DIG__ 17
#define __stackext __attribute__((__stackext__))
#define __LDBL_HAS_QUIET_NAN__ 1
#define __GNUC__ 3
#define __amigaos 1
#define MCH_AMIGA 1
#define __DBL_MAX__ 1.7976931348623157e+308
#define __DBL_HAS_INFINITY__ 1
#define __m68k__ 1
#define __mc68000__ 1
#define ixemul 1
#define __USING_SJLJ_EXCEPTIONS__ 1
#define __DBL_MAX_EXP__ 1024
#define __amigaos__ 1
#define __LONG_LONG_MAX__ 9223372036854775807LL
#define amigaos 1
#define __GXX_ABI_VERSION 1002
#define __FLT_MIN_EXP__ (-125)
#define __DBL_MIN__ 2.2250738585072014e-308
#define __DBL_HAS_QUIET_NAN__ 1
#define __mc68000 1
#define __mc68020 1
#define __REGISTER_PREFIX__ 
#define __regargs __attribute__((__regparm__))
#define __NO_INLINE__ 1
#define __FLT_MANT_DIG__ 24
#define __VERSION__ "3.4.6"
#define amiga 1
#define AMIGA 1
#define mc68000 1
#define __MCH_AMIGA__ 1
#define mc68020 1
#define __SIZE_TYPE__ long unsigned int
#define __AMIGA__ 1
#define __FLT_RADIX__ 2
#define __LDBL_EPSILON__ 2.2204460492503131e-16L
#define __stdargs __attribute__((__stkparm__))
#define __FLT_HAS_QUIET_NAN__ 1
#define __FLT_MAX_10_EXP__ 38
#define __LONG_MAX__ 2147483647L
#define __FLT_HAS_INFINITY__ 1
#define __LDBL_MANT_DIG__ 53
#define __WCHAR_TYPE__ unsigned int
#define __FLT_DIG__ 6
#define __INT_MAX__ 2147483647
#define __FLT_MAX_EXP__ 128
#define __DBL_MANT_DIG__ 53
#define __interrupt __attribute__((__interrupt__))
#define __WINT_TYPE__ unsigned int
#define __LDBL_MIN_EXP__ (-1021)
#define __LDBL_MAX_10_EXP__ 308
#define __DBL_EPSILON__ 2.2204460492503131e-16
#define __MCH_AMIGA 1
#define __ixemul 1
#define __FLT_DENORM_MIN__ 1.40129846e-45F
#define __FLT_MAX__ 3.40282347e+38F
#define __FLT_MIN_10_EXP__ (-37)
#define __GNUC_MINOR__ 4
#define __DBL_MAX_10_EXP__ 308
#define __LDBL_DENORM_MIN__ 4.9406564584124654e-324L
#define __aligned __attribute__((__aligned__(4)))
#define __PTRDIFF_TYPE__ long int
#define __amiga 1
#define __AMIGA 1
#define __LDBL_MIN_10_EXP__ (-307)
#define __LDBL_DIG__ 15
chris-y commented 3 years ago

Hmm, if I use the full command line I'm missing at least mc68000, AMIGA, amiga:

/opt/netsurf/m68k-unknown-amigaos/cross/bin/m68k-unknown-amigaos-gcc -W -Wall -Wundef -Wpointer-arith -Wcast-align -Wwrite-strings -Wmissing-declarations -Wuninitialized -Wno-unused-parameter -Wredundant-decls -Wno-cast-align -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -I. -Iinclude -Ibuild/Linux-amigaos3 -Ifrontends -Icontent/handlers  -fomit-frame-pointer -DWITH_JPEG -UWITH_PDF_EXPORT -ULIBICONV_PLUG  -I/opt/netsurf/m68k-unknown-amigaos/env/include -I/opt/netsurf/m68k-unknown-amigaos/env/include -I/opt/netsurf/m68k-unknown-amigaos/env/include -DCURL_STATICLIB -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_CURL -DUTF8PROC_EXPORTS -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_UTF8PROC -I/opt/netsurf/m68k-unknown-amigaos/env/include/libpng16 -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_PNG -I/opt/netsurf/m68k-unknown-amigaos/env/include/ -DWITH_BMP -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_GIF -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_NS_SVG -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_NSPSL -I/opt/netsurf/m68k-unknown-amigaos/env/include -DWITH_NSLOG -DNETSURF_UA_FORMAT_STRING=\""NetSurf/%d.%d (%s)"\" -DNETSURF_HOMEPAGE=\""about:welcome"\" -DNETSURF_LOG_LEVEL=VERBOSE  -DNETSURF_BUILTIN_LOG_FILTER=\""(level:WARNING || cat:jserrors)"\" -DNETSURF_BUILTIN_VERBOSE_FILTER=\""(level:VERBOSE || cat:jserrors)"\" -DSTMTEXPR=1 -std=c99 -Dnsamiga -O2 -DPATH_MAX=1024 -D__m68k__ -m68020 -DWITH_AMIGA_ICON -DWITH_AMIGA_DATATYPES -DWITH_AMISSL -DWITH_OPENSSL -D__NO_NET_API -D__NO_NETINCLUDE_ERRNO -I/opt/netsurf/m68k-unknown-amigaos/env/netinclude -I/opt/netsurf/m68k-unknown-amigaos/env/include -I/opt/netsurf/m68k-unknown-amigaos/env/include -DDUK_OPT_HAVE_CUSTOM_H  -MMD -MP -dM -E - </dev/null

#define __DBL_MIN_EXP__ (-1021)
#define WITH_AMIGA_ICON 1
#define __FLT_MIN__ 1.17549435e-38F
#define __NO_NETINCLUDE_ERRNO 1
#define __NO_NET_API 1
#define __CHAR_BIT__ 8
#define __mc68020__ 1
#define WITH_OPENSSL 1
#define __WCHAR_MAX__ 4294967295U
#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
#define __FLT_EVAL_METHOD__ 0
#define __chip __attribute__((__chip__))
#define __DBL_MIN_10_EXP__ (-307)
#define __FINITE_MATH_ONLY__ 0
#define __GNUC_PATCHLEVEL__ 6
#define __ixemul__ 1
#define PATH_MAX 1024
#define __SHRT_MAX__ 32767
#define __LDBL_MAX__ 1.7976931348623157e+308L
#define NETSURF_BUILTIN_LOG_FILTER "(level:WARNING || cat:jserrors)"
#define __saveds __attribute__((__saveds__))
#define __OPTIMIZE__ 1
#define __LDBL_MAX_EXP__ 1024
#define __SCHAR_MAX__ 127
#define __USER_LABEL_PREFIX__ _
#define __STDC_HOSTED__ 1
#define __LDBL_HAS_INFINITY__ 1
#define STMTEXPR 1
#define __DBL_DIG__ 15
#define __FLT_EPSILON__ 1.19209290e-7F
#define __amiga__ 1
#define nsamiga 1
#define __LDBL_MIN__ 2.2250738585072014e-308L
#define WITH_AMISSL 1
#define WITH_GIF 1
#define __DECIMAL_DIG__ 17
#define __stackext __attribute__((__stackext__))
#define __LDBL_HAS_QUIET_NAN__ 1
#define __GNUC__ 3
#define __amigaos 1
#define __DBL_MAX__ 1.7976931348623157e+308
#define __DBL_HAS_INFINITY__ 1
#define __m68k__ 1
#define __mc68000__ 1
#define __STRICT_ANSI__ 1
#define __USING_SJLJ_EXCEPTIONS__ 1
#define __DBL_MAX_EXP__ 1024
#define __amigaos__ 1
#define __LONG_LONG_MAX__ 9223372036854775807LL
#define NETSURF_LOG_LEVEL VERBOSE
#define UTF8PROC_EXPORTS 1
#define __GXX_ABI_VERSION 1002
#define __FLT_MIN_EXP__ (-125)
#define __DBL_MIN__ 2.2250738585072014e-308
#define __DBL_HAS_QUIET_NAN__ 1
#define __mc68000 1
#define __mc68020 1
#define __REGISTER_PREFIX__ 
#define __regargs __attribute__((__regparm__))
#define __FLT_MANT_DIG__ 24
#define __VERSION__ "3.4.6"
#define NETSURF_HOMEPAGE "about:welcome"
#define __MCH_AMIGA__ 1
#define __SIZE_TYPE__ long unsigned int
#define __AMIGA__ 1
#define __FLT_RADIX__ 2
#define __LDBL_EPSILON__ 2.2204460492503131e-16L
#define __stdargs __attribute__((__stkparm__))
#define WITH_UTF8PROC 1
#define __FLT_HAS_QUIET_NAN__ 1
#define __FLT_MAX_10_EXP__ 38
#define __LONG_MAX__ 2147483647L
#define __FLT_HAS_INFINITY__ 1
#define __STDC_VERSION__ 199901L
#define __LDBL_MANT_DIG__ 53
#define __WCHAR_TYPE__ unsigned int
#define WITH_PNG 1
#define CURL_STATICLIB 1
#define __FLT_DIG__ 6
#define __INT_MAX__ 2147483647
#define __FLT_MAX_EXP__ 128
#define __DBL_MANT_DIG__ 53
#define __interrupt __attribute__((__interrupt__))
#define __WINT_TYPE__ unsigned int
#define __LDBL_MIN_EXP__ (-1021)
#define DUK_OPT_HAVE_CUSTOM_H 1
#define __LDBL_MAX_10_EXP__ 308
#define __DBL_EPSILON__ 2.2204460492503131e-16
#define __MCH_AMIGA 1
#define __ixemul 1
#define WITH_NSLOG 1
#define WITH_CURL 1
#define __FLT_DENORM_MIN__ 1.40129846e-45F
#define WITH_AMIGA_DATATYPES 1
#define __FLT_MAX__ 3.40282347e+38F
#define WITH_BMP 1
#define __FLT_MIN_10_EXP__ (-37)
#define NETSURF_BUILTIN_VERBOSE_FILTER "(level:VERBOSE || cat:jserrors)"
#define __GNUC_MINOR__ 4
#define WITH_NS_SVG 1
#define __DBL_MAX_10_EXP__ 308
#define __LDBL_DENORM_MIN__ 4.9406564584124654e-324L
#define __aligned __attribute__((__aligned__(4)))
#define NETSURF_UA_FORMAT_STRING "NetSurf/%d.%d (%s)"
#define __PTRDIFF_TYPE__ long int
#define __amiga 1
#define WITH_JPEG 1
#define __AMIGA 1
#define __LDBL_MIN_10_EXP__ (-307)
#define __LDBL_DIG__ 15
#define WITH_NSPSL 1
chris-y commented 3 years ago

If I manually define mc68000 it throws errors about NewObject:

frontends/amiga/arexx.c: In function `ami_arexx_init':
frontends/amiga/arexx.c:144: error: `NewObject' undeclared (first use in this function)
frontends/amiga/arexx.c:144: error: (Each undeclared identifier is reported only once
frontends/amiga/arexx.c:144: error: for each function it appears in.)
frontends/amiga/arexx.c:144: error: syntax error at end of input

This is what it is complaining about:

                arexx_obj = ARexxObj,
                        AREXX_HostName,"NETSURF",
                        AREXX_Commands,Commands,
                        AREXX_NoSlot,FALSE,
                        AREXX_ReplyHook,NULL,
                        AREXX_DefExtension,"nsrx",
                        End;

ArrexxObj expands to NewObject( similar to the macros in reaction_macros.h If I change End to TAG_DONE) or put brackets around the whole thing it seems quite happy. Any idea what is causing that? I have quite a few of these macros without any brackets in them.

obarthel commented 3 years ago

[..] ArrexxObj expands to NewObject( similar to the macros in reaction_macros.h If I change End to TAG_DONE) or put brackets around the whole thing it seems quite happy. Any idea what is causing that? I have quite a few of these macros without any brackets in them.

I have a hunch that the inline header files may not work correctly. For reference, the "/opt/amiga/m68k-amigaos/ndk-include/proto/intuition.h" file will not make use of the GCC inline interface header files if the _NO_INLINE preprocessor symbol is defined. In the list of preprocessor #define instructions you posted there 's a #define __NO_INLINE__ 1 which looks out of place. No idea which impact it has, though, except for the libnix, etc. header files for which it inhibits the use of fancier local stdio functions.

obarthel commented 3 years ago

I don't think the timer stuff there needs changing. However, it would be nice if the new TimeVal and new TimeRequest were the same as on OS4. I actually think the 3.2 names are better, so this probably needs changing in the OS4 SDK.

#ifndef _TIMEVAL_DEFINED
#define _TIMEVAL_DEFINED
#endif

maybe at least that #ifdef part? Then it doesn't matter which one is included first...

Yes, we need both a guard and a symbol to test for to figure out what was defined. I'll see what I can do about it.

Incidentally, declaring the struct timeval as a union with preprocessor symbols to provide compile-time compatibility, can have side-effects by way of the #defines.

It makes sense to retain it for the sake of building code which can be run on Kickstart 1.1-3.x without binary changes to account for 3.x. But even then it would have to be documented and, if possible, made safe to use. Without those decorations I question its usefulness because its side-effects (e.g. conflicts with the inline header files) are unpredictable.

well - better support for older kickstarts would require adding something like

#if (__KICKSTART_VERSION >= 46)
...
#endif

right now the comment is evaluated and the files get truncated to provide only old stuff to 1.3

Hang on, I was wrong about that GetOutlinePen() macro (again!). That macro was added by Chris Green on February 24, 1992 to "graphics/gfxmacros.h" (for graphics.library V39, i.e. Kickstart/Workbench 3.0). The macro never existed before in the V38 or V37 header files. At this time the graphics.library function which we know today as GetOutlinePen() was still called GetOPen(). By April 6, 1992 the GetOPen() function was renamed to GetOutlinePen(), which renders the GetOutlinePen() macro in "graphics/gfxmacros.h" redundant.

The proper approach to solving the clash between the GetOutlinePen() macro in "graphics/gfxmacros.h" and the inline header file definition, etc. is to remove it from "graphics/gfxmacros.h". That macro is really just a left-over workaround which was obsoleted in 1992 already.

it contains __STDARGS__ which is reasonable. But it's missing support for gcc. I'll provide a patch soon.

I can share the unpleasantly unpublished NDK 3.2 R3 version of the "compiler-specific.h" file with you, so that you might put it to good use and wouldn't need to patch individual parts. As is common with all the recent published (and accidentally unpublished) changes, they are documented in context.

I gcc-ified that compiler-specific.h file now - feel free to look.

Is this already part of the project? The version I checked out yesterday didn't have it yet.

But more important: if there is __STDARGS__ then it must be applied to all clib/*_protos.h.

Hold on! Why? Only a small subset of functions in <clib/alib_protos.h>, <clib/debug_protos.h> and <clib/ddebug_protos.h> require parameter passing on the stack. As for the rest, the pragmas/pragma/inline header files for legacy and contemporary 'C' compilers function safely without them.

why?

  1. if inlines are used then there is no need for the prototypes
  2. if no inlines are used the functions are provided in link library with __stdargs

=> it does not hurt to __stdarg all prototypes - and it's easier to implement for me^^

Well, there are some side-effects which I for one would like to avoid.

If you don't have the inline header files for GCC then (unless I missed something here) then the function prototypes in <clib/#?_protos.h> would default to stack-based parameter passing anyway. No need to over-egg the pudding. If the developer is brave enough to link against amiga.lib and pull in a load of stub functions it might work but, arguably, this is at best a fall-back and the use of inline header files is far superior in performance and code quality. I also suspect that the fine points of what get compiled and linked are the kind of knowledge which a modern Amiga developer should not need to know in order to develop and debug software. In 1987/1988 perhaps you had to know that, but today? ☹️

What weighs more heavily in my opinion is that there will invariably be developers who copy the header files you augmented with the "standard parameter passing" attributes and which then cause all kinds of unexpected behaviour when used on legacy native Amiga 'C' compilers. There's nothing like driving away prospective Amiga 'C' developers like making it very easy to see their humble first steps erupt in a firework of warning and error messages of which only the first few lines make a little sense. I've been there, and I would not want anybody to go through the same kind of hellish education I did in learning why a 'C' compiler upchucks the source code (because: it skips tokens and tries to synchronize again so that it may process the code beyond the bit which confused it: thereby confusing the developer).

Long story short: please do not augment the interface header files. Somebody is bound to have a very hard time getting her or his feet back. If you have to add something which is beneficial for the GCC compiler you maintain, please add guards and brief documentation (as in "Only use this with the GCC compiler which you can find on https://github.com/bebbo") so that it doesn't cause surprises for other compilers.

Also supporting objective-C would be nice - this needs a patch to intuition/classes.h:

+#ifdef __OBJC__
+  IntuitionClass;
+#else
+  Class;
+#endif 

Since Class is already used in objective-C.

I'd say that Objective 'C' had its time in the sun (twice) and does not represent a significant portion of software developed for the Amiga. As such there's a greater risk of adding more entropy to the Amiga header files than they deserve without a tangible benefit. Now synchronizing the contents of the 68k assembly language header files with the 'C' language header files (which always used to be done by hand) would have a much more beneficial impact 😄

there is at least one objective-c user alive: @Midar

Then I would expect he knows how to deal with the matter at hand. Harsh as it may sound, the operating system header files are primarily supposed to be useful for everybody and not specifically for somebody. If a developer needs something particular in order to accomplish her or his goals, making local changes to header files may be the right way to go. If there is a greater need to cater for such changes, strange people such as us AmigaOS 68k developers can incorporate these, document then and release them. Well, that's the aspiration at least.

but anyway: I can live with patching the hell out of the ndk^^

... For these reasons I strongly suggest caution when changing the header files, even if it is for the better...

hehe - the community sometimes tells me that I did something insane again^^ no fun no risk

There's fun and there's cleaning up after side-effects. Developers will learn from what you produce. They may learn something which does not help them in the long run (it happened to me when I started out with 'C' programming in the 1980'ies on the Amiga) and which they struggle to make sense of (it happened to me, too). You may see your work here as a side-project which might not have a strong serious side to it. It appears to me that you are indeed doing serious work here and serious people are relying upon it. That is sure not to end with everybody only having fun. Sorry to say so: it's less fun to be more cautious than to explore how operating system header files and documentation can be mended and improved. One person's improvement may be another person's two hours spent on resolving an issue which could not be understood in context (it happened to me, etc., etc.).

Midar commented 3 years ago

there is at least one objective-c user alive: @Midar

Thanks for looping me in @bebbo!

Yep, and I would be very happy if we could keep the ObjC support working, for it is an officially supported platform in ObjFW: https://objfw.nil.im/file?name=PLATFORMS.md&ci=tip :grin:

Then I would expect he knows how to deal with the matter at hand.

I know very well how to work around it and had a workaround at some point, until bebbo changed the header. I don't remember why exactly, but I think bebbo thought it was wrong if a compiler ships with a header it chokes on ;).

Harsh as it may sound, the operating system header files are primarily supposed to be useful for everybody and not specifically for somebody. If a developer needs something particular in order to accomplish her or his goals, making local changes to header files may be the right way to go.

OMG, no. Just no. If you download some software and the build instructions tell you "Edit your system's header file", you run, you run away as fast as you can, screaming. Nope nope nope nope nope. The right way is to redefine Class to something else before including, but it is never to tell a user to edit system headers. Never ever. That just breaks the next software that user is trying to compile. Headers are immutable once installed, period ;).

If there is a greater need to cater for such changes, strange people such as us AmigaOS 68k developers can incorporate these, document then and release them. Well, that's the aspiration at least.

I think it's fine to keep it as a patch in @bebbo's GCC, as it's probably the only ObjC compiler for the Amiga ;)

acf commented 3 years ago

@Midar I realise this is off topic a bit, but ObjFW aside, does this mean I could use amiga-gcc right now to build an Amiga 3.x application with some ObjC code wrapping Intuition and GadTools?

Midar commented 3 years ago

Yes. You'd probably need a better runtime than the one that comes with GCC, but e.g. ObjFW's objfwrt1.library would work.

acf commented 3 years ago

Well this is pretty great news :-). I've been struggling to think without Objects as I've been using them so long, and I really didnt want to go near C++. adds it to the list of shiny things to investigate

rosneru commented 2 years ago

With the old 3.9 NDK I could create an array of NewMenu structs like that:

struct NewMenu newMenu[] = 
{
  { NM_TITLE,   "Project",     0 , 0, 0, 0 },
  {   NM_ITEM,    "Open...",  "o", 0, 0, pCmdOpenFilesWindow },
  {   NM_ITEM,    "About...",  0 , 0, 0, pCmdAboutRequester },
  // ...
};

With NDK 3.2 now I get some errors:

1) invalid conversion from 'UBYTE {aka unsigned char}' to 'char' [-fpermissive] 2) argument of type "UBYTE " is incompatible with parameter of type "char *"

To get it build now I must change the code:

struct NewMenu newMenu[] = 
{
  { NM_TITLE,   (const UBYTE*)"Project",                   0 , 0, 0, 0 },
  {   NM_ITEM,    (const UBYTE*)"Open...",  (const UBYTE*)"o", 0, 0, pCmdOpenFilesWindow },
  {   NM_ITEM,    (const UBYTE*)"About...",                0 , 0, 0, pCmdAboutRequester },
  // ...
};

Now it builds but it also is a bit ugly. Is there a workaround to get it done without those casts and using the 3.2 NDK?

bebbo commented 2 years ago

you have to use:

#define NO_INLINE_STDARG
#include <proto/muimaster.h>

err... wrong issue^^

I once changed the type of STRPTR to char * but @obarthel insisted on changing that back - see #234

bebbo commented 2 years ago
#define __cplusplus
#include <exec/types.H>
#undef __cplusplus

as first include might work

rosneru commented 2 years ago

I'll try it, thank you.

obarthel commented 2 years ago

With the old 3.9 NDK I could create an array of NewMenu structs like that:

[..] With NDK 3.2 now I get some errors:

  1. invalid conversion from 'UBYTE {aka unsigned char}' to 'char*' [-fpermissive]
  2. argument of type "UBYTE " is incompatible with parameter of type "char "

To get it build now I must change the code:

struct NewMenu newMenu[] = 
{
  { NM_TITLE,   (const UBYTE*)"Project",                   0 , 0, 0, 0 },
  {   NM_ITEM,    (const UBYTE*)"Open...",  (const UBYTE*)"o", 0, 0, pCmdOpenFilesWindow },
  {   NM_ITEM,    (const UBYTE*)"About...",                0 , 0, 0, pCmdAboutRequester },
  // ...
};

Now it builds but it also is a bit ugly. Is there a workaround to get it done without those casts and using the 3.2 NDK?

First off, I am sorry for dropping you into this mess. The NDK 3.2 header file changes were about cleaning up the data structure and type definitions to be more consistent. We made several passes over the header files and there are still spots left (in <libraries/gadtools.h>) where we missed left-over const UBYTE * declarations which should have been CONST_STRPTR.

Of the many changes made, the string pointer type changes are mostly for the benefit of the operating system and type checking tests. As a developer you gain nothing from these pointers generating warnings when you declare and statically initialize struct NewMenu or struct NewGadget data. A pointer to a constant string is assigned to a constant string, with the only difference being that the string constant is a signed char *. The toll you have to pay for that kind of declarations is to use pointer type casts which are ugly and here only serve to keep these warnings from drowning out the more important and relevant warnings.

You could add casts to each declaration, e.g. use (CONST_STRPTR). Ugly and not really helpful...

You could tweak the declaration of STRPTR or CONST_STRPTR in <exec/types.h> but this might just expose you to more trouble because some of the code you are depending upon might work differently if you change the signedness of the characters referenced by the respective pointer types. In AmigaOS 3.2 we had real production code which was getting tripped up by such a subtle change and it was exceptionally hard to find & fix. My advice would be to steer clear of this, because while the warning will go away any code vulnerable to changes in the string pointer signedness will be very hard to spot and will make trouble for you.

Tweaking the declaration of STRPTR or CONST_STRPTR in <exec/types.h> will not help you with these gadtools.library header files, though, because the current NDK 3.2 release still has these string pointers in struct NewMenu or struct NewGadget set to UBYTE *.

Unless GCC has changed more than I managed to keep track of over the years, changing the warning options should silence these assignment warnings: -Wno-pointer-sign would accomplish that.

Please give it a try. If it helps in reducing the number of unnecessary warnings for code which you know is not at risk, in favour of making you pick up warnings for likely defects, it might be worth trying.