[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16805
+    // definition, the declaration has no effect.
+    bool InTemplateDefinition = getLangOpts().CPlusPlus && getTemplateDepth(getCurScope()) != 0;
+
----------------
erichkeane wrote:
> Hmm... interesting way to calculate template depth, I wasn't aware of that one.  Does this cause problems in 'a template caused another template to instantiate' sorta thing? 
> 
> Also, is the "CPlusPlus" test here necessary?
It's what I came up with. Do you think there is a better way?
If you have suggestions for additional tests, I'm all ears!

The  CplusPlus tests is just there to avoid unnecessary cycles in C mode. It probably doesn't do much of a difference.


================
Comment at: clang/test/SemaTemplate/instantiate-var-template.cpp:34
   template<typename T> void f() {
-    static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // expected-error {{static assertion failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}} \
-                                                       // expected-note {{evaluates to '1 == 0'}}
+    static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // fixme: can we check a var is dependant?
   }
----------------
erichkeane wrote:
> You should be able to instantiate this template later, and probably what we now have to do.  Also, 'dependent' is the spelling in this case, 'dependant' is something different :)
I'm afraid doing though would defeat the intent of the test - it is after all named "InstantiationDependent"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144285/new/

https://reviews.llvm.org/D144285