[PATCH] Always emit function declaration when generating profile instrumentation

[Duncan P. N. Exon Smith]

> On 2014-May-27, at 16:39, Alex L <arphaman at gmail.com> wrote:
> 
> This is needed to ensure that the profile counters are emitted 
> for all functions and methods.
> 

The patch should include relevant testcases in `test/Profile/` that fail
without the code changes (and pass with them).  I think you need coverage at
least for these cases:

    static inline void foo() {}
    struct Foo { void bar() {} };

If you can think of any others, please add them as well.  (What about nested
structs that have inline functions?  Does the same fix work for them?)

I've also a spotted a few style issues, most of which `clang-format` would fix
for you [1].

[1]: http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

> -------------- next part --------------
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> 
> --- lib/CodeGen/CodeGenModule.cpp	(revision 209698)
> 
> +++ lib/CodeGen/CodeGenModule.cpp	(working copy)
> @@ -1073,6 +1073,8 @@
>    if (LangOpts.EmitAllDecls)
>      return false;
> 

It isn't obvious why this is needed.  You should add a comment, such as:

    // Always emit declarations when instrumenting.

> +  if(CodeGenOpts.ProfileInstrGenerate && isa<FunctionDecl>(Global))

Should be a space after `if`.

> 
> +    return false;
> 
>    return !getContext().DeclMustBeEmitted(Global);
>  }
>  
> @@ -3028,6 +3030,16 @@
>      break;
>  
>    // C++ Decls
> +  case Decl::CXXRecord: {

I think you need a comment here too, along the same lines.

> +    if(!CodeGenOpts.ProfileInstrGenerate)

Space after `if`.

> 
> +      break;
> 
> +    CXXRecordDecl *Record = cast<CXXRecordDecl>(D);
> +    for(CXXRecordDecl::method_iterator I = Record->method_begin(); I != Record->method_end(); ++I) {

This should be:

    for (auto I = Record->method_begin(), E = Record->method_end(); I != E;
         ++I)

Three changes there: space after `for`, 80-column violation, and only evaluate
`Record->method_end()` once per loop entry (instead of once per iteration).

If `CXXRecordDecl` has a `methods()` range accessor, this is even better:

    for (auto &I : Record->methods())

> +      if(I->hasBody())

Space after `if`.

> +        EmitTopLevelDecl(*I);
> 
> +    }
> 
> +    break;
> +  }
>    case Decl::Namespace:
>      EmitNamespace(cast<NamespaceDecl>(D));
>      break;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits