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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to