Merge branch 'main' into P2278R4_cbegin

Author: frederick-vs-ja

clang/docs/ReleaseNotes.rst

@@ -278,6 +278,8 @@ Improvements to Clang's diagnostics
 
 - Improve the diagnostics for shadows template parameter to report correct location (#GH129060).
 
+- Improve the ``-Wundefined-func-template`` warning when a function template is not instantiated due to being unreachable in modules.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/docs/analyzer/checkers.rst

@@ -543,6 +543,42 @@ Do not attempt to create a std::string from a null pointer
    }
  }
 
+.. _cplusplus-PureVirtualCall:
+
+cplusplus.PureVirtualCall (C++)
+"""""""""""""""""""""""""""""""
+
+When `virtual methods are called during construction and destruction
+<https://en.cppreference.com/w/cpp/language/virtual#During_construction_and_destruction>`__
+the polymorphism is restricted to the class that's being constructed or
+destructed because the more derived contexts are either not yet initialized or
+already destructed.
+
+This checker reports situations where this restricted polymorphism causes a
+call to a pure virtual method, which is undefined behavior. (See also the
+related checker :ref:`optin-cplusplus-VirtualCall` which reports situations
+where the restricted polymorphism affects a call and the called method is not
+pure virtual – but may be still surprising for the programmer.)
+
+.. code-block:: cpp
+
+ struct A {
+   virtual int getKind() = 0;
+
+   A() {
+     // warn: This calls the pure virtual method A::getKind().
+     log << "Constructing " << getKind();
+   }
+   virtual ~A() {
+     releaseResources();
+   }
+   void releaseResources() {
+     // warn: This can call the pure virtual method A::getKind() when this is
+     // called from the destructor.
+     callSomeFunction(getKind())
+   }
+ };
+
 .. _deadcode-checkers:
 
 deadcode
