aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a formatting nit.



================
Comment at: include/clang/AST/DeclarationName.h:735
+           Name.getNameKind() != DeclarationName::CXXConversionFunctionName)
+      return nullptr;
     return LocInfo.NamedType.TInfo;
----------------
steveire wrote:
> aaron.ballman wrote:
> > Did you investigate the callers of this function to see which ones need a 
> > null pointer check inserted into them? Otherwise, this change turns an 
> > assertion into harder to track down UB. (I'm less worried about the other 
> > changes because those will fail more gracefully.)
> All callers in clang are fine. Here's the output of `git grep -h 
> TypeSourceInfo`:
> 
>       /// getNamedTypeInfo - Returns the source type info associated to
>       TypeSourceInfo *getNamedTypeInfo() const {
>         if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
>         if (auto ToTInfoOrErr = import(From.getNamedTypeInfo()))
>           if (auto TypeNameInfo = Dtor->getNameInfo().getNamedTypeInfo()) {
>         if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
>         if (TypeSourceInfo *OldTInfo = NameInfo.getNamedTypeInfo()) {
>         if (TypeSourceInfo *TSInfo = Name.getNamedTypeInfo())
> 
> clang-tools-extra has no uses of it. Is there anywhere else to check?
> 
I double-checked the call to `import()` and it gracefully handles null pointers 
as well, so I think this change is safe (I cannot find any further instances 
that you didn't find). Thank you for looking into it!


================
Comment at: lib/AST/NestedNameSpecifier.cpp:465
 TypeLoc NestedNameSpecifierLoc::getTypeLoc() const {
-  assert((Qualifier->getKind() == NestedNameSpecifier::TypeSpec ||
-          Qualifier->getKind() == NestedNameSpecifier::TypeSpecWithTemplate) &&
-         "Nested-name-specifier location is not a type");
+  if ((Qualifier->getKind() != NestedNameSpecifier::TypeSpec &&
+          Qualifier->getKind() != NestedNameSpecifier::TypeSpecWithTemplate))
----------------
aaron.ballman wrote:
> Remove spurious parens.
Formatting is still off here for the second line of the if statement (one too 
many spaces on the indentation).


Repository:
  rC Clang

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

https://reviews.llvm.org/D56354



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to