Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Setup whitespace detection and clang-format as Github actions #66509

Closed
wants to merge 1 commit into from

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 15, 2023

Instead of using the BuildKite jobs, use GitHub actions to detect clang-format violations and trailing whitespace in PRs.

Fixes #66468

Instead of using the BuildKite jobs, use GitHub actions to detect
clang-format violations and trailing whitespace in PRs.

Fixes llvm#66468
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" github:workflow labels Sep 15, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 15, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-github-workflow

Changes Instead of using the BuildKite jobs, use GitHub actions to detect clang-format violations and trailing whitespace in PRs.

Fixes #66468

Full diff: https://github.com/llvm/llvm-project/pull/66509.diff

6 Files Affected:

  • (added) .github/workflows/clang/pr-check-trailing-whitespace.yml (+14)
  • (added) .github/workflows/clang/pr-clang-format.yml (+18)
  • (modified) clang/lib/AST/Expr.cpp (+1-1)
  • (modified) clang/lib/AST/ExprClassification.cpp (+6)
  • (modified) clang/utils/ci/buildkite-pipeline.yml (-11)
  • (modified) clang/utils/ci/run-buildbot (-3)
diff --git a/.github/workflows/clang/pr-check-trailing-whitespace.yml b/.github/workflows/clang/pr-check-trailing-whitespace.yml
new file mode 100644
index 000000000000000..b08f1e24d785ac5
--- /dev/null
+++ b/.github/workflows/clang/pr-check-trailing-whitespace.yml
@@ -0,0 +1,14 @@
+name: Ensure there are no trailing whitespace
+
+on:
+  pull_request:
+    paths:
+    - 'clang/**'
+
+jobs:
+  example:
+    name: Find Trailing Whitespace
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: harupy/find-trailing-whitespace@master
diff --git a/.github/workflows/clang/pr-clang-format.yml b/.github/workflows/clang/pr-clang-format.yml
new file mode 100644
index 000000000000000..2589b815a480f09
--- /dev/null
+++ b/.github/workflows/clang/pr-clang-format.yml
@@ -0,0 +1,18 @@
+name: clang-format Check
+
+on:
+  pull_request:
+    paths:
+    - 'clang/**'
+
+jobs:
+  formatting-check:
+    name: Formatting Check
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - name: Run clang-format style check
+      uses: jidicula/clang-format-action@v4.11.0
+      with:
+        clang-format-version: '16'
+        check-path: 'clang/'
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 4f3837371b3fc5e..342a7e3e47144bd 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -63,7 +63,7 @@ const Expr *Expr::getBestDynamicClassTypeExpr() const {
   }
 
   return E;
-}
+}   
 
 const CXXRecordDecl *Expr::getBestDynamicClassType() const {
   const Expr *E = getBestDynamicClassTypeExpr();
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 12193b7812f9bf8..18d35d1d47cfb82 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -79,6 +79,12 @@ Cl Expr::ClassifyImpl(ASTContext &Ctx, SourceLocation *Loc) const {
   return Classification(kind, modifiable);
 }
 
+static int not_well_formatted_code(int i)
+{
+   int hi = i ;
+    return hi;
+}
+
 /// Classify an expression which creates a temporary, based on its type.
 static Cl::Kinds ClassifyTemporary(QualType T) {
   if (T->isRecordType())
diff --git a/clang/utils/ci/buildkite-pipeline.yml b/clang/utils/ci/buildkite-pipeline.yml
index 7a679176038c693..22ee165d1edf4e1 100644
--- a/clang/utils/ci/buildkite-pipeline.yml
+++ b/clang/utils/ci/buildkite-pipeline.yml
@@ -17,17 +17,6 @@ env:
     # LLVM RELEASE bump version
     LLVM_HEAD_VERSION: "17"
 steps:
-  - label: "Format"
-    commands:
-      - "clang/utils/ci/run-buildbot check-format"
-    agents:
-      queue: "linux"
-    retry:
-      automatic:
-        - exit_status: -1  # Agent was lost
-          limit: 2
-    timeout_in_minutes: 120
-
   - label: "Building and testing clang (Linux)"
     commands:
       - "clang/utils/ci/run-buildbot build-clang"
diff --git a/clang/utils/ci/run-buildbot b/clang/utils/ci/run-buildbot
index d117fccc7e3fbd8..56a1a9a8c8cfa22 100755
--- a/clang/utils/ci/run-buildbot
+++ b/clang/utils/ci/run-buildbot
@@ -69,9 +69,6 @@ cmake --version
 ninja --version
 
 case "${BUILDER}" in
-check-format)
-    ! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs
-;;
 build-clang)
     mkdir install
     # We use Release here to avoid including debug information. Otherwise, the

@tru
Copy link
Collaborator

tru commented Sep 15, 2023

@ldionne thanks for the whitespace check! I have already put some work into the clang-format action on my end. Would you mind dropping it from this PR and let me push mine first? My action is not just restricted to the clang subdir.

@ldionne
Copy link
Member Author

ldionne commented Sep 15, 2023

@tru Sounds good! I gave you #66468 too.

@ldionne
Copy link
Member Author

ldionne commented Sep 15, 2023

I guess I'll close this and reopen when I have something that actually works for handling trailing whitespace.

@ldionne ldionne closed this Sep 15, 2023
@ldionne ldionne deleted the review/clang-github-actions branch September 15, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category github:workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-format job on Buildkite is bogus
3 participants