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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/docs/ReleaseNotes.rst:62
   directly rather than instantiating the definition from the standard library.
+- Implemented `CWG2518 <https://wg21.link/CWG2518>`_ which allows ``static_assert(false)``
+  not to be ill-formed when its condition is evaluated in a non-instantiated templates.
----------------
I am assuming this will be updated eventually but that version is not the final one and the one that was approved can be found from: https://github.com/cplusplus/papers/issues/1251

and it here: https://cplusplus.github.io/CWG/issues/2518.html

I think a better wording would be `would not be ill-formed when evaluated in the context of a template definition`


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806
+    bool InTemplateDefinition =
+        getLangOpts().CPlusPlus && CurContext->isDependentContext();
+
----------------
erichkeane wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > CplusPlus check is now not really beneficial?  I'm not sure how much it matters now though that these are both just bit-loads.
> > isDependentContext still does a bunch of work, recursively. I think we should keep it!
> Ah, Oh! You're right!  It is the Expr class where this is free/just checking a bit.  Disregard.
So `isDependentContext()` strictly means we are in a definition context? I think it makes sense after reading the implementation but not obvious at first. Maybe that would to document someplace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144285