[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