[cfe-commits] cfe-commits Digest, Vol 47, Issue 5

Matthieu Monrocq matthieu.monrocq at gmail.com
Sun May 1 08:54:23 PDT 2011


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

I had some time on my hands so I went ahead and did the patches.

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.

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).

Both patches are based on the patch mentionned above.

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.

Please review.
Matthieu.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110501/0063b4b6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_stringrefize_lex_pragma.diff
Type: application/octet-stream
Size: 895 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110501/0063b4b6/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_stringrefize_diags_plus.diff
Type: application/octet-stream
Size: 20804 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110501/0063b4b6/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_stringrefize_frontend_warnings.diff
Type: application/octet-stream
Size: 3999 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110501/0063b4b6/attachment-0002.obj>


More information about the cfe-commits mailing list