@@ -833,24 +869,40 @@ This checker has several options which can be set from command line (e.g.
 
 optin.cplusplus.VirtualCall (C++)
 """""""""""""""""""""""""""""""""
-Check virtual function calls during construction or destruction.
+
+When `virtual methods are called during construction and destruction
+<https://en.cppreference.com/w/cpp/language/virtual#During_construction_and_destruction>`__
+the polymorphism is restricted to the class that's being constructed or
+destructed because the more derived contexts are either not yet initialized or
+already destructed.
+
+Although this behavior is well-defined, it can surprise the programmer and
+cause unintended behavior, so this checker reports calls that appear to be
+virtual calls but can be affected by this restricted polymorphism.
+
+Note that situations where this restricted polymorphism causes a call to a pure
+virtual method (which is definitely invalid, triggers undefined behavior) are
+**reported by another checker:** :ref:`cplusplus-PureVirtualCall` and **this
+checker does not report them**.
 
 .. code-block:: cpp
 
- class A {
- public:
+ struct A {
+   virtual int getKind();
+
    A() {
-     f(); // warn
+     // warn: This calls A::getKind() even if we are constructing an instance
+     // of a different class that is derived from A.
+     log << "Constructing " << getKind();
    }
-   virtual void f();
- };
-
- class A {
- public:
-   ~A() {
-     this->f(); // warn
+   virtual ~A() {
+     releaseResources();
+   }
+   void releaseResources() {
+     // warn: This can be called within ~A() and calls A::getKind() even if
+     // we are destructing a class that is derived from A.
+     callSomeFunction(getKind())
    }
-   virtual void f();
  };
 
 .. _optin-mpi-MPI-Checker:

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5700,6 +5700,8 @@ def warn_func_template_missing : Warning<"instantiation of function %q0 "
   InGroup<UndefinedFuncTemplate>, DefaultIgnore;
 def note_forward_template_decl : Note<
   "forward declaration of template entity is here">;
+def note_unreachable_template_decl
+    : Note<"unreachable declaration of template entity is here">;
 def note_inst_declaration_hint : Note<"add an explicit instantiation "
   "declaration to suppress this warning if %q0 is explicitly instantiated in "
   "another translation unit">;

clang/include/clang/Sema/Sema.h

@@ -11268,13 +11268,11 @@ class Sema final : public SemaBase {
 
   /// Determine whether we would be unable to instantiate this template (because
   /// it either has no definition, or is in the process of being instantiated).
-  bool DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
-                                      NamedDecl *Instantiation,
-                                      bool InstantiatedFromMember,
-                                      const NamedDecl *Pattern,
-                                      const NamedDecl *PatternDef,
-                                      TemplateSpecializationKind TSK,
-                                      bool Complain = true);
+  bool DiagnoseUninstantiableTemplate(
+      SourceLocation PointOfInstantiation, NamedDecl *Instantiation,
+      bool InstantiatedFromMember, const NamedDecl *Pattern,
+      const NamedDecl *PatternDef, TemplateSpecializationKind TSK,
+      bool Complain = true, bool *Unreachable = nullptr);
 
   /// DiagnoseTemplateParameterShadow - Produce a diagnostic complaining
   /// that the template parameter 'PrevDecl' is being shadowed by a new

clang/lib/Sema/SemaTemplate.cpp

@@ -765,7 +765,7 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
                                           const NamedDecl *Pattern,
                                           const NamedDecl *PatternDef,
                                           TemplateSpecializationKind TSK,
-                                          bool Complain /*= true*/) {
+                                          bool Complain, bool *Unreachable) {
   assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
          isa<VarDecl>(Instantiation));
 
@@ -778,6 +778,8 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
     if (!hasReachableDefinition(const_cast<NamedDecl *>(PatternDef),
                                 &SuggestedDef,
                                 /*OnlyNeedComplete*/ false)) {
+      if (Unreachable)
+        *Unreachable = true;
       // If we're allowed to diagnose this and recover, do so.
       bool Recover = Complain && !isSFINAEContext();
       if (Complain)

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -5386,12 +5386,15 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
       PatternDef = nullptr;
   }
 
+  // True is the template definition is unreachable, otherwise false.
+  bool Unreachable = false;
   // FIXME: We need to track the instantiation stack in order to know which
   // definitions should be visible within this instantiation.
-  if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Function,
-                                Function->getInstantiatedFromMemberFunction(),
-                                     PatternDecl, PatternDef, TSK,
-                                     /*Complain*/DefinitionRequired)) {
+  if (DiagnoseUninstantiableTemplate(
+          PointOfInstantiation, Function,
+          Function->getInstantiatedFromMemberFunction(), PatternDecl,
+          PatternDef, TSK,
+          /*Complain*/ DefinitionRequired, &Unreachable)) {
     if (DefinitionRequired)
       Function->setInvalidDecl();
     else if (TSK == TSK_ExplicitInstantiationDefinition ||
@@ -5416,11 +5419,18 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
       if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
           !getSourceManager().isInSystemHeader(PatternDecl->getBeginLoc())) {
         Diag(PointOfInstantiation, diag::warn_func_template_missing)
-          << Function;
-        Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
-        if (getLangOpts().CPlusPlus11)
-          Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
-              << Function;
+            << Function;
+        if (Unreachable) {
+          // FIXME: would be nice to mention which module the function template
+          // comes from.
+          Diag(PatternDecl->getLocation(),
+               diag::note_unreachable_template_decl);
+        } else {
+          Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
+          if (getLangOpts().CPlusPlus11)
+            Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
+                << Function;
+        }
       }
     }
 

clang/test/Modules/diag-undefined-template.cpp

@@ -0,0 +1,39 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -I%t \
+// RUN:   -Wundefined-func-template \
+// RUN:   -fimplicit-module-maps %t/main.cpp 2>&1 | grep "unreachable declaration of template entity is here"
+
+// Note that the diagnostics are triggered when building the 'hoge' module, which is imported from the main file.
+// The "-verify" flag doesn't work in this case. Instead, we grep the expected text to verify the test.
+
+//--- shared_ptr2.h
+#pragma once
+
+template<class T>
+void x() { }
+
+//--- hoge.h
+#pragma once
+
+#include "shared_ptr2.h"
+
+inline void f() {
+  x<int>();
+}
+
+//--- module.modulemap
+module hoge {
+  header "hoge.h"
+}
+
+module shared_ptr2 {
+  header "shared_ptr2.h"
+}
+
+//--- main.cpp
+#include "hoge.h"
+
+int main() {
+  f();
+}

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -763,7 +763,8 @@ static void threadPrivatizeVars(lower::AbstractConverter &converter,
         commonSyms.insert(common);
       }
       symThreadprivateValue = lower::genCommonBlockMember(
-          converter, currentLocation, *sym, commonThreadprivateValue);
+          converter, currentLocation, sym->GetUltimate(),
+          commonThreadprivateValue);
     } else {
       symThreadprivateValue = genThreadprivateOp(*sym);
     }

flang/test/Lower/OpenMP/threadprivate-common-block-pointer.f90

@@ -0,0 +1,31 @@
+! Simple test for lowering of OpenMP Threadprivate Directive with a pointer var
+! from a common block.
+
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+!RUN: bbc -hlfir -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! Regression test for a compiler crash
+
+module mmm
+integer,pointer::nam1
+common /com1/nam1,nam2
+!$omp threadprivate(/com1/)
+end
+use mmm
+!$omp parallel copyin(nam1)
+!$omp end parallel
+end
+
+
+! CHECK-LABEL:   fir.global common @com1_(dense<0> : vector<28xi8>) {alignment = 8 : i64} : !fir.array<28xi8>
+
+! CHECK-LABEL:   func.func @_QQmain() {
+! CHECK:           %[[VAL_0:.*]] = fir.address_of(@com1_) : !fir.ref<!fir.array<28xi8>>
+! CHECK:           omp.parallel {
+! CHECK:             %[[VAL_17:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<!fir.array<28xi8>> -> !fir.ref<!fir.array<28xi8>>
+! CHECK:             %[[VAL_18:.*]] = fir.convert %[[VAL_17]] : (!fir.ref<!fir.array<28xi8>>) -> !fir.ref<!fir.array<?xi8>>
+! CHECK:             %[[VAL_19:.*]] = arith.constant 0 : index
+! CHECK:             %[[VAL_20:.*]] = fir.coordinate_of %[[VAL_18]], %[[VAL_19]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+! CHECK:             %[[VAL_21:.*]] = fir.convert %[[VAL_20]] : (!fir.ref<i8>) -> !fir.ref<!fir.box<!fir.ptr<i32>>>
+! CHECK:             %[[VAL_22:.*]]:2 = hlfir.declare %[[VAL_21]] {fortran_attrs = #{{.*}}<pointer>, uniq_name = "_QMmmmEnam1"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+

llvm/lib/Target/RISCV/RISCVInstrInfo.td

@@ -752,6 +752,7 @@ def ECALL : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "ecall", "">, Sched<[Write
   let imm12 = 0;
 }
 
+let isTrap = 1 in
 def EBREAK : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "ebreak", "">,
              Sched<[]> {
   let rs1 = 0;
@@ -762,6 +763,7 @@ def EBREAK : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "ebreak", "">,
 // This is a de facto standard (as set by GNU binutils) 32-bit unimplemented
 // instruction (i.e., it should always trap, if your implementation has invalid
 // instruction traps).
+let isTrap = 1 in
 def UNIMP : RVInstI<0b001, OPC_SYSTEM, (outs), (ins), "unimp", "">,
             Sched<[]> {
   let rs1 = 0;