sammccall added a reviewer: aaron.ballman.
sammccall marked 4 inline comments as done.
sammccall added a subscriber: aaron.ballman.
sammccall added a comment.
+Aaron for guidance & in case I'm making things up, TypeLocs/TSI have always
confused me...
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13895-13896
Context.getTrivialTypeSourceInfo(BaseCtor->getType(), UsingLoc);
FunctionProtoTypeLoc ProtoLoc =
TInfo->getTypeLoc().IgnoreParens().castAs<FunctionProtoTypeLoc>();
----------------
ymandel wrote:
> I'm now suspicious of all TypeLocs in this code, and If I'm reading this code
> correctly, this typeloc is never actually used.
My understanding is TypeLoc is really just a Type* + handle into some storage,
TypeSourceInfo is typically that storage.
So TInfo is storage to hold the location info for a function type, initialized
to ~empty ("trivial").
Then ProtoLoc is an accessor that we use to mutate that storage
(`ProtoLoc.setParam` below).
Meanwhile we pass TInfo in as the TypeSourceInfo for the function.
The net effect: if we ask for the function's typeloc, and then ask that loc for
the param decls, we'll get them, just the same as if we ask the function for
its param decls directly.
That said, see next comment.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13908
CXXConstructorDecl *DerivedCtor = CXXConstructorDecl::Create(
Context, Derived, UsingLoc, NameInfo, TInfo->getType(), TInfo,
BaseCtor->getExplicitSpecifier(), getCurFPFeatures().isFPConstrained(),
----------------
ymandel wrote:
> Once you're at it, should this use actually be nullptr?
... maybe? For implicit copy-constructors, the TSI is nullptr
(SemaDeclCXX.cpp:15425).
That suggests that being able to get at the params via the TypeLoc isn't
actually a critical invariant.
I've done this as I agree TypeSourceInfo for implicit code is dubious, and I
was able to construct a similar ASTMatcher misbehavior that this fixes.
@aaron.ballman does this seem right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158766/new/
https://reviews.llvm.org/D158766
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
