dpp icon indicating copy to clipboard operation
dpp copied to clipboard

some weird C macros replace macro with function calls separated by commas

Open Laeeth opened this issue 5 years ago • 6 comments

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
 * Copyright by The HDF Group.                                               *
 * Copyright by the Board of Trustees of the University of Illinois.         *
 * All rights reserved.                                                      *
 *                                                                           *
 * This file is part of HDF5.  The full HDF5 copyright notice, including     *
 * terms governing use, modification, and redistribution, is contained in    *
 * the COPYING file, which can be found at the root of the source code       *
 * distribution tree, or in https://support.hdfgroup.org/ftp/HDF5/releases.  *
 * If you do not have access to either file, you may request a copy from     *
 * [email protected].                                                        *
 * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

/*
 *  This example writes data to the HDF5 file.
 *  Data conversion is performed during write operation.
 */

#include "hdf5.h"

enum H5FILE_NAME    =    "SDS.h5";
enum DATASETNAME ="IntArray";
enum NX =    5;                      /* dataset dimensions */
enum NY     =6;
enum RANK   =2;


void main (string[] args)
{
    hid_t       file, dataset;         /* file and dataset handles */
    hid_t       datatype, dataspace;   /* handles */
    hsize_t[2]     dimsf;              /* dataset dimensions */
    herr_t      status;
    int[NX][NY] data;
    int         i, j;

    /*
     * Data  and output buffer initialization.
     */
    for(j = 0; j < NX; j++)
	for(i = 0; i < NY; i++)
	    data[j][i] = i + j;
    /*
     * 0 1 2 3 4 5
     * 1 2 3 4 5 6
     * 2 3 4 5 6 7
     * 3 4 5 6 7 8
     * 4 5 6 7 8 9
     */

    /*
     * Create a new file using H5F_ACC_TRUNC access,
     * default file creation properties, and default file
     * access properties.
     */
    file = H5Fcreate(H5FILE_NAME.ptr, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);

    /*
     * Describe the size of the array and create the data space for fixed
     * size dataset.
     */
    dimsf[0] = NX;
    dimsf[1] = NY;
    dataspace = H5Screate_simple(RANK, dimsf, null);

    /*
     * Define datatype for the data in the file.
     * We will store little endian INT numbers.
     */
    datatype = H5Tcopy(H5T_NATIVE_INT);
    status = H5Tset_order(datatype, H5T_ORDER_LE);

    /*
     * Create a new dataset within the file using defined dataspace and
     * datatype and default dataset creation properties.
     */
    dataset = H5Dcreate2(file, DATASETNAME, datatype, dataspace, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);

    /*
     * Write the data to the dataset using default transfer properties.
     */
    status = H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);

    /*
     * Close/release resources.
     */
    H5Sclose(dataspace);
    H5Tclose(datatype);
    H5Dclose(dataset);
    H5Fclose(file);
}

➜ examples d++ foo.dpp

Error: Could not execute `dmd foo.d -offoo`:
foo.d(4327): Error: C style cast illegal, use `cast(hid_t)0`
foo.d(4327): Error: C style cast illegal, use `cast(hid_t)0`

offending line:

    status = H5Dwrite(dataset, (H5open(), H5T_NATIVE_INT_g), (hid_t)0, (hid_t)0, cast(hid_t)0, data);

original line:
    status = H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);

Note also H5S_ALL and H5P_DEFAULT have cast and if possible D-style explicit cast should be added. For comma operator would opening new scope and turning it into ; work?

In this case it isn't that bad - easy to remedy with minor work.

Laeeth avatar Aug 05 '18 21:08 Laeeth

The macros are doing what they're supposed to - unfortunately (or not) the comma operator is deprecated in D. I'll have to think of what to do with that, it's not obvious.

The real issue here is the casts aren't caught by the heuristic that translates macros.

atilaneves avatar Aug 06 '18 10:08 atilaneves

According to the dlang site section on deprecations the solution is to use a lambda, i.e.

// instead of `auto result = foo(), bar();`
auto result = () { foo(); return bar(); }();

Figuring out how to recognise that in a macro is going to be challenging.

atilaneves avatar Aug 06 '18 10:08 atilaneves

Yes - I understand the macros are just doing what poor macros do, but for purpose at hand obviously to be useful one might want to find a way to do something so it just works. And agree that the casts are a second problem but distinct (they both matter).

Presumably you have from clang the mini-AST of the post-preprocessor expression the macro evaluates to? And comma operator is a kind of node there? I could be completely off the mark as I got a bit lost in clang docs.

Laeeth avatar Aug 06 '18 13:08 Laeeth

Presumably you have from clang the mini-AST of the post-preprocessor expression the macro evaluates to?

Very unfortunately, no. Macros aren't C code, they're text substitutions. I have the text of the macro, which is its own separate minilanguage that, in normal C code, will eventually get parsed by the compiler if it's expanded anywhere. Until it's expanded, it's not code, and hence doesn't have an AST.

I considered forcing the expansion of the macro to then parse it, but then what do you expand function-like macros with? A string? An integer literal? And that's just one parameter, if it's three and the second one is a string but not the others, how is the translator generically to know?

atilaneves avatar Aug 06 '18 13:08 atilaneves

A horrible answer that would work I think. The comma operator is still in D syntax right now, but is just banned. So one could find in D code - using libdparse- where the comma operator is introduced as a result of an expanded macro and fix it up as per the extract from the guide you posted. The problem is that it may disappear from grammar at some stage if it ends up being used for tuples etc and then it won't even be recognised but banned.

Anyway it's not worth the trouble for now. Probably mostly the user should just either manually redefine the comma-ridden macro with a different name in an extra include file or translate it to D. Could sometimes be difficult to do so I suppose but such is life.

Laeeth avatar Aug 06 '18 17:08 Laeeth

I'd rather use dmd as a library (now that's a thing) than libdparse. It's good but it's not a proper frontend. That would massively complicate things though. I'm not sure what the best solution is.

atilaneves avatar Aug 07 '18 07:08 atilaneves