preprocessor: Better line info for <builtin> & <command-line>

Message ID 7176e381-ef7b-4d12-2af1-c7ab556a849a@acm.org
State New
Headers show
Series
  • preprocessor: Better line info for <builtin> & <command-line>
Related show

Commit Message

Nathan Sidwell July 7, 2020, 6:43 p.m.
With C++ module header units it becomes important to distinguish between 
macros defined in forced headers (& commandline & builtins) from those 
defined in the header file being processed.  We weren't making that easy 
because we treated the builtins and command-line locations somewhat 
file-like, with incrementing line numbers, and showing them as included 
from line 1 of the main file.  This patch does 3 things:

0) extend the idiom that 'line 0' of a file means 'the file as a whole'

1) builtins and command-line macros are shown as-if included from line zero.

2) when emitting preprocessed output we keep resetting the line number 
so that re-reading that preprocessed output will get the same set of 
locations for the command line etc.

For instance the new c-c++-common/cpp/line-2.c test, now emits

     In file included from <command-line>:
     ./line-2.h:4:2: error: #error wrong
         4 | #error wrong
           |  ^~~~~
     line-2.c:3:11: error: macro "bill" passed 1 arguments, but takes just 0
         3 | int bill(1);
           |           ^
    In file included from <command-line>:
     ./line-2.h:3: note: macro "bill" defined here
         3 | #define bill() 2
           |

Before it told you about including from <command-line>:31.

the preprocessed output looks like:
# 0 "line-2.c"
# 0 "<built-in>"
#define __STDC__ 1
# 0 "<built-in>"
#define __cplusplus 201703L
# 0 "<built-in>"
...

(There's a new optimization in do_line_marker to stop each of these line 
markers causing a new line map.  We can simply rewind the location, and 
keep using the same line map.)

             libcpp/
             * directives.c (do_linemarker): Optimize rewinding to line 
zero.
             * files.c (_cpp_stack_file): Start on line zero when about 
to inject
             headers.
             (cpp_push_include, cpp_push_default_include): Use 
highest_line as
             the location.
             * include/cpplib.h (cpp_read_main_file): Add injecting parm.
             * init.c (cpp_read_main_file): Likewise, inform 
_cpp_stack_file.
             * internal.h (enum include_type): Add IT_MAIN_INJECT.
             gcc/c-family/
             * c-opts.c (c_common_post_options): Add 'injecting' arg to
             cpp_read_main_file.
             (c_finish_options): Add linemap_line_start calls for 
builtin and cmd
             maps.  Force token position to line_table's highest line.
             * c-ppoutput.c (print_line_1): Refactor, print line zero.
             (cb_define): Always increment source line.
             gcc/testsuite/
             * c-c++-common/cpp/line-2.c: New.
             * c-c++-common/cpp/line-2.h: New.
             * c-c++-common/cpp/line-3.c: New.
             * c-c++-common/cpp/line-4.c: New.
             * c-c++-common/cpp/line-4.h: New.

-- 
Nathan Sidwell

Patch

diff --git c/ChangeLog w/ChangeLog
index 8c254769017..481f54a856e 100644
--- c/ChangeLog
+++ w/ChangeLog
@@ -1,3 +1,28 @@ 
+2020-07-07  Nathan Sidwell  <nathan@acm.org>
+
+	gcc/c-family/
+	* c-opts.c (c_common_post_options): Add 'injecting' arg to
+	cpp_read_main_file.
+	(c_finish_options): Add linemap_line_start calls for builtin and cmd
+	maps.  Force token position to line_table's highest line.
+	* c-ppoutput.c (print_line_1): Refactor, print line zero.
+	(cb_define): Always increment source line.
+	gcc/testsuite/
+	* c-c++-common/cpp/line-2.c: New.
+	* c-c++-common/cpp/line-2.h: New.
+	* c-c++-common/cpp/line-3.c: New.
+	* c-c++-common/cpp/line-4.c: New.
+	* c-c++-common/cpp/line-4.h: New.
+	libcpp/
+	* directives.c (do_linemarker): Optimize rewinding to line zero.
+	* files.c (_cpp_stack_file): Start on line zero when about to inject
+	headers.
+	(cpp_push_include, cpp_push_default_include): Use highest_line as
+	the location.
+	* include/cpplib.h (cpp_read_main_file): Add injecting parm.
+	* init.c (cpp_read_main_file): Likewise, inform _cpp_stack_file.
+	* internal.h (enum include_type): Add IT_MAIN_INJECT.
+
 2020-06-12  Martin Liska  <mliska@suse.cz>
 
 	* .gitignore: Add .clang-tidy.
