[patch] tblgen: Modularize the diagnostic emitter

Tobias Grosser tobias at grosser.es
Thu Feb 27 12:34:45 PST 2014


Copying Craig as he was the last one with several major edits to the file.

I do not think we currently need this for the 'remark' change, but the
patch by itself remains useful.

Cheers,
Tobias

On 02/26/2014 01:23 PM, 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
>

-------------- next part --------------
>From a8e2a43457e2bed7be7f119a08f889c86e5cdbfa Mon Sep 17 00:00:00 2001
From: Tobias Grosser <tobias at grosser.es>
Date: Wed, 26 Feb 2014 11:48:17 +0100
Subject: [PATCH] tblgen: Modularize the diagnostic emitter

Replace a large monolitic 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(-)

diff --git a/utils/TableGen/ClangDiagnosticsEmitter.cpp b/utils/TableGen/ClangDiagnosticsEmitter.cpp
index db159d1..c429cb3 100644
--- a/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -581,58 +581,26 @@ static std::string getDiagCategoryEnum(llvm::StringRef name) {
   return enumName.str();
 }
 
-namespace clang {
-void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
-  // Compute a mapping from a DiagGroup to all of its parents.
-  DiagGroupParentMap DGParentMap(Records);
-
-  std::vector<Record*> Diags =
-    Records.getAllDerivedDefinitions("Diagnostic");
-
-  std::vector<Record*> DiagGroups
-    = Records.getAllDerivedDefinitions("DiagGroup");
-
-  std::map<std::string, GroupInfo> DiagsInGroup;
-  groupDiagnostics(Diags, DiagGroups, DiagsInGroup);
-
-  // All extensions are implicitly in the "pedantic" group.  Record the
-  // implicit set of groups in the "pedantic" group, and use this information
-  // later when emitting the group information for Pedantic.
-  RecordVec DiagsInPedantic;
-  RecordVec GroupsInPedantic;
-  InferPedantic inferPedantic(DGParentMap, Diags, DiagGroups, DiagsInGroup);
-  inferPedantic.compute(&DiagsInPedantic, &GroupsInPedantic);
-
-  // Walk through the groups emitting an array for each diagnostic of the diags
-  // that are mapped to.
-  OS << "\n#ifdef GET_DIAG_ARRAYS\n";
-  unsigned MaxLen = 0;
-  OS << "static const int16_t DiagArrays[] = {\n"
-     << "  /* Empty */ -1,\n";
-  for (std::map<std::string, GroupInfo>::const_iterator
-       I = DiagsInGroup.begin(), E = DiagsInGroup.end(); I != E; ++I) {
-    MaxLen = std::max(MaxLen, (unsigned)I->first.size());
-    const bool IsPedantic = I->first == "pedantic";
-
-    const std::vector<const Record*> &V = I->second.DiagsInGroup;
-    if (!V.empty() || (IsPedantic && !DiagsInPedantic.empty())) {
-      OS << "  /* DiagArray" << I->second.IDNo << " */ ";
-      for (unsigned i = 0, e = V.size(); i != e; ++i)
-        OS << "diag::" << V[i]->getName() << ", ";
-      // Emit the diagnostics implicitly in "pedantic".
-      if (IsPedantic) {
-        for (unsigned i = 0, e = DiagsInPedantic.size(); i != e; ++i)
-          OS << "diag::" << DiagsInPedantic[i]->getName() << ", ";
-      }
-      OS << "-1,\n";
-    }
-  }
-  OS << "};\n\n";
-
+/// @brief Emit the array of diagnostic subgroups.
+///
+/// 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
+///   }
+///
+static void emitDiagSubGroups(std::map<std::string, GroupInfo> &DiagsInGroup,
+                              RecordVec &GroupsInPedantic, raw_ostream &OS) {
   OS << "static const int16_t DiagSubGroups[] = {\n"
      << "  /* Empty */ -1,\n";
   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) {
     const bool IsPedantic = I->first == "pedantic";
 
     const std::vector<std::string> &SubGroups = I->second.SubGroups;
@@ -640,7 +608,7 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
       OS << "  /* DiagSubGroup" << I->second.IDNo << " */ ";
       for (unsigned i = 0, e = SubGroups.size(); i != e; ++i) {
         std::map<std::string, GroupInfo>::const_iterator RI =
-          DiagsInGroup.find(SubGroups[i]);
+            DiagsInGroup.find(SubGroups[i]);
         assert(RI != DiagsInGroup.end() && "Referenced without existing?");
         OS << RI->second.IDNo << ", ";
       }
@@ -648,9 +616,9 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
       if (IsPedantic) {
         for (unsigned i = 0, e = GroupsInPedantic.size(); i != e; ++i) {
           const std::string &GroupName =
-            GroupsInPedantic[i]->getValueAsString("GroupName");
+              GroupsInPedantic[i]->getValueAsString("GroupName");
           std::map<std::string, GroupInfo>::const_iterator RI =
-            DiagsInGroup.find(GroupName);
+              DiagsInGroup.find(GroupName);
           assert(RI != DiagsInGroup.end() && "Referenced without existing?");
           OS << RI->second.IDNo << ", ";
         }
@@ -660,34 +628,127 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
     }
   }
   OS << "};\n\n";
+}
 
-  StringToOffsetTable GroupNames;
+/// @brief Emit the list of diagnostic arrays.
+///
+/// This data structure is a large array that contains itself arrays of varying
+/// size. Each array represents a list of diagnostics. The different arrays are
+/// separated by the value '-1'.
+///
+///   static const int16_t DiagArrays[] = {
+///     /* Empty */ -1,
+///     /* DiagArray1 */ diag::warn_pragma_message,
+///                      -1,
+///     /* DiagArray2 */ diag::warn_abs_too_small,
+///                      diag::warn_unsigned_abs,
+///                      diag::warn_wrong_absolute_value_type,
+///                      -1
+///   };
+///
+static void emitDiagArrays(std::map<std::string, GroupInfo> &DiagsInGroup,
+                           RecordVec &DiagsInPedantic, raw_ostream &OS) {
+  OS << "static const int16_t DiagArrays[] = {\n"
+     << "  /* Empty */ -1,\n";
   for (std::map<std::string, GroupInfo>::const_iterator
-         I = DiagsInGroup.begin(), E = DiagsInGroup.end(); I != E; ++I) {
-    // Store a pascal-style length byte at the beginning of the string.
-    std::string Name = char(I->first.size()) + I->first;
-    GroupNames.GetOrAddStringOffset(Name, false);
+           I = DiagsInGroup.begin(),
+           E = DiagsInGroup.end();
+       I != E; ++I) {
+    const bool IsPedantic = I->first == "pedantic";
+
+    const std::vector<const Record *> &V = I->second.DiagsInGroup;
+    if (!V.empty() || (IsPedantic && !DiagsInPedantic.empty())) {
+      OS << "  /* DiagArray" << I->second.IDNo << " */ ";
+      for (unsigned i = 0, e = V.size(); i != e; ++i)
+        OS << "diag::" << V[i]->getName() << ", ";
+      // Emit the diagnostics implicitly in "pedantic".
+      if (IsPedantic) {
+        for (unsigned i = 0, e = DiagsInPedantic.size(); i != e; ++i)
+          OS << "diag::" << DiagsInPedantic[i]->getName() << ", ";
+      }
+      OS << "-1,\n";
+    }
   }
+  OS << "};\n\n";
+}
 
+/// @brief Emit a list of group names.
+///
+/// This creates a long string which by itself contains a list of pascal style
+/// strings, which consist of a length byte directly followed by the string.
+///
+///   static const char DiagGroupNames[] = {
+///     \000\020#pragma-messages\t#warnings\020CFString-literal"
+///   };
+static void emitDiagGroupNames(StringToOffsetTable &GroupNames,
+                               raw_ostream &OS) {
   OS << "static const char DiagGroupNames[] = {\n";
   GroupNames.EmitString(OS);
   OS << "};\n\n";
+}
 
+/// @brief Emit diagnostic arrays and related data structures.
+///
+/// This creates the actual diagnostic array, an array of diagnostic subgroups
+/// and an array of subgroup names.
+///
+///  #ifdef GET_DIAG_ARRAYS
+///     static const int16_t DiagArrays[];
+///     static const int16_t DiagSubGroups[];
+///     static const char DiagGroupNames[];
+///  #endif
+static void emitAllDiagArrays(std::map<std::string, GroupInfo> &DiagsInGroup,
+                              RecordVec &DiagsInPedantic,
+                              RecordVec &GroupsInPedantic,
+                              StringToOffsetTable &GroupNames,
+                              raw_ostream &OS) {
+  OS << "\n#ifdef GET_DIAG_ARRAYS\n";
+  emitDiagArrays(DiagsInGroup, DiagsInPedantic, OS);
+  emitDiagSubGroups(DiagsInGroup, GroupsInPedantic, OS);
+  emitDiagGroupNames(GroupNames, OS);
   OS << "#endif // GET_DIAG_ARRAYS\n\n";
+}
+
+/// @brief Emit diagnostic table.
+///
+/// The table is sorted by the name of the diagnostic group. Each element
+/// consists of the name of the diagnostic group (given as offset in the
+/// group name table), a reference to a list of diagnostics (optional) and a
+/// reference to a set of subgroups (optional).
+///
+/// #ifdef GET_DIAG_TABLE
+///  {/* abi */              159, /* DiagArray11 */ 19, /* Empty */          0},
+///  {/* aggregate-return */ 180, /* Empty */        0, /* Empty */          0},
+///  {/* all */              197, /* Empty */        0, /* DiagSubGroup13 */ 3},
+///  {/* deprecated */       1981,/* DiagArray1 */ 348, /* DiagSubGroup3 */  9},
+/// #endif
+static void emitDiagTable(std::map<std::string, GroupInfo> &DiagsInGroup,
+                          RecordVec &DiagsInPedantic,
+                          RecordVec &GroupsInPedantic,
+                          StringToOffsetTable &GroupNames, raw_ostream &OS) {
+  unsigned MaxLen = 0;
+
+  for (std::map<std::string, GroupInfo>::const_iterator
+           I = DiagsInGroup.begin(),
+           E = DiagsInGroup.end();
+       I != E; ++I)
+    MaxLen = std::max(MaxLen, (unsigned)I->first.size());
 
-  // Emit the table now.
   OS << "\n#ifdef GET_DIAG_TABLE\n";
   unsigned SubGroupIndex = 1, DiagArrayIndex = 1;
   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) {
     // Group option string.
     OS << "  { /* ";
     if (I->first.find_first_not_of("abcdefghijklmnopqrstuvwxyz"
                                    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-                                   "0123456789!@#$%^*-+=:?")!=std::string::npos)
-      PrintFatalError("Invalid character in diagnostic group '" +
-                      I->first + "'");
-    OS << I->first << " */ " << std::string(MaxLen-I->first.size(), ' ');
+                                   "0123456789!@#$%^*-+=:?") !=
+        std::string::npos)
+      PrintFatalError("Invalid character in diagnostic group '" + I->first +
+                      "'");
+    OS << I->first << " */ " << std::string(MaxLen - I->first.size(), ' ');
     // Store a pascal-style length byte at the beginning of the string.
     std::string Name = char(I->first.size()) + I->first;
     OS << GroupNames.GetOrAddStringOffset(Name, false) << ", ";
@@ -696,12 +757,12 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
     const bool IsPedantic = I->first == "pedantic";
 
     // Diagnostics in the group.
