[llvm-commits] [llvm] r158800 - in /llvm/trunk: include/llvm/ADT/STLExtras.h include/llvm/Analysis/Dominators.h include/llvm/Analysis/LoopInfo.h include/llvm/Bitcode/ReaderWriter.h include/llvm/CodeGen/MachineInstrBundle.h include/llvm/Instructio

[David Blaikie]

On Wed, Jun 20, 2012 at 10:45 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> On Wed, Jun 20, 2012 at 10:00 PM, Chris Lattner <clattner at apple.com> wrote:
>>
>>
>> On Jun 20, 2012, at 1:39 AM, Chandler Carruth wrote:
>>
>> > Author: chandlerc
>> > Date: Wed Jun 20 03:39:33 2012
>> > New Revision: 158800
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=158800&view=rev
>> > Log:
>> > Remove 'static' from inline functions defined in header files.
>> >
>> > There is a pretty staggering amount of this in LLVM's header files, this
>> > is not all of the instances I'm afraid. These include all of the
>> > functions that (in my build) are used by a non-static inline (or
>> > external) function. Specifically, these issues were caught by the new
>> > '-Winternal-linkage-in-inline' warning.
>>
>> Should this patch be reverted?  I thought it was decided that that flag
>> was a bad idea for C++ code, since the whole concept is predicated on
>> "inline" working like the C99 inline semantics, which are bone-headed...
>
>
> I think the patches are still fine -- I didn't commit any of the really
> questionable changes. I don't see why we would want both 'static' and
> 'inline', especially in a header file? I chatted w/ a few others and none
> had any theories as to why this would be desirable...
>
> I don't feel particularly strongly about it though, especially if there is a
> benefit to the pattern that I'm not aware of. If there *is* a benefit,
> should we systematically follow this pattern?

If anything I'd assume the benefit is in favor of the change you made
- with 'static' wouldn't we end up with duplicate copies of all these
functions (if they don't get entirely inlined to all call sites) from
each TU that includes them?