diff --git a/toolchain/check/cpp/generate_ast.cpp b/toolchain/check/cpp/generate_ast.cpp index 31b0353fa5e0..ff3efedeacc2 100644 --- a/toolchain/check/cpp/generate_ast.cpp +++ b/toolchain/check/cpp/generate_ast.cpp @@ -681,8 +681,6 @@ class GenerateASTAction : public clang::ASTFrontendAction { auto BeginSourceFileAction(clang::CompilerInstance& /*clang_instance*/) -> bool override { - // TODO: `clang.getPreprocessor().enableIncrementalProcessing();` to avoid - // the TU scope getting torn down before we're done parsing macros. return true; } @@ -699,6 +697,7 @@ class GenerateASTAction : public clang::ASTFrontendAction { clang_instance.getSema(), /*SkipFunctionBodies=*/false); + clang_instance.getPreprocessor().enableIncrementalProcessing(); clang_instance.getPreprocessor().EnterMainSourceFile(); parser_->Initialize(); diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index 90fcc9041efe..8281bf66e4d8 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -2502,9 +2502,10 @@ static auto IsIncompleteClass(Context& context, SemIR::NameScopeId scope_id) // TODO: Add support for other macro types and non-integer literal values. static auto ImportMacro(Context& context, SemIR::LocId loc_id, SemIR::NameScopeId scope_id, SemIR::NameId name_id, + clang::IdentifierInfo* identifier_info, clang::MacroInfo* macro_info) -> SemIR::ScopeLookupResult { - auto inst_id = TryEvaluateMacro(context, loc_id, name_id, macro_info); + auto inst_id = TryEvaluateMacro(context, loc_id, identifier_info, macro_info); if (inst_id == SemIR::ErrorInst::InstId) { return SemIR::ScopeLookupResult::MakeNotFound(); } @@ -2570,7 +2571,8 @@ auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id, if (clang::MacroInfo* macro_info = LookupMacro(context, scope_id, identifier_info)) { - return ImportMacro(context, loc_id, scope_id, name_id, macro_info); + return ImportMacro(context, loc_id, scope_id, name_id, identifier_info, + macro_info); } auto lookup = ClangLookupName(context, scope_id, identifier_info); if (!lookup) { diff --git a/toolchain/check/cpp/macros.cpp b/toolchain/check/cpp/macros.cpp index 8267af39c8cd..a344e21fd0a8 100644 --- a/toolchain/check/cpp/macros.cpp +++ b/toolchain/check/cpp/macros.cpp @@ -11,7 +11,9 @@ #include "clang/Sema/Sema.h" #include "common/check.h" #include "toolchain/check/cpp/constant.h" +#include "toolchain/check/cpp/generate_ast.h" #include "toolchain/check/cpp/import.h" +#include "toolchain/check/cpp/location.h" #include "toolchain/check/literal.h" namespace Carbon::Check { @@ -46,52 +48,49 @@ static auto MapConstant(Context& context, SemIR::LocId loc_id, } auto TryEvaluateMacro(Context& context, SemIR::LocId loc_id, - SemIR::NameId name_id, clang::MacroInfo* macro_info) - -> SemIR::InstId { - auto name_str_opt = context.names().GetAsStringIfIdentifier(name_id); + clang::IdentifierInfo* identifier_info, + clang::MacroInfo* macro_info) -> SemIR::InstId { CARBON_CHECK(macro_info, "macro info missing"); - if (macro_info->getNumTokens() == 0) { context.TODO(loc_id, "Unsupported: macro with 0 replacement tokens"); return SemIR::ErrorInst::InstId; } - clang::Sema& sema = context.clang_sema(); - clang::Preprocessor& preprocessor = sema.getPreprocessor(); + auto& ast_context = context.cpp_context()->ast_context(); auto& parser = context.cpp_context()->parser(); + auto& preprocessor = context.cpp_context()->sema().getPreprocessor(); - llvm::SmallVector tokens(macro_info->tokens().begin(), - macro_info->tokens().end()); - - clang::Token current_token = parser.getCurToken(); - - // Add eof token - clang::Token eof; - eof.startToken(); - eof.setKind(clang::tok::eof); - eof.setLocation(current_token.getEndLoc()); - tokens.push_back(eof); - - tokens.push_back(current_token); - - preprocessor.EnterTokenStream(tokens, /*DisableMacroExpansion=*/false, + // Enter the macro name, not the macro body, so we properly suppress recursive + // expansion. + clang::Token macro_name[1]; + macro_name[0].startToken(); + macro_name[0].setKind(clang::tok::identifier); + macro_name[0].setIdentifierInfo(identifier_info); + macro_name[0].setLocation(GetCppLocation(context, loc_id)); + preprocessor.EnterTokenStream(macro_name, /*DisableMacroExpansion=*/false, /*IsReinject=*/false); - parser.ConsumeAnyToken(true); + + CARBON_CHECK(parser.getCurToken().is(clang::tok::eof)); + parser.ConsumeToken(); clang::ExprResult result = parser.ParseConstantExpression(); - clang::Expr* result_expr = result.get(); + // Consume any remaining tokens to advance the preprocessor and parser past + // the end of the macro. bool success = !result.isInvalid() && parser.getCurToken().is(clang::tok::eof); + while (!parser.getCurToken().is(clang::tok::eof)) { + parser.ConsumeAnyToken(true); + } if (!success) { - parser.SkipUntil(clang::tok::eof); CARBON_DIAGNOSTIC( InCppMacroEvaluation, Error, "failed to parse macro Cpp.{0} to a valid constant expression", std::string); - context.emitter().Emit(loc_id, InCppMacroEvaluation, (*name_str_opt).str()); + context.emitter().Emit(loc_id, InCppMacroEvaluation, + identifier_info->getName().str()); return SemIR::ErrorInst::InstId; } @@ -103,8 +102,7 @@ auto TryEvaluateMacro(Context& context, SemIR::LocId loc_id, } clang::Expr::EvalResult evaluated_result; - if (!result_expr->EvaluateAsConstantExpr(evaluated_result, - sema.getASTContext())) { + if (!result_expr->EvaluateAsConstantExpr(evaluated_result, ast_context)) { CARBON_FATAL("failed to evaluate macro as constant expression"); } diff --git a/toolchain/check/cpp/macros.h b/toolchain/check/cpp/macros.h index dbf046770852..c68129d6e102 100644 --- a/toolchain/check/cpp/macros.h +++ b/toolchain/check/cpp/macros.h @@ -8,6 +8,7 @@ #include "toolchain/check/context.h" namespace clang { +class IdentifierInfo; class MacroInfo; } // namespace clang @@ -17,8 +18,8 @@ namespace Carbon::Check { // constant if possible. Returns an `InstId` on success or // `SemIR::ErrorInst::InstId` otherwise. auto TryEvaluateMacro(Context& context, SemIR::LocId loc_id, - SemIR::NameId name_id, clang::MacroInfo* macro_info) - -> SemIR::InstId; + clang::IdentifierInfo* identifier_info, + clang::MacroInfo* macro_info) -> SemIR::InstId; } // namespace Carbon::Check diff --git a/toolchain/check/diagnostic_emitter.cpp b/toolchain/check/diagnostic_emitter.cpp index d7152e6d009c..0c2070167fa6 100644 --- a/toolchain/check/diagnostic_emitter.cpp +++ b/toolchain/check/diagnostic_emitter.cpp @@ -43,6 +43,8 @@ auto DiagnosticEmitter::ConvertLoc(LocIdForDiagnostics loc_id, break; case Carbon::SemIR::DiagnosticLocConverter::ImportLoc::CppMacroExpansion: // TODO: Include the macro name in the note. + // TODO: Include the Clang-generated snippet here rather than with the + // main diagnostic. context_fn(import.loc, InCppMacroExpansion); break; } diff --git a/toolchain/check/testdata/interop/cpp/basics/inline/modules.carbon b/toolchain/check/testdata/interop/cpp/basics/inline/modules.carbon index d5d2b2886784..5620d4464558 100644 --- a/toolchain/check/testdata/interop/cpp/basics/inline/modules.carbon +++ b/toolchain/check/testdata/interop/cpp/basics/inline/modules.carbon @@ -17,8 +17,12 @@ library "[[@TEST_NAME]]"; // TODO: This diagnostic isn't very good. import Cpp inline ''' +// CHECK:STDERR: fail_export_module_in_inline_cpp.carbon:[[@LINE+11]]:8: error: module declaration must not come from an #include directive [CppInteropParseError] +// CHECK:STDERR: 17 | export module Foo; +// CHECK:STDERR: | ^~~~~~~~~~~ +// CHECK:STDERR: // CHECK:STDERR: fail_export_module_in_inline_cpp.carbon:[[@LINE+7]]:8: error: module declaration must occur at the start of the translation unit [CppInteropParseError] -// CHECK:STDERR: 13 | export module Foo; +// CHECK:STDERR: 17 | export module Foo; // CHECK:STDERR: | ^ // CHECK:STDERR: :1:1: note: add 'module;' to the start of the file to introduce a global module fragment [CppInteropParseNote] // CHECK:STDERR: 1 | ; @@ -33,15 +37,19 @@ library "[[@TEST_NAME]]"; // TODO: This diagnostic isn't very good. import Cpp inline ''' +// CHECK:STDERR: fail_module_in_inline_cpp.carbon:[[@LINE+15]]:1: error: module declaration must not come from an #include directive [CppInteropParseError] +// CHECK:STDERR: 21 | module Foo; +// CHECK:STDERR: | ^~~~~~~~~~~ +// CHECK:STDERR: // CHECK:STDERR: fail_module_in_inline_cpp.carbon:[[@LINE+11]]:1: error: module declaration must occur at the start of the translation unit [CppInteropParseError] -// CHECK:STDERR: 17 | module Foo; +// CHECK:STDERR: 21 | module Foo; // CHECK:STDERR: | ^ // CHECK:STDERR: :1:1: note: add 'module;' to the start of the file to introduce a global module fragment [CppInteropParseNote] // CHECK:STDERR: 1 | ; // CHECK:STDERR: | ^ // CHECK:STDERR: // CHECK:STDERR: fail_module_in_inline_cpp.carbon:[[@LINE+4]]:8: error: module 'Foo' not found [CppInteropParseError] -// CHECK:STDERR: 17 | module Foo; +// CHECK:STDERR: 21 | module Foo; // CHECK:STDERR: | ^~~~ // CHECK:STDERR: module Foo; @@ -53,23 +61,31 @@ library "[[@TEST_NAME]]"; // TODO: This diagnostic isn't very good. import Cpp inline ''' +// CHECK:STDERR: fail_global_module_in_inline_cpp.carbon:[[@LINE+8]]:1: error: module declaration must not come from an #include directive [CppInteropParseError] +// CHECK:STDERR: 14 | module; +// CHECK:STDERR: | ^~~~~~~ +// CHECK:STDERR: // CHECK:STDERR: fail_global_module_in_inline_cpp.carbon:[[@LINE+4]]:1: error: 'module;' introducing a global module fragment can appear only at the start of the translation unit [CppInteropParseError] -// CHECK:STDERR: 10 | module; +// CHECK:STDERR: 14 | module; // CHECK:STDERR: | ^~~~~~~ // CHECK:STDERR: module; int n; +// CHECK:STDERR: fail_global_module_in_inline_cpp.carbon:[[@LINE+15]]:1: error: module declaration must not come from an #include directive [CppInteropParseError] +// CHECK:STDERR: 33 | module Foo; +// CHECK:STDERR: | ^~~~~~~~~~~ +// CHECK:STDERR: // CHECK:STDERR: fail_global_module_in_inline_cpp.carbon:[[@LINE+11]]:1: error: module declaration must occur at the start of the translation unit [CppInteropParseError] -// CHECK:STDERR: 25 | module Foo; +// CHECK:STDERR: 33 | module Foo; // CHECK:STDERR: | ^ // CHECK:STDERR: :1:1: note: add 'module;' to the start of the file to introduce a global module fragment [CppInteropParseNote] // CHECK:STDERR: 1 | ; // CHECK:STDERR: | ^ // CHECK:STDERR: // CHECK:STDERR: fail_global_module_in_inline_cpp.carbon:[[@LINE+4]]:8: error: module 'Foo' not found [CppInteropParseError] -// CHECK:STDERR: 25 | module Foo; +// CHECK:STDERR: 33 | module Foo; // CHECK:STDERR: | ^~~~ // CHECK:STDERR: module Foo; diff --git a/toolchain/check/testdata/interop/cpp/macros/macros.carbon b/toolchain/check/testdata/interop/cpp/macros/macros.carbon index c778a3b188ba..3712ffa1ace3 100644 --- a/toolchain/check/testdata/interop/cpp/macros/macros.carbon +++ b/toolchain/check/testdata/interop/cpp/macros/macros.carbon @@ -40,19 +40,21 @@ fn F() { } // --- bad_suffix.h +// CHECK:STDERR: ./bad_suffix.h:[[@LINE+1]]:24: in expansion of macro defined here [InCppMacroExpansion] #define CONFIG_VALUE 42f // --- fail_import_bad_suffix.carbon library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_import_bad_suffix.carbon:[[@LINE+4]]:10: in file included here [InCppInclude] -// CHECK:STDERR: ./bad_suffix.h:1:24: error: invalid digit 'f' in decimal constant [CppInteropParseError] -// CHECK:STDERR: 1 | #define CONFIG_VALUE 42f -// CHECK:STDERR: | ^ import Cpp library "bad_suffix.h"; fn F() { + // CHECK:STDERR: fail_import_bad_suffix.carbon:[[@LINE+20]]:6: error: invalid digit 'f' in decimal constant [CppInteropParseError] + // CHECK:STDERR: 27 | Cpp.CONFIG_VALUE; + // CHECK:STDERR: | ^ + // CHECK:STDERR: 2 | #define CONFIG_VALUE 42f + // CHECK:STDERR: | ^ // CHECK:STDERR: fail_import_bad_suffix.carbon:[[@LINE+15]]:3: note: in `Cpp` name lookup for `CONFIG_VALUE` [InCppNameLookup] // CHECK:STDERR: Cpp.CONFIG_VALUE; // CHECK:STDERR: ^~~~~~~~~~~~~~~~ @@ -72,19 +74,21 @@ fn F() { } // --- integer_literal_too_big.h +// CHECK:STDERR: ./integer_literal_too_big.h:[[@LINE+1]]:22: in expansion of macro defined here [InCppMacroExpansion] #define CONFIG_VALUE 18446744073709551616 // --- fail_import_integer_literal_too_big.carbon library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_import_integer_literal_too_big.carbon:[[@LINE+4]]:10: in file included here [InCppInclude] -// CHECK:STDERR: ./integer_literal_too_big.h:1:22: error: integer literal is too large to be represented in any integer type [CppInteropParseError] -// CHECK:STDERR: 1 | #define CONFIG_VALUE 18446744073709551616 -// CHECK:STDERR: | ^ import Cpp library "integer_literal_too_big.h"; fn F() { + // CHECK:STDERR: fail_import_integer_literal_too_big.carbon:[[@LINE+9]]:6: error: integer literal is too large to be represented in any integer type [CppInteropParseError] + // CHECK:STDERR: 16 | Cpp.CONFIG_VALUE; + // CHECK:STDERR: | ^ + // CHECK:STDERR: 2 | #define CONFIG_VALUE 18446744073709551616 + // CHECK:STDERR: | ^ // CHECK:STDERR: fail_import_integer_literal_too_big.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `CONFIG_VALUE` [InCppNameLookup] // CHECK:STDERR: Cpp.CONFIG_VALUE; // CHECK:STDERR: ^~~~~~~~~~~~~~~~ @@ -453,19 +457,21 @@ fn F() { // --- multiple_characters.h +// CHECK:STDERR: ./multiple_characters.h:[[@LINE+1]]:24: in expansion of macro defined here [InCppMacroExpansion] #define MULTIPLE_CHARS 'AB' // --- import_multiple_characters.carbon library "[[@TEST_NAME]]"; -// CHECK:STDERR: import_multiple_characters.carbon:[[@LINE+4]]:10: in file included here [InCppInclude] -// CHECK:STDERR: ./multiple_characters.h:2:24: warning: multi-character character constant [CppInteropParseWarning] -// CHECK:STDERR: 2 | #define MULTIPLE_CHARS 'AB' -// CHECK:STDERR: | ^ import Cpp library "multiple_characters.h"; fn F() { + // CHECK:STDERR: import_multiple_characters.carbon:[[@LINE+9]]:25: warning: multi-character character constant [CppInteropParseWarning] + // CHECK:STDERR: 16 | let unused a: i32 = Cpp.MULTIPLE_CHARS; + // CHECK:STDERR: | ^ + // CHECK:STDERR: 3 | #define MULTIPLE_CHARS 'AB' + // CHECK:STDERR: | ^ // CHECK:STDERR: import_multiple_characters.carbon:[[@LINE+4]]:22: note: in `Cpp` name lookup for `MULTIPLE_CHARS` [InCppNameLookup] // CHECK:STDERR: let unused a: i32 = Cpp.MULTIPLE_CHARS; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~ @@ -971,6 +977,17 @@ fn F() { Cpp.m = 2; } +// --- self_expand.carbon + +import Cpp inline ''' +const int COUNT = 0; +#define COUNT COUNT + 1 +'''; + +class C(N: i32) {} +// Expansion of `COUNT` should be suppressed inside itself. +let _: C(1) = {} as C(Cpp.COUNT); + // CHECK:STDOUT: --- import_integer_literal_replacement_token.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants {