EricWF marked 4 inline comments as done. EricWF added a comment. In https://reviews.llvm.org/D46740#1097014, @rsmith wrote:
> Thanks, this is great. > > In https://reviews.llvm.org/D46740#1096859, @EricWF wrote: > > > Turns out we have to fully parse the diagnostic text in order to substitute > > modifier indexes, which is a bit of a complication. This patch does exactly > > that. > > > I like that you were able to reuse the existing parsing code for > documentation generation here. With heavy modifications :-) ================ Comment at: docs/InternalsManual.rst:341-342 + ``"candidate %select{function|constructor}3%select{| template| %1}2 not viable"``. +Class: + ``Integers`` +Description: ---------------- rsmith wrote: > I don't think "Class:" makes sense here, since what this specifier consumes > depends on its replacement string. "Class: varies" or similar might work, but > maybe just drop it? Dropping it seems clearer. ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3526-3544 def note_ovl_candidate : Note<"candidate " "%select{function|function|constructor|" - "function |function |constructor |" "is the implicit default constructor|" "is the implicit copy constructor|" "is the implicit move constructor|" "is the implicit copy assignment operator|" "is the implicit move assignment operator|" ---------------- rsmith wrote: > Is there a reason this one wasn't changed to use `%sub`? It refers to implicit special members differently ("is the implicit default constructor" -> "constructor (the implicit default constructor"). I think I mistakenly thought transforming it would be grammatically incorrect. But looking further it seems fine. https://reviews.llvm.org/D46740 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
