[NFC] Avoid potential dereferencing of nullptr.

[aaron.ballman]

clang/lib/AST/ItaniumMangle.cpp

1659

This seems incorrect -- if the declaration name is a destructor, then ND should always be null, right?

clang/lib/CodeGen/CGObjCMac.cpp

1806 ↗

(On Diff #544140)

Hmmm, is assert the correct approach here? It seems to me that a Complex rvalue will trigger this assertion. I think this deserves some additional thinking rather than silencing the static analysis warning. CC @efriedma @rjmccall who might have ideas.

[efriedma]

clang/lib/AST/ItaniumMangle.cpp

1659

Seems okay to me? ND is the destructor declaration.

clang/lib/CodeGen/CGObjCMac.cpp

1806 ↗

(On Diff #544140)

Yes, if a complex value hit this codepath with a null contBB, it would just crash, so the analysis is flagging a real issue.

I'm not sure how to write ObjC code to actually trigger this codepath, though. Probably involves a _Complex _BitInt(128) or something like that.

[aaron.ballman]

clang/lib/AST/ItaniumMangle.cpp

1659

Yeah, you're correct -- I was thinking CXXDestructorDecl was not a NamedDecl, but it's a CXXMethodDecl without a name, so this is fine.

[schittir]

Thank you for the review, @aaron.ballman and @efriedma
Do you recommend any changes here?

[aaron.ballman]

In D156274#4539522, @schittir wrote:

Thank you for the review, @aaron.ballman and @efriedma
Do you recommend any changes here?

The changes to SemaCodeComplete.cpp and ItaniumMangle.cpp are good, the changes in CGObjCMac.cpp require some more work because the static analysis tool found a real bug and we should fix the bug rather than assert it won't be hit. I'd recommend landing the good changes now (they're NFC) and start a new review for the codegen changes.

[schittir]

Remove changes in CGObjCMac.cpp

[aaron.ballman]

LGTM! Are you planning to work on the ObjC issues separately? (Not suggesting you have to! But if you don't intend to, can you file an issue in GitHub so we don't lose track of the problem?)

[schittir]

Test failure seems unrelated to change. Driver/fsanitize.c lit test passes in my local testing. Restarting build.

[schittir]

In D156274#4544804, @aaron.ballman wrote:

LGTM! Are you planning to work on the ObjC issues separately? (Not suggesting you have to! But if you don't intend to, can you file an issue in GitHub so we don't lose track of the problem?)

Hi Aaron,
I will make sure to file a bug report.
Thank you!

[schittir]

In D156274#4545581, @schittir wrote:

Test failure seems unrelated to change. Driver/fsanitize.c lit test passes in my local testing. Restarting build.

The build is now green. Landing the patch.

clang/lib/AST/ItaniumMangle.cpp

case DeclarationName::CXXDestructorName:
     if (ND == Structor)
       // If the named decl is the C++ destructor we're mangling, use the type we
       // were given.
       mangleCXXDtorType(static_cast<CXXDtorType>(StructorType));
     else
       // Otherwise, use the complete destructor name. This is relevant if a
       // class with a destructor is declared within a destructor.
       mangleCXXDtorType(Dtor_Complete);
+    assert(ND);
     writeAbiTags(ND, AdditionalAbiTags);
     break;
 
   case DeclarationName::CXXOperatorName:
     if (ND && Arity == UnknownArity) {
       Arity = cast<FunctionDecl>(ND)->getNumParams();
 
       // If we have a member function, we need to include the 'this' pointer.

clang/lib/Sema/SemaCodeComplete.cpp

ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(), CC);
   if (!PreferredType.isNull())
     Results.setPreferredType(PreferredType);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   // FIXME: results is always empty, this appears to be dead.
-  if (!Results.empty() && NNS->isDependent())
+  if (!Results.empty() && NNS && NNS->isDependent())
     Results.AddResult("template");
 
   // If the scope is a concept-constrained type parameter, infer nested
   // members based on the constraints.
   if (const auto *TTPT =
           dyn_cast_or_null<TemplateTypeParmType>(NNS->getAsType())) {
     for (const auto &R : ConceptInfo(*TTPT, S).members()) {
       if (R.Operator != ConceptInfo::Member::Colons)