Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 15, 2024

Use const pointers for various Init objects in Diagnostic Emitter. This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

@jurahul jurahul changed the title [Clang][TableGen] Use const pointer for various Init pointers. [Clang][TableGen] Use const pointers for various Init objects in Diagnostic Emitter Oct 15, 2024
@jurahul jurahul force-pushed the const_init_clang_diagnostic_emitter branch from a9ded78 to 2fe2957 Compare October 15, 2024 15:14
@jurahul jurahul marked this pull request as ready for review October 15, 2024 19:40
@jurahul jurahul requested a review from erichkeane October 15, 2024 19:40
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 15, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/pr-subscribers-clang

Author: Rahul Joshi (jurahul)

Changes

Use const pointers for various Init objects in Diagnostic Emitter. This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089


Full diff: https://github.com/llvm/llvm-project/pull/112318.diff

1 Files Affected:

  • (modified) clang/utils/TableGen/ClangDiagnosticsEmitter.cpp (+6-6)
diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 325d63de1563de..72fb8eec94db31 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -83,7 +83,7 @@ getCategoryFromDiagGroup(const Record *Group,
 static std::string getDiagnosticCategory(const Record *R,
                                          DiagGroupParentMap &DiagGroupParents) {
   // If the diagnostic is in a group, and that group has a category, use it.
-  if (DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
+  if (const DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
     // Check the diagnostic's diag group for a category.
     std::string CatName = getCategoryFromDiagGroup(Group->getDef(),
                                                    DiagGroupParents);
@@ -161,7 +161,7 @@ static void groupDiagnostics(ArrayRef<const Record *> Diags,
 
   for (unsigned i = 0, e = Diags.size(); i != e; ++i) {
     const Record *R = Diags[i];
-    DefInit *DI = dyn_cast<DefInit>(R->getValueInit("Group"));
+    const DefInit *DI = dyn_cast<DefInit>(R->getValueInit("Group"));
     if (!DI)
       continue;
     assert(R->getValueAsDef("Class")->getName() != "CLASS_NOTE" &&
@@ -359,7 +359,7 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic,
     const Record *R = Diags[i];
     if (isExtension(R) && isOffByDefault(R)) {
       DiagsSet.insert(R);
-      if (DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
+      if (const DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
         const Record *GroupRec = Group->getDef();
         if (!isSubGroupOfGroup(GroupRec, "pedantic")) {
           markGroup(GroupRec);
@@ -378,7 +378,7 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic,
     // Check if the group is implicitly in -Wpedantic.  If so,
     // the diagnostic should not be directly included in the -Wpedantic
     // diagnostic group.
-    if (DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group")))
+    if (const DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group")))
       if (groupInPedantic(Group->getDef()))
         continue;
 
@@ -1443,7 +1443,7 @@ void clang::EmitClangDiagsDefs(const RecordKeeper &Records, raw_ostream &OS,
     // Check if this is an error that is accidentally in a warning
     // group.
     if (isError(R)) {
-      if (DefInit *Group = dyn_cast<DefInit>(R.getValueInit("Group"))) {
+      if (const DefInit *Group = dyn_cast<DefInit>(R.getValueInit("Group"))) {
         const Record *GroupRec = Group->getDef();
         const std::string &GroupName =
             std::string(GroupRec->getValueAsString("GroupName"));
@@ -1478,7 +1478,7 @@ void clang::EmitClangDiagsDefs(const RecordKeeper &Records, raw_ostream &OS,
 
     // Warning group associated with the diagnostic. This is stored as an index
     // into the alphabetically sorted warning group table.
-    if (DefInit *DI = dyn_cast<DefInit>(R.getValueInit("Group"))) {
+    if (const DefInit *DI = dyn_cast<DefInit>(R.getValueInit("Group"))) {
       std::map<std::string, GroupInfo>::iterator I = DiagsInGroup.find(
           std::string(DI->getDef()->getValueAsString("GroupName")));
       assert(I != DiagsInGroup.end());

@jurahul jurahul force-pushed the const_init_clang_diagnostic_emitter branch from 2fe2957 to 1629f6e Compare October 15, 2024 20:12
Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the const auto * suggestion from @erichkeane .

@github-actions
Copy link

github-actions bot commented Oct 15, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@jurahul jurahul force-pushed the const_init_clang_diagnostic_emitter branch from 1629f6e to ba4f37f Compare October 15, 2024 20:17
@jurahul jurahul merged commit ffc5b19 into llvm:main Oct 15, 2024
8 checks passed
@jurahul jurahul deleted the const_init_clang_diagnostic_emitter branch October 15, 2024 22:49
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…nostic Emitter (llvm#112318)

Use const pointers for various Init objects in Diagnostic Emitter. This
is a part of effort to have better const correctness in TableGen
backends:


https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants