C++ instrumentation: fix for ranges support

The instrumentation scheme for C++ for ranges resulted in incorrect code
when initializer lists were involved, e.g.:

```
for (auto i : {1, 2) {
   <for_body>
}
```

being instrumented as

```
for (auto i : (witness(), {1, 2}) {
   <for_body>
}
```

which is invalid C++ code.

To fix this, we now instrument such expressions as followed:

```
witness();
for (auto i : {1, 2}) {
   <for_body>
}
```

We can't execute the loop statement without executing the witness
with a goto as it is forbidden to bypass initialization statements in C++.

When there is an initialization statement, e.g.:

```
for (int j = 0; auto i : {1, 2}){}
```

we actually introduce an outer scope and instrument as followed:

```
{
   witness_j();
   int j = 0;
   witness_i();
   for (auto i : {1, 2}){}
}
```

Author: Jugst3r

testsuite/tests/C++/stmt/ForRange/WithInitializerRange/src/test_for_range.cpp

@@ -0,0 +1,18 @@
+#include <initializer_list>
+
+int
+main (void)
+{
+  int sum = 0;               // # init
+  for (auto i : { 1, 2, 3 }) // # for-range
+    {
+      sum += i;              // # for-body
+    }
+  return 0;
+}
+
+//# test_for_range.cpp
+//
+//  /init/      l+ ## 0
+//  /for-range/ l+ ## 0
+//  /for-body/  l+ ## 0

testsuite/tests/C++/stmt/ForRange/WithInitializerRange/test.py

@@ -0,0 +1,13 @@
+"""
+Test that gnatcov correctly instruments a for range with an initializer as the
+range expression and an initialization statement, e.g.
+```
+for (int i = 0; auto j : {1, 2}){}
+```
+"""
+
+from SCOV.tc import TestCase
+from SUITE.context import thistest
+
+TestCase().run()
+thistest.result()

tools/gnatcov/clang-extensions.adb

@@ -132,6 +132,29 @@ package body Clang.Extensions is
       CX_Rewriter_Insert_Text_Before_Token_C (Rew, Loc, Insert & ASCII.NUL);
    end CX_Rewriter_Insert_Text_Before_Token;
 
+   ------------------------------------
+   -- CX_Rewriter_Get_Rewritten_Text --
+   ------------------------------------
+
+   function CX_Rewriter_Get_Rewritten_Text
+     (Rew : Rewriter_T;
+      R   : Source_Range_T) return String
+   is
+      function CX_Rewriter_Get_Rewritten_Text
+        (Rew : Rewriter_T;
+         R   : Source_Range_T) return String_T
+        with
+          Import, Convention => C,
+          External_Name => "clang_CXRewriter_getRewrittenText";
+
+      Rewritten_Text_C : constant String_T :=
+        CX_Rewriter_Get_Rewritten_Text (Rew, R);
+      Rewritten_Text   : constant String := Get_C_String (Rewritten_Text_C);
+   begin
+      Dispose_String (Rewritten_Text_C);
+      return Rewritten_Text;
+   end CX_Rewriter_Get_Rewritten_Text;
+
    -----------------------
    -- Spelling_Location --
    -----------------------

tools/gnatcov/clang-extensions.ads

@@ -124,6 +124,12 @@ package Clang.Extensions is
    --  Insert the text Insert before the token at the given location, and after
    --  any previously inserted string (at the same location).
 
+   function CX_Rewriter_Get_Rewritten_Text
+     (Rew : Rewriter_T;
+      R   : Source_Range_T) return String
+     with Inline;
+   --  Return the rewritten text for the given source range.
+
    function Get_Cursor_TU (C : Cursor_T) return Translation_Unit_T
    with
      Import, Convention => C,

tools/gnatcov/clang-wrapper.cc

@@ -608,10 +608,19 @@ clang_CXRewriter_insertTextBeforeToken (CXRewriter Rew, CXSourceLocation Loc,
   R.InsertTextAfter (Prv, Insert);
 }
 
-/* Wrappers around source location analysis functions.  */
+extern "C" CXString
+clang_CXRewriter_getRewrittenText (CXRewriter Rew, CXSourceRange Rng)
+{
+  assert (Rew);
+  Rewriter &R = *reinterpret_cast<Rewriter *> (Rew);
+  SourceRange SR = translateCXSourceRange (Rng);
+  return createDup (R.getRewrittenText (SR).c_str ());
+}
 
