aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplate.cpp:6490-6491 // C++11 [temp.arg.nontype]p1: // - an address constant expression of type std::nullptr_t if (Arg->getType()->isNullPtrType()) ---------------- It's worth noting that the rules we follow changed slightly: http://eel.is/c++draft/temp.arg.nontype#2 so if we're making repairs in this area, are there broader changes we need to consider? (That's fine as a FIXME comment if the changes end up being unrelated to this specific fix.) ================ Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-11.cpp:31 IP<&tl> ip7; // expected-error{{non-type template argument of type 'int *' is not a constant expression}} +IP<(int*)1> ip8; // expected-error {{non-type template argument does not refer to any declaration}} ---------------- shafik wrote: > rsmith wrote: > > efriedma wrote: > > > erichkeane wrote: > > > > shafik wrote: > > > > > shafik wrote: > > > > > > It looks like in C++17 mode we catch this case: > > > > > > https://godbolt.org/z/s43oE5qWE > > > > > Another case to check for: > > > > > > > > > > ``` > > > > > IP<(int*)(1-1)> ip9; > > > > > ``` > > > > > > > > > > In C++11 the wording use to allow `integer constant expressions` > > > > The new diagnostic here is unfortunate. That 'does not refer to any > > > > declaration' doesn't really let me know that it is illegal because that > > > > isn't a NPE. > > > > > > > > The 'treat it as a null ptr' here is obviously awful, but I find myself > > > > wondering if we can do better on this diagnostic trivially enough? > > > It's easy enough to use a different message; we just need to detect the > > > particular case we're considering, print an error, and return NPV_Error. > > > But I'm not sure what a better diagnostic looks like. "standard C++ > > > does not allow using '(int*)1' as a non-type template argument"? (The > > > fact that null is allowed isn't really relevant here.) > > > It looks like in C++17 mode we catch this case: > > > https://godbolt.org/z/s43oE5qWE > > > > That's diagnosing the cast, not the template argument value. You can get > > around the cast diagnostic by forcing constant folding with a > > `__builtin_constant_p` conditional > > ([example](https://godbolt.org/z/8f8WWTGeM)). Then we diagnose as > > > <source>:7:4: error: non-type template argument refers to subobject '(int > > > *)1' > > ... which seems like a worse diagnostic than the C++11 one, because it's > > not even true. > > > That's diagnosing the cast, not the template argument value. You can get > > around the cast diagnostic by forcing constant folding with a > > `__builtin_constant_p` conditional > > ([example](https://godbolt.org/z/8f8WWTGeM)). Then we diagnose as > > > <source>:7:4: error: non-type template argument refers to subobject '(int > > > *)1' > > ... which seems like a worse diagnostic than the C++11 one, because it's > > not even true. > > Great point, I missed that earlier. > > My point was that if we are catching this in C++17 mode maybe we can reuse > the machinery to catch this other modes as well and fix the diagnostic > quality as well. Although I realize that may not be easy but worth at least > understanding. For comparison, ICC diagnoses this with: `error: invalid nontype template argument of type "int *"` and GCC diagnoses it as: `error: 'reinterpret_cast' from integer to pointer` and `error: '1' is not a valid template argument for 'int*' because it is not the address of a variable` Either one of those diagnostics seems like an improvement over our status quo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134928/new/ https://reviews.llvm.org/D134928 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
