Mordante marked an inline comment as done.
Mordante added inline comments.

================
Comment at: clang/lib/Parse/ParseDecl.cpp:6673
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {
----------------
rjmccall wrote:
> rjmccall wrote:
> > We don't usually cite bugs like this in the main source code except as 
> > "there's a bug with this, here's where we tracking fixing it".  The comment 
> > explains the purpose of the diagnostic well enough (but please include a 
> > period).
> > 
> > Can you just drop the excess arguments here instead of cutting off parsing?
> Or actually — let's move this out of the parser entirely, because (unlike 
> prototype scope depth) we actually need to diagnose it in Sema as well 
> because of variadic templates.  `Sema::BuildFunctionType` seems like the 
> right place.
> 
> Also, can you `static_assert` that function declarations support at least as 
> many parameter declarations as we allow in types?
I tried your suggestion to move the test to `Sema::BuildFunctionType` but that 
function is only used for templates so it doesn't show the warning for other 
cases. Also the error generated will also print the 
`note_ovl_candidate_substitution_failure` diagnostic making the output rather 
unreadable. So I'll look for a better way to handle the variadic template case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64811



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

Reply via email to