diff --git c/gcc/c-family/c-opts.c w/gcc/c-family/c-opts.c
index 9b6300f330f..ec3de868dd4 100644
--- c/gcc/c-family/c-opts.c
+++ w/gcc/c-family/c-opts.c
@@ -1110,7 +1110,11 @@  c_common_post_options (const char **pfilename)
   input_location = UNKNOWN_LOCATION;
 
   *pfilename = this_input_filename
-    = cpp_read_main_file (parse_in, in_fnames[0]);
+    = cpp_read_main_file (parse_in, in_fnames[0],
+			  /* We'll inject preamble pieces if this is
+			     not preprocessed.  */
+			  !cpp_opts->preprocessed);
+
   /* Don't do any compilation or preprocessing if there is no input file.  */
   if (this_input_filename == NULL)
     {
@@ -1429,6 +1433,7 @@  c_finish_options (void)
 	= linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0,
 					       _("<built-in>"), 0));
       cb_file_change (parse_in, bltin_map);
+      linemap_line_start (line_table, 0, 1);
 
       /* Make sure all of the builtins about to be declared have
 	 BUILTINS_LOCATION has their location_t.  */
@@ -1452,9 +1457,10 @@  c_finish_options (void)
 	= linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0,
 					       _("<command-line>"), 0));
       cb_file_change (parse_in, cmd_map);
+      linemap_line_start (line_table, 0, 1);
 
       /* All command line defines must have the same location.  */
-      cpp_force_token_locations (parse_in, cmd_map->start_location);
+      cpp_force_token_locations (parse_in, line_table->highest_line);
       for (size_t i = 0; i < deferred_count; i++)
 	{
 	  struct deferred_opt *opt = &deferred_opts[i];
diff --git c/gcc/c-family/c-ppoutput.c w/gcc/c-family/c-ppoutput.c
index 1e2b32b46ac..44c6f30e06b 100644
--- c/gcc/c-family/c-ppoutput.c
+++ w/gcc/c-family/c-ppoutput.c
@@ -560,26 +560,23 @@  print_line_1 (location_t src_loc, const char *special_flags, FILE *stream)
   if (src_loc != UNKNOWN_LOCATION && !flag_no_line_commands)
     {
       const char *file_path = LOCATION_FILE (src_loc);
-      int sysp;
       size_t to_file_len = strlen (file_path);
       unsigned char *to_file_quoted =
          (unsigned char *) alloca (to_file_len * 4 + 1);
-      unsigned char *p;
 
       print.src_line = LOCATION_LINE (src_loc);
       print.src_file = file_path;
 
       /* cpp_quote_string does not nul-terminate, so we have to do it
 	 ourselves.  */
-      p = cpp_quote_string (to_file_quoted,
-			    (const unsigned char *) file_path,
-			    to_file_len);
+      unsigned char *p = cpp_quote_string (to_file_quoted,
+					   (const unsigned char *) file_path,
+					   to_file_len);
       *p = '\0';
       fprintf (stream, "# %u \"%s\"%s",
-	       print.src_line == 0 ? 1 : print.src_line,
-	       to_file_quoted, special_flags);
+	       print.src_line, to_file_quoted, special_flags);
 
-      sysp = in_system_header_at (src_loc);
+      int sysp = in_system_header_at (src_loc);
       if (sysp == 2)
 	fputs (" 3 4", stream);
       else if (sysp == 1)
@@ -677,8 +674,7 @@  cb_define (cpp_reader *pfile, location_t line, cpp_hashnode *node)
   linemap_resolve_location (line_table, line,
 			    LRK_MACRO_DEFINITION_LOCATION,
 			    &map);
-  if (LINEMAP_LINE (map) != 0)
-    print.src_line++;
+  print.src_line++;
 }
 
 static void
