Skip to content

Conversation

@PikachuHyA
Copy link
Contributor

@PikachuHyA PikachuHyA commented Jun 14, 2024

This patch refactors the C++ compilation actions in CcCompilationHelper. It introduces a helper method createSourceActionHelper to streamline the creation of source actions and reduces duplicated code related to PIC and non-PIC compilation.

Changes

  1. Refactored Source Action Creation:

    • Introduced createSourceActionHelper to handle the common logic for creating compile actions.
    • This method consolidates the setup and finalization of compile actions for both PIC and non-PIC variants.
    • The helper method now handles setting up variables, creating temp actions, and registering the compile actions.
  2. Updated createSourceAction Method:

    • The createSourceAction method now calls createSourceActionHelper for both PIC and non-PIC compile actions.
    • This change simplifies the createSourceAction method and removes redundancy.

Background

I'm working on support C++20 Modules in Bazel, see #19940

While constructing the action graph for compiling C++20 Modules, I noticed that the createSourceAction code could be reused. Therefore, I refactored createSourceAction. One potentially unusual aspect is the handling of ArtifactCategory.CPP_MODULE at the end. Since the logic for handling C++20 Modules differs from that of Clang Modules, this part of the logic was placed outside of createSourceActionHelper.

the code of constructing the action graph for compiling C++20 Modules is #22553

@PikachuHyA PikachuHyA requested a review from lberki as a code owner June 14, 2024 07:53
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Jun 14, 2024
@PikachuHyA
Copy link
Contributor Author

cc @comius

@sgowroji sgowroji added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jun 15, 2024
@PikachuHyA PikachuHyA force-pushed the refactor_createSourceAction branch from 4e03084 to 09ed307 Compare July 22, 2024 09:06
@PikachuHyA
Copy link
Contributor Author

@comius @trybka
Could you please review this patch? It was split out from #22553 , which is still quite large.

@PikachuHyA
Copy link
Contributor Author

@comius @trybka
ping

@PikachuHyA PikachuHyA force-pushed the refactor_createSourceAction branch 2 times, most recently from 58d0397 to 09eb1d9 Compare September 4, 2024 04:31
@PikachuHyA
Copy link
Contributor Author

hi @lberki,

could you please assign this PR to @comius and @trybka? It would be great to have this patch reviewed and merged before we tackle #22553.

thank you

@PikachuHyA
Copy link
Contributor Author

cc @katre

@fmeum fmeum requested review from comius and trybka September 6, 2024 06:47
@fmeum fmeum added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Sep 6, 2024
@fmeum fmeum removed the request for review from lberki September 6, 2024 06:48
@PikachuHyA PikachuHyA force-pushed the refactor_createSourceAction branch from 09eb1d9 to b4396a4 Compare December 16, 2024 07:28
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 24, 2025
fmeum pushed a commit to fmeum/bazel that referenced this pull request Jan 29, 2025
This patch refactors the C++ compilation actions in `CcCompilationHelper`. It introduces a helper method `createSourceActionHelper` to streamline the creation of source actions and reduces duplicated code related to PIC and non-PIC compilation.

## Changes
1. **Refactored Source Action Creation**:
    - Introduced `createSourceActionHelper` to handle the common logic for creating compile actions.
    - This method consolidates the setup and finalization of compile actions for both PIC and non-PIC variants.
    - The helper method now handles setting up variables, creating temp actions, and registering the compile actions.

2. **Updated `createSourceAction` Method**:
    - The `createSourceAction` method now calls `createSourceActionHelper` for both PIC and non-PIC compile actions.
    - This change simplifies the `createSourceAction` method and removes redundancy.

## Background

I'm working on support C++20 Modules in Bazel, see bazelbuild#19940

While constructing the action graph for compiling C++20 Modules, I noticed that the `createSourceAction` code could be reused. Therefore, I refactored `createSourceAction`. One potentially unusual aspect is the handling of `ArtifactCategory.CPP_MODULE` at the end. Since the logic for handling C++20 Modules differs from that of Clang Modules, this part of the logic was placed outside of `createSourceActionHelper`.

the code of constructing the action graph for compiling C++20 Modules is bazelbuild#22553

Closes bazelbuild#22744.

PiperOrigin-RevId: 719290267
Change-Id: I62cd8826261866ce3bd4225c099a479288a449aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Rules-CPP Issues for C++ rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants