From 48a84ca55d3da26aacde796cff2bae369e87e803 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 3 Dec 2024 12:47:19 -0800 Subject: [PATCH] Restrict the Cpp package name (#4618) We're looking at using this for imports, and it gets awkward if we allow users to declare entries. --- toolchain/check/check.cpp | 21 ++- .../packages/fail_package_main.carbon | 106 ------------- .../restricted_package_names.carbon | 148 ++++++++++++++++++ toolchain/diagnostics/diagnostic_kind.def | 1 + 4 files changed, 165 insertions(+), 111 deletions(-) delete mode 100644 toolchain/check/testdata/packages/fail_package_main.carbon create mode 100644 toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 2767e681a80cc..871337e4a951c 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -545,11 +545,12 @@ static auto GetImportKey(UnitInfo& unit_info, IdentifierId file_package_id, return {package_name, library_name}; } -static constexpr llvm::StringLiteral ExplicitMainName = "Main"; +static constexpr llvm::StringLiteral CppPackageName = "Cpp"; +static constexpr llvm::StringLiteral MainPackageName = "Main"; static auto RenderImportKey(ImportKey import_key) -> std::string { if (import_key.first.empty()) { - import_key.first = ExplicitMainName; + import_key.first = MainPackageName; } if (import_key.second.empty()) { return import_key.first.str(); @@ -573,7 +574,7 @@ static auto TrackImport(Map& api_map, // True if the import has `Main` as the package name, even if it comes from // the file's packaging (diagnostics may differentiate). - bool is_explicit_main = import_key.first == ExplicitMainName; + bool is_explicit_main = import_key.first == MainPackageName; // Explicit imports need more validation than implicit ones. We try to do // these in an order of imports that should be removed, followed by imports @@ -688,6 +689,11 @@ static auto TrackImport(Map& api_map, } else { // The imported api is missing. package_imports.has_load_error = true; + if (!explicit_import_map && import_key.first == CppPackageName) { + // Don't diagnose the implicit import in `impl package Cpp`, because we'll + // have diagnosed the use of `Cpp` in the declaration. + return; + } CARBON_DIAGNOSTIC(LibraryApiNotFound, Error, "corresponding API for '{0}' not found", std::string); CARBON_DIAGNOSTIC(ImportNotFound, Error, "imported API '{0}' not found", @@ -714,9 +720,9 @@ static auto BuildApiMapAndDiagnosePackaging( // Construct a boring key for Main//default. : ImportKey{"", ""}; - // Diagnose explicit `Main` uses before they become marked as possible + // Diagnose restricted package names before they become marked as possible // APIs. - if (import_key.first == ExplicitMainName) { + if (import_key.first == MainPackageName) { CARBON_DIAGNOSTIC(ExplicitMainPackage, Error, "`Main//default` must omit `package` declaration"); CARBON_DIAGNOSTIC( @@ -726,6 +732,11 @@ static auto BuildApiMapAndDiagnosePackaging( import_key.second.empty() ? ExplicitMainPackage : ExplicitMainLibrary); continue; + } else if (import_key.first == CppPackageName) { + CARBON_DIAGNOSTIC(CppPackageDeclaration, Error, + "`Cpp` cannot be used by a `package` declaration"); + unit_info.emitter.Emit(packaging->names.node_id, CppPackageDeclaration); + continue; } bool is_impl = packaging && packaging->is_impl; diff --git a/toolchain/check/testdata/packages/fail_package_main.carbon b/toolchain/check/testdata/packages/fail_package_main.carbon deleted file mode 100644 index fee19e4d79e96..0000000000000 --- a/toolchain/check/testdata/packages/fail_package_main.carbon +++ /dev/null @@ -1,106 +0,0 @@ -// Part of the Carbon Language project, under the Apache License v2.0 with LLVM -// Exceptions. See /LICENSE for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -// AUTOUPDATE -// TIP: To test this file alone, run: -// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/fail_package_main.carbon -// TIP: To dump output, run: -// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/fail_package_main.carbon - -// --- fail_main.carbon - -// CHECK:STDERR: fail_main.carbon:[[@LINE+4]]:1: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] -// CHECK:STDERR: package Main; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -package Main; - -// --- fail_main_impl.carbon - -// CHECK:STDERR: fail_main_impl.carbon:[[@LINE+4]]:6: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] -// CHECK:STDERR: impl package Main; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -impl package Main; - -// --- fail_raw_main.carbon - -// `Main` isn't a keyword, so this fails the same way. -// CHECK:STDERR: fail_raw_main.carbon:[[@LINE+4]]:1: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] -// CHECK:STDERR: package r#Main; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -package r#Main; - -// --- fail_main_lib.carbon - -// CHECK:STDERR: fail_main_lib.carbon:[[@LINE+3]]:1: error: use `library` declaration in `Main` package libraries [ExplicitMainLibrary] -// CHECK:STDERR: package Main library "main_lib"; -// CHECK:STDERR: ^~~~~~~ -package Main library "[[@TEST_NAME]]"; - -// CHECK:STDOUT: --- fail_main.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { -// CHECK:STDOUT: import Core//prelude -// CHECK:STDOUT: import Core//prelude/... -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .Core = imports.%Core -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: --- fail_main_impl.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { -// CHECK:STDOUT: import Core//prelude -// CHECK:STDOUT: import Core//prelude/... -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .Core = imports.%Core -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: --- fail_raw_main.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { -// CHECK:STDOUT: import Core//prelude -// CHECK:STDOUT: import Core//prelude/... -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .Core = imports.%Core -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: --- fail_main_lib.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { -// CHECK:STDOUT: import Core//prelude -// CHECK:STDOUT: import Core//prelude/... -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .Core = imports.%Core -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: } -// CHECK:STDOUT: diff --git a/toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon b/toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon new file mode 100644 index 0000000000000..e02d02f26539b --- /dev/null +++ b/toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon @@ -0,0 +1,148 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/no_prelude/restricted_package_names.carbon + +// --- lowercase_main.carbon + +// The restriction is case-sensitive. +package main; + +// --- fail_main.carbon + +// CHECK:STDERR: fail_main.carbon:[[@LINE+4]]:1: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] +// CHECK:STDERR: package Main; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +package Main; + +// --- fail_main_impl.carbon + +// CHECK:STDERR: fail_main_impl.carbon:[[@LINE+4]]:6: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] +// CHECK:STDERR: impl package Main; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +impl package Main; + +// --- fail_raw_main.carbon + +// `Main` isn't a keyword, so this fails the same way. +// CHECK:STDERR: fail_raw_main.carbon:[[@LINE+4]]:1: error: `Main//default` must omit `package` declaration [ExplicitMainPackage] +// CHECK:STDERR: package r#Main; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +package r#Main; + +// --- fail_main_lib.carbon + +// CHECK:STDERR: fail_main_lib.carbon:[[@LINE+4]]:1: error: use `library` declaration in `Main` package libraries [ExplicitMainLibrary] +// CHECK:STDERR: package Main library "main_lib"; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +package Main library "[[@TEST_NAME]]"; + +// --- lowercase_cpp.carbon + +// The restriction is case-sensitive. +package cpp; + +// --- fail_cpp.carbon + +// CHECK:STDERR: fail_cpp.carbon:[[@LINE+4]]:1: error: `Cpp` cannot be used by a `package` declaration [CppPackageDeclaration] +// CHECK:STDERR: package Cpp; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +package Cpp; + +// --- fail_cpp.impl.carbon + +// CHECK:STDERR: fail_cpp.impl.carbon:[[@LINE+4]]:6: error: `Cpp` cannot be used by a `package` declaration [CppPackageDeclaration] +// CHECK:STDERR: impl package Cpp; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +impl package Cpp; + +// --- fail_raw_cpp.carbon + +// `cpp` isn't a keyword, so this fails the same way. +// CHECK:STDERR: fail_raw_cpp.carbon:[[@LINE+4]]:1: error: `Cpp` cannot be used by a `package` declaration [CppPackageDeclaration] +// CHECK:STDERR: package r#Cpp; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +package r#Cpp; + +// --- fail_cpp_lib.carbon + +// CHECK:STDERR: fail_cpp_lib.carbon:[[@LINE+3]]:1: error: `Cpp` cannot be used by a `package` declaration [CppPackageDeclaration] +// CHECK:STDERR: package Cpp library "cpp_lib"; +// CHECK:STDERR: ^~~~~~~ +package Cpp library "[[@TEST_NAME]]"; + +// CHECK:STDOUT: --- lowercase_main.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_impl.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_raw_main.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_lib.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- lowercase_cpp.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_cpp.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_cpp.impl.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } +// CHECK:STDOUT: %default.import = import +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_raw_cpp.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_cpp_lib.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 0c1ca5e674cdf..97cd57425936b 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -152,6 +152,7 @@ CARBON_DIAGNOSTIC_KIND(ImportNotFound) CARBON_DIAGNOSTIC_KIND(ImportCycleDetected) CARBON_DIAGNOSTIC_KIND(ExplicitMainPackage) CARBON_DIAGNOSTIC_KIND(ExplicitMainLibrary) +CARBON_DIAGNOSTIC_KIND(CppPackageDeclaration) CARBON_DIAGNOSTIC_KIND(ImportMainPackage) CARBON_DIAGNOSTIC_KIND(ImportMainDefaultLibrary) CARBON_DIAGNOSTIC_KIND(ImportCurrentPackageByName)