llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Daniel Grumberg (daniel-grumberg) <details> <summary>Changes</summary> Extension SGFs require the module system to be enabled in order to discover which module defines the extended external type. This patch ensures the following: - Associate symbols with their top level module name, - Ensure we don't drop macro definitions that came from a submodule but that should be included in the SGF output. --- Patch is 38.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105868.diff 10 Files Affected: - (modified) clang/include/clang/ExtractAPI/DeclarationFragments.h (+2-2) - (modified) clang/include/clang/ExtractAPI/ExtractAPIVisitor.h (+1-1) - (modified) clang/lib/ExtractAPI/DeclarationFragments.cpp (+1-3) - (modified) clang/lib/ExtractAPI/ExtractAPIConsumer.cpp (+30-54) - (modified) clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp (+2-2) - (modified) clang/test/ExtractAPI/bool.c (+4-4) - (modified) clang/test/ExtractAPI/emit-symbol-graph/multi_file.c (+4-207) - (modified) clang/test/ExtractAPI/emit-symbol-graph/single_file.c (+1-104) - (modified) clang/test/ExtractAPI/macros.c (+349-407) - (added) clang/test/ExtractAPI/submodule-macro.m (+25) ``````````diff diff --git a/clang/include/clang/ExtractAPI/DeclarationFragments.h b/clang/include/clang/ExtractAPI/DeclarationFragments.h index 535da90b98284b..4ac744459031eb 100644 --- a/clang/include/clang/ExtractAPI/DeclarationFragments.h +++ b/clang/include/clang/ExtractAPI/DeclarationFragments.h @@ -411,9 +411,9 @@ class DeclarationFragmentsBuilder { /// Build DeclarationFragments for a macro. /// /// \param Name name of the macro. - /// \param MD the associated MacroDirective. + /// \param MI the associated MacroInfo. static DeclarationFragments getFragmentsForMacro(StringRef Name, - const MacroDirective *MD); + const MacroInfo *MI); /// Build DeclarationFragments for a typedef \p TypedefNameDecl. static DeclarationFragments diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h index 67659f5a25037c..b09b8b44d9abaa 100644 --- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h +++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h @@ -213,7 +213,7 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> { StringRef getOwningModuleName(const Decl &D) { if (auto *OwningModule = D.getImportedOwningModule()) - return OwningModule->Name; + return OwningModule->getTopLevelModule()->Name; return {}; } diff --git a/clang/lib/ExtractAPI/DeclarationFragments.cpp b/clang/lib/ExtractAPI/DeclarationFragments.cpp index 6b85c7db90349e..d77bb1d424f7cf 100644 --- a/clang/lib/ExtractAPI/DeclarationFragments.cpp +++ b/clang/lib/ExtractAPI/DeclarationFragments.cpp @@ -1327,14 +1327,12 @@ DeclarationFragmentsBuilder::getFragmentsForFunctionTemplateSpecialization( DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForMacro(StringRef Name, - const MacroDirective *MD) { + const MacroInfo *MI) { DeclarationFragments Fragments; Fragments.append("#define", DeclarationFragments::FragmentKind::Keyword) .appendSpace(); Fragments.append(Name, DeclarationFragments::FragmentKind::Identifier); - auto *MI = MD->getMacroInfo(); - if (MI->isFunctionLike()) { Fragments.append("(", DeclarationFragments::FragmentKind::Text); unsigned numParameters = MI->getNumParams(); diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index d6335854cbf262..b23a62267651c8 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -286,78 +286,54 @@ class MacroCallback : public PPCallbacks { MacroCallback(const SourceManager &SM, APISet &API, Preprocessor &PP) : SM(SM), API(API), PP(PP) {} - void MacroDefined(const Token &MacroNameToken, - const MacroDirective *MD) override { - auto *MacroInfo = MD->getMacroInfo(); + void EndOfMainFile() override { + for (const auto &M : PP.macros()) { + auto *II = M.getFirst(); + auto MD = PP.getMacroDefinition(II); + auto *MI = MD.getMacroInfo(); - if (MacroInfo->isBuiltinMacro()) - return; + if (!MI) + continue; - auto SourceLoc = MacroNameToken.getLocation(); - if (SM.isWrittenInBuiltinFile(SourceLoc) || - SM.isWrittenInCommandLineFile(SourceLoc)) - return; + // Ignore header guard macros + if (MI->isUsedForHeaderGuard()) + continue; - PendingMacros.emplace_back(MacroNameToken, MD); - } + // Ignore builtin macros and ones defined via the command line. + if (MI->isBuiltinMacro()) + continue; - // If a macro gets undefined at some point during preprocessing of the inputs - // it means that it isn't an exposed API and we should therefore not add a - // macro definition for it. - void MacroUndefined(const Token &MacroNameToken, const MacroDefinition &MD, - const MacroDirective *Undef) override { - // If this macro wasn't previously defined we don't need to do anything - // here. - if (!Undef) - return; - - llvm::erase_if(PendingMacros, [&MD, this](const PendingMacro &PM) { - return MD.getMacroInfo()->isIdenticalTo(*PM.MD->getMacroInfo(), PP, - /*Syntactically*/ false); - }); - } + auto DefLoc = MI->getDefinitionLoc(); - void EndOfMainFile() override { - for (auto &PM : PendingMacros) { - // `isUsedForHeaderGuard` is only set when the preprocessor leaves the - // file so check for it here. - if (PM.MD->getMacroInfo()->isUsedForHeaderGuard()) + if (SM.isWrittenInBuiltinFile(DefLoc) || SM.isWrittenInCommandLineFile(DefLoc)) continue; - if (!shouldMacroBeIncluded(PM)) + auto AssociatedModuleMacros = MD.getModuleMacros(); + StringRef OwningModuleName; + if (!AssociatedModuleMacros.empty()) + OwningModuleName = AssociatedModuleMacros.back()->getOwningModule()->getTopLevelModuleName(); + + if (!shouldMacroBeIncluded(DefLoc, OwningModuleName)) continue; - StringRef Name = PM.MacroNameToken.getIdentifierInfo()->getName(); - PresumedLoc Loc = SM.getPresumedLoc(PM.MacroNameToken.getLocation()); + StringRef Name = II->getName(); + PresumedLoc Loc = SM.getPresumedLoc(DefLoc); SmallString<128> USR; - index::generateUSRForMacro(Name, PM.MacroNameToken.getLocation(), SM, - USR); - + index::generateUSRForMacro(Name, DefLoc, SM, USR); API.createRecord<extractapi::MacroDefinitionRecord>( USR, Name, SymbolReference(), Loc, - DeclarationFragmentsBuilder::getFragmentsForMacro(Name, PM.MD), + DeclarationFragmentsBuilder::getFragmentsForMacro(Name, MI), DeclarationFragmentsBuilder::getSubHeadingForMacro(Name), - SM.isInSystemHeader(PM.MacroNameToken.getLocation())); - } + SM.isInSystemHeader(DefLoc)); - PendingMacros.clear(); + } } -protected: - struct PendingMacro { - Token MacroNameToken; - const MacroDirective *MD; - - PendingMacro(const Token &MacroNameToken, const MacroDirective *MD) - : MacroNameToken(MacroNameToken), MD(MD) {} - }; - - virtual bool shouldMacroBeIncluded(const PendingMacro &PM) { return true; } + virtual bool shouldMacroBeIncluded(const SourceLocation &MacroLoc, StringRef ModuleName) { return true; } const SourceManager &SM; APISet &API; Preprocessor &PP; - llvm::SmallVector<PendingMacro> PendingMacros; }; class APIMacroCallback : public MacroCallback { @@ -366,9 +342,9 @@ class APIMacroCallback : public MacroCallback { LocationFileChecker &LCF) : MacroCallback(SM, API, PP), LCF(LCF) {} - bool shouldMacroBeIncluded(const PendingMacro &PM) override { + bool shouldMacroBeIncluded(const SourceLocation &MacroLoc, StringRef ModuleName) override { // Do not include macros from external files - return LCF(PM.MacroNameToken.getLocation()); + return LCF(MacroLoc) || API.ProductName == ModuleName; } private: diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp index 1bce9c59b19791..54124ddbb2f58a 100644 --- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp +++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp @@ -928,8 +928,8 @@ bool SymbolGraphSerializer::traverseObjCCategoryRecord( return true; auto *CurrentModule = ModuleForCurrentSymbol; - if (Record->isExtendingExternalModule()) - ModuleForCurrentSymbol = &ExtendedModules[Record->Interface.Source]; + if (auto ModuleExtendedByRecord= Record->getExtendedExternalModule()) + ModuleForCurrentSymbol = &ExtendedModules[*ModuleExtendedByRecord]; if (!walkUpFromObjCCategoryRecord(Record)) return false; diff --git a/clang/test/ExtractAPI/bool.c b/clang/test/ExtractAPI/bool.c index efab6dfeef03b2..7a4d34acb0d269 100644 --- a/clang/test/ExtractAPI/bool.c +++ b/clang/test/ExtractAPI/bool.c @@ -2,8 +2,8 @@ // RUN: split-file %s %t // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \ // RUN: %t/reference.output.json.in >> %t/reference.output.json -// RUN: %clang -extract-api --pretty-sgf -target arm64-apple-macosx \ -// RUN: %t/input.h -o %t/output.json +// RUN: %clang_cc1 -extract-api --product-name=BoolTest --pretty-sgf -triple arm64-apple-macosx \ +// RUN: %t/input.h -o %t/output.json // Generator version is not consistent across test runs, normalize it. // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \ @@ -15,7 +15,7 @@ bool Foo; bool IsFoo(bool Bar); -/// expected-no-diagnostics +// expected-no-diagnostics //--- reference.output.json.in { @@ -28,7 +28,7 @@ bool IsFoo(bool Bar); "generator": "?" }, "module": { - "name": "", + "name": "BoolTest", "platform": { "architecture": "arm64", "operatingSystem": { diff --git a/clang/test/ExtractAPI/emit-symbol-graph/multi_file.c b/clang/test/ExtractAPI/emit-symbol-graph/multi_file.c index e668f69bc7e05f..651e0df2cd93a0 100644 --- a/clang/test/ExtractAPI/emit-symbol-graph/multi_file.c +++ b/clang/test/ExtractAPI/emit-symbol-graph/multi_file.c @@ -27,9 +27,6 @@ #ifndef TEST_H #define TEST_H -#define testmarcro1 32 -#define testmacro2 42 - int testfunc (int param1, int param2); void testfunc2 (); #endif /* TEST_H */ @@ -185,7 +182,7 @@ int main () "location": { "position": { "character": 4, - "line": 6 + "line": 3 }, "uri": "file://INPUT_DIR/test.h" }, @@ -249,7 +246,7 @@ int main () "location": { "position": { "character": 5, - "line": 7 + "line": 4 }, "uri": "file://INPUT_DIR/test.h" }, @@ -335,106 +332,6 @@ int main () "pathComponents": [ "main" ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:test.h@39@macro@testmarcro1" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 3 - }, - "uri": "file://INPUT_DIR/test.h" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "title": "testmarcro1" - }, - "pathComponents": [ - "testmarcro1" - ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:test.h@62@macro@testmacro2" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 4 - }, - "uri": "file://INPUT_DIR/test.h" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "title": "testmacro2" - }, - "pathComponents": [ - "testmacro2" - ] } ] } @@ -573,7 +470,7 @@ int main () "location": { "position": { "character": 4, - "line": 6 + "line": 3 }, "uri": "file://INPUT_DIR/test.h" }, @@ -637,7 +534,7 @@ int main () "location": { "position": { "character": 5, - "line": 7 + "line": 4 }, "uri": "file://INPUT_DIR/test.h" }, @@ -659,106 +556,6 @@ int main () "pathComponents": [ "testfunc2" ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:test.h@39@macro@testmarcro1" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 3 - }, - "uri": "file://INPUT_DIR/test.h" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "testmarcro1" - } - ], - "title": "testmarcro1" - }, - "pathComponents": [ - "testmarcro1" - ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:test.h@62@macro@testmacro2" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 4 - }, - "uri": "file://INPUT_DIR/test.h" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "testmacro2" - } - ], - "title": "testmacro2" - }, - "pathComponents": [ - "testmacro2" - ] } ] } diff --git a/clang/test/ExtractAPI/emit-symbol-graph/single_file.c b/clang/test/ExtractAPI/emit-symbol-graph/single_file.c index b00b5f5237c9a3..feb759f5947bc9 100644 --- a/clang/test/ExtractAPI/emit-symbol-graph/single_file.c +++ b/clang/test/ExtractAPI/emit-symbol-graph/single_file.c @@ -15,9 +15,6 @@ // CHECK-NOT: warning: //--- main.c -#define TESTMACRO1 2 -#define TESTMARCRO2 5 - int main () { return 0; @@ -87,7 +84,7 @@ int main () "location": { "position": { "character": 4, - "line": 3 + "line": 0 }, "uri": "file://INPUT_DIR/main.c" }, @@ -109,106 +106,6 @@ int main () "pathComponents": [ "main" ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "TESTMACRO1" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:main.c@8@macro@TESTMACRO1" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 0 - }, - "uri": "file://INPUT_DIR/main.c" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "TESTMACRO1" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "TESTMACRO1" - } - ], - "title": "TESTMACRO1" - }, - "pathComponents": [ - "TESTMACRO1" - ] - }, - { - "accessLevel": "public", - "declarationFragments": [ - { - "kind": "keyword", - "spelling": "#define" - }, - { - "kind": "text", - "spelling": " " - }, - { - "kind": "identifier", - "spelling": "TESTMARCRO2" - } - ], - "identifier": { - "interfaceLanguage": "c", - "precise": "c:main.c@29@macro@TESTMARCRO2" - }, - "kind": { - "displayName": "Macro", - "identifier": "c.macro" - }, - "location": { - "position": { - "character": 8, - "line": 1 - }, - "uri": "file://INPUT_DIR/main.c" - }, - "names": { - "navigator": [ - { - "kind": "identifier", - "spelling": "TESTMARCRO2" - } - ], - "subHeading": [ - { - "kind": "identifier", - "spelling": "TESTMARCRO2" - } - ], - "title": "TESTMARCRO2" - }, - "pathComponents": [ - "TESTMARCRO2" - ] } ] } diff --git a/clang/test/ExtractAPI/macros.c b/clang/test/ExtractAPI/macros.c index 10003fe6f6e40f..15eb5f6a7f66fe 100644 --- a/clang/test/ExtractAPI/macros.c +++ b/clang/test/ExtractAPI/macros.c @@ -1,416 +1,358 @@ // RUN: rm -rf %t -// RUN: split-file %s %t -// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \ -// RUN: %t/reference.output.json.in >> %t/reference.output.json -// RUN: %clang -extract-api --pretty-sgf --product-name=Macros -target arm64-apple-macosx \ -// RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s +// RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing --product-name=Macros -triple arm64-apple-macosx \ +// RUN: -isystem %S -x objective-c-header %s -o %t/output.symbols.json -// Generator version is not consistent across test runs, normalize it. -// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \ -// RUN: %t/output.json >> %t/output-normalized.json -// RUN: diff %t/reference.output.json %t/output-normalized.json - -// CHECK-NOT: error: -// CHECK-NOT: warning: - -//--- input.h +// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix HELLO #define HELLO 1 +// HELLO-LABEL: "!testLabel": "c:@macro@HELLO" +// HELLO: "accessLevel": "public", +// HELLO-NEXT: "declarationFragments": [ +// HELLO-NEXT: { +// HELLO-NEXT: "kind": "keyword", +// HELLO-NEXT: "spelling": "#define" +// HELLO-NEXT: }, +// HELLO-NEXT: { +// HELLO-NEXT: "kind": "text", +// HELLO-NEXT: "spelling": " " +// HELLO-NEXT: }, +// HELLO-NEXT: { +// HELLO-NEXT: "kind": "identifier", +// HELLO-NEXT: "spelling": "HELLO" +// HELLO-NEXT: } +// HELLO-NEXT: ], +// HELLO: "kind": { +// HELLO-NEXT: "displayName": "Macro", +// HELLO-NEXT: "identifier": "objective-c.macro" +// HELLO-NEXT: }, +// HELLO-NEXT: "location": { +// HELLO-NEXT: "position": { +// HELLO-NEXT: "character": 8, +// HELLO-NEXT: "line": [[# @LINE - 25]] +// HELLO-NEXT: }, +// HELLO-NEXT: "uri": "file://{{.*}}/macros.c" +// HELLO-NEXT: }, +// HELLO-NEXT: "names": { +// HELLO-NEXT: "navigator": [ +// HELLO-NEXT: { +// HELLO-NEXT: "kind": "identifier", +// HELLO-NEXT: "spelling": "HELLO" +// HELLO-NEXT: } +// HELLO-NEXT: ], +// HELLO-NEXT: "subHeading": [ +// HELLO-NEXT: { +// HELLO-NEXT: "kind": "identifier", +// HELLO-NEXT: "spelling": "HELLO" +// HELLO-NEXT: } +// HELLO-NEXT: ], +// HELLO-NEXT: "title": "HELLO" +// HELLO-NEXT: }, +// HELLO... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/105868 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits