royjacobson added inline comments.

================
Comment at: clang/lib/Sema/SemaOverload.cpp:2967
+         "parameters!");
+  for (FunctionProtoType::param_type_iterator
+           O = OldType->param_type_begin(),
----------------
erichkeane wrote:
> Thanks for the clarification on 'Reversed'.  The comment makes it more clear.
> 
> This whole 'for' header is... really tough to mentally parse, even BEFORE 
> this, and now it is even worse with 'Reversed' involved.  I would prefer that 
> the iterators be initialized ahead of time.  Additionally, can we use 
> `reverse_iterator` for the 'NewType' instead of branching on `Reversed` here? 
>  
> 
> Any other suggestions you have to simplify this loop would also be 
> appreciated.  I might ALSO consider using 'zip' here?
I made it index based which IMO is easier to understand now.

I thought reverse_iterator would be annoying because I would need to make the 
code a template since it's a different type.

Also - ping about the comment in isBetterOverloadCandidate :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to