[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

[Aaron Ballman via Phabricator via cfe-commits]

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