[PATCH] D158766: [AST] TypeSourceInfo for params of inherited constructor should be null

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13895-13896
       Context.getTrivialTypeSourceInfo(BaseCtor->getType(), UsingLoc);
   FunctionProtoTypeLoc ProtoLoc =
       TInfo->getTypeLoc().IgnoreParens().castAs<FunctionProtoTypeLoc>();
 
----------------
I'm now suspicious of all TypeLocs in this code, and If I'm reading this code correctly, this typeloc is never actually used.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13908
   CXXConstructorDecl *DerivedCtor = CXXConstructorDecl::Create(
       Context, Derived, UsingLoc, NameInfo, TInfo->getType(), TInfo,
       BaseCtor->getExplicitSpecifier(), getCurFPFeatures().isFPConstrained(),
----------------
Once you're at it, should this use actually be nullptr?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13931
         Context, DerivedCtor, UsingLoc, UsingLoc, /*IdentifierInfo=*/nullptr,
-        FPT->getParamType(I), TInfo, SC_None, /*DefArg=*/nullptr);
+        FPT->getParamType(I), nullptr, SC_None, /*DefArg=*/nullptr);
     PD->setScopeInfo(0, I);
----------------
nit: label with `/* TypeSourceInfo=*/`?


================
Comment at: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp:2142-2144
+              typeLoc(loc(asString("int")),
+                      hasAncestor(cxxRecordDecl(hasAnyName("Derived"))))));
+}
----------------
why phrase it bottom up, instead of top-down (matching the record decl and then using hasDescendant)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158766/new/

https://reviews.llvm.org/D158766