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

[Steven R. Brandt]

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

[Steven R. Brandt]

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).

[Erik Schnetter]

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?

[Steven R. Brandt]

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.

[Erik Schnetter]

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

[anonymous]

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.

[Erik Schnetter]

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.

[Erik Schnetter]

Please modify lib/make/force-rebuild as well together with these changes.

[Steven R. Brandt]

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.

[Erik Schnetter]

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.

[Roland Haas]

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.

[Steven R. Brandt]

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.

[Roland Haas]

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

[Steven R. Brandt]

Commit revision is 4839