r205368 - Partially revert r204517 and fix a different way:

[David Blaikie]

On Tue, Apr 1, 2014 at 3:25 PM, Eric Christopher <echristo at gmail.com> wrote:
> Author: echristo
> Date: Tue Apr  1 17:25:28 2014
> New Revision: 205368
>
> URL: http://llvm.org/viewvc/llvm-project?rev=205368&view=rev
> Log:
> Partially revert r204517 and fix a different way:
>
> We don't want to encourage the code to emit a lexical block for
> a function that needs one in order for the line table to change,
> we need to grab the line information from the body of the pattern
> that we were instantiated from, this code should do that.
>
> Modify the test case to ensure that we're still looking in the
> right place for all of the scopes and also that we haven't
> created a lexical block where we didn't need one.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>     cfe/trunk/test/CodeGenCXX/linetable-fnbegin.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205368&r1=205367&r2=205368&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Apr  1 17:25:28 2014
> @@ -2497,14 +2497,25 @@ void CGDebugInfo::EmitFunctionStart(Glob
>    FnBeginRegionCount.push_back(LexicalBlockStack.size());
>
>    const Decl *D = GD.getDecl();
> -  // Function may lack declaration in source code if it is created by Clang
> -  // CodeGen (examples: _GLOBAL__I_a, __cxx_global_array_dtor, thunk).
> +
> +  // Use the location of the start of the function to determine where
> +  // the function definition is located.

I'm not sure this first sentence adds a lot, and seems either
ambiguous ("where's the start of the function?" in the AST, that'd be
the start of the return type - but we're certainly not using that
anywhere here) or incorrect.

> By default use the location
> +  // of the declaration

This could probably say "definition" rather than "declaration" (we
should always be handed a definition here - could include an assert to
that effect if desired).

> as the location for the subprogram. A function
> +  // may lack a declaration in the source code if it is created by code
> +  // gen. (examples: _GLOBAL__I_a, __cxx_global_array_dtor, thunk).
>    bool HasDecl = (D != 0);
> -  // Use the location of the declaration.
>    SourceLocation Loc;
> -  if (HasDecl)
> +  if (HasDecl) {
>      Loc = D->getLocation();
>
> +    // If this is a function specialization then use the pattern body

Slightly misleading - we're not using the body, just the pattern definition.

Honestly I might consider breaking this whole location computation bit
into a separate function and writing a paragraph-long screed about the
quirky nature of the AST here & what we're dealing with.

> +    // as the location for the function.
> +    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
> +      if (const FunctionDecl *SpecDecl = FD->getTemplateInstantiationPattern())
> +        if (SpecDecl->hasBody(SpecDecl))
> +          Loc = SpecDecl->getLocation();
> +  }
> +
>    unsigned Flags = 0;
>    llvm::DIFile Unit = getOrCreateFile(Loc);
>    llvm::DIDescriptor FDContext(Unit);
> @@ -2585,8 +2596,6 @@ void CGDebugInfo::EmitFunctionStart(Glob
>    // Push the function onto the lexical block stack.
>    llvm::MDNode *SPN = SP;
>    LexicalBlockStack.push_back(SPN);
> -  // Initialize PrevLoc to the location of the function header.
> -  PrevLoc = Loc;
>
>    if (HasDecl)
>      RegionMap[D] = llvm::WeakVH(SP);
>
> Modified: cfe/trunk/test/CodeGenCXX/linetable-fnbegin.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/linetable-fnbegin.cpp?rev=205368&r1=205367&r2=205368&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/linetable-fnbegin.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/linetable-fnbegin.cpp Tue Apr  1 17:25:28 2014
> @@ -3,10 +3,13 @@
>  // right header file.
>  // CHECK: define{{.*}}bar
>  // CHECK-NOT: define
> -// CHECK: ret {{.*}}, !dbg ![[DBG:.*]]
> -// CHECK: ![[HPP:.*]] = metadata !{metadata !"./template.hpp",
> -// CHECK: ![[DBG]] = metadata !{i32 23, i32 0, metadata ![[BLOCK:.*]], null}
> -// CHECK: ![[BLOCK]] = metadata !{{{.*}}, metadata ![[HPP]], {{.*}}} ; [ DW_TAG_lexical_block ]
> +// CHECK: ret {{.*}}, !dbg [[DBG:.*]]
> +// CHECK: [[HPP:.*]] = metadata !{metadata !"./template.hpp",
> +// CHECK: [[SP:.*]] = metadata !{i32 786478, metadata [[HPP]], metadata !"_ZTS3FooIiE", metadata !"bar", metadata !"bar", metadata !"_ZN3FooIiE3barEv", i32 22, metadata !8, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, i32 (%class.Foo*)* @_ZN3FooIiE3barEv, null, metadata !7, metadata !2, i32 22} ; [ DW_TAG_subprogram ] [line 22] [def] [bar]
> +// We shouldn't need a lexical block for this function.
> +// CHECK: [[DBG]] = metadata !{i32 23, i32 0, metadata [[SP]], null}
> +
> +
>  # 1 "./template.h" 1
>  template <typename T>
>  class Foo {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits