clangupc / clang-upc

Clang UPC Front-End
https://clangupc.github.io/
Other
17 stars 5 forks source link

[CG]UPC should not place upc_*.h headers in the include path for UPCR mode #101

Open bonachea opened 6 years ago

bonachea commented 6 years ago

I'm in the process of developing some feature upgrades for UPCR that will involve meaningful changes to public UPC-level headers such as upc_types.h. These changes will only be "active" for some versions of UPCR, and not others.

In the process of doing this work, I discovered that both for CUPC+UPCR and GUPC+UPCR, the compiler driver (not upcc) adds a directory to the system include path like CUPC:/lib/clang/4.0.1/include/ GUPC:/lib/gcc/x86_64-pc-linux-gnu/8.0.0/include/ that contains a number of C/POSIX headers (eg stddef.h) that are presumably necessary for correct operation. However, the same directory also contains UPC-specific headers (upc_*.h, gasp*.h, pupc.h) that directly collide with header files of the same name in UPCR/upcr_preinclude. Presumably these problematic versions of the headers are only intended for use in non-UPCR modes. Unfortunately, the current design implies that for [CG]UPC+UPCR modes, both "versions" of these headers are present in the application include path, and the version actually substituted when expanding an #include depends on directory ordering of the include path and the details of the argument to #include.

IMHO this is a fragile design. The only reason it has not caused a problem already is because the headers either have compatible content (which is about to change) or in at least one case happen to use the same multi-header-include-protection #ifdef symbol and the UPCR version happens to be pulled into the compilation unit first. A more robust design would place these libupc headers in a dedicated subdirectory that is excluded from the include path entirely when compiling in UPCR mode.

Note that simply updating the contents of these [CG]UPC headers to track changes in UPCR is not a feasible solution, because the new values will be UPCR-version-dependent and will #include a GASNet-level dependence.

CC: @PHHargrove