Fix Driver/driver-compile.swift

[cltnschlosser]

• include swiftDeps without output file map
• diagnose multiple files with same name

TODO

• add XCTests
• Open PR for minor change needed to test file: swift-driver test tweaks swift#35843

[cltnschlosser]

@swift-ci please test

[CodaFi]

I wonder if we can clean things up by running this check when we're planning the build in the first place.

[cltnschlosser]

Yeah, that's where it was before, in the creation of IncrementalCompilationState, but I needed this for computeSupplementaryOutputPath for -emit-reference-dependencies-path. It likely makes sense to delay this check until later, currently if incremental builds are later disabled we still pass -emit-reference-dependencies-path to the frontend. That's the c++ driver logic that I'm imitating here, so that the tests pass, but long term it's worth evaluating if that's the right approach. (I honestly don't know much about incremental builds and swiftDeps, so I can't say for sure one way or the other. My approach so far with swift-driver is trying to get it to feature parity with c++, making sure it can stand in and have the lit tests still pass.)

[CodaFi]

⛵

[davidungar]

I didn't get a change to look at this before it was merge. I doubt I would have approved it because it diffuses the knowledge about incremental compilation into the Driver, instead of the IncrementalCompilationState.
I don't understand the need to compute the reference dependencies path. Doesn't it come from the OutputFileMap?
What am I missing?
Is this mirroring some change in the legacy driver?

[cltnschlosser]

@davidungar

I didn't get a change to look at this before it was merge. I doubt I would have approved it because it diffuses the knowledge about incremental compilation into the Driver, instead of the IncrementalCompilationState.
I don't understand the need to compute the reference dependencies path. Doesn't it come from the OutputFileMap?
What am I missing?
Is this mirroring some change in the legacy driver?

It's mirroring existing logic, but not a change. Maybe the old logic is all wrong, but I'll layout what I was working through.

This is the code that adds the swiftDeps, the only requirement is that Compilation.getIncrementalBuildEnabled() is true. https://github.com/apple/swift/blob/377f23f5dab2413104a1ef38c347e3b6f5d5242b/lib/Driver/Driver.cpp#L3305
This is a value set early in the driver, roughly here: https://github.com/apple/swift/blob/377f23f5dab2413104a1ef38c347e3b6f5d5242b/lib/Driver/Driver.cpp#L934

In most real world cases the path likely comes from OutputFileMap, but in the c++ driver, it was possible to output a swiftDeps file without using an OutputFileMap. This test(taken from: https://github.com/apple/swift/blob/main/test/Driver/driver-compile.swift), shows that behavior:

// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 %s -sdk %S/../Inputs/clang-importer-sdk -Xfrontend -foo -Xfrontend -bar -Xllvm -baz -Xcc -garply -F /path/to/frameworks -Fsystem /path/to/systemframeworks -F /path/to/more/frameworks -I /path/to/headers -I path/to/more/headers -module-cache-path /tmp/modules -incremental 2>&1 > %t.complex.txt
// RUN: %FileCheck %s < %t.complex.txt
// RUN: %FileCheck -check-prefix COMPLEX %s < %t.complex.txt

// COMPLEX-DAG: -emit-reference-dependencies-path {{(.*(/|\\))?driver-compile[^ /]*}}.swiftdeps

This was the failing test that now passes with this change.

[cltnschlosser]

It's also worth noting that I'm pretty sure the computed path is almost never used, the important piece here is that the value is non-nil. (Kinda a hack...)

Looking at the following code, the input is only ever nil when !compilerMode.usesPrimaryFileInputs, but we short-circuit early if referenceDependenciesPath is nil.

      addOutputOfType(
        outputType: .swiftDeps,
        finalOutputPath: referenceDependenciesPath,
        input: input,
        flag: "-emit-reference-dependencies-path")
        
func addOutputOfType(
        outputType: FileType, finalOutputPath: VirtualPath?,
        input: TypedVirtualPath?, flag: String
    ) {
      // If there is no final output, there's nothing to do.
      guard let finalOutputPath = finalOutputPath else { return }

      // If the whole of the compiler output is this type, there's nothing to
      // do.
      if outputType == compilerOutputType { return }

      // Compute the output path based on the input path (if there is one), or
      // use the final output.
      let outputPath: VirtualPath
      if let input = input {
        if let outputFileMapPath = outputFileMap?.existingOutput(inputFile: input.file, outputType: outputType) {
          outputPath = outputFileMapPath
        } else if let output = inputOutputMap[input], output.file != .standardOutput, compilerOutputType != nil {
          // Alongside primary output
          outputPath = output.file.replacingExtension(with: outputType)
        } else {
          outputPath = .temporary(RelativePath(input.file.basenameWithoutExt.appendingFileTypeExtension(outputType)))
        }
      } else {
        outputPath = finalOutputPath
      }

      flaggedInputOutputPairs.append((flag: flag, input: input, output: TypedVirtualPath(file: outputPath, type: outputType)))
    }