aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.
Adding Erich to look at this WIP for early feedback as he's recently been
looking at template instantiation guts rather deeply, but I added some
questions.
================
Comment at: clang/include/clang/AST/DeclCXX.h:280-282
+ LDKUnknown = 0,
+ LDKAlwaysDependent,
+ LDKNeverDependent,
----------------
(Matches the naming style used just above.)
================
Comment at: clang/lib/Sema/TreeTransform.h:12934
+ // substituting an unevaluated lambda inside of a function's parameter's type
+ // - as parameter type are not instanciated from within a function's DC. We
+ // use isUnevaluatedContext() to distinguish the function parameter case.
----------------
================
Comment at: clang/lib/Sema/TreeTransform.h:12948
+
+ if (OldClass->isDependentContext())
+ getDerived().transformedLocalDecl(OldClass, {Class});
----------------
Can you explain why you only do these transformations when the old class is a
dependent context? Also, should this be looking at
`getDerived().AlwaysRebuild()`? I'm not certain -- we're not calling any
`Rebuild*` functions here because lambdas are not rebuilt but generated
entirely from scratch. So I'm guessing we don't need to look at
`AlwaysRebuild`, but it's a bit unclear.
================
Comment at: clang/lib/Sema/TreeTransform.h:12968-12971
+ if (E->getCallOperator()->isDependentContext()) {
+ getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
+ getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
+ }
----------------
Are these changes necessary as part of this patch? I'm a bit worried that only
doing this in dependent contexts may change behavior for code like this:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L769
================
Comment at: clang/lib/Sema/TreeTransform.h:13040
- getDerived().transformedLocalDecl(OldVD, NewVDs);
+ if (OldVD->isTemplated())
+ getDerived().transformedLocalDecl(OldVD, NewVDs);
----------------
This also seems a bit surprising -- can't the variable declaration be
non-templated but still dependent (e.g., an `auto` variable whose
initialization is dependent?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121532/new/
https://reviews.llvm.org/D121532
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
