[PATCH] D100854: [TableGen] Change assertion information from a tuple to a struct [NFC]

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/TableGen/Record.h:1479
+
+    AssertionInfo(SMLoc Loc, Init *Condition, Init *Message) :
+        Loc(Loc), Condition(Condition), Message(Message) {}
----------------
Could add a comment saying this ctor is here only so you can use make_unique (& I think C++20 will make this work without a user-defined/declared ctor like this (when we eventually adopt C++20, which is a long way off))

Alternatively, since there's only one make_unique, might be just as well to skip the ctor and have the construction code do this:
```
std::unique_ptr<AssertionInfo> x(new AssertionInfo{Loc, Condition, Message});
```
(maybe also with a comment there about why make_unique here until C++20 due to the lack of a ctor)


================
Comment at: llvm/lib/TableGen/TGParser.cpp:368
   if (E.Assertion) {
-    CheckAssert(std::get<0>(*E.Assertion), std::get<1>(*E.Assertion), 
-                std::get<2>(*E.Assertion));
+    CheckAssert((*E.Assertion).AssertionInfo::Loc,
+                (*E.Assertion).AssertionInfo::Condition, 
----------------
This is some very strange/non-idiomatic code. Why are the member accesses qualified like this?

I'd expect this code to look more like: `E.Assertion->Loc` for instance


================
Comment at: llvm/lib/TableGen/TGParser.cpp:3227
   } else {
-    std::unique_ptr<Record::AssertionTuple> Tuple =
-         std::make_unique<Record::AssertionTuple>(ConditionLoc, Condition, Message);
-    addEntry(std::move(Tuple));
+    std::unique_ptr<Record::AssertionInfo> Info =
+         std::make_unique<Record::AssertionInfo>(ConditionLoc, Condition,
----------------
You can use 'auto' for the type here, since it's clear from the make_unique on the right what the type of this variable is.

Or you can roll it all into the addEntry call below like this:
```
addEntry(std::make_unique<Record::AssertionInfo>(ConditionLoc, Condition, Message));
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100854/new/

https://reviews.llvm.org/D100854