Merge pull request #9615 from github/redsun82/swift-fix-synthesized-entities

Swift: fix emission of synthesized entities

Author: AlexDenisov

swift/extractor/SwiftDispatcher.h

@@ -10,29 +10,25 @@
 
 namespace codeql {
 
-enum class SwiftExtractionMode { Module, PrimaryFile };
-
 // The main responsibilities of the SwiftDispatcher are as follows:
 // * redirect specific AST node emission to a corresponding visitor (statements, expressions, etc.)
 // * storing TRAP labels for emitted AST nodes (in the TrapLabelStore) to avoid re-emission
 // Since SwiftDispatcher sees all the AST nodes, it also attaches a location to every 'locatable'
 // node (AST nodes that are not types: declarations, statements, expressions, etc.).
 class SwiftDispatcher {
  public:
-  // all references passed as parameters to this constructor are supposed to outlive the
-  // SwiftDispatcher
+  // all references and pointers passed as parameters to this constructor are supposed to outlive
+  // the SwiftDispatcher
   SwiftDispatcher(const swift::SourceManager& sourceManager,
                   TrapArena& arena,
                   TrapOutput& trap,
-                  SwiftExtractionMode extractionMode,
                   swift::ModuleDecl& currentModule,
-                  llvm::StringRef currentFileName)
+                  swift::SourceFile* currentPrimarySourceFile = nullptr)
       : sourceManager{sourceManager},
         arena{arena},
         trap{trap},
-        extractionMode(extractionMode),
         currentModule{currentModule},
-        currentFileName(currentFileName) {}
+        currentPrimarySourceFile{currentPrimarySourceFile} {}
 
   template <typename Entry>
   void emit(const Entry& entry) {
@@ -135,29 +131,22 @@ class SwiftDispatcher {
 
   // In order to not emit duplicated entries for declarations, we restrict emission to only
   // Decls declared within the current "scope".
-  // Depending on the SwiftExtractionMode the scope is defined as follows:
-  //  - SwiftExtractionMode::Module: the current scope means the current module. This is used in
-  //    the case of system or builtin modules.
-  //  - SwiftExtractionMode::PrimaryFile: in this mode, we extract several files belonging to the
+  // Depending on the whether we are extracting a primary source file or not the scope is defined as
+  // follows:
+  //  - not extracting a primary source file (`currentPrimarySourceFile == nullptr`): the current
+  //    scope means the current module. This is used in the case of system or builtin modules.
+  //  - extracting a primary source file: in this mode, we extract several files belonging to the
   //    same module one by one. In this mode, we restrict emission only to the same file ignoring
   //    all the other files.
-  // TODO this currently does not extract compiler-synthesized entities without a valid location,
-  // this will be fixed in an upcoming PR
   bool shouldEmitDeclBody(swift::Decl* decl) {
-    switch (extractionMode) {
-      case SwiftExtractionMode::Module: {
-        return &currentModule == decl->getModuleContext();
-      } break;
-      case SwiftExtractionMode::PrimaryFile: {
-        swift::SourceLoc location = decl->getStartLoc();
-        if (!location.isValid()) {
-          return false;
-        }
-        auto declFileName = sourceManager.getDisplayNameForLoc(location).str();
-        return &currentModule == decl->getModuleContext() && declFileName == currentFileName;
-      } break;
-      default:
-        return false;
+    if (decl->getModuleContext() != &currentModule) {
+      return false;
+    }
+    if (!currentPrimarySourceFile) {
+      return true;
+    }
+    if (auto context = decl->getDeclContext()) {
+      return currentPrimarySourceFile == context->getParentSourceFile();
     }
     return false;
   }
@@ -241,9 +230,8 @@ class SwiftDispatcher {
   TrapOutput& trap;
   Store store;
   Store::Handle waitingForNewLabel{std::monostate{}};
-  SwiftExtractionMode extractionMode;
   swift::ModuleDecl& currentModule;
-  llvm::StringRef currentFileName;
+  swift::SourceFile* currentPrimarySourceFile;
 };
 
 }  // namespace codeql

swift/extractor/SwiftExtractor.cpp

@@ -50,16 +50,16 @@ static void archiveFile(const SwiftExtractorConfiguration& config, swift::Source
 }
 
 static void extractDeclarations(const SwiftExtractorConfiguration& config,
+                                llvm::ArrayRef<swift::Decl*> topLevelDecls,
                                 swift::CompilerInstance& compiler,
-                                SwiftExtractionMode extractionMode,
                                 swift::ModuleDecl& module,
-                                llvm::StringRef fileName,
-                                llvm::ArrayRef<swift::Decl*> topLevelDecls) {
+                                swift::SourceFile* primaryFile = nullptr) {
   // The extractor can be called several times from different processes with
   // the same input file(s)
   // We are using PID to avoid concurrent access
   // TODO: find a more robust approach to avoid collisions?
-  std::string tempTrapName = fileName.str() + '.' + std::to_string(getpid()) + ".trap";
+  llvm::StringRef filename = primaryFile ? primaryFile->getFilename() : module.getModuleFilename();
+  std::string tempTrapName = filename.str() + '.' + std::to_string(getpid()) + ".trap";
   llvm::SmallString<PATH_MAX> tempTrapPath(config.trapDir);
   llvm::sys::path::append(tempTrapPath, tempTrapName);
 
@@ -96,22 +96,22 @@ static void extractDeclarations(const SwiftExtractorConfiguration& config,
   trap.assignKey(unknownLocationLabel, "unknown");
   trap.emit(LocationsTrap{unknownLocationLabel, unknownFileLabel});
 
-  SwiftVisitor visitor(compiler.getSourceMgr(), arena, trap, extractionMode, module, fileName);
+  SwiftVisitor visitor(compiler.getSourceMgr(), arena, trap, module, primaryFile);
   for (auto decl : topLevelDecls) {
     visitor.extract(decl);
   }
   if (topLevelDecls.empty()) {
     // In the case of empty files, the dispatcher is not called, but we still want to 'record' the
     // fact that the file was extracted
-    llvm::SmallString<PATH_MAX> name(fileName);
+    llvm::SmallString<PATH_MAX> name(filename);
     llvm::sys::fs::make_absolute(name);
     auto fileLabel = arena.allocateLabel<FileTag>();
     trap.assignKey(fileLabel, name.str().str());
     trap.emit(FilesTrap{fileLabel, name.str().str()});
   }
 
   // TODO: Pick a better name to avoid collisions
-  std::string trapName = fileName.str() + ".trap";
+  std::string trapName = filename.str() + ".trap";
   llvm::SmallString<PATH_MAX> trapPath(config.trapDir);
   llvm::sys::path::append(trapPath, trapName);
 
@@ -133,15 +133,14 @@ void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
       llvm::SmallVector<swift::Decl*> decls;
       module->getTopLevelDecls(decls);
       // TODO: pass ModuleDecl directly when we have module extraction in place?
-      extractDeclarations(config, compiler, SwiftExtractionMode::Module, *module,
-                          module->getModuleFilename(), decls);
+      extractDeclarations(config, decls, compiler, *module);
     } else {
       // The extraction will only work if one (or more) `-primary-file` CLI option is provided,
       // which is what always happens in case of `swift build` and `xcodebuild`
       for (auto primaryFile : module->getPrimarySourceFiles()) {
         archiveFile(config, *primaryFile);
-        extractDeclarations(config, compiler, SwiftExtractionMode::PrimaryFile, *module,
-                            primaryFile->getFilename(), primaryFile->getTopLevelDecls());
+        extractDeclarations(config, primaryFile->getTopLevelDecls(), compiler, *module,
+                            primaryFile);
       }
     }
   }

swift/ql/test/extractor-tests/generated/decl/EnumDecl/EnumDecl.ql

@@ -0,0 +1,12 @@
+// generated by codegen/codegen.py
+import codeql.swift.elements
+import TestUtils
+
+from EnumDecl x, Type getInterfaceType, string getName, Type getType
+where
+  toBeTested(x) and
+  not x.isUnknown() and
+  getInterfaceType = x.getInterfaceType() and
+  getName = x.getName() and
+  getType = x.getType()
+select x, "getInterfaceType:", getInterfaceType, "getName:", getName, "getType:", getType

swift/ql/test/extractor-tests/generated/decl/EnumDecl/EnumDecl_getBaseType.ql

@@ -0,0 +1,7 @@
+// generated by codegen/codegen.py
+import codeql.swift.elements
+import TestUtils
+
+from EnumDecl x, int index
+where toBeTested(x) and not x.isUnknown()
+select x, index, x.getBaseType(index)

swift/ql/test/extractor-tests/generated/decl/EnumDecl/EnumDecl_getGenericTypeParam.ql

@@ -0,0 +1,7 @@
+// generated by codegen/codegen.py
+import codeql.swift.elements
+import TestUtils
+
+from EnumDecl x, int index
+where toBeTested(x) and not x.isUnknown()
+select x, index, x.getGenericTypeParam(index)

swift/ql/test/extractor-tests/generated/decl/EnumDecl/EnumDecl_getMember.ql

@@ -0,0 +1,7 @@
+// generated by codegen/codegen.py
+import codeql.swift.elements
+import TestUtils
+
+from EnumDecl x, int index
+where toBeTested(x) and not x.isUnknown()
+select x, index, x.getMember(index)