Swift support

[xinzhengzhang]

Reimplement for https://github.com/hedronvision/bazel-compile-commands-extractor/pull/89

[cpsauer]

Hey, @xinzhengzhang! Thanks for all your good work on this. I remain eager to get swift support in--and this is certainly a simpler diff--but I have some questions.

• Do you not still need exclude_swift? What happened there?
• Should we have the missing file warning for swift? On one hand, we'll be able to generate all the right commands without, so we can avoid bothering the user. On the other hand, it might be handy in helping people understand when they've got the wrong flags or why there are errors that are just because files are missing.
  • If we do choose to omit it, we might choose to move the warning into _get_headers for consistency, but that can be my job.
• For finding swift sources:
  • Very interesting that there are multiple! I'd have guessed that would have triggered a lot of unnecessary recompilation. If you know why, I'd be very curious. (But if you don't, no need to research it for me.)
  • How about conditioning on mnemonic at the start (if compile_action.mnemonic == 'SwiftCompile') and then doing the filtering separately, just so it's totally clear that it's a separate codepath? If we do that, we should probably rename _get_files.source_extensions -> _get_files.c_family_source_extensions.
  • Another reason for this is that we're going to be able to remove all the other language-specific flag logic as soon as the next version of clangd ships. They fixed the underlying bug that we were working around.
• Similarly, _get_cpp_command_for_files should probably now be _get_command_for_files!
• Might there be ways to simplify _get_apple_platform? (I assume we do really need the environment variable based logic? Is the platform path not passed on Swift.) Feel free also to just pass the whole of compile_action into the _platform_patch functions.
• As before, I don't think we can assume that we'll be in the __BAZEL_XCODE_ branch for swiftc, since it can be run on linux. How about if we bring back _swift_patch? Could be something like: if swift mnemonic action, pop first arg if ends with worker, swap in swiftc, remove all -Xwapped-swift (assuming you don't need any of those arguments?). Then _apple_platform_patch can only set compile_args[0] = 'clang' if the mnemonic is not swift?
• As before, we're also going to at least need some docs before we merge. Again, if you want to just outline them, I'm happy to flesh them out. But in particular, I think it'd be good to have figured out pointing sourcekit-lsp into the accumulating index from building.

Sorry to be the one saying that this needs a bit more discussion and work--but I really appreciate your continued efforts!

Cheers, Chris

[xinzhengzhang]

"Very interesting that there are multiple! I'd have guessed that would have triggered a lot of unnecessary recompilation. If you know why, I'd be very curious."

That's how I understand it. Because in a module, swift files are mutually visible and referenced. When any file changes, it is similar to the change of the header file, and other files must be recompiled. Since this is the case, it is reasonable to put all the sources of the entire module in one compilation task.

In addition, you can see many parameters features with the module cache keyword, swift itself has been doing a lot of efforts to speed up its incremental compilation speed.

As a user, our best practice is to dismantle the library as small as possible and make full use of bazel cache

[xinzhengzhang]

Do you not still need exclude_swift? What happened there?

Thank you for your kindly permission but it is not needed now~ I have already filtered the swift source before sending to the infer.

Should we have the missing file warning for swift?

Yes, I just lost it and not added it.

For finding swift sources Similarly, _get_cpp_command_for_files should probably now be _get_command_for_files! Feel free also to just pass the whole of compile_action into the _platform_patch functions. How about if we bring back _swift_patch?

Done

As before, we're also going to at least need some docs before we merge. Again, if you want to just outline them, I'm happy to flesh them out. But in particular, I think it'd be good to have figured out pointing sourcekit-lsp into the accumulating index from building.

Because English is not my mother tongue, I can still write some short sentences, but I can't do it when it comes to the whole paragraph... So I may need your help here and I will write some outline about how the things work.

1. We need to install sourcekit-lsp. We can install it by installing the Xcode toolchain or manually compiling.
2. There are three modes for lsp server to work.
   • Specify Package.swift to work with swift package manager mode
   • Specify buildServer.json to work with build server protocl mode
   • Specify compile_commands.json to work with CompilationDatabaseBuildSystem mode
3. We need a lsp client for example vscode
   • Install sswg.swift-lang
4. Run bazel-command-extractor to extractor compile_commands.json to active plugin
   • Since swift is organized based on module the dependency converts to compile_commands will be something like -fmodule=xxxx.module
   • So just having compile_commands is not enough, we have to pre-compile once
   • Notice: Compilation and extractor need to have the same configuration

[cpsauer]

Hello again @xinzhengzhang! Again, sorry for being slow--got buried in tasks over here.

Thanks so much for teaching me more about the swift compilation model. That helps me be more effective, now and into the future.

I think we'll still need exclude_swift for others, otherwise we'll break their fb Infer setups! Could I ask you to re-add it for now? Could you also file an issue (or PR) with infer (and any other static analysis tools you use that crash on swift), asking them to automatically ignore swift files, listing the links of those issues back here? The goal is to let them know that compile_commands.json will increasingly contain swift files and maybe even encourage them to help analyze swift :) And we should also keep track of what all that flag is needed for.

Thanks so much for working through some of the others! If anything from above remains, could I ask you to work through that, too? (Update: after reading through the code and pushing some minor fixes, some definitely remain. Could I ask you to see how clean you can make things?)

On instructions for users: I'm more than happy to convert bullet points into paragraphs and generally handle the English writing part. (But you should feel good about your English! You write in English far better than I write in any other language.) But I will need slightly more detailed instructions. Do users need to manually specify the compile_commands.json mode? Is there a way to remove the dependency on a preexisting build, by, say, removing the -fmodule flags like we do for C++? (If so, we should definitely do that, I think.) And again, is there configuration that's important for Indexing While Building?

Thanks, Chris

[xinzhengzhang]

I think we'll still need exclude_swift for others, otherwise we'll break their fb Infer setups! Could I ask you to re-add it for now? Could you also file an issue (or PR) with infer (and any other static analysis tools you use that crash on swift), asking them to automatically ignore swift files, listing the links of those issues back here? The goal is to let them know that compile_commands.json will increasingly contain swift files and maybe even encourage them to help analyze swift :) And we should also keep track of what all that flag is needed for.

