[PATCH] D49522: [DWARF v5] Don't emit duplicate DW_AT_rnglists_base and address review comments from previous review

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Is this still missing test coverage for the case where the CU has low/high_pc, but also needs a ranges_base because a function has ranges for a local scope? Looks like the bug got fixed, but test coverage is still missing?



================
Comment at: test/DebugInfo/X86/rnglists_base_attr.ll:4
+
+; Make sure we don't generate a duplicate DW_AT_rnglists_base attribue in the CU DIE.
+; From the source:
----------------
dblaikie wrote:
> I'd probably introduce a separate f1/f2 - so that it doesn't look like the recursion is an integral part of the test:
> 
>   void f1();
>   void f2() {
>     f1(); //reorder the IR for this call between the two below to produce ranges
>     {
>       bool b; // force a DW_AT_lexical_scope to be created
>       f1();
>       f1();
>     }
>   }
> 
Maybe expound on this a bit more:

"... in the CU DIE when more than one range list is emitted."

To help explain why the source example code looks the way it does - maybe include some comment about "force a non-contiguous local-scope DIEs for f1 and a non-contiguous CU by having functions in different sections" or the like.

(I know I'm really bad at commenting test cases & explaining why they're constructed the way they are - so this is more a "do as I say, not as I do" - but also please do poke me about doing this myself when I'm a bit opaque in my test cases!)


================
Comment at: test/DebugInfo/X86/rnglists_base_attr.ll:4-5
+
+; Make sure we don't generate a duplicate DW_AT_rnglists_base attribue in the CU DIE.
+; From the source:
+; bool x;
----------------
I'd probably introduce a separate f1/f2 - so that it doesn't look like the recursion is an integral part of the test:

  void f1();
  void f2() {
    f1(); //reorder the IR for this call between the two below to produce ranges
    {
      bool b; // force a DW_AT_lexical_scope to be created
      f1();
      f1();
    }
  }



https://reviews.llvm.org/D49522