Skip to content

[Support] Fix CRTP for GraphWriter (NFC) #152811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 10, 2025

Conversation

naveen-seth
Copy link
Contributor

Commit 474bbc1 forgot to change GraphWriterBase's members from private to protected.
Because of this, GraphWriter can't be properly partially specialized using CRTP.
This commit corrects that, allowing GraphWriterBase to be partially specialized using CRTP as intended.

@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2025

@llvm/pr-subscribers-llvm-support

Author: Naveen Seth Hanig (naveen-seth)

Changes

Commit 474bbc1 forgot to change GraphWriterBase's members from private to protected.
Because of this, GraphWriter can't be properly partially specialized using CRTP.
This commit corrects that, allowing GraphWriterBase to be partially specialized using CRTP as intended.


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

1 Files Affected:

  • (modified) llvm/include/llvm/Support/GraphWriter.h (+2)
diff --git a/llvm/include/llvm/Support/GraphWriter.h b/llvm/include/llvm/Support/GraphWriter.h
index af2e5016298e6..58e5b59751b67 100644
--- a/llvm/include/llvm/Support/GraphWriter.h
+++ b/llvm/include/llvm/Support/GraphWriter.h
@@ -62,6 +62,7 @@ LLVM_ABI bool DisplayGraph(StringRef Filename, bool wait = true,
                            GraphProgram::Name program = GraphProgram::DOT);
 
 template <typename GraphType, typename Derived> class GraphWriterBase {
+protected:
   raw_ostream &O;
   const GraphType &G;
   bool RenderUsingHTML = false;
@@ -73,6 +74,7 @@ template <typename GraphType, typename Derived> class GraphWriterBase {
   using child_iterator = typename GTraits::ChildIteratorType;
   DOTTraits DTraits;
 
+private:
   static_assert(std::is_pointer_v<NodeRef>,
                 "FIXME: Currently GraphWriterBase requires the NodeRef type to "
                 "be a pointer.\nThe pointer usage should be moved to "

Commit 474bbc1 forgot to change GraphWriterBase's members from private
to protected. Because of this, GraphWriter cant’t be properly
partially specialized using CRTP.
This commit corrects that, allowing GraphWriterBase to be partially
specialized using CRTP as intended.
@naveen-seth naveen-seth force-pushed the fix-graph-writer-crtp branch from b4bd1a9 to f3a0281 Compare August 10, 2025 18:50
@naveen-seth naveen-seth merged commit a2d353e into llvm:main Aug 10, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants