[cfe-commits] Patch - PR9548 - "no known conversion from 'vector<string>' to 'vector<string>'"

[Chandler Carruth]

On Mon, Apr 25, 2011 at 8:38 PM, Richard Trieu <rtrieu at google.com> wrote:

> Ping.  Any comments on this patch?


I like the general direction of this patch, but I have one nit, and one more
serious comment.

nit: You should switch back to the enum for the ArgumentKind in your
interface rather than exposing the raw 'char' storage type. This may be made
irrelevant by the major comment though....

My big issue is similar to Sebastian's. This slows down the *building* of
diagnostics. It would be much better to do all of this logic in the
Diagnostic class.

To give a concrete reason, SFINAE: we sometimes build many many diagnostics
merely to detect the presence of them, discard a particular type with those
diagnostics, and then begin semantically analyzing something else. We don't
want to pay for this type of formatting logic in that case. We only want to
pay for it when we are *emitting* the diagnostic.

Another reason; after we've hit the max diagnostic emitted limit, it would
be good to cut any and every corner we can to speed things up.

Finally, I think it will be more semantically clear. No need to funnel an
enum through so many layers, etc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110526/4babd29f/attachment.html>