diff --git c/gcc/testsuite/c-c++-common/cpp/line-2.c w/gcc/testsuite/c-c++-common/cpp/line-2.c
new file mode 100644
index 00000000000..97cf398f64c
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/line-2.c
@@ -0,0 +1,11 @@ 
+int line1;
+int f = bob;
+int bill(1);
+int line4;
+
+// { dg-do preprocess }
+// { dg-options "-dD -include $srcdir/c-c++-common/cpp/line-2.h -nostdinc" }
+
+// { dg-regexp {In file included from <command-line>:\n[^\n]*/line-2.h:4:2: error: #error wrong\n} }
+
+// { dg-regexp {[^\n]*/line-2.c:3:11: error: macro "bill" passed 1 arguments, but takes just 0\nIn file included from <command-line>:\n[^\n]*/line-2.h:3: note: macro "bill" defined here\n} }
diff --git c/gcc/testsuite/c-c++-common/cpp/line-2.h w/gcc/testsuite/c-c++-common/cpp/line-2.h
new file mode 100644
index 00000000000..737bdfa2926
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/line-2.h
@@ -0,0 +1,5 @@ 
+#define bob 1
+/* Comment */
+#define bill() 2
+#error wrong
+
diff --git c/gcc/testsuite/c-c++-common/cpp/line-3.c w/gcc/testsuite/c-c++-common/cpp/line-3.c
new file mode 100644
index 00000000000..2ffc44907a2
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/line-3.c
@@ -0,0 +1,20 @@ 
+# 0 ".../line-1.c"
+# 0 "<built-in>"
+# 0 "<command-line>"
+# 1 "./line-2.h" 1
+#define bob 1
+
+#define bill() 2
+#error wrong
+# 0 "<command-line>" 2
+# 1 ".../line-3.c"
+int line1;
+int f = bob;
+int bill(1);
+int line4;
+
+// { dg-regexp {In file included from <command-line>:\n[^\n]*/line-2.h:4:2: error: #error wrong\n} }
+
+// { dg-regexp {[^\n]*/line-3.c:3:11: error: macro "bill" passed 1 arguments, but takes just 0\nIn file included from <command-line>:\n[^\n]*/line-2.h:3: note: macro "bill" defined here\n} }
+
+// { dg-options "-fpreprocessed -fdirectives-only" }
diff --git c/gcc/testsuite/c-c++-common/cpp/line-4.c w/gcc/testsuite/c-c++-common/cpp/line-4.c
new file mode 100644
index 00000000000..810f15929c2
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/line-4.c
@@ -0,0 +1,11 @@ 
+int line1;
+int f = bob;
+int b = bill();
+int line4;
+
+// { dg-do preprocess }
+// { dg-options "-dD -include $srcdir/c-c++-common/cpp/line-4.h -nostdinc" }
+
+// { dg-final { scan-file line-4.i {# 0 "[^\n]*/line-4.c"\n# 0 "<built-in>"\n} } }
+// { dg-final { scan-file line-4.i {# 0 "<command-line>"\n# 1 "[^\n]*/line-4.h" 1\n#define bob 1\n} } }
+// { dg-final { scan-file line-4.i {#define bill\(\) 2\n# 0 "<command-line>" 2\n# 1 "[^\n]*/line-4.c"\nint line1;\n} } }
diff --git c/gcc/testsuite/c-c++-common/cpp/line-4.h w/gcc/testsuite/c-c++-common/cpp/line-4.h
new file mode 100644
index 00000000000..6ee05a3d137
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/line-4.h
@@ -0,0 +1,3 @@ 
+#define bob 1
+/* Comment */
+#define bill() 2
diff --git c/libcpp/directives.c w/libcpp/directives.c
index bbfdfcd3368..f59718708e4 100644
--- c/libcpp/directives.c
+++ w/libcpp/directives.c
@@ -940,7 +940,7 @@  strtolinenum (const uchar *str, size_t len, linenum_type *nump, bool *wrapped)
 
 /* Interpret #line command.
    Note that the filename string (if any) is a true string constant
-   (escapes are interpreted), unlike in #line.  */
+   (escapes are interpreted).  */
 static void
 do_line (cpp_reader *pfile)
 {
@@ -1115,27 +1115,43 @@  do_linemarker (cpp_reader *pfile)
   line_table->seen_line_directive = true;
 }
 
