Skip to content

Clean up labelled NOTEs and FIXMEs to use consistent style and wording. #33124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions include/swift/AST/PrintOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,7 @@ struct PrintOptions {
};

/// Whether to print function @convention attribute on function types.
// FIXME: [clang-function-type-serialization] Once we start serializing Clang
// types, we should also start printing the full type in the swiftinterface.
// [TODO: Clang-type-plumbing] Print the full type in the swiftinterface.
FunctionRepresentationMode PrintFunctionRepresentationAttrs =
FunctionRepresentationMode::NameOnly;

Expand Down
5 changes: 2 additions & 3 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -3013,8 +3013,7 @@ class AnyFunctionType : public TypeBase {
static void assertIsFunctionType(const clang::Type *);

ExtInfo(unsigned Bits, Uncommon Other) : Bits(Bits), Other(Other) {
// TODO: [clang-function-type-serialization] Once we start serializing
// the Clang type, we should also assert that the pointer is non-null.
// [TODO: Clang-type-plumbing] Assert that the pointer is non-null.
auto Rep = Representation(Bits & RepresentationMask);
if ((Rep == Representation::CFunctionPointer) && Other.ClangFunctionType)
assertIsFunctionType(Other.ClangFunctionType);
Expand Down Expand Up @@ -4372,7 +4371,7 @@ class SILFunctionType final : public TypeBase, public llvm::FoldingSetNode,
unsigned NumAnyResults : 16; // Not including the ErrorResult.
unsigned NumAnyIndirectFormalResults : 16; // Subset of NumAnyResults.

// [SILFunctionType-layout]
// [NOTE: SILFunctionType-layout]
// The layout of a SILFunctionType in memory is:
// SILFunctionType
// SILParameterInfo[NumParameters]
Expand Down
3 changes: 1 addition & 2 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ namespace swift {
/// Use Clang function types for computing canonical types.
/// If this option is false, the clang function types will still be computed
/// but will not be used for checking type equality.
/// FIXME: [clang-function-type-serialization] This option should be turned
/// on once we start serializing clang function types.
/// [TODO: Clang-type-plumbing] Turn on for feature rollout.
bool UseClangFunctionTypes = false;

/// Whether to use the import as member inference system
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Frontend/ModuleInterfaceSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct ModuleInterfaceOptions {
bool PreserveTypesAsWritten = false;

/// Should we emit the cType when printing @convention(c) or no?
/// FIXME: [clang-function-type-serialization] This check should go away.
/// [TODO: Clang-type-plumbing] This check should go away.
bool PrintFullConvention = false;

/// Copy of all the command-line flags passed at .swiftinterface
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3453,7 +3453,7 @@ CanSILFunctionType SILFunctionType::get(

// All SILFunctionTypes are canonical.

// See [SILFunctionType-layout]
// See [NOTE: SILFunctionType-layout]
bool hasResultCache = normalResults.size() > 1;
size_t bytes =
totalSizeToAlloc<SILParameterInfo, SILResultInfo, SILYieldInfo,
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTDemangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ Type ASTBuilder::createImplFunctionType(
break;
}

// TODO: [store-sil-clang-function-type]
// [TODO: Store-SIL-Clang-type]
auto einfo = SILFunctionType::ExtInfo(representation, flags.isPseudogeneric(),
!flags.isEscaping(), diffKind,
/*clangFunctionType*/ nullptr);
Expand Down
6 changes: 2 additions & 4 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4019,8 +4019,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
break;
case SILFunctionType::Representation::CFunctionPointer:
Printer << "c";
// FIXME: [clang-function-type-serialization] Once we start serializing
// Clang function types, we should be able to remove the second check.
// [TODO: Clang-type-plumbing] Remove the second check.
if (printNameOnly || !info.getUncommonInfo().hasValue())
break;
printCType(Ctx, Printer, info);
Expand Down Expand Up @@ -4086,8 +4085,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
break;
case SILFunctionType::Representation::CFunctionPointer:
Printer << "c";
// FIXME: [clang-function-type-serialization] Once we start serializing
// Clang function types, we should be able to remove the second check.
// [TODO: Clang-type-plumbing] Remove the second check.
if (printNameOnly || !info.getUncommonInfo().hasValue())
break;
printCType(Ctx, Printer, info);
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3407,7 +3407,7 @@ const clang::Type *AnyFunctionType::getCanonicalClangFunctionType() const {
return ty ? ty->getCanonicalTypeInternal().getTypePtr() : nullptr;
}

// TODO: [store-sil-clang-function-type]
// [TODO: Store-SIL-Clang-type]
const clang::FunctionType *SILFunctionType::getClangFunctionType() const {
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3409,7 +3409,7 @@ void ClangModuleUnit::getImportedModules(
if (filter.containsOnly(ModuleDecl::ImportFilterKind::ImplementationOnly))
return;

// [Note: Pure-Clang-modules-privately-import-stdlib]:
// [NOTE: Pure-Clang-modules-privately-import-stdlib]:
// Needed for implicitly synthesized conformances.
if (filter.contains(ModuleDecl::ImportFilterKind::Private))
if (auto stdlib = owner.getStdlibModule())
Expand Down
8 changes: 4 additions & 4 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,10 @@ void CompilerInstance::setUpDiagnosticOptions() {
// this modules was specified as an explicit input to the compiler.
// 4. ModuleInterfaceLoader: Tries to find an up-to-date swiftmodule. If it
// succeeds, it issues a particular "error" (see
// [Note: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]), which
// is interpreted by the overarching loader as a command to use the
// ImplicitSerializedModuleLoader. If we failed to find a .swiftmodule, this falls
// back to using an interface. Actual errors lead to diagnostics.
// [NOTE: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]),
// which is interpreted by the overarching loader as a command to use the
// ImplicitSerializedModuleLoader. If we failed to find a .swiftmodule,
// this falls back to using an interface. Actual errors lead to diagnostics.
// 5. ImplicitSerializedModuleLoader: Loads a serialized module if it can.
// Used for implicit loading of modules from the compiler's search paths.
// 6. ClangImporter: This must come after all the Swift module loaders because
Expand Down
2 changes: 1 addition & 1 deletion lib/Frontend/ModuleInterfaceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ class ModuleInterfaceLoaderImpl {
case ModuleLoadingMode::OnlySerialized:
llvm_unreachable("module interface loader should not have been created");
}
// [Note: ModuleInterfaceLoader-defer-to-SerializedModuleLoader]
// [NOTE: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]
// If there's a module adjacent to the .swiftinterface that we can
// _likely_ load (it validates OK and is up to date), bail early with
// errc::not_supported, so the next (serialized) loader in the chain will
Expand Down
16 changes: 8 additions & 8 deletions lib/FrontendTool/FrontendTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class ABIDependencyEvaluator {
};
} // end anonymous namespace

// See [Note: Bailing-vs-crashing-in-trace-emission].
// See [NOTE: Bailing-vs-crashing-in-trace-emission].
// TODO: Use PrettyStackTrace instead?
void ABIDependencyEvaluator::crashOnInvariantViolation(
llvm::function_ref<void (llvm::raw_string_ostream &)> f) const {
Expand All @@ -380,7 +380,7 @@ void ABIDependencyEvaluator::crashOnInvariantViolation(
#endif
}

// [Note: Trace-Clang-submodule-complexity]
// [NOTE: Trace-Clang-submodule-complexity]
//
// A Clang module may have zero or more submodules. In practice, when traversing
// the imports of a module, we observe that different submodules of the same
Expand All @@ -393,7 +393,7 @@ void ABIDependencyEvaluator::crashOnInvariantViolation(
// branches, so long as we don't try to visit an ancestor when one of its
// descendants is still on the traversal stack, so that we don't end up with
// arbitrarily complex intra-module cycles.
// See also: [Note: Intra-module-leafwards-traversal].
// See also: [NOTE: Intra-module-leafwards-traversal].
// 2. When adding entries to the ABI export map, we need to avoid marking
// dependencies within the same top-level module. This step is needed in
// addition to step 1 to avoid creating cycles like
Expand Down Expand Up @@ -421,7 +421,7 @@ void ABIDependencyEvaluator::reexposeImportedABI(
&& module->isNonSwiftModule()
&& module->getTopLevelModule() == reexport->getTopLevelModule()) {
// Dependencies within the same top-level Clang module are not useful.
// See also: [Note: Trace-Clang-submodule-complexity].
// See also: [NOTE: Trace-Clang-submodule-complexity].
return;
}

Expand Down Expand Up @@ -504,18 +504,18 @@ void ABIDependencyEvaluator::computeABIDependenciesForClangModule(
// There are three cases here which can potentially create cycles:
//
// 1. Clang modules importing the stdlib.
// See [Note: Pure-Clang-modules-privately-import-stdlib].
// See [NOTE: Pure-Clang-modules-privately-import-stdlib].
// 2. Overlay S @_exported-imports underlying module S' and another Clang
// module C'. C' (transitively) #imports S' but it gets treated as if
// C' imports S. This creates a cycle: S -> C' -> ... -> S.
// In practice, this case is hit for
// Darwin (Swift) -> SwiftOverlayShims (Clang) -> Darwin (Swift).
// 3. [Note: Intra-module-leafwards-traversal]
// 3. [NOTE: Intra-module-leafwards-traversal]
// Cycles within the same top-level module.
// These don't matter for us, since we only care about the dependency
// graph at the granularity of top-level modules. So we ignore these
// by only considering parent -> submodule dependencies.
// See also [Note: Trace-Clang-submodule-complexity].
// See also [NOTE: Trace-Clang-submodule-complexity].
if (import->isStdlibModule()) {
continue;
}
Expand Down Expand Up @@ -720,7 +720,7 @@ static void computeSwiftModuleTraceInfo(
});
}

// [Note: Bailing-vs-crashing-in-trace-emission] There are certain edge cases
// [NOTE: Bailing-vs-crashing-in-trace-emission] There are certain edge cases
// in trace emission where an invariant that you think should hold does not hold
// in practice. For example, sometimes we have seen modules without any
// corresponding filename.
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2213,7 +2213,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
}

// Resolve the function type directly with these attributes.
// TODO: [store-sil-clang-function-type]
// [TODO: Store-SIL-Clang-type]
SILFunctionType::ExtInfo extInfo(rep, attrs.has(TAK_pseudogeneric),
attrs.has(TAK_noescape), diffKind,
nullptr);
Expand Down