[PATCH] D44478: TableGen: Streamline how defs are instantiated
Artem Belevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 14 12:15:11 PDT 2018
tra added a comment.
Just in time. :-) I've ran into the broken foreach-in-multiclass yesterday.
================
Comment at: lib/TableGen/TGParser.cpp:2453
+ CurRec = make_unique<Record>(Records.getNewAnonymousName(), DefLoc, Records,
+ true);
+ else
----------------
Nit: `/*Anonymous=*/true`.
================
Comment at: lib/TableGen/TGParser.cpp:2798
// Keep track of the new generated record definitions.
- std::vector<Record*> NewRecDefs;
+ std::vector<std::unique_ptr<Record>> NewRecDefs;
----------------
Number of items in a multiclass is relatively small. Perhaps we could/should use SmallVector here.
================
Comment at: lib/TableGen/TGParser.cpp:2853-2856
+ // TODO: This MUST happen before template argument resolution. This
+ // does not make sense and should be changed, but at the time of
+ // writing, there are existing .td files which rely on this
+ // implementation detail. It's a bad idea and should be fixed.
----------------
It would be good to have an example or a pointer to something specific so whoever gets to read it later has an ises what needs to be fixed. Perhaps it would make sense to add a disabled test case that would demonstrate the issue.
================
Comment at: lib/TableGen/TGParser.cpp:2867-2868
+ //
+ // TODO: Whether the name is resolved is basically determined by magic.
+ // Unfortunately, existing .td files depend on it.
+ R.set(StringInit::get("NAME"), DefmName);
----------------
Same here.
Repository:
rL LLVM
https://reviews.llvm.org/D44478
More information about the llvm-commits
mailing list