-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[Flang] Use a module directory to avoid race condition #123215
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
Conversation
Use a module directory in a test that uses another fortran test to avoid race conditions in module creation.
@llvm/pr-subscribers-flang-fir-hlfir Author: Kiran Chandramohan (kiranchandramohan) ChangesUse a module directory in a test that uses another fortran test to avoid race conditions in module creation. Full diff: https://github.com/llvm/llvm-project/pull/123215.diff 1 Files Affected:
diff --git a/flang/test/Lower/module_use.f90 b/flang/test/Lower/module_use.f90
index ad43865470b681..c7b29df882fadc 100644
--- a/flang/test/Lower/module_use.f90
+++ b/flang/test/Lower/module_use.f90
@@ -1,5 +1,6 @@
-! RUN: bbc -emit-fir %S/module_definition.f90
-! RUN: bbc -emit-fir %s -o - | FileCheck %s
+! RUN: rm -fr %t && mkdir %t
+! RUN: bbc -emit-fir -module %t %S/module_definition.f90
+! RUN: bbc -emit-fir -J %t %s -o - | FileCheck %s
! Test use of module data not defined in this file.
! The modules are defined in module_definition.f90
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the -p
suggestion, this looks ok. Hopefully it fixes the other issue.
flang/test/Lower/module_use.f90
Outdated
@@ -1,5 +1,6 @@ | |||
! RUN: bbc -emit-fir %S/module_definition.f90 | |||
! RUN: bbc -emit-fir %s -o - | FileCheck %s | |||
! RUN: rm -fr %t && mkdir %t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mkdir -p
would be safer, just in case there is something that doesn't exist along the way
flang/test/Lower/module_use.f90
Outdated
@@ -1,5 +1,6 @@ | |||
! RUN: bbc -emit-fir %S/module_definition.f90 | |||
! RUN: bbc -emit-fir %s -o - | FileCheck %s | |||
! RUN: rm -fr %t && mkdir %t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this break Windows bots?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have other similar tests that seem to work OK.
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/write-module.f90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, some other tests use rm
and mkdir
without excluding Windows, so I guess this is fine.
Sorry for the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Kiran!
Use a module directory in a test that uses another fortran test to avoid race conditions in module creation.