Is the compiler more likely to inline a function in the class declaration with the inline keyword specified?

Question

I was reviewing a colleague's code recently and noticed he had put the "inline" keyword in in front of a bunch of Getter functions that were defined in a class declaration.

eg

class Foo
{
public:
    inline bool GetBar() const { return m_Bar; }
private:
    bool m_Bar;
};

I suggested in the code review that he remove the inline keywords, as I've read in a number of different places that defining a function in the class declaration is interpreted by the compiler (MSVC in this case, but apparently part of the C++ standard) as an indication that the author wants the functions inlined. My feeling is that if the extra text doesn't serve any purpose, it's just unnecessary clutter and should be removed.

His response was as follows:

1. The inline keyword makes it clear to other programmers interacting with this code that these functions are/should be inlined.

2. Many compilers still take the inline keyword into account in this case and use it to affect (read: increase) some kind of weighting that is used to decide whether or not said function would in fact be inlined.

3. Having the inline keyword there means that "warn if not inlined" warnings will be triggered (if enabled) if said functions are not inlined for whatever reason.

Personally, I disagree with the first reason altogether. To me, having the functions defined in the class declaration is enough to show the intention.

I'm skeptical about the last two reasons. I can't find any information that either confirms or denies the the point about the inline keyword affecting some sort of weighting. I'm also having trouble triggering the "warn if not inlined" warning for a function defined in a class declaration.

If you've read this far, I was wondering if you might have any insight into any of the above points? Additionally, if you could point me to any relevant articles/documentation, I'd really appreciate it.

Thanks!

Answer 1

Edit 1: Added LLVM (in other words "clang") inlining code

Edit 2: Add clarification as to how to "resolve" this.

The actual correct

Point 1 is of course self-explanatory.

Point 2 is nonsense - all modern compilers (at least MS, GCC and Clang [aka XCode]) completely ignore inline keywords and decides purely based on frequency/size critera (determining "code-bloat factor" based on size * number times, so small functions or functions called only a few times are more likely to be inlined - of course a getter would be a perfect choice for inlining always by the compiler, since it will be only two or three instructions, and most likely shorter than loading up this, then calling the getter function.

The inline keyword doesn't make ANY difference at all [and the C++ standard states that a definition inside the class is inline anyway].

Point 3 is another plausible scenario, but I would have thought that the fact that it is implicitly inline by it's defintion should give the same result. There was discussion about the inline keyword and its meaning on Clang mailing list a while back, with the conclusion that "the compiler usually knows best".

It is also typically completely useless to use inline together with virtual functions, as they will nearly always be called through a vtable entry, and can't be inlined.

Edit 1:

Code taken from LLVM's "InlineCost.cpp":

InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee,
                                             int Threshold) {
  // Cannot inline indirect calls.
  if (!Callee)
    return llvm::InlineCost::getNever();

  // Calls to functions with always-inline attributes should be inlined
  // whenever possible.
  if (CS.hasFnAttr(Attribute::AlwaysInline)) {
    if (isInlineViable(*Callee))
      return llvm::InlineCost::getAlways();
    return llvm::InlineCost::getNever();
  }

  // Never inline functions with conflicting attributes (unless callee has
  // always-inline attribute).
  if (!functionsHaveCompatibleAttributes(CS.getCaller(), Callee,
                                         TTIWP->getTTI(*Callee)))
    return llvm::InlineCost::getNever();

  // Don't inline this call if the caller has the optnone attribute.
  if (CS.getCaller()->hasFnAttribute(Attribute::OptimizeNone))
    return llvm::InlineCost::getNever();

  // Don't inline functions which can be redefined at link-time to mean
  // something else.  Don't inline functions marked noinline or call sites
  // marked noinline.
  if (Callee->mayBeOverridden() ||
      Callee->hasFnAttribute(Attribute::NoInline) || CS.isNoInline())
    return llvm::InlineCost::getNever();

  DEBUG(llvm::dbgs() << "      Analyzing call of " << Callee->getName()
        << "...\n");

  CallAnalyzer CA(TTIWP->getTTI(*Callee), ACT, *Callee, Threshold, CS);
  bool ShouldInline = CA.analyzeCall(CS);

  DEBUG(CA.dump());

  // Check if there was a reason to force inlining or no inlining.
  if (!ShouldInline && CA.getCost() < CA.getThreshold())
    return InlineCost::getNever();
  if (ShouldInline && CA.getCost() >= CA.getThreshold())
    return InlineCost::getAlways();

  return llvm::InlineCost::get(CA.getCost(), CA.getThreshold());
}

As can be seen (with some digging around in the rest of the code), there is only checks for the "always" and "never" inline options. None for the inline keyword itself.

[Note that this is the inliner code for clang and clang++ - clang itself does not do anything particularly clever when it generates code, it's "just" (upsetting hundreds of programmers who have spent hundreds of manyears on that project!) a parser for C and C++ that translates to LLVM IR, all the good, clever stuff is done at the LLVM layer - this is really a good way to provide a "multilanguage" compiler framework. I have written a Pascal compiler, and despite being a complete novice on compiler work, my compiler (which uses LLVM to generate actual machine code) is better in benchmarks (of the generated code) than Free Pascal - all thanks to LLVM, nearly none of that is my work - except for some code to inline a few commonly called functions in one particular benchmark]

I don't have access sources for MS compiler (doh!), and I can't be bothered to download gcc just to check this. I know, from experience, that all three will inline functions without the inline keyword, and gcc will aggressively inline functions that it can determine has only one caller (e.g. large static helper functions)

Edit 2:

The right way to resolve this sort of issue is to have a coding standard that says, clearly, when and where inline [and functions defined inside the class] should be used, and when this shouldn't be done. If currently, none of the other small getter functions in other classes have inline on them, then it would be strange and stand out that this function does. If all except some have inline, that probably should be fixed too.

Another anectdote: I personally like to write if-statements as

if (some stuff goes here)

(space between if and parenthesis, but not around the thing inside) but the coding standard at work says:

if( some stuff goes here )

(no space between if and parenthesis, but space around the thing inside)

I had accidentally got something like this:

if ( some stuff goes here )

in some code that was up for review. I fixed that line, but decided to ALSO fix the 175 other if-statements with a space after if - in total there were less than 350 if-statements in that file, so MORE than half of them were incorrect...