Skip to content
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

Add support for importing a trivial global C++ function #5033

Merged
merged 18 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions toolchain/check/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ cc_library(
"//toolchain/sem_ir:inst",
"//toolchain/sem_ir:typed_insts",
"@llvm-project//clang:frontend",
"@llvm-project//clang:sema",
"@llvm-project//clang:tooling",
"@llvm-project//llvm:Support",
],
Expand Down
3 changes: 3 additions & 0 deletions toolchain/check/check.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ struct Unit {

// The unit's SemIR, provided as empty and filled in by CheckParseTrees.
SemIR::File* sem_ir;

// The Clang AST owned by `CompileSubcommand`.
std::unique_ptr<clang::ASTUnit>* cpp_ast;
};

// Checks a group of parse trees. This will use imports to decide the order of
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/check_unit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ auto CheckUnit::InitPackageScopeAndImports() -> void {
CARBON_CHECK(context_.scope_stack().PeekIndex() == ScopeIndex::Package);
ImportOtherPackages(namespace_type_id);
ImportCppFiles(context_, unit_and_imports_->unit->sem_ir->filename(),
unit_and_imports_->unit->cpp_ast,
unit_and_imports_->cpp_import_names, fs_);
}

Expand Down
148 changes: 146 additions & 2 deletions toolchain/check/import_cpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <string>

#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Sema/Lookup.h"
#include "clang/Tooling/Tooling.h"
#include "common/raw_string_ostream.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
Expand Down Expand Up @@ -61,7 +62,9 @@ static auto GenerateAst(Context& context, llvm::StringRef importing_file_path,
diagnostic_options.get());
// TODO: Share compilation flags with ClangRunner.
auto ast = clang::tooling::buildASTFromCodeWithArgs(
GenerateCppIncludesHeaderCode(context, imports), {},
GenerateCppIncludesHeaderCode(context, imports),
{// Parse C++ (and not C)
"-x", "c++"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{// Parse C++ (and not C)
"-x", "c++"},
// Parse C++ (and not C).
{"-x", "c++"},

Per style: "Only use line comments (with //, not /* ... */), on a line by themselves"
https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/cpp_style_guide.md

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(importing_file_path + ".generated.cpp_imports.h").str(), "clang-tool",
std::make_shared<clang::PCHContainerOperations>(),
clang::tooling::getClangStripDependencyFileAdjuster(),
Expand Down Expand Up @@ -113,14 +116,19 @@ static auto AddNamespace(Context& context, PackageNameId cpp_package_id,
}

auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path,
std::unique_ptr<clang::ASTUnit>* ast,
llvm::ArrayRef<Parse::Tree::PackagingNames> imports,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs)
-> void {
if (imports.empty()) {
return;
}

auto [ast, ast_has_error] =
CARBON_CHECK(ast);
CARBON_CHECK(!ast->get());
CARBON_CHECK(!context.sem_ir().cpp_ast());

auto [generated_ast, ast_has_error] =
GenerateAst(context, importing_file_path, imports, fs);

PackageNameId package_id = imports.front().package_id;
Expand All @@ -131,10 +139,146 @@ auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path,
auto name_scope_id = AddNamespace(context, package_id, imports);
SemIR::NameScope& name_scope = context.name_scopes().Get(name_scope_id);
name_scope.set_is_closed_import(true);
name_scope.set_is_cpp_scope(true);

*ast = std::move(generated_ast);
context.sem_ir().set_cpp_ast(ast->get());

if (ast_has_error) {
name_scope.set_has_error();
}
}

static auto ClangLookup(Context& context, SemIR::NameId name_id)
-> std::optional<clang::LookupResult> {
clang::ASTUnit* ast = context.sem_ir().cpp_ast();
CARBON_CHECK(ast);
CARBON_CHECK(ast->hasSema());
clang::Sema& sema = ast->getSema();

auto name = context.names().GetAsStringIfIdentifier(name_id);
CARBON_CHECK(name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we try to look up a non-identifier Carbon name, such as via Cpp.Self or Cpp.base? Perhaps we should produce a lookup failure for non-identifier names for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test and now using GetIRBaseName().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My general understanding is we want to require raw keyword syntax in this sort of situation: i.e., Cpp.r#base, with Cpp.base being an error. So GetAsStringIfIdentifier was correct, rather than GetIRBaseName (which I think mainly exists for debug output).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


clang::LookupResult lookup(
sema,
clang::DeclarationNameInfo(
clang::DeclarationName(
sema.getPreprocessor().getIdentifierInfo(*name)),
clang::SourceLocation()),
clang::Sema::LookupNameKind::LookupOrdinaryName);

if (!sema.LookupQualifiedName(
lookup, ast->getASTContext().getTranslationUnitDecl())) {
return std::nullopt;
}

return lookup;
}

static auto ImportFunctionDecl(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id,
SemIR::NameId name_id,
const clang::FunctionDecl* clang_decl)
-> SemIR::InstId {
if (clang_decl->isVariadic()) {
context.TODO(loc_id, "Unsupported: Variadic function");
return SemIR::InstId::None;
}
if (!clang_decl->isGlobal()) {
context.TODO(loc_id, "Unsupported: Non-global function");
return SemIR::InstId::None;
}
if (clang_decl->getTemplatedKind() != clang::FunctionDecl::TK_NonTemplate) {
context.TODO(loc_id, "Unsupported: Template function");
return SemIR::InstId::None;
}
if (!clang_decl->param_empty()) {
context.TODO(loc_id, "Unsupported: Function with parameters");
return SemIR::InstId::None;
}
if (!clang_decl->getReturnType()->isVoidType()) {
context.TODO(loc_id, "Unsupported: Function with non-void return type");
return SemIR::InstId::None;
}

auto function_decl = SemIR::FunctionDecl{
SemIR::TypeId::None, SemIR::FunctionId::None, SemIR::InstBlockId::Empty};
auto decl_id = AddPlaceholderInst(
context, SemIR::LocIdAndInst(Parse::NodeId::None, function_decl));

auto function_info = SemIR::Function{
SemIR::EntityWithParamsBase{
.name_id = name_id,
.parent_scope_id = scope_id,
.generic_id = SemIR::GenericId::None,
.first_param_node_id = Parse::NodeId::None,
.last_param_node_id = Parse::NodeId::None,
.pattern_block_id = SemIR::InstBlockId::Empty,
.implicit_param_patterns_id = SemIR::InstBlockId::Empty,
.param_patterns_id = SemIR::InstBlockId::Empty,
.call_params_id = SemIR::InstBlockId::Empty,
.is_extern = false,
.extern_library_id = SemIR::LibraryNameId ::None,
.non_owning_decl_id = SemIR::InstId::None,
.first_owning_decl_id = decl_id,
.definition_id = SemIR::InstId::None},
SemIR::FunctionFields{
.return_slot_pattern_id = SemIR::InstId::None,
.virtual_modifier = SemIR::FunctionFields::VirtualModifier::None,
.self_param_id = SemIR::InstId::None}};

function_decl.function_id = context.functions().Add(function_info);

function_decl.type_id = GetFunctionType(context, function_decl.function_id,
SemIR::SpecificId::None);

ReplaceInstBeforeConstantUse(context, decl_id, function_decl);

return decl_id;
}

static auto ImportNameDecl(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id, SemIR::NameId name_id,
const clang::NamedDecl* clang_decl)
-> SemIR::InstId {
if (const auto* clang_function_decl =
clang::dyn_cast<clang::FunctionDecl>(clang_decl)) {
return ImportFunctionDecl(context, loc_id, scope_id, name_id,
clang_function_decl);
}

context.TODO(loc_id, llvm::formatv("Unsupported: Declaration type {0}",
clang_decl->getDeclKindName())
.str());
return SemIR::InstId::None;
}

auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id, SemIR::NameId name_id)
-> SemIR::InstId {
auto lookup = ClangLookup(context, name_id);
if (!lookup) {
return SemIR::InstId::None;
}

DiagnosticAnnotationScope annotate_diagnostics(
&context.emitter(), [&](auto& builder) {
CARBON_DIAGNOSTIC(InCppNameLookup, Note,
"in `Cpp` name lookup for `{0}`", SemIR::NameId);
builder.Note(loc_id, InCppNameLookup, name_id);
});

if (!lookup->isSingleResult()) {
context.TODO(loc_id,
llvm::formatv("Unsupported: Lookup succeeded but couldn't "
"find a single result; LookupResultKind: {0}",
lookup->getResultKind())
.str());
return SemIR::InstId::None;
}

return ImportNameDecl(context, loc_id, scope_id, name_id,
lookup->getFoundDecl());
}

} // namespace Carbon::Check
12 changes: 10 additions & 2 deletions toolchain/check/import_cpp.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,20 @@

namespace Carbon::Check {

// Generates a C++ header that includes the imported cpp files, parses it and
// report errors and warnings. If successful, adds a `Cpp` namespace.
// Generates a C++ header that includes the imported cpp files, parses it,
// generates the AST from it and links `SemIR::File` to it. Report C++ errors
// and warnings. If successful, adds a `Cpp` namespace.
auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path,
std::unique_ptr<clang::ASTUnit>* ast,
llvm::ArrayRef<Parse::Tree::PackagingNames> imports,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) -> void;

// Looks up the given name in the Clang AST generated when importing C++ code.
// If successful, generates the instruction and returns the new `InstId`.
auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id, SemIR::NameId name_id)
-> SemIR::InstId;

} // namespace Carbon::Check

#endif // CARBON_TOOLCHAIN_CHECK_IMPORT_CPP_H_
14 changes: 14 additions & 0 deletions toolchain/check/name_lookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

#include "toolchain/check/generic.h"
#include "toolchain/check/import.h"
#include "toolchain/check/import_cpp.h"
#include "toolchain/check/import_ref.h"
#include "toolchain/check/type_completion.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/sem_ir/name_scope.h"

namespace Carbon::Check {

Expand Down Expand Up @@ -150,6 +152,18 @@ auto LookupNameInExactScope(Context& context, SemIR::LocId loc_id,
scope.import_ir_scopes(), name_id),
SemIR::AccessKind::Public);
}

if (scope.is_cpp_scope()) {
SemIR::InstId imported_inst_id =
ImportNameFromCpp(context, loc_id, scope_id, name_id);
if (imported_inst_id.has_value()) {
SemIR::ScopeLookupResult result = SemIR::ScopeLookupResult::MakeFound(
imported_inst_id, SemIR::AccessKind::Public);
scope.AddRequired({.name_id = name_id, .result = result});
return result;
}
}

return SemIR::ScopeLookupResult::MakeNotFound();
}

Expand Down
Loading