[Clang importer + macros] Handle name lookup and type checking for expanded macros #77580

DougGregor · 2024-11-13T06:42:11Z

Introduce a number of fixes to allow us to fully use declarations that are produced by applying a peer macro to an imported declarations.

These changes include:

Treating the source file containing swift_attr macros as one that stores macros, so we don't parse it as top-level code
Ensuring that we have the right set of imports in the source file containing the macro expansion, because it depends only on the module it comes from
Ensuring that name lookup looks in that file even when the DeclContext hierarchy doesn't contain the source file (because it's based on the Clang module structure)

Expand testing to be sure that we're getting the right calls, diagnostics, printing APIs, and generated IR symbols.

DougGregor · 2024-11-13T06:42:17Z

@swift-ci please smoke test

hnrklssn · 2024-11-13T18:18:56Z

lib/IRGen/IRGenDebugInfo.cpp

-            generatedInfo->kind != GeneratedSourceInfo::PrettyPrinted)
+            generatedInfo->kind != GeneratedSourceInfo::PrettyPrinted &&
+            generatedInfo->kind != GeneratedSourceInfo::DefaultArgument &&
+            generatedInfo->kind != GeneratedSourceInfo::Attribute)


The comment above says "We only care about macros, so skip everything else", but aren't macros attributes in the grammar? I wouldn't expect us to skip GeneratedSourceInfo::Attribute here.

The comment means "we only care about macro expansion buffers." I'll clarify it

hnrklssn · 2024-11-13T18:33:00Z

test/Macros/print_clang_expand_on_imported.swift

 // RUN: %FileCheck -input-file %t/imported.printed.txt %s
+// RUN: echo "" >> %t/stderr.log
+// RUN: %FileCheck -check-prefix DIAGS -input-file %t/stderr.log %s


Is it not possible to instead have swift-ide-test return an error code if the input has errors? That seems like a simpler solution. Not sure what effect they have, but it sounds like --test-input-complete or --typecheck could be useful.

rintaro · 2024-11-13T21:42:36Z

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -122,7 +122,8 @@ public func parseSourceFile(
      .preambleMacroExpansion,
      .replacedFunctionBody,
      .prettyPrinted,
-      .defaultArgument:
+      .defaultArgument,
+      .attribute:


Could you move .attribute to . memberAttributeMacroExpansion branch below, it's still not implemented and doing the same thing, but just not to forget to move it later.

Okay, done!

rintaro · 2024-11-13T21:45:22Z

While you are here, could you make the parsing use ParseSourceFileRequest? (rdar://139119159)

rintaro · 2024-11-13T21:52:04Z

lib/Parse/ParseRequests.cpp

@@ -364,7 +366,8 @@ SourceFileParsingResult parseSourceFile(SourceFile &SF) {
      break;
    }

-    case GeneratedSourceInfo::MemberAttributeMacroExpansion: {
+    case GeneratedSourceInfo::MemberAttributeMacroExpansion:
+    case GeneratedSourceInfo::Attribute: {
      parser.parseExpandedAttributeList(items);


Oh apparently, swift_attr accepts modifiers OR attributes, but parseExpandedAttributeList only accepts attributes? 🤔

swift/lib/ClangImporter/ImportDecl.cpp

Lines 8411 to 8427 in 0bcd251

if (parser.Tok.is(tok::at_sign)) {

SourceLoc atEndLoc = parser.Tok.getRange().getEnd();

SourceLoc atLoc = parser.consumeToken(tok::at_sign);

hadError = parser

.parseDeclAttribute(MappedDecl->getAttrs(), atLoc,

atEndLoc, initContext,

/*isFromClangAttribute=*/true)

.isError();

} else {

SourceLoc staticLoc;

StaticSpellingKind staticSpelling;

hadError = parser

.parseDeclModifierList(MappedDecl->getAttrs(), staticLoc,

staticSpelling,

/*isFromClangAttribute=*/true)

.isError();

}

Yeah, and we have some things that are permitted but are neither attributes nor modifiers, which is odd. I'm going to have to think about this refactoring more.

… as attributes Previously, they were being parsed as top-level code, which would cause errors because there are no definitions. Introduce a new GeneratedSourceInfo kind to mark the purpose of these buffers so the parser can handle them appropriately.

…ule-sensitive We need different buffers for each imported module that has swift_attr attributes, so cache them appropriately.

Right now, we're creating a bunch of throwaway ErrorExpr nodes in the AST.

…panded macros Introduce a number of fixes to allow us to fully use declarations that are produced by applying a peer macro to an imported declarations. These changes include: * Ensuring that we have the right set of imports in the source file containing the macro expansion, because it depends only on the module it comes from * Ensuring that name lookup looks in that file even when the DeclContext hierarchy doesn't contain the source file (because it's based on the Clang module structure) Expand testing to be sure that we're getting the right calls, diagnostics, and generated IR symbols.

DougGregor · 2024-11-14T05:24:26Z

@swift-ci please smoke test

DougGregor · 2024-11-14T06:02:08Z

@swift-ci please smoke test

DougGregor · 2024-11-14T07:29:01Z

@swift-ci please smoke test

DougGregor requested review from ahoppen, bnbarham, hamishknight, rintaro, jckarter, CodaFi, adrian-prantl, zoecarver, hyp, egorzhdan, beccadax, ian-twilightcoder, Xazax-hun, hborla, slavapestov and xedin as code owners November 13, 2024 06:42

DougGregor enabled auto-merge November 13, 2024 06:42

DougGregor force-pushed the swift-attr-parsing-fixes branch from 39afec7 to 0bcd251 Compare November 13, 2024 18:07

hnrklssn reviewed Nov 13, 2024

View reviewed changes

rintaro reviewed Nov 13, 2024

View reviewed changes

DougGregor added 5 commits November 13, 2024 21:19

Clang importer: switch swift_attr attribute text cache over to be mod…

aa4c548

…ule-sensitive We need different buffers for each imported module that has swift_attr attributes, so cache them appropriately.

Only create ErrorExpr when the CallerSideDefaultArgExprRequest is cyclic

1769ae9

Right now, we're creating a bunch of throwaway ErrorExpr nodes in the AST.

[ASTGen] Move unimplemented macro role case down

8f5e941

DougGregor force-pushed the swift-attr-parsing-fixes branch from c579a63 to 8f5e941 Compare November 14, 2024 05:22

DougGregor requested a review from tshortli as a code owner November 14, 2024 05:22

DougGregor changed the title ~~Ensure that buffers containing Clang swift_attr attributes are parsed as attributes~~ [Clang importer + macros] Handle name lookup and type checking for expanded macros Nov 14, 2024

DougGregor mentioned this pull request Nov 14, 2024

Infer @PointerBounds macro from clang __counted_by parameters #77387

Merged

Fix tests for Linux

75aae22

DougGregor merged commit f802b67 into swiftlang:main Nov 14, 2024
3 checks passed

DougGregor deleted the swift-attr-parsing-fixes branch November 14, 2024 14:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang importer + macros] Handle name lookup and type checking for expanded macros #77580

[Clang importer + macros] Handle name lookup and type checking for expanded macros #77580

DougGregor commented Nov 13, 2024 •

edited

Loading

DougGregor commented Nov 13, 2024

hnrklssn Nov 13, 2024

DougGregor Nov 13, 2024

hnrklssn Nov 13, 2024

rintaro Nov 13, 2024

DougGregor Nov 14, 2024

rintaro commented Nov 13, 2024 •

edited

Loading

rintaro Nov 13, 2024 •

edited

Loading

DougGregor Nov 13, 2024

DougGregor commented Nov 14, 2024

DougGregor commented Nov 14, 2024

DougGregor commented Nov 14, 2024

	if (parser.Tok.is(tok::at_sign)) {
	SourceLoc atEndLoc = parser.Tok.getRange().getEnd();
	SourceLoc atLoc = parser.consumeToken(tok::at_sign);
	hadError = parser
	.parseDeclAttribute(MappedDecl->getAttrs(), atLoc,
	atEndLoc, initContext,
	/isFromClangAttribute=/true)
	.isError();
	} else {
	SourceLoc staticLoc;
	StaticSpellingKind staticSpelling;
	hadError = parser
	.parseDeclModifierList(MappedDecl->getAttrs(), staticLoc,
	staticSpelling,
	/isFromClangAttribute=/true)
	.isError();
	}

[Clang importer + macros] Handle name lookup and type checking for expanded macros #77580

[Clang importer + macros] Handle name lookup and type checking for expanded macros #77580

Conversation

DougGregor commented Nov 13, 2024 • edited Loading

DougGregor commented Nov 13, 2024

hnrklssn Nov 13, 2024

Choose a reason for hiding this comment

DougGregor Nov 13, 2024

Choose a reason for hiding this comment

hnrklssn Nov 13, 2024

Choose a reason for hiding this comment

rintaro Nov 13, 2024

Choose a reason for hiding this comment

DougGregor Nov 14, 2024

Choose a reason for hiding this comment

rintaro commented Nov 13, 2024 • edited Loading

rintaro Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

DougGregor Nov 13, 2024

Choose a reason for hiding this comment

DougGregor commented Nov 14, 2024

DougGregor commented Nov 14, 2024

DougGregor commented Nov 14, 2024

DougGregor commented Nov 13, 2024 •

edited

Loading

rintaro commented Nov 13, 2024 •

edited

Loading

rintaro Nov 13, 2024 •

edited

Loading