Ok, I will add exclude_swift and list issue link later

Do users need to manually specify the compile_commands.json mode?

No, they don't. Sourcekit-lsp will be automatically selected according to the files in the current workspace. If there are both Package.swift and compile_commands.json exists compile_commands.json have the lowest priority

Is there a way to remove the dependency on a preexisting build, by, say, removing the -fmodule flags like we do for C++? (If so, we should definitely do that, I think.)

Because swift interacts with C and swift using .module and .swiftmodule. As far as I know I don't know of any other way to get sourcekit to work without precompiling the module. However, we can precompile once in the extractor process like get_headers, but because this path is in the same location as the bazel build, we have to modify the search path of the module in compile_commands, do you think it is necessary? (I will continue to dig to see if there are other ways)

is there configuration that's important for Indexing While Building?

After my test, no matter whether the index-store of sourcekit-lsp and rule_swift are consistent when compiling, they can work normally. But setting the same store should speed up indexing. (I can make a benchmark after I haven't tested it here) In rules_swift we need to enable features SWIFT_FEATURE_INDEX_WHILE_BUILDING and SWIFT_FEATURE_USE_GLOBAL_INDEX_STORE using build --features=swift.index_while_building build --features=swift.use_global_module_cache. In sourcekit-lsp we need to set --index-store-path to the same path

[cpsauer]

Sweet. Thanks.

x2

I see. Should we automatically kick off a bazel build if there are swift compile actions, then, if there's really no way to skip building & modules? (I didn't quite understand what you were saying about the paths, but maybe you were proposing just generating the module files?)

Sounds good. When you're done, could I ask you to add that (and the above) to the instructions bullet points so I can flesh them out?

[snowp]

Gave this a shot locally but seems like it doesn't quit work:

Traceback (most recent call last):
  File "/private/var/tmp/_bazel_snowp/c1591b76b8a2fcd15b6ff3cd34fdd7da/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/external/hedron_compile_commands/refresh_all.runfiles/hedron_compile_commands/refresh_all.py", line 675, in <module>
    _get_files.source_extensions = _get_files.c_families_source_extensions + '.swift'
AttributeError: 'function' object has no attribute 'c_families_source_extensions'. Did you mean: 'c_family_source_extensions'?

This is with Bazel 6.0.0

I would love to get support for this landed so let me know if there is anything I can do to help push this forward!

[xinzhengzhang]

Gave this a shot locally but seems like it doesn't quit work:

Traceback (most recent call last):
  File "/private/var/tmp/_bazel_snowp/c1591b76b8a2fcd15b6ff3cd34fdd7da/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/external/hedron_compile_commands/refresh_all.runfiles/hedron_compile_commands/refresh_all.py", line 675, in <module>
    _get_files.source_extensions = _get_files.c_families_source_extensions + '.swift'
AttributeError: 'function' object has no attribute 'c_families_source_extensions'. Did you mean: 'c_family_source_extensions'?

This is with Bazel 6.0.0

I would love to get support for this landed so let me know if there is anything I can do to help push this forward!

Because there is only one file in the core of this project, there are many conflicts between the two features (https://github.com/hedronvision/bazel-compile-commands-extractor/pull/99 and https://github.com/hedronvision/bazel-compile-commands-extractor/pull/96) . I am going to deal with swift after the file-based pr is completed. If you need it, you can use https://github.com/xinzhengzhang/bazel-compile-commands-extractor/commits/feature/bis-support-rb3 this branch which was rebased last week and was used in the vscode plugin https://github.com/xinzhengzhang/bis.

As for the patch related to swift, I am also independent it it https://github.com/xinzhengzhang/bis/blob/main/patches/swift_support.patch

[xinzhengzhang]

Currently feature/swift-support is broken, but it conflicts too much with feature/file-based-filter. I temporarily rebase the two features into one branch https://github.com/xinzhengzhang/bazel-compile-commands-extractor/commits/feature/bis-support-rb4. I will continue to work after #99

[cpsauer]

Agree re ordering. Working on it--sorry for the backlog, guys.

[joprice]

@xinzhengzhang I tried the bis-support-rb4 branch and it works great on my project, except when I add a library that uses swift_compiler_plugin, where I get the error external macro implementation type 'ComposableArchitectureMacros.ReducerMacro' could not be found for macro 'Reducer()'. Not sure if I'm doing something wrong configuring things or if the branch doesn't support macros yet.

[joprice]

I think this might be something with how the platform is detected. I added a config setting like: https://github.com/bazelbuild/rules_swift/blob/61df2d5e9f549fa47bfe38fae7829408d16bafed/examples/xplatform/macros/BUILD#L6-L9 and I get an error like the following:

    @ComposableArchitecture//:ComposableArchitectureMacros (8eb6a2)   <-- target platform (@local_config_platform//:host) didn't satisfy constraint @platforms//:incompatible
Bazel aquery failed. Command: ['bazel', 'aquery', "mnemonic('(Objc|Cpp|Swift)Compile', deps(//:App))", '--output=jsonproto', '--include_artifacts=false', '--ui_event_filters=-info', '--noshow_progress', '--features=-compiler_param_file', '--features=-layering_check']

Should I be adding a flag to the refresh_compile_commands target to set the platform to ios?

[xinzhengzhang]

@xinzhengzhang I tried the bis-support-rb4 branch and it works great on my project, except when I add a library that uses swift_compiler_plugin, where I get the error external macro implementation type 'ComposableArchitectureMacros.ReducerMacro' could not be found for macro 'Reducer()'. Not sure if I'm doing something wrong configuring things or if the branch doesn't support macros yet.

It is true that swift macro is not currently supported. I will see how to support it recently.

[joprice]

I solved the first issue by modifying the json to remove the "-Xfrontend", arguments around the -load-plugin-executable flag.

[xinzhengzhang]

@joprice Delayed for a few days due to the need to upgrade the swift version

I would like to ask what is your environment? My environment is Sourcekit-lsp: Built-in version of Xcode 15.0.1 Swift: 5.9 Bazel: 6.4.0

Your problem is not reproduced. There is nothing wrong with the code prompt and automatic completion, but there are two flaws found so far.

1. Unable to jump
2. The macros will be marked in red

Maybe we need to wait until https://github.com/apple/sourcekit-lsp/pull/892 this is solved.

---

Test case

I have added the case of swift macros to my plugin version, its code should be consistent with bis-support-rb4

https://github.com/xinzhengzhang/bis/commit/31efb35e9470f51c02ccce5bae5d4ecba8825cdb

---

https://github.com/bazelbuild/rules_swift/blob/61df2d5e9f549fa47bfe38fae7829408d16bafed/examples/xplatform/macros/BUILD#L6-L9

I looked at the implementation of rules. This define should be completely unnecessary. It should be placed here mainly for explicit enable.

"-Xfrontend"

I didn't reproduce it. Maybe there is a problem with the version of sourcekit-lsp you are using?

[joprice]

I have the same environment. The only difference I see is freestanding vs attached macros, where I'm using attached ones like @CasePaths and @Reducer. I'll try a freestanding one and see if I get similar errors.