Change per thorn -DTHORN_IS_xxx to a per thorn -I bindings/include/xxx

Issue #768 closed
Steven R. Brandt created an issue

It is not possible for Mojave to display many include files correctly because the way they should be viewed depends on context. E.g. what THORN_IS_ construct should not be greyed out when viewing definethisthorn.h?

The plan is to create many definethisthorn.h files in many directories. Similar things need to be done with other files. A start, something that compiles and splits up only definethisthorn.h and cctk_Functions.h is attached.

Keyword: build
Keyword: system

Comments (16)

  1. Steven R. Brandt reporter
    • removed comment

    This patch can build the entire Einstein Toolkit (except for the LORENE and Meudon thorns. They don't build on my laptop, so I didn't test them). The test suite runs as well with the patch as without (fails one test on my laptop).

  2. Erik Schnetter
    • removed comment

    I have a few questions on this patch. (These are just questions, not comments asking for action.)

    What does this line

    my($header1,$header2,$header3,@header3,$thorn,$nthorns); do? Are the Perl variables $header3 and @header3 independent, or is one the scalar version of the other?

    In these two lines:

    push(@data, "#include \"$thorn/cctk_Arguments.h\""); push(@data, "#include \"cctk_Parameter.h\""); Why is cctk_Arguments.h located in a subdirectory, but cctk_Parameter.h is not?

    Also: Why did you rename cctk_Schedule.h to cctk_ScheduleFunctions.h?

  3. Steven R. Brandt reporter
    • removed comment

    The perl variables $header3 and @header3 are independent. The issue is that CreateHeaderThorns used to create just one file with header3, and now I needed to create N of them. Actually, $header3 is not used and I should just delete it.

    cctk_Parameter.h was not an N-way switch, so I didn't move it.

    cctk_ScheduleFunctions.h used to be an N-way switch function. It was no longer needed, but it was still getting included as a way of getting to the underlying ${thorn}_Schedule.h function (which I assume is what you mean instead of cctk_Schedule.h). My fix was to create a per-thorn cctk_ScheduleFunctions.h that has the same content as the ${thorn}_Schedule.h file. So this is a case where I could generate one less file.

  4. Erik Schnetter
    • removed comment

    This patch fails for me when I build with DEBUG=yes. The errors look like

    /home/eschnett/Cvanilla/src/include/cctk.h(309): error: identifier "CCTK_THORNSTRING" is undefined LINE, FILE, CCTK_THORNSTRING,

    compilation aborted for /home/eschnett/Cvanilla/configs/sim- debug/build/CactusBindings/Variables/ADM.c (code 2)

    This is because the auto-generated file ADM.c includes cctk, which defines static inline functions that use CCTK_THORNSTRING. However, at the including place this is not defined.

    I can see three solutions: (1) require every place that include cctk.h to define CCTK_THORNSTRING (possibly explicitly) (2) augment definethisthorn.h to define CCTK_THORNSTRING to "no thorn active" if no thorn is active (3) put some preprocessor logic into cctk.h to ensure CCTK_THORNSTRING is defined there

  5. anonymous
    • removed comment

    Thanks for the feedback.

    This patch is actually still under development. While it does compile (not in Debug mode) it turns out Mojave was still confused by header files in common areas that included thorn specific files. That case turns out to have essentially the same problem as per thorn defines. You put your curser on "definethisthorn.h" and the browser doesn't know where to take you.

    The newest version of the patch fixes this by (for example) making cctk.h a very small per-thorn include file that includes definethisthorn.h and a new header called cctk_core.h. I plan to attach the newer patch to this ticket when it's been tested a bit more.

  6. Erik Schnetter
    • removed comment

    I looked at the patch, and the main logic seems fine. I obviously didn't check all the fine print. I also haven't tried applying it.

    Large sections of code seem to be commented out. Please remove this code instead.

  7. Steven R. Brandt reporter
    • removed comment

    Replying to [comment:7 eschnett]:

    I looked at the patch, and the main logic seems fine. I obviously didn't check all the fine print. I also haven't tried applying it.

    Large sections of code seem to be commented out. Please remove this code instead.

    Not quite sure what you're referring to. I think the only sections that are commented out are small and relate to the dependency fixer.

  8. Erik Schnetter
    • removed comment

    I believe that the file cctk_core.h reverts some changes to cctk.h that happened in the past weeks. Please check. This concerns handling the "inline" keyword.

    The "commented out" refers to file "lib/sbin/GridFuncStuff.pl", lines 149 to 689. These are 540 new lines introducing code that is commented out.

  9. Roland Haas
    • removed comment

    With the patch (and copying cctk_core.h to src/include) I get compilation errors when compiling ET-trunk (using simfactory and the debian-lenny configuration file for a --debug build). I get:

    COMPILING /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/ScalarBoundary.c In file included from /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/ScalarBoundary.c:25:0: /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/Boundary.h:29:1: error: unknown type name ‘cGH’ which then repeats a number of times.

  10. Steven R. Brandt reporter
    • removed comment

    Replying to [comment:11 rhaas]:

    With the patch (and copying cctk_core.h to src/include) I get compilation errors when compiling ET-trunk (using simfactory and the debian-lenny configuration file for a --debug build). I get: ` COMPILING /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/ScalarBoundary.c In file included from /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/ScalarBoundary.c:25:0: /mnt/data/rhaas/postdoc/gr/ET_trunk/arrangements/CactusBase/Boundary/src/Boundary.h:29:1: error: unknown type name ‘cGH’ ` which then repeats a number of times.

    For some reason the patch doesn't remove src/include/cctk.h. Do that by hand and I think it will work.

  11. Roland Haas
    • removed comment

    Thank you. I should have used the "-E" switch to patch I guess. After removing the file the patched version of ET compiles.

  12. Log in to comment