[clang] [clangl[TableGen] Change Diagnostic Emitter to use const RecordKeeper (PR #108209)

Rahul Joshi via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 11 08:22:32 PDT 2024


================
@@ -255,20 +257,18 @@ class InferPedantic {
       GMap;
 
   DiagGroupParentMap &DiagGroupParents;
-  const std::vector<Record*> &Diags;
-  const std::vector<Record*> DiagGroups;
+  ArrayRef<const Record *> Diags;
----------------
jurahul wrote:

Yes. If you look at https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

one of the changes in that RecordKeeper::getAllDerivedDefinitions() returns an ArrayRef to an internal cached vector, so this memory is owned by RecordKeeper, which for all TableGen backends is guaranteed to be live.

Here, Diags is set using  getAllDerivedDefinitions(), so should be safe. 

Note that previous code also uses a reference to a std::vector for Diags. However, in the earlier version getAllDerivedDefinitions() returned a vector by value. So, this is a reference to an unnamed temporary. I think C++ requires that the compiler extends the lifetime of the unnamed temp is it sees a reference to it being used, I am assuming that is correct even in the older version when we had the reference pointing to an unnamed temp return value of getAllDerivedDefinitions().

https://github.com/llvm/llvm-project/pull/108209


More information about the cfe-commits mailing list