fix bugzilla Issue 24397 ImportC: support function-like macros

[WalterBright]

Finally! Now macros like:

#define ADD(a, b) a + b

are rewritten as:

auto ADD(__MP1, __MP2)(__MP1 a, __MP2 b) { return a + b; }

It's a miracle!

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	24397	enhancement	Support C preprocessor function-like macros

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16199"

[WalterBright]

It's very frustrating when all the test suite Ubuntus fail with a seg fault, yet everything works fine on my local Ubuntu box.

[WalterBright]

I'm dead in the water on this. I cannot repro any failures on my Ubuntu machine. None of the test machines build the compiler with -debug, so the crash dumps are nonexistent or useless. If I add logging printfs, the resulting output is so large the ci machines hang.

[ibuclaw]

I'm dead in the water on this. I cannot repro any failures on my Ubuntu machine. None of the test machines build the compiler with -debug, so the crash dumps are nonexistent or useless. If I add logging printfs, the resulting output is so large the ci machines hang.

Have you tried podman/docker to run it in a 22.04 system environment?

[tim-dlang]

I can reproduce it locally. The failure happens in addVar, because symbols is null for this line:

defineTab[cast(void*)s.ident] = symbols.length;

[ibuclaw]

@WalterBright this seems to be the reduction that confirms my observation.

#define pr16199_trigger(cond,func,args) _Generic (cond, default: func args)
#define pr16199_skipped1(a) (1)   /* This is incorrectly parsed by cparseExpression */
#define pr16199_skipped2(b) (2)   /* This is incorrectly skipped by skipToNextLine */
#define pr16199_ice         0x3   /* segfault */

[WalterBright]

@tim-dlang @ibuclaw I sure appreciate your help with this! I will set about fixing this.

[WalterBright]

@ibuclaw I can confirm that your reduction fails on my machine. Thanks!

[WalterBright]

@tim-dlang you were right about the location of the segfault. It was the result of memory corruption. @ibuclaw your diagnosis was also correct. I was able to fix it. But now a new problem has arisen with the coverage tests. Sigh. I'll look into that tomorrow. At least I can repro it!

[ibuclaw]

@tim-dlang you were right about the location of the segfault. It was the result of memory corruption. @ibuclaw your diagnosis was also correct. I was able to fix it. But now a new problem has arisen with the coverage tests. Sigh. I'll look into that tomorrow. At least I can repro it!

I see two kinds of failures.

1. cov2.d fails - is using \0 as a delimiter doing something strange with expression location information?

2. Freebsd can't do any link/run tests - looks like importC is miscompiling zlib

2024-02-18T08:21:18.7897880Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to adler32 [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7901120Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to crc32 [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7905670Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to compress2 [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7910300Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to inflate [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7924510Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to inflateEnd [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7926850Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to deflateEnd [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7931400Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to deflate [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7934360Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to deflateInit_ [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7946600Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to inflateInit_ [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7958940Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to deflateInit2_ [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7970620Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to inflateInit2_ [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7975020Z ld: error: /Users/runner/work/dmd/dmd/compiler/test/../../../phobos/generated/freebsd/release/64/libphobos2.so: undefined reference to inflateBackInit_ [--no-allow-shlib-undefined] 
2024-02-18T08:21:18.7976780Z cc: error: linker command failed with exit code 1 (use -v to see invocation) 2024-02-18T08:21:18.7977450Z Error: linker exited with status 1

[WalterBright]

cov2.d fails - is using \0 as a delimiter doing something strange with expression location information?

Close to the mark - the bug was already there, the more expansive use of the macros exposed the bug. Fixed now.

Freebsd can't do any link/run tests - looks like importC is miscompiling zlib

I seem to recall this was a pre-existing problem with FreeBSD. Something to do with new system .h files in FreeBSD.

[ibuclaw]

Freebsd can't do any link/run tests - looks like importC is miscompiling zlib

I seem to recall this was a pre-existing problem with FreeBSD. Something to do with new system .h files in FreeBSD.

That's in FreeBSD 14, not the version we're currently testing, and only this PR is triggering the failure.

Pausing to look at one of the functions that are undefined.

/* zconf.h */
#ifndef ZEXTERN
#  define ZEXTERN extern
#endif
#ifndef ZEXPORT
#  define ZEXPORT
#endif
#ifndef FAR
#  define FAR
#endif
#ifndef OF
#  define OF(args)  args
#endif

/* zlib.h */
#define ZLIB_VERSION "1.2.12"

typedef struct z_stream_s {
    /* snipped fields. */
    int opaque;
} z_stream;
typedef z_stream *z_streamp;

ZEXTERN int ZEXPORT inflateInit2_ OF((z_streamp strm, int  windowBits,
                                      const char *version, int stream_size));

#  define inflateInit2(strm, windowBits) \
          inflateInit2_((strm), (windowBits), ZLIB_VERSION, \
                        (int)sizeof(z_stream))

/* inflate.c */
int ZEXPORT inflateInit2_(strm, windowBits, version, stream_size)
z_streamp strm;
int windowBits;
const char *version;
int stream_size;
{
    /* snipped implementation. */
    return 0;
}

Could the importC parser actually be doing something wrong here it wasn't doing before?

[WalterBright]

I'm mystified by this, as this PR is only about D code using #defines, it doesn't have anything to do with how the C preprocessor works. Maybe the FreeBSD preprocessor is different? Unfortunately, my FreeBSD machine is broken at the moment.

[WalterBright]

The OMF DMD is also failing, and I can repro that, so I'll investigate it first.

[ibuclaw]

I'm mystified by this, as this PR is only about D code using #defines, it doesn't have anything to do with how the C preprocessor works. Maybe the FreeBSD preprocessor is different? Unfortunately, my FreeBSD machine is broken at the moment.

I have a VM, and I see that this PR causes the regression.

Command:

../dmd/generated/freebsd/release/64/dmd -c -conf= -I../dmd/druntime/import  -w -de -preview=dip1000 -preview=dtorfields -preview=fieldwise -m64 -fPIC -O -release -P=-Ietc/c/zlib -P=-DHAVE_UNISTD_H -ofgenerated/freebsd/release/64/zlib.o etc/c/zlib/adler32.c etc/c/zlib/compress.c etc/c/zlib/crc32.c etc/c/zlib/deflate.c etc/c/zlib/gzclose.c etc/c/zlib/gzlib.c etc/c/zlib/gzread.c etc/c/zlib/gzwrite.c etc/c/zlib/infback.c etc/c/zlib/inffast.c etc/c/zlib/inflate.c etc/c/zlib/inftrees.c etc/c/zlib/trees.c etc/c/zlib/uncompr.c etc/c/zlib/zutil.c

Before:

$ nm generated/freebsd/release/64/zlib.o | wc -l
     360

After:

$ nm generated/freebsd/release/64/zlib.o | wc -l
       3
$ nm generated/freebsd/release/64/zlib.o 
                 U __start_minfo
                 U __stop_minfo
                 U _d_dso_registry

No C symbols are being emitted. I have an idea of what this might be.

[ibuclaw]

This is the reproducer:

#define M16199C(X,S,M) ({ int __x; })

int pr16199c()
{
    return 0;
}

All functions after the macro definition get erroneously swallowed up, so the result ends up being:

extern (C) auto M16199C(__MP1, __MP2, __MP3)(__MP1 X, __MP2 S, __MP3 M)
{
	return delegate ()
	{
		{
			extern (C) int pr16199c()
			{
				return 0;
			}
			extern (C) int __x = void;
			;
		}
	}
	();
}

[WalterBright]

damn, you're good, @ibuclaw !

[ibuclaw]

damn, you're good, @ibuclaw !

This is the behaviour seen on FreeBSD that causes nothing to be emitted.

#define M16199Da(TYPE,VAR) ((TYPE)(VAR))
#define M16199D(X,S,M) ({ int *__x = (X); M16199Da(S *, __x); })
int pr16199d() { return 0; }

Tweak it a bit more, and we get a segfault in cparse.

#define M16199Ea(TYPE) (TYPE __x;)
#define M16199E(X,S,M) ({ M16199Ea(S *); })

[WalterBright]

Ouch! Anyhow, the OMF problem is fixed.

[WalterBright]

@ibuclaw the problems you identified seem to be fixed now. There were 3 bugs I found and fixed.

[thewilsonator]

generated/windows/debug/64/linkD.exe 
LINK : generated\windows\debug\64\dynamiccast.exe not found or not built by the last incremental link; performing full link
LINK : warning LNK4217: symbol '_D11dynamiccast1C7__ClassZ' defined in 'dynamiccast.obj' is imported by 'dynamiccast.obj' in function '_D4core8lifetime__T12_d_newclassTTC11dynamiccast1CZQBgFNaNbNeZQBc'

[dkorpel]

The translation of macro functions to D functions is not correct in general because of arguments with side-effects. Consider:

#define DOUBLE(x) (x) + (x)

void f(void)
{
    int i = 0;
    DOUBLE(i++);
    assert(i == 2); // fails after D translation
}

Which is a silly example, but I've encountered real C macros that rely on this behavior.

[ntrel]

#define DOUBLE(x) (x) + (x)

Maybe it would be enough to ignore macros that reuse an argument?

[WalterBright]

@dkorpel of course you're right. There are other problems like if the macro body deliberately did not enclose a parameter in ( ), or even the entire expansion in ( ). More or less, if the macros are intended for use as metaprogramming rather than as template-like, there's going to be trouble.

I don't think it is possible for ImportC to promise to replicate C preprocessor semantics. The best we can do is make our best shot. And document things that won't work, like your example.

[WalterBright]

@ntrel it's a good idea, but it also then may annoyingly refuse to compile things that would otherwise work as intended.

[dkorpel]

The best we can do is make our best shot.

Not always. I've mistranslated macros to functions before, and it's annoying to debug. I'd rather have ImportC fail to translate a macro than produce an incorrect result.

[WalterBright]

Doc PR: https://github.com/dlang/dlang.org/pull/3771

[WalterBright]

I understand your concern, and it bothers me, too. I pointed out some other cases in the doc PR. A couple people have pointed out that not supporting function-like macros makes some C packages more or less unusable.

[Herringway]

shouldn't new features like this get a changelog entry? it seems like a lot of new features get introduced without them these days

[dkorpel]

A couple people have pointed out that not supporting function-like macros makes some C packages more or less unusable.

Not unusable, it just requires more manual work. I'm using ImportC to import ffmpeg headers, and manually translate the macros ImportC doesn't parse yet:

import ffmpegh; // .c file with #include <libavcodec/avcodec.h> etc.

auto MKTAG(int a, int b, int c, int d) => (a | (b << 8) | (c << 16) | (cast(uint) d << 24));
enum AVERROR_EOF = -MKTAG( 'E','O','F',' '); ///< End of file

I'm not objecting to this PR by the way. It's nice to reduce the amount of manual translations necessary. I'm only pointing out something to watch out for.