Speculative PrintAsObjC crasher fix

[beccadax]

Before printing the top-level decls in the generated header, PrintAsObjC sorts them to ensure they will be printed in a stable order. This sort starts by evaluating several criteria which ought to distinguish any ValueDecls, then includes extra criteria to sort ExtensionDecls on the same type.

Apple has received a number of automated crash reports on this extension-only part of the sort comparison. Unfortunately, nobody has filed a bug report about this crash and we have not been able to devise a reproducer, but the crash reports are consistent with the comparison misinterpreting other declaration kinds as ExtensionDecls. In theory, the code above this point ought to always return when it isn't processing two ExtensionDecls, but in practice it seems like it doesn't.

Without a reproducer for this bug, we cannot fully fix it, but we can at least make the code more defensive by adding an explicit check that it is only handling ExtensionDecls. This will cause declarations to be printed in the header in a nondeterministic order, so we will emit a warning soliciting a bug report to gather more information about this problem.

Hopefully fixes rdar://81811616, at least to the point of no longer crashing. No new tests because we don't know how to cause this crash.

[beccadax]

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - b73aba4

[beccadax]

@swift-ci smoke test

[xymus]

From the crashlogs these may be triggered by allowing modules with errors. @bnbarham do you think these could be an expected invalid state during such builds?

[bnbarham]

Allowing modules with errors does still output the ObjectiveC header. I have a reproducer based on Becca's comments:

// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-module -o %t/errors.swiftmodule -emit-objc-header-path %t/errors.h -module-name errors -experimental-allow-module-with-compiler-errors %s

import Foundation
@objc class Foo {
}
@objc class Foo {
}

I can look into fixing this one if you'd like @beccadax - it sounds like it may be better to completely drop the duplicate declaration?

[beccadax]

Oh, of course allowing compiler errors could reproduce this. 🤦‍♀️ Do we mark one of the decls invalid? If so, it should be easy to modify DeclAndTypePrinter::shouldInclude() to exclude invalid decls. If not, I'm not quite sure how to handle this—the middle of a sort is a pretty bad time to try to drop an element.

In any case, @bnbarham, I'm happy to let you take the bug if you can fix it properly.

[bnbarham]

Do we mark one of the decls invalid? If so, it should be easy to modify DeclAndTypePrinter::shouldInclude() to exclude invalid decls. If not, I'm not quite sure how to handle this—the middle of a sort is a pretty bad time to try to drop an element.

I was thinking of dropping much earlier, but setting it to invalid is probably a better idea. Thanks so much for investigating this enough to come to the conclusion that duplicate decls could cause it! I'll have a look at fixing this one today/tomorrow.

[bnbarham]

Duplicate decls are indeed marked as invalid @beccadax. I've put up a PR to ignore them in shouldInclude: #38923

[beccadax]

Closing in favor of #38923.