[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

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