Handles expressions such as:
- std::is_const<T>()
- std::is_const<T>()();
- std::is_same(decltype(U()), V>::value;
Handles expressions such as:
lib/Sema/SemaTemplate.cpp | ||
---|---|---|
3089 ↗ | (On Diff #177681) | It might be more maintainer-proof to write this as std::pair<const char *, const char *> Braces; if (Node->isStdInitListInitialization()) Braces = std::make_pair("", ""); else if (Node->isListInitialization()) Braces = std::make_pair("{", "}"); else Braces = std::make_pair("(", ")"); OS << Braces.first; for (...) { ... } OS << Braces.second; return true; |
test/SemaCXX/static-assert.cpp | ||
136 ↗ | (On Diff #177681) | Conspicuously missing any test for lines 3081–3106 above. IIUC, those lines would trigger on things like template<class T> struct X { int i=0, j=0; constexpr operator bool() const { return false; } }; template<class T> void test() { static_assert(X<T>{}); static_assert(X<T>{1,2}); static_assert(T{0}); static_assert(T(0)); } template void test<int>(); But I guess I don't see why extra code is needed to handle those; shouldn't the pretty-printer handle them already? What do the current diagnostics look like for my example? |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
136 ↗ | (On Diff #177681) | Note: Your examples are CXXFunctionalCastExpr, and CXXUnresolvedConstructExpr respectively, not CXXTemporaryObjectExpr, so they are not handled by this change. They are correctly printed before this change with T==int. The issue comes when adding an extra layer of indirection: struct ExampleTypes { explicit ExampleTypes(int); using T = int; using U = float; }; template<class T> struct X { int i=0; int j=0; constexpr operator bool() const { return false; } }; template<class T> void foo6() { static_assert(X<typename T::T>()); static_assert(X<typename T::T>{}); static_assert(X<typename T::T>{1,2}); static_assert(X<typename T::T>({1,2})); static_assert(typename T::T{0}); static_assert(typename T::T(0)); } template void foo6<ExampleTypes>(); The errors before the change are: File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 71: static_assert failed due to requirement 'X<typename ExampleTypes::T>()' File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 72: static_assert failed due to requirement 'X<typename ExampleTypes::T>{}' File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 73: static_assert failed due to requirement 'X<typename ExampleTypes::T>{1, 2}' File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 74: static_assert failed due to requirement 'X<typename ExampleTypes::T>({1, 2})' File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 75: static_assert failed due to requirement 'typename ExampleTypes::T{0}' File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 76: static_assert failed due to requirement 'typename ExampleTypes::T(0)' I must admit that I did not think of handling these, and now I think it would actually be better to tap into the pretty printer to avoid code duplication. |
include/clang/AST/Stmt.h | ||
---|---|---|
847 ↗ | (On Diff #178012) | I'm pretty wary of long lists of parameters that include booleans with default values. Why is PrintCanonicalTypes not part of the printing policy? I think that's where we should keep all the knobs relating to how the printing happens. |
move PrintQualifiedTypes to PrintingPolicy
include/clang/AST/Stmt.h | ||
---|---|---|
847 ↗ | (On Diff #178012) | Good point, done. |
lib/AST/StmtPrinter.cpp | ||
---|---|---|
78 ↗ | (On Diff #178882) | Spurious formatting changes -- can revert this file. |
lib/AST/TypePrinter.cpp | ||
165 ↗ | (On Diff #178882) | t doesn't meet the usual naming requirements; how about QT? |
lib/Sema/SemaTemplate.cpp | ||
3062 ↗ | (On Diff #178882) | Why are you dropping the explicit here? |
3124 ↗ | (On Diff #178882) | Don't use auto here, please. |
LGTM, but you should wait a few days before committing in case @Quuxplusone has comments.
LGTM. Tiny style suggestions, which I won't mind if you ignore.
include/clang/AST/Type.h | ||
---|---|---|
995 ↗ | (On Diff #178888) | Nit: could this function have been left in-line, and just changed split() to splitAccordingToPolicy(this, Policy)? void getAsStringInternal(std::string &Str, const PrintingPolicy &Policy) const { return getAsStringInternal(splitAccordingToPolicy(Policy), Str, Policy); } But if that would make the code harder to read instead of easier, then don't mind me. |
998 ↗ | (On Diff #178888) | Nit: this function's body hasn't changed, so personally I would have left it alone (not out-of-lined it). |
Thanks for the comments !
include/clang/AST/Type.h | ||
---|---|---|
995 ↗ | (On Diff #178888) | I'd rather avoid polluting the QualType API with splitAccordingToPolicy() which is really only useful for this use case.
Actually this is independent of the above change, done. |