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

[flang] Fix ISO_Fortran_binding.h to work better with C++ code #82556

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

psteinfeld
Copy link
Contributor

[flang] Fix ISO_Fortran_binding.h to work better with C++ code

This stems from working on LANL's "dopey" project -- https://github.com/lanl/dopey.

That project contains C++ code which includes the header file "ISO_Fortran_binding.h". The dopey code wraps that include with an 'extern "C"' clause since the standard (18.5.1, paragraph 1) says that the file" shall contain C structure definitions, typedef declarations, ...". But the clang++ compiler emits error messages objecting to the fact that ISO_Fortran_binding.h contains templates.

This change fixes that by preceding the problematic code in ISO_Fortran_binding.h with language linkage clauses that specify that they contain C++ code rather than C code.

In the accompanying test, I needed to account for the fact that some people build the compiler by doing a make check-flang. In this case, the clang compiler which is required by the test will not be built.

Here's an example of a C++ program that shows the problem:

  extern "C" {
  #include "ISO_Fortran_binding.h"
  }
  int main() {
    return 0;
  }

[flang] Fix ISO_Fortran_binding.h to work better with C++ code

This stems from working on LANL's "dopey" project --
https://github.com/lanl/dopey.

That project contains C++ code which includes the header file
"ISO_Fortran_binding.h".  The dopey code  wraps that include with an
'extern "C"' clause since the standard (18.5.1, paragraph 1) says that
the file" shall contain C structure definitions, typedef declarations,
...".  But the clang++ compiler emits error messages objecting to the
fact that ISO_Fortran_binding.h contains templates.

This change fixes that by preceding the problematic code in
ISO_Fortran_binding.h with language linkage clauses that specify that
they contain C++ code rather than C code.

Here's an example of a C++ program that shows the problem:
```
  extern "C" {
  #include "ISO_Fortran_binding.h"
  }
  int main() {
    return 0;
  }
```
@llvmbot llvmbot added flang:driver flang Flang issues not falling into any other category labels Feb 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2024

@llvm/pr-subscribers-flang-driver

Author: Pete Steinfeld (psteinfeld)

Changes

[flang] Fix ISO_Fortran_binding.h to work better with C++ code

This stems from working on LANL's "dopey" project -- https://github.com/lanl/dopey.

That project contains C++ code which includes the header file "ISO_Fortran_binding.h". The dopey code wraps that include with an 'extern "C"' clause since the standard (18.5.1, paragraph 1) says that the file" shall contain C structure definitions, typedef declarations, ...". But the clang++ compiler emits error messages objecting to the fact that ISO_Fortran_binding.h contains templates.

This change fixes that by preceding the problematic code in ISO_Fortran_binding.h with language linkage clauses that specify that they contain C++ code rather than C code.

In the accompanying test, I needed to account for the fact that some people build the compiler by doing a make check-flang. In this case, the clang compiler which is required by the test will not be built.

Here's an example of a C++ program that shows the problem:

  extern "C" {
  #include "ISO_Fortran_binding.h"
  }
  int main() {
    return 0;
  }

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

2 Files Affected:

  • (modified) flang/include/flang/ISO_Fortran_binding.h (+4-4)
  • (added) flang/test/Driver/iso-fortran-binding.cpp (+32)
diff --git a/flang/include/flang/ISO_Fortran_binding.h b/flang/include/flang/ISO_Fortran_binding.h
index 4a28d3322a38f0..3f74a7e56f1755 100644
--- a/flang/include/flang/ISO_Fortran_binding.h
+++ b/flang/include/flang/ISO_Fortran_binding.h
@@ -125,7 +125,7 @@ namespace cfi_internal {
 // The below structure emulates a flexible array. This structure does not take
 // care of getting the memory storage. Note that it already contains one element
 // because a struct cannot be empty.
-template <typename T> struct FlexibleArray : T {
+extern "C++" template <typename T> struct FlexibleArray : T {
   RT_API_ATTRS T &operator[](int index) { return *(this + index); }
   const RT_API_ATTRS T &operator[](int index) const { return *(this + index); }
   RT_API_ATTRS operator T *() { return this; }
@@ -163,12 +163,12 @@ typedef struct CFI_cdesc_t {
 // needed, for C++'s CFI_cdesc_t's emulated flexible
 // dim[] array.
 namespace cfi_internal {
-template <int r> struct CdescStorage : public CFI_cdesc_t {
+extern "C++" template <int r> struct CdescStorage : public CFI_cdesc_t {
   static_assert((r > 1 && r <= CFI_MAX_RANK), "CFI_INVALID_RANK");
   CFI_dim_t dim[r - 1];
 };
-template <> struct CdescStorage<1> : public CFI_cdesc_t {};
-template <> struct CdescStorage<0> : public CFI_cdesc_t {};
+extern "C++" template <> struct CdescStorage<1> : public CFI_cdesc_t {};
+extern "C++" template <> struct CdescStorage<0> : public CFI_cdesc_t {};
 } // namespace cfi_internal
 #define CFI_CDESC_T(rank) \
   FORTRAN_ISO_NAMESPACE_::cfi_internal::CdescStorage<rank>
diff --git a/flang/test/Driver/iso-fortran-binding.cpp b/flang/test/Driver/iso-fortran-binding.cpp
new file mode 100644
index 00000000000000..f977451c729f8a
--- /dev/null
+++ b/flang/test/Driver/iso-fortran-binding.cpp
@@ -0,0 +1,32 @@
+! UNSUPPORTED: system-windows
+! RUN: split-file %s %t
+! RUN: chmod +x %t/runtest.sh
+! RUN: %t/runtest.sh %t %t/cppfile.cpp %flang | FileCheck %s
+
+!--- cppfile.cpp
+extern "C" {
+#include "ISO_Fortran_binding.h"
+}
+#include<iostream>
+
+int main() {
+  std::cout << "PASS\n";
+  return 0;
+}
+
+// CHECK: PASS
+!--- runtest.sh
+#!/bin/bash
+TMPDIR=$1
+CPPFILE=$2
+FLANG=$3
+BINDIR=`dirname $FLANG`
+CPPCOMP=$BINDIR/clang++
+if [ -x $CCOMP ]
+then
+  $CPPCOMP $CPPFILE -o $TMPDIR/a.out
+  $TMPDIR/a.out # should print "PASS"
+else
+  # No clang compiler, just pass by default
+  echo "PASS"
+fi

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you, Pete!

Copy link

github-actions bot commented Feb 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

There is a flang/test/Integration directory that might be a good place for the test. I don't know whether cpp tests run there though.

From the test/Driver directory to the test/Integration directory as
suggested by Kiran.

Thanks, Kiran!
@psteinfeld
Copy link
Contributor Author

There is a flang/test/Integration directory that might be a good place for the test. I don't know whether cpp tests run there though.

Good suggestion. I'll move it.

@psteinfeld
Copy link
Contributor Author

In separate testing (see PR #82811), I convinced my selv that the failing Windows build of these changes has nothing to do with the changes themselves.

@psteinfeld psteinfeld merged commit 796d26a into llvm:main Feb 26, 2024
3 of 4 checks passed
@psteinfeld psteinfeld deleted the ps-fix-ISO_Fortran_binding.h branch July 9, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants