heasm66 / mdlzork

Different versions of original mainframe Zork reconstructed and patched to run under Confusion.
15 stars 6 forks source link

macros.cpp seems to be missing #include <time.h> #31

Closed eriktorbjorn closed 1 year ago

eriktorbjorn commented 1 year ago

When I compile confusion_patched I get a number of errors complaining about various time-related things not being declared:

macros.cpp: In function ‘mdl_value_t* mdl_builtin_eval_gettimedate(mdl_value_t*, mdl_value_t*)’:
macros.cpp:7910:15: error: aggregate ‘tm broketime’ has incomplete type and cannot be defined
 7910 |     struct tm broketime;
      |               ^~~~~~~~~
macros.cpp:7940:9: error: ‘localtime_r’ was not declared in this scope; did you mean ‘locale_t’?
 7940 |         localtime_r(&tv.tv_sec, &broketime);
      |         ^~~~~~~~~~~
      |         locale_t
macros.cpp:7944:9: error: ‘gmtime_r’ was not declared in this scope; did you mean ‘time_t’?
 7944 |         gmtime_r(&tv.tv_sec, &broketime);
      |         ^~~~~~~~
      |         time_t
make: *** [<builtin>: macros.o] Error 1

Adding #include <time.h> seems to fix this.

heasm66 commented 1 year ago

Thanks!

I don't get that error on my platforms but I havn't updated gcc there yet. What platform are you on?

I've added the line you suggested to macros.cpp as it doesn't do any damage. Could be that it was included lower down in the chain before and now it isn't?

eriktorbjorn commented 1 year ago

I don't get that error on my platforms but I havn't updated gcc there yet. What platform are you on?

Linux, specifically the unstable version of Debian. It's using GCC 12.2.0, and glibc 2.36.

But I did try compiling it on Ubuntu, whatever version I got running in Windows 10 at work, and I didn't get any errors there. So I don't know what's up with that.

larsbrinkhoff commented 1 year ago

I don't get the error on my machine, which runs Ubuntu 22.04. By compiling macros.cpp with the -E option, I can examine the preprocessor output to see where time.h is included. It goes like this: macros.cpp includes "macros.hpp", which includes \ → <bits/basic_string.h> → <ext/atomicity.h> → <bits/gthr.h> → <bits/gthr-default.h> → . Maybe it's not the same across all system versions, but the gist of it that some include file has a lapse of hygiene and is including a header file the user didn't request. This is generally frowned upon, or is even in error.

eriktorbjorn commented 1 year ago

Well, the inclusion of ext/atomicity.h seems to have been removed fairly recently, as in less than a year ago. Perhaps that's why?

https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=f3e22baec0290c23654e99bf184153765944f4aa