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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 20 15:17:41 PDT 2021


dblaikie added a comment.

(also a bunch of clang-format auto-comments on this review, please run this change through clang-format-patch or however else you like to do it to address those comments)



================
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, 
----------------
Paul-C-Anagnostopoulos wrote:
> dblaikie wrote:
> > 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
> I was hoping someone would tell me how to simplify this. Why do I need 'AssertionInfo::' this way, but not with '->Loc'?
I don't think you do? You could write it as `(*E.Assertion).Loc` as well. It's pretty uncommon to need to scope a member access like you have here (how'd you came across this syntax/end up writing it this way?) - /sometimes/ it may be useful to scope a member function access (`x.Base::y()`) to access a base member function rather than an override or shadowing in a derived class. (eg: if you want to override a function but delegate some functionality to the orgiinal base function, you could use `Base::func()` to do that delegation - but very rarely for member variable access)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100854



More information about the llvm-commits mailing list