From c7c7992293532d417e188e7cd4690a194e779a8d Mon Sep 17 00:00:00 2001 From: Snorlax Date: Fri, 23 Dec 2022 19:39:34 +0800 Subject: [PATCH 1/9] Swift support --- refresh.template.py | 58 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index f985ba5..537cd8d 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -553,11 +553,17 @@ def _get_headers(compile_action, source_path: str): def _get_files(compile_action): """Gets the ({source files}, {header files}) clangd should be told the command applies to.""" + # If we've got swift action just return sources + if compile_action.mnemonic == 'SwiftCompile': + source_files = set(filter(lambda arg: arg.endswith(_get_files.swift_source_extension), compile_action.arguments)) + return source_files, set() + # Getting the source file is a little trickier than it might seem. # First, we do the obvious thing: Filter args to those that look like source files. - source_file_candidates = [arg for arg in compile_action.arguments if not arg.startswith('-') and arg.endswith(_get_files.source_extensions)] + source_file_candidates = [arg for arg in compile_action.arguments if not arg.startswith('-') and arg.endswith(_get_files.c_family_source_extensions)] assert source_file_candidates, f"No source files found in compile args: {compile_action.arguments}.\nPlease file an issue with this information!" + source_file = source_file_candidates[0] # If we've got multiple candidates for source files, apply heuristics based on how Bazel tends to format commands. @@ -584,7 +590,7 @@ def _get_files(compile_action): source_index = compile_action.arguments.index('/c') + 1 source_file = compile_action.arguments[source_index] - assert source_file.endswith(_get_files.source_extensions), f"Source file candidate, {source_file}, seems to be wrong.\nSelected from {compile_action.arguments}.\nPlease file an issue with this information!" + assert source_file.endswith(_get_files.c_family_source_extensions), f"Source file candidate, {source_file}, seems to be wrong.\nSelected from {compile_action.arguments}.\nPlease file an issue with this information!" # Warn gently about missing files if not os.path.isfile(source_file): @@ -640,7 +646,8 @@ def _get_files(compile_action): _get_files.openclxx_source_extensions = ('.clcpp',) _get_files.assembly_source_extensions = ('.s', '.asm') _get_files.assembly_needing_c_preprocessor_source_extensions = ('.S',) -_get_files.source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions +_get_files.swift_source_extension = '.swift' +_get_files.c_family_source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions _get_files.extensions_to_language_args = { # Note that clangd fails on the --language or -ObjC or -ObjC++ forms. See https://github.com/clangd/clangd/issues/1173#issuecomment-1226847416 _get_files.c_source_extensions: '-xc', _get_files.cpp_source_extensions: '-xc++', @@ -673,17 +680,25 @@ def _get_apple_SDKROOT(SDK_name: str): # Traditionally stored in SDKROOT environment variable, but not provided by Bazel. See https://github.com/bazelbuild/bazel/issues/12852 -def _get_apple_platform(compile_args: typing.List[str]): +def _get_apple_platform(compile_action): """Figure out which Apple platform a command is for. Is the name used by Xcode in the SDK files, not the marketing name. e.g. iPhoneOS, not iOS. """ # A bit gross, but Bazel specifies the platform name in one of the include paths, so we mine it from there. + compile_args = compile_action.arguments for arg in compile_args: match = re.search('/Platforms/([a-zA-Z]+).platform/Developer/', arg) if match: return match.group(1) + if getattr(compile_action, 'environmentVariables', None): + match = next( + filter(lambda x: x.key == "APPLE_SDK_PLATFORM", compile_action.environmentVariables), + None + ) + if match: + return match.value return None @@ -695,18 +710,21 @@ def _get_apple_DEVELOPER_DIR(): # Traditionally stored in DEVELOPER_DIR environment variable, but not provided by Bazel. See https://github.com/bazelbuild/bazel/issues/12852 -def _apple_platform_patch(compile_args: typing.List[str]): +def _apple_platform_patch(compile_action): """De-Bazel the command into something clangd can parse. This function has fixes specific to Apple platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not. """ # Bazel internal environment variable fragment that distinguishes Apple platforms that need unwrapping. # Note that this occurs in the Xcode-installed wrapper, but not the CommandLineTools wrapper, which works fine as is. + compile_args = compile_action.arguments if any('__BAZEL_XCODE_' in arg for arg in compile_args): # Undo Bazel's Apple platform compiler wrapping. # Bazel wraps the compiler as `external/local_config_cc/wrapped_clang` and exports that wrapped compiler in the proto. However, we need a clang call that clangd can introspect. (See notes in "how clangd uses compile_commands.json" in ImplementationReadme.md for more.) # Removing the wrapper is also important because Bazel's Xcode (but not CommandLineTools) wrapper crashes if you don't specify particular environment variables (replaced below). We'd need the wrapper to be invokable by clangd's --query-driver if we didn't remove the wrapper. - compile_args[0] = 'clang' + + if compile_action.mnemonic != 'SwiftCompile': + compile_args[0] = 'clang' # We have to manually substitute out Bazel's macros so clang can parse the command # Code this mirrors is in https://github.com/bazelbuild/bazel/blob/master/tools/osx/crosstool/wrapped_clang.cc @@ -715,13 +733,29 @@ def _apple_platform_patch(compile_args: typing.List[str]): # We also have to manually figure out the values of SDKROOT and DEVELOPER_DIR, since they're missing from the environment variables Bazel provides. # Filed Bazel issue about the missing environment variables: https://github.com/bazelbuild/bazel/issues/12852 compile_args = [arg.replace('__BAZEL_XCODE_DEVELOPER_DIR__', _get_apple_DEVELOPER_DIR()) for arg in compile_args] - apple_platform = _get_apple_platform(compile_args) + apple_platform = _get_apple_platform(compile_action) assert apple_platform, f"Apple platform not detected in CMD: {compile_args}" compile_args = [arg.replace('__BAZEL_XCODE_SDKROOT__', _get_apple_SDKROOT(apple_platform)) for arg in compile_args] return compile_args +def _swift_patch(compile_args: typing.List[str]): + + # rules_swift add a worker for wrapping if enable --persistent_worker flag (https://bazel.build/remote/persistent) + # https://github.com/bazelbuild/rules_swift/blob/master/swift/internal/actions.bzl#L236 + # We need to remove it (build_bazel_rules_swift/tools/worker/worker) + while len(compile_args) > 0 and (not 'swiftc' in compile_args[0]): + compile_args.pop(0) + + assert len(compile_args) > 0, "No compiler found in swift_path" + compile_args[0] = 'swiftc' + + # Remove -Xwrapped-swift introduced by rules_swift + compile_args = [arg for arg in compile_args if not arg.startswith('-Xwrapped-swift')] + + return compile_args + def _all_platform_patch(compile_args: typing.List[str]): """Apply de-Bazeling fixes to the compile command that are shared across target platforms.""" # clangd writes module cache files to the wrong place @@ -758,14 +792,16 @@ def _all_platform_patch(compile_args: typing.List[str]): return compile_args -def _get_cpp_command_for_files(compile_action): +def _get_command_for_files(compile_action): """Reformat compile_action into a compile command clangd can understand. Undo Bazel-isms and figures out which files clangd should apply the command to. """ # Patch command by platform compile_action.arguments = _all_platform_patch(compile_action.arguments) - compile_action.arguments = _apple_platform_patch(compile_action.arguments) + compile_action.arguments = _apple_platform_patch(compile_action) + if compile_action.mnemonic == 'SwiftCompile': + compile_action.arguments = _swift_patch(compile_action.arguments) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. source_files, header_files = _get_files(compile_action) @@ -803,7 +839,7 @@ def _amend_action_as_external(action): with concurrent.futures.ThreadPoolExecutor( max_workers=min(32, (os.cpu_count() or 1) + 4) # Backport. Default in MIN_PY=3.8. See "using very large resources implicitly on many-core machines" in https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.ThreadPoolExecutor ) as threadpool: - outputs = threadpool.map(_get_cpp_command_for_files, aquery_output.actions) + outputs = threadpool.map(_get_command_for_files, aquery_output.actions) # Yield as compile_commands.json entries header_files_already_written = set() @@ -863,7 +899,7 @@ def _get_commands(target: str, flags: str): # Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html # Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto # One bummer, not described in the docs, is that aquery filters over *all* actions for a given target, rather than just those that would be run by a build to produce a given output. This mostly isn't a problem, but can sometimes surface extra, unnecessary, misconfigured actions. Chris has emailed the authors to discuss and filed an issue so anyone reading this could track it: https://github.com/bazelbuild/bazel/issues/14156. - f"mnemonic('(Objc|Cpp)Compile',{target_statment})", + f"mnemonic('(Objc|Cpp|Swift)Compile',{target_statment})", # We switched to jsonproto instead of proto because of https://github.com/bazelbuild/bazel/issues/13404. We could change back when fixed--reverting most of the commit that added this line and tweaking the build file to depend on the target in that issue. That said, it's kinda nice to be free of the dependency, unless (OPTIMNOTE) jsonproto becomes a performance bottleneck compated to binary protos. '--output=jsonproto', # We'll disable artifact output for efficiency, since it's large and we don't use them. Small win timewise, but dramatically less json output from aquery. From b042f0d686fb978c4361ffc7f8dbbb87c1f7ce6c Mon Sep 17 00:00:00 2001 From: Snorlax Date: Fri, 23 Dec 2022 20:18:10 +0800 Subject: [PATCH 2/9] Add _warn_if_file_doesnt_exist for reminding missing files --- refresh.template.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 537cd8d..b9b6292 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -94,6 +94,25 @@ def _print_header_finding_warning_once(): _print_header_finding_warning_once.has_logged = False +def _warn_if_file_doesnt_exist(source_file): + if not os.path.isfile(source_file): + if not _warn_if_file_doesnt_exist.has_logged_missing_file_error: # Just log once; subsequent messages wouldn't add anything. + _warn_if_file_doesnt_exist.has_logged_missing_file_error = True + log_warning(f""">>> A source file you compile doesn't (yet) exist: {source_file} + It's probably a generated file, and you haven't yet run a build to generate it. + That's OK; your code doesn't even have to compile for this tool to work. + If you can, though, you might want to run a build of your code. + That way everything is generated, browsable and indexed for autocomplete. + However, if you have *already* built your code, and generated the missing file... + Please make sure you're supplying this tool with the same flags you use to build. + You can either use a refresh_compile_commands rule or the special -- syntax. Please see the README. + [Supplying flags normally won't work. That just causes this tool to be built with those flags.] + Continuing gracefully...""") + return True + return False +_warn_if_file_doesnt_exist.has_logged_missing_file_error = False + + @functools.lru_cache(maxsize=None) def _get_bazel_cached_action_keys(): """Gets the set of actionKeys cached in bazel-out.""" @@ -556,6 +575,9 @@ def _get_files(compile_action): # If we've got swift action just return sources if compile_action.mnemonic == 'SwiftCompile': source_files = set(filter(lambda arg: arg.endswith(_get_files.swift_source_extension), compile_action.arguments)) + for source_file in source_files: + _warn_if_file_doesnt_exist(source_file) + return source_files, set() # Getting the source file is a little trickier than it might seem. @@ -593,19 +615,7 @@ def _get_files(compile_action): assert source_file.endswith(_get_files.c_family_source_extensions), f"Source file candidate, {source_file}, seems to be wrong.\nSelected from {compile_action.arguments}.\nPlease file an issue with this information!" # Warn gently about missing files - if not os.path.isfile(source_file): - if not _get_files.has_logged_missing_file_error: # Just log once; subsequent messages wouldn't add anything. - _get_files.has_logged_missing_file_error = True - log_warning(f""">>> A source file you compile doesn't (yet) exist: {source_file} - It's probably a generated file, and you haven't yet run a build to generate it. - That's OK; your code doesn't even have to compile for this tool to work. - If you can, though, you might want to run a build of your code. - That way everything is generated, browsable and indexed for autocomplete. - However, if you have *already* built your code, and generated the missing file... - Please make sure you're supplying this tool with the same flags you use to build. - You can either use a refresh_compile_commands rule or the special -- syntax. Please see the README. - [Supplying flags normally won't work. That just causes this tool to be built with those flags.] - Continuing gracefully...""") + if _warn_if_file_doesnt_exist(source_file): return {source_file}, set() # Note: We need to apply commands to headers and sources. From d6296e62fae5fca09d7272f928fcafb6ac008a76 Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 21:31:19 -0800 Subject: [PATCH 3/9] Move _warn_if_file_doesnt_exist into inverted tree prefix order --- refresh.template.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index b9b6292..1f2b21c 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -94,25 +94,6 @@ def _print_header_finding_warning_once(): _print_header_finding_warning_once.has_logged = False -def _warn_if_file_doesnt_exist(source_file): - if not os.path.isfile(source_file): - if not _warn_if_file_doesnt_exist.has_logged_missing_file_error: # Just log once; subsequent messages wouldn't add anything. - _warn_if_file_doesnt_exist.has_logged_missing_file_error = True - log_warning(f""">>> A source file you compile doesn't (yet) exist: {source_file} - It's probably a generated file, and you haven't yet run a build to generate it. - That's OK; your code doesn't even have to compile for this tool to work. - If you can, though, you might want to run a build of your code. - That way everything is generated, browsable and indexed for autocomplete. - However, if you have *already* built your code, and generated the missing file... - Please make sure you're supplying this tool with the same flags you use to build. - You can either use a refresh_compile_commands rule or the special -- syntax. Please see the README. - [Supplying flags normally won't work. That just causes this tool to be built with those flags.] - Continuing gracefully...""") - return True - return False -_warn_if_file_doesnt_exist.has_logged_missing_file_error = False - - @functools.lru_cache(maxsize=None) def _get_bazel_cached_action_keys(): """Gets the set of actionKeys cached in bazel-out.""" @@ -569,6 +550,25 @@ def _get_headers(compile_action, source_path: str): _get_headers.has_logged = False +def _warn_if_file_doesnt_exist(source_file): + if not os.path.isfile(source_file): + if not _warn_if_file_doesnt_exist.has_logged_missing_file_error: # Just log once; subsequent messages wouldn't add anything. + _warn_if_file_doesnt_exist.has_logged_missing_file_error = True + log_warning(f""">>> A source file you compile doesn't (yet) exist: {source_file} + It's probably a generated file, and you haven't yet run a build to generate it. + That's OK; your code doesn't even have to compile for this tool to work. + If you can, though, you might want to run a build of your code. + That way everything is generated, browsable and indexed for autocomplete. + However, if you have *already* built your code, and generated the missing file... + Please make sure you're supplying this tool with the same flags you use to build. + You can either use a refresh_compile_commands rule or the special -- syntax. Please see the README. + [Supplying flags normally won't work. That just causes this tool to be built with those flags.] + Continuing gracefully...""") + return True + return False +_warn_if_file_doesnt_exist.has_logged_missing_file_error = False + + def _get_files(compile_action): """Gets the ({source files}, {header files}) clangd should be told the command applies to.""" From 1e0f4a75d7d2b579758e9826b392ca55b9a6e502 Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 21:37:37 -0800 Subject: [PATCH 4/9] Swift source detection: Use comprehension and tuple for parallelism --- refresh.template.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 1f2b21c..a2cc281 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -574,7 +574,7 @@ def _get_files(compile_action): # If we've got swift action just return sources if compile_action.mnemonic == 'SwiftCompile': - source_files = set(filter(lambda arg: arg.endswith(_get_files.swift_source_extension), compile_action.arguments)) + source_files = set(arg for arg in compile_action.arguments if arg.endswith(_get_files.swift_source_extensions)) for source_file in source_files: _warn_if_file_doesnt_exist(source_file) @@ -656,7 +656,7 @@ def _get_files(compile_action): _get_files.openclxx_source_extensions = ('.clcpp',) _get_files.assembly_source_extensions = ('.s', '.asm') _get_files.assembly_needing_c_preprocessor_source_extensions = ('.S',) -_get_files.swift_source_extension = '.swift' +_get_files.swift_source_extensions = ('.swift',) _get_files.c_family_source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions _get_files.extensions_to_language_args = { # Note that clangd fails on the --language or -ObjC or -ObjC++ forms. See https://github.com/clangd/clangd/issues/1173#issuecomment-1226847416 _get_files.c_source_extensions: '-xc', From c2f06830f71d23dd8a5fb06c1b5e695f91298c76 Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 21:38:23 -0800 Subject: [PATCH 5/9] Remove unused _get_files.has_logged_missing_file_error --- refresh.template.py | 1 - 1 file changed, 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index a2cc281..04614be 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -644,7 +644,6 @@ def _get_files(compile_action): compile_action.arguments.insert(1, lang_flag) return {source_file}, header_files -_get_files.has_logged_missing_file_error = False # Setup extensions and flags for the whole C-language family. # Clang has a list: https://github.com/llvm/llvm-project/blob/b9f3b7f89a4cb4cf541b7116d9389c73690f78fa/clang/lib/Driver/Types.cpp#L293 _get_files.c_source_extensions = ('.c', '.i') From 4fa5a0192231e35cfd836ead378e7ca58d3c859c Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 21:43:00 -0800 Subject: [PATCH 6/9] Fold swift extension variable --- refresh.template.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 04614be..9764a90 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -574,7 +574,7 @@ def _get_files(compile_action): # If we've got swift action just return sources if compile_action.mnemonic == 'SwiftCompile': - source_files = set(arg for arg in compile_action.arguments if arg.endswith(_get_files.swift_source_extensions)) + source_files = set(arg for arg in compile_action.arguments if arg.endswith('.swift')) for source_file in source_files: _warn_if_file_doesnt_exist(source_file) @@ -655,7 +655,6 @@ def _get_files(compile_action): _get_files.openclxx_source_extensions = ('.clcpp',) _get_files.assembly_source_extensions = ('.s', '.asm') _get_files.assembly_needing_c_preprocessor_source_extensions = ('.S',) -_get_files.swift_source_extensions = ('.swift',) _get_files.c_family_source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions _get_files.extensions_to_language_args = { # Note that clangd fails on the --language or -ObjC or -ObjC++ forms. See https://github.com/clangd/clangd/issues/1173#issuecomment-1226847416 _get_files.c_source_extensions: '-xc', From da11703709881f6315ff9bb7370c788fa34f85f3 Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 21:45:38 -0800 Subject: [PATCH 7/9] Re-add _get_files For simpler merging with the "file based filter" PR moving to end for greater clarity --- refresh.template.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index 9764a90..841a549 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -655,7 +655,6 @@ def _get_files(compile_action): _get_files.openclxx_source_extensions = ('.clcpp',) _get_files.assembly_source_extensions = ('.s', '.asm') _get_files.assembly_needing_c_preprocessor_source_extensions = ('.S',) -_get_files.c_family_source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions _get_files.extensions_to_language_args = { # Note that clangd fails on the --language or -ObjC or -ObjC++ forms. See https://github.com/clangd/clangd/issues/1173#issuecomment-1226847416 _get_files.c_source_extensions: '-xc', _get_files.cpp_source_extensions: '-xc++', @@ -668,6 +667,8 @@ def _get_files(compile_action): _get_files.assembly_needing_c_preprocessor_source_extensions: '-xassembler-with-cpp', } _get_files.extensions_to_language_args = {ext : flag for exts, flag in _get_files.extensions_to_language_args.items() for ext in exts} # Flatten map for easier use +_get_files.c_family_source_extensions = _get_files.c_source_extensions + _get_files.cpp_source_extensions + _get_files.objc_source_extensions + _get_files.objcpp_source_extensions + _get_files.cuda_source_extensions + _get_files.opencl_source_extensions + _get_files.openclxx_source_extensions + _get_files.assembly_source_extensions + _get_files.assembly_needing_c_preprocessor_source_extensions +_get_files.source_extensions = _get_files.c_families_source_extensions + '.swift' @functools.lru_cache(maxsize=None) From 7efd4b2c9fe13b878a68b3171599f7d54d5cb32e Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 22:08:29 -0800 Subject: [PATCH 8/9] Make command patch functions more parallel Defensive instead of arms length protection All take the compile_acion and return a modified args array. --- refresh.template.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 841a549..70fef31 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -724,9 +724,9 @@ def _apple_platform_patch(compile_action): This function has fixes specific to Apple platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not. """ + compile_args = compile_action.arguments # Bazel internal environment variable fragment that distinguishes Apple platforms that need unwrapping. # Note that this occurs in the Xcode-installed wrapper, but not the CommandLineTools wrapper, which works fine as is. - compile_args = compile_action.arguments if any('__BAZEL_XCODE_' in arg for arg in compile_args): # Undo Bazel's Apple platform compiler wrapping. # Bazel wraps the compiler as `external/local_config_cc/wrapped_clang` and exports that wrapped compiler in the proto. However, we need a clang call that clangd can introspect. (See notes in "how clangd uses compile_commands.json" in ImplementationReadme.md for more.) @@ -749,7 +749,15 @@ def _apple_platform_patch(compile_action): return compile_args -def _swift_patch(compile_args: typing.List[str]): +def _swift_patch(compile_action): + """De-Bazel the command into something sourecekit-lsp can parse. + + This function has fixes specific to Swift, but you should call it on all platforms. It'll determine whether the fixes should be applied or not. + """ + + compile_args = compile_action.arguments + if compile_action.mnemonic != 'SwiftCompile': + return # rules_swift add a worker for wrapping if enable --persistent_worker flag (https://bazel.build/remote/persistent) # https://github.com/bazelbuild/rules_swift/blob/master/swift/internal/actions.bzl#L236 @@ -765,8 +773,12 @@ def _swift_patch(compile_args: typing.List[str]): return compile_args -def _all_platform_patch(compile_args: typing.List[str]): + +def _all_platform_patch(compile_action): """Apply de-Bazeling fixes to the compile command that are shared across target platforms.""" + + compile_args = compile_action.arguments + # clangd writes module cache files to the wrong place # Without this fix, you get tons of module caches dumped into the VSCode root folder. # Filed clangd issue at: https://github.com/clangd/clangd/issues/655 @@ -807,10 +819,9 @@ def _get_command_for_files(compile_action): Undo Bazel-isms and figures out which files clangd should apply the command to. """ # Patch command by platform - compile_action.arguments = _all_platform_patch(compile_action.arguments) + compile_action.arguments = _all_platform_patch(compile_action) compile_action.arguments = _apple_platform_patch(compile_action) - if compile_action.mnemonic == 'SwiftCompile': - compile_action.arguments = _swift_patch(compile_action.arguments) + compile_action.arguments = _swift_patch(compile_action) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. source_files, header_files = _get_files(compile_action) From 5dd390c8904ba59694e8786a661417204208bd59 Mon Sep 17 00:00:00 2001 From: Christopher Peterson Sauer Date: Sun, 29 Jan 2023 22:10:30 -0800 Subject: [PATCH 9/9] Fix _swift_patch early return --- refresh.template.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 70fef31..baffd52 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -756,20 +756,18 @@ def _swift_patch(compile_action): """ compile_args = compile_action.arguments - if compile_action.mnemonic != 'SwiftCompile': - return - - # rules_swift add a worker for wrapping if enable --persistent_worker flag (https://bazel.build/remote/persistent) - # https://github.com/bazelbuild/rules_swift/blob/master/swift/internal/actions.bzl#L236 - # We need to remove it (build_bazel_rules_swift/tools/worker/worker) - while len(compile_args) > 0 and (not 'swiftc' in compile_args[0]): - compile_args.pop(0) + if compile_action.mnemonic == 'SwiftCompile': + # rules_swift add a worker for wrapping if enable --persistent_worker flag (https://bazel.build/remote/persistent) + # https://github.com/bazelbuild/rules_swift/blob/master/swift/internal/actions.bzl#L236 + # We need to remove it (build_bazel_rules_swift/tools/worker/worker) + while len(compile_args) > 0 and (not 'swiftc' in compile_args[0]): + compile_args.pop(0) - assert len(compile_args) > 0, "No compiler found in swift_path" - compile_args[0] = 'swiftc' + assert len(compile_args) > 0, "No compiler found in swift_path" + compile_args[0] = 'swiftc' - # Remove -Xwrapped-swift introduced by rules_swift - compile_args = [arg for arg in compile_args if not arg.startswith('-Xwrapped-swift')] + # Remove -Xwrapped-swift introduced by rules_swift + compile_args = [arg for arg in compile_args if not arg.startswith('-Xwrapped-swift')] return compile_args