-/* Arrange the file_change callback.  pfile->line has changed to
-   FILE_LINE of TO_FILE, for reason REASON.  SYSP is 1 for a system
-   header, 2 for a system header that needs to be extern "C" protected,
-   and zero otherwise.  */
+/* Arrange the file_change callback.  Changing to TO_FILE:TO_LINE for
+   REASON.  SYSP is 1 for a system header, 2 for a system header that
+   needs to be extern "C" protected, and zero otherwise.  */
 void
 _cpp_do_file_change (cpp_reader *pfile, enum lc_reason reason,
-		     const char *to_file, linenum_type file_line,
+		     const char *to_file, linenum_type to_line,
 		     unsigned int sysp)
 {
   linemap_assert (reason != LC_ENTER_MACRO);
-  const struct line_map *map = linemap_add (pfile->line_table, reason, sysp,
-					    to_file, file_line);
+
   const line_map_ordinary *ord_map = NULL;
-  if (map != NULL)
-    {
-      ord_map = linemap_check_ordinary (map);
-      linemap_line_start (pfile->line_table,
-			  ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map),
-			  127);
+  if (!to_line && reason == LC_RENAME_VERBATIM)
+    {
+      /* A linemarker moving to line zero.  If we're on the second
+         line of the current map, and it also starts at zero, just
+         rewind -- we're probably reading the builtins of a
+         preprocessed source.  */
+      line_map_ordinary *last = LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table);
+      if (!ORDINARY_MAP_STARTING_LINE_NUMBER (last)
+	  && SOURCE_LINE (last, pfile->line_table->highest_line) == 2)
+	{
+	  ord_map = last;
+	  pfile->line_table->highest_location
+	    = pfile->line_table->highest_line = MAP_START_LOCATION (last);
+	}
     }
 
+  if (!ord_map)
+    if (const line_map *map = linemap_add (pfile->line_table, reason, sysp,
+					   to_file, to_line))
+      {
+	ord_map = linemap_check_ordinary (map);
+	linemap_line_start (pfile->line_table,
+			    ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map),
+			    127);
+      }
+
   if (pfile->cb.file_change)
     pfile->cb.file_change (pfile, ord_map);
 }
diff --git c/libcpp/files.c w/libcpp/files.c
index 85c79a1ef93..3d48c38fc0a 100644
--- c/libcpp/files.c
+++ w/libcpp/files.c
@@ -947,7 +947,11 @@  _cpp_stack_file (cpp_reader *pfile, _cpp_file *file, include_type type,
     pfile->line_table->highest_location--;
 
   /* Add line map and do callbacks.  */
-  _cpp_do_file_change (pfile, LC_ENTER, file->path, 1, sysp);
+  _cpp_do_file_change (pfile, LC_ENTER, file->path,
+		       /* With preamble injection, start on line zero, so
+			  the preamble doesn't appear to have been
+			  included from line 1.  */
+		       type == IT_MAIN_INJECT ? 0 : 1, sysp);
 
   return true;
 }
@@ -1475,7 +1479,8 @@  _cpp_compare_file_date (cpp_reader *pfile, const char *fname,
 bool
 cpp_push_include (cpp_reader *pfile, const char *fname)
 {
-  return _cpp_stack_include (pfile, fname, false, IT_CMDLINE, 0);
+  return _cpp_stack_include (pfile, fname, false, IT_CMDLINE,
+			     pfile->line_table->highest_line);
 }
 
 /* Pushes the given file, implicitly included at the start of a
@@ -1484,7 +1489,8 @@  cpp_push_include (cpp_reader *pfile, const char *fname)
 bool
 cpp_push_default_include (cpp_reader *pfile, const char *fname)
 {
-  return _cpp_stack_include (pfile, fname, true, IT_DEFAULT, 0);
+  return _cpp_stack_include (pfile, fname, true, IT_DEFAULT,
+			     pfile->line_table->highest_line);
 }
 
 /* Do appropriate cleanup when a file INC's buffer is popped off the
diff --git c/libcpp/include/cpplib.h w/libcpp/include/cpplib.h
index 544735a51af..e8bb15d1f09 100644
--- c/libcpp/include/cpplib.h
+++ w/libcpp/include/cpplib.h
@@ -979,7 +979,8 @@  extern class mkdeps *cpp_get_deps (cpp_reader *) ATTRIBUTE_PURE;
    input file, except for preprocessed input.  This will generate at
    least one file change callback, and possibly a line change callback
    too.  If there was an error opening the file, it returns NULL.  */
-extern const char *cpp_read_main_file (cpp_reader *, const char *);
+extern const char *cpp_read_main_file (cpp_reader *, const char *,
+				       bool injecting = false);
 
 /* Set up built-ins with special behavior.  Use cpp_init_builtins()
    instead unless your know what you are doing.  */
diff --git c/libcpp/init.c w/libcpp/init.c
index 63124c8161e..d641d0a1e3a 100644
--- c/libcpp/init.c
+++ w/libcpp/init.c
@@ -657,10 +657,11 @@  cpp_post_options (cpp_reader *pfile)
 }
 
 /* Setup for processing input from the file named FNAME, or stdin if
-   it is the empty string.  Return the original filename
-   on success (e.g. foo.i->foo.c), or NULL on failure.  */
+   it is the empty string.  Return the original filename on success
+   (e.g. foo.i->foo.c), or NULL on failure.  INJECTING is true if
+   there may be injected headers before line 1 of the main file.  */
 const char *
-cpp_read_main_file (cpp_reader *pfile, const char *fname)
+cpp_read_main_file (cpp_reader *pfile, const char *fname, bool injecting)
 {
   if (CPP_OPTION (pfile, deps.style) != DEPS_NONE)
     {
@@ -677,16 +678,16 @@  cpp_read_main_file (cpp_reader *pfile, const char *fname)
   if (_cpp_find_failed (pfile->main_file))
     return NULL;
 
-  _cpp_stack_file (pfile, pfile->main_file, IT_MAIN, 0);
+  _cpp_stack_file (pfile, pfile->main_file,
+		   injecting ? IT_MAIN_INJECT : IT_MAIN, 0);
 
   /* For foo.i, read the original filename foo.c now, for the benefit
      of the front ends.  */
   if (CPP_OPTION (pfile, preprocessed))
     {
       read_original_filename (pfile);
-      fname =
-	ORDINARY_MAP_FILE_NAME
-	((LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table)));
+      fname = (ORDINARY_MAP_FILE_NAME
+	       ((LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table))));
     }
   return fname;
 }
diff --git c/libcpp/internal.h w/libcpp/internal.h
index 049ee175317..4bafe1cf353 100644
--- c/libcpp/internal.h
+++ w/libcpp/internal.h
@@ -123,7 +123,9 @@  enum include_type
    /* Non-directive including mechanisms.  */
    IT_CMDLINE,  /* -include */
    IT_DEFAULT,  /* forced header  */
-   IT_MAIN,     /* main  */
+   IT_MAIN,     /* main, start on line 1 */
+   IT_MAIN_INJECT,  /* main, but there will be an injected preamble
+		       before line 1 */
 
    IT_DIRECTIVE_HWM = IT_IMPORT + 1,  /* Directives below this.  */
    IT_HEADER_HWM = IT_DEFAULT + 1     /* Header files below this.  */