tahonermann added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2460-2475
- FunctionDecl *OwningFunc = cast<FunctionDecl>(OldParm->getDeclContext());
- if (OwningFunc->isInLocalScopeForInstantiation()) {
- // Instantiate default arguments for methods of local classes (DR1484)
- // and non-defining declarations.
- Sema::ContextRAII SavedContext(*this, OwningFunc);
- LocalInstantiationScope Local(*this, true);
- ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
----------------
This functionality is now incorporated into `Sema::SubstDefaultArguments()` and
postponed until after construction of enclosing declaration contexts.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2592-2613
+ if (ForCallExpr) {
+ // Check the expression as an initializer for the parameter.
+ InitializedEntity Entity
+ = InitializedEntity::InitializeParameter(Context, Param);
+ InitializationKind Kind = InitializationKind::CreateCopy(
+ Param->getLocation(),
+ /*FIXME:EqualLoc*/ PatternExpr->getBeginLoc());
----------------
I find this code exceedingly troubling, but I haven't been able to figure out
how to unify it. Conceptually, instantiation of a default argument should
produce the same result (the instantiated default argument is cached after
all!) whether it is implicitly always instantiated (as in a declaration in
local scope or a friend function definition) or instantiated on demand for a
call expression. However, my attempts to unify this code have all resulted in
test failures that I have, so far, been unable to explain. Note that much of
the code in the `ForCallExpr` case is replicated in the
`ConvertParamDefaultArgument()` function. I think this should be investigated
further and may be a reason to postpone approving this change.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2138
+ else if (D->isLocalExternDecl()) {
+ LexicalDC = SemaRef.CurContext;
+ }
----------------
This seems highly questionable to me. I suspect there is a better way to
identify the correct context, but I have not so far found it elsewhere. Note
that the context needed is for the enclosing function template specialization
(not the enclosing template).
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2296-2311
+ for (ParmVarDecl *PVD : Function->parameters()) {
+ if (!PVD->hasDefaultArg())
+ continue;
+ if (SemaRef.SubstDefaultArgument(D->getInnerLocStart(), PVD,
TemplateArgs)) {
+ // If substitution fails, the default argument is set to a
+ // RecoveryExpr that wraps the uninstantiated default argument so
+ // that downstream diagnostics are omitted.
----------------
This code is effectively replicated in three distinct locations. It is probably
worth moving it into a helper function somewhere.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4565
- /*DiscardedValue*/ false);
- if (Result.isInvalid())
return true;
----------------
The above removed code has been incorporated into
`Sema::SubstDefaultArgument()`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133500/new/
https://reviews.llvm.org/D133500
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