-    const std::vector<const Record*> &V = I->second.DiagsInGroup;
-    const bool hasDiags = !V.empty() ||
-                          (IsPedantic && !DiagsInPedantic.empty());
+    const std::vector<const Record *> &V = I->second.DiagsInGroup;
+    const bool hasDiags =
+        !V.empty() || (IsPedantic && !DiagsInPedantic.empty());
     if (hasDiags) {
-      OS << "/* DiagArray" << I->second.IDNo << " */ "
-         << DiagArrayIndex << ", ";
+      OS << "/* DiagArray" << I->second.IDNo << " */ " << DiagArrayIndex
+         << ", ";
       if (IsPedantic)
         DiagArrayIndex += DiagsInPedantic.size();
       DiagArrayIndex += V.size() + 1;
@@ -711,8 +772,8 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
 
     // Subgroups.
     const std::vector<std::string> &SubGroups = I->second.SubGroups;
-    const bool hasSubGroups = !SubGroups.empty() ||
-                              (IsPedantic && !GroupsInPedantic.empty());
+    const bool hasSubGroups =
+        !SubGroups.empty() || (IsPedantic && !GroupsInPedantic.empty());
     if (hasSubGroups) {
       OS << "/* DiagSubGroup" << I->second.IDNo << " */ " << SubGroupIndex;
       if (IsPedantic)
@@ -721,18 +782,70 @@ void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
     } else {
       OS << "/* Empty */         0";
     }
+
     OS << " },\n";
   }
   OS << "#endif // GET_DIAG_TABLE\n\n";
+}
 
-  // Emit the category table next.
+/// @brief Emit the table of diagnostic categories.
+///
+/// The table has the form of macro calls that have two parameters. The
+/// category's name as well as an enum that represents the category. The
+/// table can be used by defining the macro 'CATEGORY' and including this
+/// table right after.
+///
+/// #ifdef GET_CATEGORY_TABLE
+///   CATEGORY("Semantic Issue", DiagCat_Semantic_Issue)
+///   CATEGORY("Lambda Issue", DiagCat_Lambda_Issue)
+/// #endif
+static void emitCategoryTable(RecordKeeper &Records, raw_ostream &OS) {
   DiagCategoryIDMap CategoriesByID(Records);
   OS << "\n#ifdef GET_CATEGORY_TABLE\n";
   for (DiagCategoryIDMap::const_iterator I = CategoriesByID.begin(),
-       E = CategoriesByID.end(); I != E; ++I)
+                                         E = CategoriesByID.end();
+       I != E; ++I)
     OS << "CATEGORY(\"" << *I << "\", " << getDiagCategoryEnum(*I) << ")\n";
   OS << "#endif // GET_CATEGORY_TABLE\n\n";
 }
+
+namespace clang {
+void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS) {
+  // Compute a mapping from a DiagGroup to all of its parents.
+  DiagGroupParentMap DGParentMap(Records);
+
+  std::vector<Record *> Diags = Records.getAllDerivedDefinitions("Diagnostic");
+
+  std::vector<Record *> DiagGroups =
+      Records.getAllDerivedDefinitions("DiagGroup");
+
+  std::map<std::string, GroupInfo> DiagsInGroup;
+  groupDiagnostics(Diags, DiagGroups, DiagsInGroup);
+
+  // All extensions are implicitly in the "pedantic" group.  Record the
+  // implicit set of groups in the "pedantic" group, and use this information
+  // later when emitting the group information for Pedantic.
+  RecordVec DiagsInPedantic;
+  RecordVec GroupsInPedantic;
+  InferPedantic inferPedantic(DGParentMap, Diags, DiagGroups, DiagsInGroup);
+  inferPedantic.compute(&DiagsInPedantic, &GroupsInPedantic);
+
+  StringToOffsetTable GroupNames;
+  for (std::map<std::string, GroupInfo>::const_iterator
+           I = DiagsInGroup.begin(),
+           E = DiagsInGroup.end();
+       I != E; ++I) {
+    // Store a pascal-style length byte at the beginning of the string.
+    std::string Name = char(I->first.size()) + I->first;
+    GroupNames.GetOrAddStringOffset(Name, false);
+  }
+
+  emitAllDiagArrays(DiagsInGroup, DiagsInPedantic, GroupsInPedantic, GroupNames,
+                    OS);
+  emitDiagTable(DiagsInGroup, DiagsInPedantic, GroupsInPedantic, GroupNames,
+                OS);
+  emitCategoryTable(Records, OS);
+}
 } // end namespace clang
 
 //===----------------------------------------------------------------------===//
-- 
1.8.1.2



More information about the cfe-commits mailing list