configure should warn or prohibit mixed-family/mixed-version compilers
UPC++ configure currently silently accepts mixed-family/mixed-version compiler setups, where CC and CXX specify a different compiler family, or a different major version of the same compiler family.
Example:
dirac$ configure CC=gcc CXX=clang++ --without-mpicc && make -j16 && make -j16 check
Fetching https://gasnet-bugs.lbl.gov/nightly/unlisted/GASNet-stable.tar.gz
Unpacking GASNet-stable.tar.gz
System: Linux pcp-d-5 3.10.0-693.1.1.el7.x86_64 #1 SMP Tue Aug 15 08:36:44 CDT 2017 x86_64 x86_64 x86_64 GNU/Linux
LSB Version: :core-4.1-amd64:core-4.1-noarch
Distributor ID: Scientific
Description: Scientific Linux release 7.3 (Nitrogen)
Release: 7.3
Codename: Nitrogen
Date: Sat Jan 15 15:55:46 PST 2022
Current directory:<redacted>
Install directory: /usr/local/upcxx
Configure command: ../upcxx/configure CC=gcc CXX=clang++ --without-mpicc
Configure environment:
GASNET='https://gasnet-bugs.lbl.gov/nightly/unlisted/GASNet-stable.tar.gz'
GASNET_CONFIGURE_ARGS='--disable-kind-cuda-uva'
GMAKE='/usr/local/pkg/gmake/newest/bin/make'
/usr/local/pkg/clang/12.0.0/bin/clang++
clang version 12.0.0
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/pkg/clang/12.0.0/bin
/usr/local/pkg/gcc/11.2.0/bin/gcc
gcc (GCC) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
...
These "mixed" configures are a Very Bad Idea for several reasons:
- We don't test them at all
- Many such combinations will break in subtle or confusing ways (including the combo shown above)
- All C++ compilers we support come bundled with a matching C compiler, so there's no good reason to not use matched compilers.
Based on a quick skim of INSTALL.md, we don't appear to even mention this important potential pitfall (although we talk plenty about the C++ compiler in isolation). However thanks to our defaulting CC=gcc
, this mistake is very easy to accidentally make -- i.e. configure CXX=clang++
is a mixed-family config equivalent to the one shown above!
GASNet configure DOES generate a warning for mixing, ie:
The compiler families of $CC and $CXX do not match:
CC: COMPILER_FAMILY:GNU COMPILER_VERSION:11.2.0
CXX: COMPILER_FAMILY:CLANG COMPILER_VERSION:12.0.0
If this is not what you intended, then set $CXX to select a different C++ compiler.
but this happens after the UPC++ configure step finishes and it's very easy for this to get lost in the middle of the huge library build step.
I propose we apply some subset of the following corrections in rough order of increasing effort, to help users avoid this problem:
- Update INSTALL.md to clarify that mixing compiler families or major versions is NOT supported and highly discouraged
- Add configure-time checks for mixed-family/mixed-version compiler setups and issue a loud warning before the end of UPC++ configure.
- Better yet default to treating a mixed configure as a fatal error and require an override option for "yes, I really want to do this unsupported thing"
- Add a mechanism to grep the GASNet configure warnings and summary from bld/gasnet.debug/config.txt and echo it to the console at the end of libupcxx build. This is probably worth doing for other reasons as well.
Comments (5)
-
-
reporter extracts the requested portion of the outputs from both configure runs.
I was thinking along similar lines, but would additionally include the subsequent "GASNet configuration" summary, since it provides generally useful information. The slightly more "difficult" part would be removing unnecessary duplication in the warnings from gasnet.debug and gasnet.opt (which are often but not always identical), and from the configuration summary (which is basically always the same between debug/opt, so we should only show it once).
-
- changed status to open
Proposed resolution (including items 1, 2 and 3) appears in pull request 396.
I would like to see item 4 (for printing GASNet's configure summary) split off into a distinct issue.
-
I've created a distinct issue 519 for the printing of GASNet's configure summary.
Pull request 396 covers the remaining scope of this issue. -
- changed status to resolved
Configure time version checks
This commit resolves issue 518 by ensuring that "mismatched" C and C++ compilers are rejected by default. The
INSTALL.md
section regarding theconfigure
step is updated to mention the requirement and the (unsupported) means to bypass it.→ <<cset 369ff491df9a>>
- Log in to comment
I agree we can (and so should if time allows) do better for this type of mismatch.
My initial thoughts and opinions:
This seems like something we should consider a "blocker" for the next release. There is no reason NOT to fix the docs.
This actually sounds to me like the most difficult of the four items. However, I am pretty sure use of portable_platform.h should be sufficient to "fingerprint" the two compilers for comparison purposes.
I think this is just an incremental improvement on item 2. So assuming we agree this is the right approach, it should be simple to add.
Don't know why this was listed last by "rough order of increasing effort".
The following (to go in the
all-hook
inbld/Makefile.rules
) may need some "cosmetics", but extracts the requested portion of the outputs from both configure runs.