[PATCH] D125421: [TableGen] Add generation of argument register lists

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/utils/TableGen/CallingConvEmitter.cpp:29
+  std::map<std::string, std::set<std::string>> AssignedSwiftRegsMap;
+  std::map<std::string, std::set<std::string>> DelegateToMap;
+
----------------
nickdesaulniers wrote:
> void wrote:
> > nickdesaulniers wrote:
> > > I know the current code is using `std::string` without explicitly including `<string>`, but would you mind please cleaning that up here (in this patch)?
> > > 
> > > Also curious if we should be preferring containers from ADT?
> > I *really* hate adding headers that aren't necessary. Is this some kind of style guide violation?
> It's definitely a google3 style violation.
> https://google.github.io/styleguide/cppguide.html#Include_What_You_Use
> 
> Looks like it's not though for LLVM:
> https://llvm.org/docs/CodingStandards.html#include-as-little-as-possible
> > You must include all of the header files that you are using — you can include them either directly or indirectly through another header file.
> There's also https://llvm.org/docs/CodingStandards.html#include-style, but that doesn't seem to make a call either way either.
> 
> OK, I'll let that slide, but the comment still stands about using containers from ADT. We have a StringMap and StringSet
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringset-h
> Looks like it's not though for LLVM:

Right. LLVM does things very differently from Google.


================
Comment at: llvm/utils/TableGen/CallingConvEmitter.cpp:327
+void CallingConvEmitter::EmitArgRegisterLists(raw_ostream &O) {
+  using EntryTy = std::pair<std::string, std::set<std::string>>;
+
----------------
nickdesaulniers wrote:
> void wrote:
> > nickdesaulniers wrote:
> > > This is a common case for just using `auto &` in your range for loops.
> > I wish people would make up their minds about whether I should use `auto` or not. One day I shouldn't use it then the next I should. I don't see why I would suddenly use it here.
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> Encourages the use of `auto` when the type doesn't already appear in the RHS of an assignment using a cast (in which case the type would be specified twice) or for iteration variable types.
Using `auto` during in a `for` loop is exactly the complaint I got last time. The type of `Entry`, et al, isn't an iterator but the actual elements. So instead of going back and forth arguing about this, I'm keeping it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125421