rjmccall 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()) { ---------------- Mordante wrote: > 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. > Hmm, yes, we should find a way to suppress follow-on errors like that. But we do need to be able to avoid problems even when building parameter lists that don't correspond to a declared function, e.g. with `template <class... T> void foo(void (*fp)(T...));`. (You'll need to pass the template arguments explicitly here, since we obviously can't infer an excessive number of parameters from a function whose parameter count is limited by the same constraint.) And I always forget that function type parsing doesn't go through that function, sorry. 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
