[cfe-commits] r149083 - in /cfe/trunk: include/clang/Driver/ToolChain.h lib/Driver/Driver.cpp lib/Driver/ToolChain.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/WindowsToolChain.cpp test/Driver/prefixed-tools.c test/Driver/pref

[Douglas Gregor]

On Jan 26, 2012, at 10:14 PM, Joerg Sonnenberger wrote:

> On Thu, Jan 26, 2012 at 08:26:12PM -0800, Chandler Carruth wrote:
>> On Thu, Jan 26, 2012 at 8:22 PM, Chandler Carruth <chandlerc at google.com>wrote:
>> 
>>> On Thu, Jan 26, 2012 at 1:56 PM, Joerg Sonnenberger <joerg at bec.de> wrote:
>>> 
>>>> Author: joerg
>>>> Date: Thu Jan 26 15:56:28 2012
>>>> New Revision: 149083
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=149083&view=rev
>>>> Log:
>>>> Keep track of the original target the user specified before
>>>> normalization. This used to be captured in DefaultTargetTriple and is
>>>> used for the (optional) $triple-$tool lookup for cross-compilation.
>>>> Do this properly by making it an attribute of the toolchain and use it
>>>> in combination with the computed triple as index for the toolchain
>>>> lookup.
>>>> 
>>> 
>>> Why wasn't this (non-trivial or obvious cleanup) patch mailed for code
>>> review? Or generally discussed? Please don't commit w/o review.
>>> 
>>> This is I suspect similar to the approach I had in mind, but I haven't had
>>> time to review it or make sure it is actually sane. Personally, I would
>>> like you to revert and follow the standard developer policy by mailing a
>>> patch for review. I suspect that functionality like this is exactly the
>>> right way to go, but it needs to be discussed first.
>>> 
>> 
>> Having now read the patch, I definitely want this to be reverted. It is
>> similar to the design I had planned for this feature, but it introduces a
>> lot of boiler plate that is not needed, and misses some key elements. We
>> need to discuss the design and implementation of this before you commit,
>> and that should have happened with a patch, not a commit.
> 
> Reverting just this change is inacceptable. It fixes a major regression
> for cross-compiling support from your earlier changes, without
> introducing a bag of new ones.

There is a legitimate concern that your change is not the right direction. Please revert, and we can discuss how best to implement this functionality.

	- Doug