-extern "C" unsigned
-clang_isMacroLocation (CXSourceLocation Loc)
+  /* Wrappers around source location analysis functions.  */
+
+  extern "C" unsigned
+  clang_isMacroLocation (CXSourceLocation Loc)
 {
   const SourceLocation SLoc = translateSourceLocation (Loc);
   return SLoc.isMacroID () ? 1 : 0;

tools/gnatcov/instrument-c.adb

@@ -37,7 +37,6 @@ with Coverage;            use Coverage;
 with Coverage_Options;
 with Hex_Images;          use Hex_Images;
 with Inputs;              use Inputs;
-with Instrument.C_Utils;  use Instrument.C_Utils;
 with Outputs;             use Outputs;
 with Paths;               use Paths;
 with SCOs;
@@ -213,6 +212,11 @@ package body Instrument.C is
       Loc  : Source_Location_T;
       Text : String);
 
+   overriding procedure Register_CXX_For_Range
+     (Pass : Instrument_Pass_Kind;
+      UIC  : in out C_Unit_Inst_Context'Class;
+      N    : Cursor_T);
+
    Record_PP_Info_Pass : aliased Record_PP_Info_Pass_Kind;
    Instrument_Pass     : aliased Instrument_Pass_Kind;
 
@@ -273,6 +277,54 @@ package body Instrument.C is
    function Insert_MCDC_State
      (UIC : in out C_Unit_Inst_Context'Class; Name : String) return String;
 
+   procedure Fix_CXX_For_Ranges (UIC : in out C_Unit_Inst_Context);
+   --  This procedure fixes C++ for ranges that were purposefully instrumented
+   --  wrongly. To instrument a C++ for range, we actually need to turn
+   --
+   --  for (int i = 0; auto j : {1, 2}) {}
+   --
+   --  into
+   --
+   --  {
+   --     witness_i(); int i = 0;
+   --     witness_j();
+   --     for (auto j : {1, 2}) {}
+   --  }
+   --
+   --  We introduce an outer scope to leave the initializer declaration
+   --  lifetime unchanged, and to be able to easily instrument both the
+   --  initializer declaration and the range expression.
+   --
+   --  Note that this is valid because you can't goto inside the loop
+   --  (and thus skip the execution of witness_j) as it is
+   --  forbidden to bypass declarations with initialization in C++,
+   --  which "auto j : {1, 2}" is.
+   --
+   --  Though we can't do that all at once, as this changes the shape as the
+   --  AST. As we rely on the AST node location to instrument statements and
+   --  decisions, we would be instrumenting the decision at the wrong place,
+   --  as we would instrument the initialization statement by moving it.
+   --
+   --  Thus, we do the normal instrumentation process, which will yield an
+   --  unvalid C++ sequence, that we can easily fix in this procedure, by
+   --  moving around the rewritten code.
+   --
+   --  The for ranges at the entry of this procedure will have been
+   --  instrumented as followed (the added code is identified by <>):
+   --
+   --  <witness_j();> for (<witness_i();> int i = 0; auto j : {1, 2}) {}<}>
+   --
+   --  Two things to note here: the witness_j is executed before "int i = 0;"
+   --  (which is wrong), and there is an unmatched closing brace.
+   --
+   --  To fix this, we actually need to move both the added code, _and_ the
+   --  initializer statement before the <witness_j();>, and insert an opening
+   --  brace before:
+   --
+   --  <{><witness_i();> int i = 0; <witness_j();> for (auto j : {1, 2}) {}<}>
+   --
+   --  and now we have valid, and correctly instrumented code.
+
    ----------------------------
    -- Source level rewriting --
    ----------------------------
@@ -977,6 +1029,18 @@ package body Instrument.C is
       CX_Rewriter_Insert_Text_After (Rew, Loc, Text);
    end Insert_Text_After;
 
+   ----------------------------
+   -- Register_CXX_For_Range --
+   ----------------------------
+
+   overriding procedure Register_CXX_For_Range
+     (Pass : Instrument_Pass_Kind;
+      UIC  : in out C_Unit_Inst_Context'Class;
+      N    : Cursor_T) is
+   begin
+      UIC.Instrumented_CXX_For_Ranges.Append (N);
+   end Register_CXX_For_Range;
+
    --------------------------
    -- Instrument_Statement --
    --------------------------
@@ -1222,6 +1286,43 @@ package body Instrument.C is
       return Name;
    end Insert_MCDC_State;
 
