Shameless bump:<br><br>clang_stringrefize_diags_plus.diff replaces char const* by llvm::StringRef in the DiagnosticIDs interface, and updates the  ProcessWarningOptions in lib/FrontEnd/Warnings.cpp  and PragmaDiagnosticHandler::HandlePragma in lib/Lex/Pragma.cpp to take advantage of this.<br>
<br>Please review.<br><br><div class="gmail_quote">2011/5/1 Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com">matthieu.monrocq@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="gmail_quote"><div><div></div><div class="h5"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex">


Date: Sun, 1 May 2011 15:45:33 +0200<br>
From: Matthieu Monrocq <<a href="mailto:matthieu.monrocq@gmail.com" target="_blank">matthieu.monrocq@gmail.com</a>><br>
Subject: [cfe-commits] Improving Clang Diagnostics [1.3]: StringRefize<br>
        DiagnosticIDs API<br>
To: <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
Message-ID: <BANLkTim=<a href="mailto:6Zk6VYSWQ0-EDKZ3xYUJmATktw@mail.gmail.com" target="_blank">6Zk6VYSWQ0-EDKZ3xYUJmATktw@mail.gmail.com</a>><br>
Content-Type: text/plain; charset="iso-8859-1"<br>
<br>
Hello,<br>
<br>
This patch StringRefize DiagnosticIDs API (and internals).<br>
<br>
I had some troubles because of macro expansion: not easy to see that a<br>
StringRef was built from a null pointer and track down the crash... but<br>
could the issue via an assert in the StringRef class itself. I've submitted<br>
the patch for this already, hopefully it'll get applied so that others have<br>
an easier time figuring out what's going wrong when they introduce<br>
StringRef.<br>
<br>
Apart from this, there are two functions that could perhaps benefit from<br>
deeper changes, in the clients of DiagnosticIDs:<br>
- ProcessWarningOptions in lib/FrontEnd/Warnings.cpp, which deals with<br>
processing the -W family of options. There is a lot of manipulation of<br>
OptStart / OptEnd pointers that I think could be easily translated into<br>
using StringRef<br>
- PragmaDiagnosticHandler::HandlePragma in lib/Lex/Pragma.cpp, I believe<br>
WarningName could be a StringRef, but I need to investigate what<br>
Literal.GetString returns to be sure it's possible<br>
<br>
I'll handle this as two follow-up patches, dependent on this one, unless you<br>
think it's better to merge the changes into this patch.<br>
<br>
Please review.<br>
Matthieu.<br>
-------------- next part --------------<br>
An HTML attachment was scrubbed...<br>
URL: <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110501/51ef5ee2/attachment.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110501/51ef5ee2/attachment.html</a><br>


-------------- next part --------------<br>
A non-text attachment was scrubbed...<br>
Name: clang_stringrefize_diags.diff<br>
Type: application/octet-stream<br>
Size: 16999 bytes<br>
Desc: not available<br>
Url : <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110501/51ef5ee2/attachment.obj" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110501/51ef5ee2/attachment.obj</a><br>


<br>
------------------------------<br></blockquote></div></div><div><br>I had some time on my hands so I went ahead and did the patches.<br><br>The patch in lib/Lex/Pragma.cpp is relatively innocuous and to be frank does not bring much to the table (syntaxically), it removes a construction of  std::string  anyway so might be worth it.<br>

<br>The patch in lib/Frontend/Warnings.cpp is much more important. It really cleans up the code consequently, trading awkward pointer differences and manual call to memcmp for  llvm::StringRef  smart interface (especially for prefix comparisons).<br>

<br>Both patches are based on the patch mentionned above.<br><br>I also provide a "all-in-one" patch (clang_stringrefize_diags_plus.diff), which bundles the StringRefication of DiagnosticIDs API with those two patches, for simpler application.<br>

<br>Please review.<br>Matthieu.<br></div></div>
</blockquote></div><br>