Re: [PATCH] diagnostics: Allow FEs to keep customizations for middle end [PR101551, PR106274]

[Lewis Hyatt <lhyatt@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]

On Fri, Oct 28, 2022 at 10:28:21AM +0200, Richard Biener wrote:
> Yes, the idea was also to free up memory but then that part never
> really materialized - the idea was to always run free-lang-data, not
> just when later outputting LTO bytecode.  The reason is probably
> mainly the diagnostic regressions you observe.
> 
> Maybe a better strathegy than your patch would be to work towards
> that goal but reduce the number of "freeings", instead adjusting the
> LTO streamer to properly ignore frontend specific bits where clearing
> conflicts with the intent to preserve accurate diagnostics throughout
> the compilation.
> 
> If you see bits that when not freed would fix some of the observed
> issues we can see to replicate the freeing in the LTO output machinery.
> 
> Richard.

Thanks again for the suggestions. I took a look and it seems pretty doable to
just stop resetting all the diagnostics hooks in free-lang-data. Once that's
done, the only problematic part that I have been able to identify is here in
ipa-free-lang-data.c around line 674:

====
  /* We need to keep field decls associated with their trees. Otherwise tree
     merging may merge some fields and keep others disjoint which in turn will
     not do well with TREE_CHAIN pointers linking them.

     Also do not drop containing types for virtual methods and tables because
     these are needed by devirtualization.
     C++ destructors are special because C++ frontends sometimes produces
     virtual destructor as an alias of non-virtual destructor.  In
     devirutalization code we always walk through aliases and we need
     context to be preserved too.  See PR89335  */
  if (TREE_CODE (decl) != FIELD_DECL
      && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
          || (!DECL_VIRTUAL_P (decl)
	      && (TREE_CODE (decl) != FUNCTION_DECL
		  || !DECL_CXX_DESTRUCTOR_P (decl)))))
    DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl));
====

The C++ implementations of the decl_printable_name langhook and the diagnostic
starter callback do not work as-is when the DECL_CONTEXT for class member
functions disappears.  So I did have a patch that changes the C++
implementations to work in this case, but attached here is a new one along the
lines of what you suggested, rather changing the above part of free-lang-data
so it doesn't activate as often. The patch is pretty complete (other than
missing a commit message) and bootstrap + regtest all languages looks good
with no regressions. I tried the same with BUILD_CONFIG=bootstrap-lto as well,
and that also looked good when it eventually finished. I added testcases for
several frontends to verify that the diagnostics still work with -flto. I am
not sure what are the implications for LTO itself, of changing this part of
the pass, so I would have to ask you to weigh in on that aspect please. Thanks!

-Lewis

[-- Attachment #2: free-lang-data-diag.txt --]
[-- Type: text/plain, Size: 17061 bytes --]

[PATCH] middle-end: Preserve frontend diagnostics in free-lang-data [PR101551, PR106274]

gcc/ChangeLog:

	PR lto/106274
        PR middle-end/101551
	* ipa-free-lang-data.cc (free_lang_data_in_decl): Preserve
	DECL_CONTEXT for class member functions.
	(free_lang_data): Do not reset frontend diagnostics customizations.

gcc/testsuite/ChangeLog:

	PR lto/106274
        PR middle-end/101551
	* c-c++-common/diag-after-fld-1.c: New test.
	* g++.dg/diag-after-fld-1.C: New test.
	* g++.dg/diag-after-fld-2.C: New test.
	* gfortran.dg/allocatable_uninitialized_2.f90: New test.
	* objc.dg/diag-after-fld-1.m: New test.

diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc
index ccdbf849c25..391b7689639 100644
--- a/gcc/ipa-free-lang-data.cc
+++ b/gcc/ipa-free-lang-data.cc
@@ -682,10 +682,8 @@ free_lang_data_in_decl (tree decl, class free_lang_data_d *fld)
      devirutalization code we always walk through aliases and we need
      context to be preserved too.  See PR89335  */
   if (TREE_CODE (decl) != FIELD_DECL
-      && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
-          || (!DECL_VIRTUAL_P (decl)
-	      && (TREE_CODE (decl) != FUNCTION_DECL
-		  || !DECL_CXX_DESTRUCTOR_P (decl)))))
+      && TREE_CODE (decl) != VAR_DECL
+      && TREE_CODE (decl) != FUNCTION_DECL)
     DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl));
 }
 
@@ -1115,7 +1113,6 @@ free_lang_data (void)
   /* Reset some langhooks.  Do not reset types_compatible_p, it may
      still be used indirectly via the get_alias_set langhook.  */
   lang_hooks.dwarf_name = lhd_dwarf_name;