+   ------------------------
+   -- Fix_CXX_For_Ranges --
+   ------------------------
+
+   procedure Fix_CXX_For_Ranges (UIC : in out C_Unit_Inst_Context) is
+   begin
+      for N of UIC.Instrumented_CXX_For_Ranges loop
+         declare
+            Loc           : constant Source_Location_T := Start_Sloc (N);
+            For_Init_Stmt : constant Cursor_T := Get_For_Init (N);
+            For_Init_Rng  : constant Source_Range_T :=
+              Get_Cursor_Extent (For_Init_Stmt);
+         begin
+            --  Get the rewritten text for the initializing statement and
+            --  move it before any rewritten text before the for statement.
+
+            CX_Rewriter_Insert_Text_Before
+              (UIC.Rewriter,
+               Loc,
+               CX_Rewriter_Get_Rewritten_Text (UIC.Rewriter, For_Init_Rng));
+
+            --  Open the outer scope. It was closed during the instrumentation
+            --  process, so we do not need to take care of that.
+
+            CX_Rewriter_Insert_Text_Before (UIC.Rewriter, Loc, "{");
+            CX_Rewriter_Remove_Text (UIC.Rewriter, For_Init_Rng);
+
+            --  for (; auto i : {1, 2}) is invalid C++ code so insert a dummy
+            --  initializing expression to avoid the null statement here, as
+            --  it is easier than trying to move around the comma.
+
+            CX_Rewriter_Insert_Text_Before
+              (UIC.Rewriter, Start_Sloc (For_Init_Stmt), "true");
+         end;
+      end loop;
+   end Fix_CXX_For_Ranges;
+
    type SC_Entry is record
       N : Cursor_T;
       --  Original statement node, used to compute macro expansion information
@@ -2033,14 +2134,28 @@ package body Instrument.C is
                   --  the range declaration initialization expression. Like all
                   --  statements, they can contain nested decisions.
 
-                  Extend_Statement_Sequence
-                    (For_Init_Stmt, ' ', Insertion_N => N);
-                  Process_Decisions_Defer (For_Init_Stmt, 'X');
+                  if not Is_Null (For_Init_Stmt) then
+
+                     --  See Fix_CXX_For_Ranges for an explanation of the
+                     --  below code.
+
+                     Extend_Statement_Sequence
+                       (For_Init_Stmt, ' ', Insertion_N  => For_Init_Stmt);
+                     Process_Decisions_Defer (For_Init_Stmt, 'X');
+
+                     --  Preemptively end the introduced outer scope as it is
+                     --  easier done when traversing the AST.
+
+                     Append (Trailing_Braces, '}');
+                     UIC.Pass.Register_CXX_For_Range (UIC, N);
+                  end if;
+
+                  --  Instrument the range as mentioned above
 
                   Extend_Statement_Sequence
                     (For_Range_Decl, ' ',
-                     Insertion_N  => For_Range_Decl,
-                     Instr_Scheme => Instr_Expr);
+                     Insertion_N  => N,
+                     Instr_Scheme => Instr_Stmt);
                   Process_Decisions_Defer (For_Range_Decl, 'X');
 
                   Set_Statement_Entry;
@@ -3179,6 +3294,11 @@ package body Instrument.C is
          end loop;
       end;
 
+      --  Once everything has been instrumented, fixup the for ranges. See the
+      --  documentation of Fix_CXX_For_Ranges.
+
+      Fix_CXX_For_Ranges (UIC);
+
       --  We import the extern declaration of symbols instead of including the
       --  header where they are defined.
       --

tools/gnatcov/instrument-c.ads

@@ -31,6 +31,7 @@ with Clang.Index;              use Clang.Index;
 with Clang.Rewrite;            use Clang.Rewrite;
 
 with Files_Table;       use Files_Table;
+with Instrument.C_Utils; use Instrument.C_Utils;
 with Instrument.Common; use Instrument.Common;
 with Slocs;             use Slocs;
 
@@ -343,6 +344,11 @@ package Instrument.C is
 
          Current_File_Scope : Source_File_Index;
          --  Source file in wich the last scope encountered was opened.
+
+         Instrumented_CXX_For_Ranges : Cursor_Vectors.Vector;
+         --  List of instrumented for ranges. For an explanation of why we need
+         --  to store these, see the documentation of the Fix_CXX_For_Ranges
+         --  subprogram.
       end record;
 
    type C_Source_Rewriter is tagged limited private;
@@ -428,6 +434,12 @@ private
       Loc  : Source_Location_T;
       Text : String) is null;
 
+   procedure Register_CXX_For_Range
+     (Pass : Pass_Kind;
+      UIC  : in out C_Unit_Inst_Context'Class;
+      N    : Cursor_T) is null;
+   --  See the documentation of Fix_CXX_For_Ranges
+
    type C_Source_Rewriter is limited new Ada.Finalization.Limited_Controlled
    with record
       CIdx     : Index_T;