[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