[patch] tblgen: Modularize the diagnostic emitter

Ben Langmuir blangmuir at apple.com
Tue May 6 14:17:49 PDT 2014


Hi Tobias,

LGTM, your refactoring made this much clearer to me as a first-time reader of this code. A couple of minor things:

> +/// @brief Emit the array of diagnostic subgroups.
> +///

nitpick: \brief seems to be the generally accepted style

> +/// The array of diagnostic subgroups contains for each group a list of its
> +/// subgroups. The individual lists are separated by '-1'. Groups with no
> +/// subgroups are skipped.
> +///
> +///   static const int16_t DiagSubGroups[] = {
> +///     /* Empty */ -1,
> +///     /* DiagSubGroup0 */ 142, -1,
> +///     /* DiagSubGroup13 */ 265, 322, 399, -1
> +///   }

\code, \endcode

>    for (std::map<std::string, GroupInfo>::const_iterator
> -       I = DiagsInGroup.begin(), E = DiagsInGroup.end(); I != E; ++I) {
> +           I = DiagsInGroup.begin(),
> +           E = DiagsInGroup.end();
> +       I != E; ++I) {

Range-based for maybe?

Ben


On May 6, 2014, at 12:49 AM, Tobias Grosser <tobias at grosser.es> wrote:

> Ping.
> 
> On 26/02/2014 13:23, Tobias Grosser wrote:
>> Hi,
>> 
>> I would like to commit this patch (mostly mechanical)
>> 
>> [PATCH] tblgen: Modularize the diagnostic emitter
>> 
>> Replace a large monolithic function with per-table functions which all
>> nicely fit on my screen. I also added documentation to each function
>> that describes what kind of tables are generated and which information
>> is contained. Finally, I run clang-format over the moved code.
>> 
>> I spent a significant amount of time to understand this code when
>> reasoning about possible extensions to the diagnostic interface to
>> support 'remark' diagnostics. This change will definitely help such an
>> implementation, but already by itself it will save other people a lot of
>> time when trying to understand this functionality.
>> 
>> Even though the patch touches the full function, it is mostly
>> mechanical. No functional change intended. The generated tblgen files
>> are identical.
>> ---
>>  utils/TableGen/ClangDiagnosticsEmitter.cpp | 257
>> +++++++++++++++++++++--------
>>  1 file changed, 185 insertions(+), 72 deletions(-)
>> 
>> As it touches a couple of lines, I would appreciate a quick review.
>> 
>> Cheers,
>> Tobias
>> 
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> 
> <0001-tblgen-Modularize-the-diagnostic-emitter.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list