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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806
+    bool InTemplateDefinition =
+        getLangOpts().CPlusPlus && CurContext->isDependentContext();
+
----------------
shafik wrote:
> 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?
Shafik: (if you mean Template Definition Context), yes more or less.  IIRC, that term was added to the standard after the clang Templates were implemented (I remember someone mentioning a big standards 'template' rewrite a few years back), so thats perhaps why it wasn't named that.  You can get the same from an expression (isInstantiationDependent, isValueDependent, and isTypeDependent, the latter two having language equivalence, and the former just a catch-all).

Feel free to write up some documentation on this in the internals manual, as long as you properly explain the Template Definition Context/etc means.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144285