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

Revision history for this message
Andrew Johnson (anj) wrote :

Hi Jeff,

On 2011-08-31 Jeff Hill wrote:
> your changes look good

Thanks.

> 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 removed it after testing on Linux, and I added a #error in the gcc-specific
header if it gets used by clang.

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

You're probably right about adding *-x86-clang targets, although there may be
some complications on MacOS/X, see the discussion of gcc_select at
https://trac.macports.org/wiki/UsingTheRightCompiler

I'll talk to Janet about this.

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

Forget that idea of dropping 12229, it's not a problem.

I will try to merge this branch into lp:epics-base/3.15 today.

- Andrew
--
Optimization is the process of taking something that works and
replacing it with something that almost works, but costs less.
-- Roger Needham

« Back to merge proposal