erichkeane added a comment.
Patch seems sound, just a nit that we don't need a bunch of the asserts here,
since the 'get' function already asserts it looks. I'd like others to take a
look too, but this looks acceptable to me.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1157
+ continue;
+ assert(PVD->hasUninstantiatedDefaultArg());
+ Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
----------------
This assert is unncessary, the line below will assert. Same with the similar
'assert' later on.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1159
+ Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
+ // FIXME: Obtain the source location for the '=' token.
+ SourceLocation EqualLoc = UninstExpr->getBeginLoc();
----------------
Looks like we don't store the equals-token location, so any attempt here
probably would have to have ParmVarDecl store this location somewhere?
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2545
+ bool ForCallExpr) {
+ assert(Param->hasUninstantiatedDefaultArg());
+
----------------
Here too.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2303
+ // that downstream diagnostics are omitted.
+ assert(PVD->hasUninstantiatedDefaultArg());
+ Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
----------------
See above.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2705
+ // that downstream diagnostics are omitted.
+ assert(Params[P]->hasUninstantiatedDefaultArg());
+ Expr *UninstExpr = Params[P]->getUninstantiatedDefaultArg();
----------------
And again.
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
