Use of class template argument deduction

LLVM Project
1

Google’s style guide/experience suggests that misuse of CTAD can lead to maintenance problems in terms of more difficulty changing APIs when users depend on implicit CTAD even where the API design isn’t intended for it: Google C++ Style Guide

Would folks be open to the idea of enabling the -Wctad-maybe-unsupported warning in LLVM to enforce this stylistic choice? So far we’ve only seen one instance of these cropping up with RDFGraph’s Print type being called with CTAD. It looks pretty harmless/probably good for readability, the patch to enable this would be:

diff --git a/llvm/include/llvm/CodeGen/RDFGraph.h b/llvm/include/llvm/CodeGen/RDFGraph.h
index 4a8428fce20a..89811da048d1 100644
--- a/llvm/include/llvm/CodeGen/RDFGraph.h
+++ b/llvm/include/llvm/CodeGen/RDFGraph.h
@@ -934,6 +934,9 @@ namespace rdf {
     const DataFlowGraph &G;
   };

+  template<typename T>
+  Print(const T&, const DataFlowGraph&) -> Print<T>;
+
   template <typename T>
   struct PrintNode : Print<NodeAddr<T>> {
     PrintNode(const NodeAddr<T> &x, const DataFlowGraph &g)

I’ll send out a phab review for the flag addition and this patch shortly - but figured it was worth some broader discussion too.

2

Posted for review here: ⚙ D131727 Enable -Wctad-maybe-unsupported in LLVM build

3

This seems a pretty reasonable tradeoff to me. One question though: What does the warning actually look like? Does it reasonably guide developers into adding explicit deduction guides?

4 
<source>:5:7: warning: 't1<T>' may not intend to support class template argument deduction [-Wctad-maybe-unsupported]
    5 | t1 v(3);
      |       ^
<source>:5:7: note: add a deduction guide to suppress this warning
2 Likes