-  lang_hooks.decl_printable_name = gimple_decl_printable_name;
   lang_hooks.gimplify_expr = lhd_gimplify_expr;
   lang_hooks.overwrite_decl_assembler_name = lhd_overwrite_decl_assembler_name;
   lang_hooks.print_xnode = lhd_print_tree_nothing;
@@ -1141,9 +1138,6 @@ free_lang_data (void)
      make sure we never call decl_assembler_name on local symbols and
      devise a separate, middle-end private scheme for it.  */
 
-  /* Reset diagnostic machinery.  */
-  tree_diagnostics_defaults (global_dc);
-
   rebuild_type_inheritance_graph ();
 
   delete fld_incomplete_types;
diff --git a/gcc/testsuite/c-c++-common/diag-after-fld-1.c b/gcc/testsuite/c-c++-common/diag-after-fld-1.c
new file mode 100644
index 00000000000..c1fc87a03f3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/diag-after-fld-1.c
@@ -0,0 +1,25 @@
+/* Make sure that post-ipa-free-lang-data diagnostics expand macros as
+   expected.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto -O2 -Wnonnull-compare" } */
+
+#define X(p) p == 0 /* { dg-warning {-Wnonnull-compare} } */
+int f (void *) __attribute__((nonnull));
+int f (void *p)
+{
+  return X (p); /* { dg-note {in expansion of macro 'X'} } */
+}
+
+#define X2(p) p == 0 /* { dg-warning {-Wnonnull-compare} } */
+#define Y2(p) X2(p) /* { dg-note {in expansion of macro 'X2'} } */
+
+#define MAKE_F2 \
+  int f2 (void *) __attribute__((nonnull)); \
+  int f2 (void *p) \
+  { \
+    return Y2 (p); /* { dg-note {in expansion of macro 'Y2'} } */ \
+  }
+
+MAKE_F2 /* { dg-note {in expansion of macro 'MAKE_F2'} } */
diff --git a/gcc/testsuite/g++.dg/diag-after-fld-1.C b/gcc/testsuite/g++.dg/diag-after-fld-1.C
new file mode 100644
index 00000000000..0f8d10b7848
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diag-after-fld-1.C
@@ -0,0 +1,470 @@
+/* Based on 20090121-1.C; verify these middle-end diagnostics work fine after
+   ipa-free-lang data pass.  Try to exercise most code paths in cp/error.cc:
+   dump_function_decl(), i.e. various combinations of templates and member
+   functions.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto -Wuninitialized -O2" } */
+
+int y;
+
+int w1 (int)
+{
+  struct S
+  {
+    S () /* { dg-regexp {.*: In constructor 'w1\(int\)::S::S\(\)':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S(), y;
+}
+
+template<typename T>
+int w2 (T)
+{
+  struct S
+  {
+    S () /* { dg-regexp {.*: In constructor 'w2\(T\)::S::S\(\) \[with T = char\]':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S (), y;
+}
+int w2a ()
+{
+  return w2('\0');
+}
+
+int w3 (int)
+{
+  struct S
+  {
+    void f2 () /* { dg-regexp {.*: In member function 'void w3\(int\)::S::f2\(\)':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S().f2 (), y;
+}
+
+template<typename T>
+int w4 (T)
+{
+  struct S
+  {
+    void f2 () /* { dg-regexp {.*: In member function 'void w4\(T\)::S::f2\(\) \[with T = char\]':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S().f2 (), y;
+}
+int w4a ()
+{
+  return w4 ('\0');
+}
+
+struct A1
+{
+  A1 () /* { dg-regexp {.*: In constructor 'A1::A1\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  void f2 () /* { dg-regexp {.*: In member function 'void A1::f2\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename T>
+  void f3 () /* { dg-regexp {.*: In member function 'void A1::f3\(\) \[with T = char\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  static int f4() /* { dg-regexp {.*: In static member function 'static int A1::f4\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  operator int () /* { dg-regexp {.*: In member function 'A1::operator int\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename T>
+  operator T* () /* { dg-regexp {.*: In member function 'A1::operator T\*\(\) \[with T = char\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+    return 0;
+  }
+};
+
+int f1 ()
+{
+  return A1 (), y;
+}
+
+int f2 ()
+{
+  return A1 ().f2 (), y;
+}
+
+int f3 ()
+{
+  return A1 ().f3<char> (), y;
+}
+
+int f4 ()
+{
+  return A1 ().f4 ();
+}
+
+int f5 ()
+{
+  return A1 ();
+}
+
+int f6 ()
+{
+  char *s = A1 ();
+  return y;
+}
+
+template<typename T>
+struct A2
+{
+  A2 () /* { dg-regexp {.*: In constructor 'A2<T>::A2\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  void f2 () /* { dg-regexp {.*: In member function 'void A2<T>::f2\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename U>
+  void f3 () /* { dg-regexp {.*: In member function 'void A2<T>::f3\(\) \[with U = char; T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  static int f4() /* { dg-regexp {.*: In static member function 'static int A2<T>::f4\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  operator int () /* { dg-regexp {.*: In member function 'A2<T>::operator int\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename U>
+  operator U* () /* { dg-regexp {.*: In member function 'A2<T>::operator U\*\(\) \[with U = char; T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+    return 0;
+  }
+};
+
+int g1 ()
+{
+  return A2<void> (), y;
+}
+
+int g2 ()
+{
+  return A2<void> ().f2 (), y;
+}
+
+int g3 ()
+{
+  return A2<void> ().f3<char> (), y;
+}
+
+int g4 ()
+{
+  return A2<void> ().f4 ();
+}
+
+int g5 ()
+{
+  return A2<void> ();
+}
+
+int g6 ()
+{
+  char *s = A2<void> ();
+  return y;
+}
+
+struct A3
+{
+  ~A3 () /* { dg-regexp {.*: In destructor 'A3::~A3\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+};
+
+int h1 ()
+{
+  {
+    A3 a;
+  }
+  return y;
+}
+
+/* Let's do it all again inside a namespace too.  */
+namespace N {
+
+int w1 (int)
+{
+  struct S
+  {
+    S () /* { dg-regexp {.*: In constructor 'N::w1\(int\)::S::S\(\)':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S(), y;
+}
+
+template<typename T>
+int w2 (T)
+{
+  struct S
+  {
+    S () /* { dg-regexp {.*: In constructor 'N::w2\(T\)::S::S\(\) \[with T = char\]':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S (), y;
+}
+int w2a ()
+{
+  return w2('\0');
+}
+
+int w3 (int)
+{
+  struct S
+  {
+    void f2 () /* { dg-regexp {.*: In member function 'void N::w3\(int\)::S::f2\(\)':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S().f2 (), y;
+}
+
+template<typename T>
+int w4 (T)
+{
+  struct S
+  {
+    void f2 () /* { dg-regexp {.*: In member function 'void N::w4\(T\)::S::f2\(\) \[with T = char\]':} } */
+    {
+      int x; /* { dg-note {'x' was declared here} } */
+      y = x; /* { dg-warning {'x' is used uninitialized} } */
+    }
+  };
+  return S().f2 (), y;
+}
+int w4a ()
+{
+  return w4 ('\0');
+}
+
+struct A1
+{
+  A1 () /* { dg-regexp {.*: In constructor 'N::A1::A1\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  void f2 () /* { dg-regexp {.*: In member function 'void N::A1::f2\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename T>
+  void f3 () /* { dg-regexp {.*: In member function 'void N::A1::f3\(\) \[with T = char\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  static int f4() /* { dg-regexp {.*: In static member function 'static int N::A1::f4\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  operator int () /* { dg-regexp {.*: In member function 'N::A1::operator int\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename T>
+  operator T* () /* { dg-regexp {.*: In member function 'N::A1::operator T\*\(\) \[with T = char\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+    return 0;
+  }
+
+};
+
+int f1 ()
+{
+  return A1 (), y;
+}
+
+int f2 ()
+{
+  return A1 ().f2 (), y;
+}
+
+int f3 ()
+{
+  return A1 ().f3<char> (), y;
+}
+
+int f4 ()
+{
+  return A1 ().f4 ();
+}
+
+int f5 ()
+{
+  return A1 ();
+}
+
+int f6 ()
+{
+  char *s = A1 ();
+  return y;
+}
+
+template<typename T>
+struct A2
+{
+  A2 () /* { dg-regexp {.*: In constructor 'N::A2<T>::A2\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  void f2 () /* { dg-regexp {.*: In member function 'void N::A2<T>::f2\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename U>
+  void f3 () /* { dg-regexp {.*: In member function 'void N::A2<T>::f3\(\) \[with U = char; T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  static int f4() /* { dg-regexp {.*: In static member function 'static int N::A2<T>::f4\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  operator int () /* { dg-regexp {.*: In member function 'N::A2<T>::operator int\(\) \[with T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+
+  template<typename U>
+  operator U* () /* { dg-regexp {.*: In member function 'N::A2<T>::operator U\*\(\) \[with U = char; T = void\]':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+    return 0;
+  }
+
+};
+
+int g1 ()
+{
+  return A2<void> (), y;
+}
+
+int g2 ()
+{
+  return A2<void> ().f2 (), y;
+}
+
+int g3 ()
+{
+  return A2<void> ().f3<char> (), y;
+}
+
+int g4 ()
+{
+  return A2<void> ().f4 ();
+}
+
+int g5 ()
+{
+  return A2<void> ();
+}
+
+int g6 ()
+{
+  char *s = A2<void> ();
+  return y;
+}
+
+struct A3
+{
+  ~A3 () /* { dg-regexp {.*: In destructor 'N::A3::~A3\(\)':} } */
+  {
+    int x; /* { dg-note {'x' was declared here} } */
+    y = x; /* { dg-warning {'x' is used uninitialized} } */
+  }
+};
+
+int h1 ()
+{
+  {
+    A3 a;
+  }
+  return y;
+}
+
+
+} /* namespace N */
diff --git a/gcc/testsuite/g++.dg/diag-after-fld-2.C b/gcc/testsuite/g++.dg/diag-after-fld-2.C
new file mode 100644
index 00000000000..ffc5ae5c635
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diag-after-fld-2.C
@@ -0,0 +1,27 @@
+/* Continuation of diag-after-fld-1.C for C++11 features.  */
+/* { dg-do compile { target c++11 } } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto -Wuninitialized -O2" } */
+
+int y;
+
+int f1 (int)
+{
+  return [] () { /* { dg-regexp {.*: In lambda function:} } */
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  } (); 
+}
+
+template<typename T>
+int f2 (T)
+{
+  return [] () { /* { dg-regexp {.*: In lambda function:} } */
+    int x; /* { dg-note {'x' was declared here} } */
+    return y = x; /* { dg-warning {'x' is used uninitialized} } */
+  } (); 
+}
+int f2a ()
+{
+  return f2('\0');
+}
diff --git a/gcc/testsuite/gfortran.dg/allocatable_uninitialized_2.f90 b/gcc/testsuite/gfortran.dg/allocatable_uninitialized_2.f90
new file mode 100644
index 00000000000..9664ad63ba9
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocatable_uninitialized_2.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-require-effective-target lto }
+! { dg-options "-O -Wall -flto" }
+! { dg-additional-options "-fdiagnostics-show-caret -fdiagnostics-show-line-numbers" }
+
+program main
+  real,allocatable:: a(:),b(:)
+
+   a(1)=2*b(1)
+
+end
+
+! { dg-allow-blank-lines-in-output 1 }
+! { dg-regexp {.*allocatable_uninitialized_2\.f90:9:14:\n\n    9 \|    a\(1\)=2\*b\(1\)\n      \|              \^\nWarning: 'b.offset' is used uninitialized \[-Wuninitialized\]\n} }
+! { dg-regexp {.*allocatable_uninitialized_2\.f90:7:30:\n\n    7 \|   real,allocatable:: a\(:\),b\(:\)\n      \|                              \^\nnote: 'b' declared here\n} }
+! { dg-regexp {.*allocatable_uninitialized_2\.f90:9:14:\n\n    9 \|    a\(1\)=2\*b\(1\)\n      \|              \^\nWarning: 'a.offset' is used uninitialized \[-Wuninitialized\]\n} }
+! { dg-regexp {.*allocatable_uninitialized_2\.f90:7:25:\n\n    7 \|   real,allocatable:: a\(:\),b\(:\)\n      \|                         \^\nnote: 'a' declared here\n} }
diff --git a/gcc/testsuite/objc.dg/diag-after-fld-1.m b/gcc/testsuite/objc.dg/diag-after-fld-1.m
new file mode 100644
index 00000000000..3bc4aa18997
--- /dev/null
+++ b/gcc/testsuite/objc.dg/diag-after-fld-1.m
@@ -0,0 +1,24 @@
+/* Make sure that post-ipa-free-lang-data diagnostics call objc_printable_name
+   as expected.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto -O2 -Wuninitialized" } */
+
+#include "../objc-obj-c++-shared/TestsuiteObject.m"
+
+@interface T : TestsuiteObject
++ (void) f;
+@end
+
+int y;
+
+@implementation T
+/* This dg-regexp is the main test; prior to this patch, we output the mangled
+   name '_c_T__f' here.  */
++ (void) f /* { dg-regexp {.*: In function '\+\[T f\]':} } */
+{
+  int x; /* { dg-note {'x' was declared here} } */
+  y = x; /* { dg-warning {-Wuninitialized} } */
+}
+@end