Code review comment for lp:~epics-core/epics-base/3.15-buildCompilerSpecific

Revision history for this message
Jeff Hill (johill-lanl) wrote :

your changes look good

> Both darwin and linux can build Base using clang, but when I use clang on Linux
> with the gcc header it doesn't hit the #error in compiler/gcc/compilerSpecific.h
> since clang defines __GNUC__ anyway. There needs to be an easy way to make that
> switch (including setting CC and CCC) based on a single CONFIG_SITE setting.

I suspect that you now see that I did implement a clang specific compilerDependent.h.
I did include a #warning in that file because I don't yet have clang installed here
and it hasn't been tested.

I see that you added a commented out compiler class definition (which will cause the
clang specific file to be used) for clang in multiple places. Perhaps we can
conditionally include a clang specific configure file as we do with gcc so that
this can be consolidated? The typical approach is to add a suffix to the
arch - i.e. linux-x86-clang. That probably needs to be done, but it probably
should be considered orthogonal to this merge proposal (I prefer not to
block other layered merges)

> Have you based your atomic branch on this one? I'd like to drop your 12229 merge
> commit, but that would probably mess you up if I did.

For this branch I started with R3.15, and copied stuff over from the atomics branch.
I do have the following code in the atomics branch so it does apparently have 12229 but I
might have manually copied in the change.

+/*
833 + * Deprecation marker if possible
834 + */
835 +#if (__GNUC__ > 2)
836 +# define EPICS_DEPRECATED __attribute__((deprecated))
837 +#else
838 +# define EPICS_DEPRECATED
839 +#endif

« Back to merge proposal