[flang] Move internal Fortran::ISO namespace out of user-facing ISO_F… #72909
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vzakhari
approved these changes
Nov 20, 2023
psteinfeld
requested changes
Nov 21, 2023
There was a problem hiding this comment.
Thanks for the fast response!
Line 172 of the updated version of ISO_Fortran_binding.h defines the macro CFI_CDESC_T using ::Fortran::ISO::cfi_internal... This no longer works because the namespaces Fortran and ISO have been removed.
Also, existing C++ code that wraps an include of ISO_Fortran_binding.h with extern "C"will stil l run into problems because it will think that the header file contains pure C code. That is, it will not allow templates or namespaces. This has the relatively easy fix of just removing the extern "C" declaration, though.
Sorry that I missed that, will try again. |
…ortran_binding.h
... and into the ISO_Fortran_binding_wrapper.h header, through which
the compiler and runtime access its contents. This change ensures that
user code that #includes ISO_Fortran_binding.h within 'extern "C" {'
doesn't encounter mysterious namespace errors.
|
@llvm/pr-subscribers-flang-codegen Author: Peter Klausler (klausler) Changes…ortran_binding.h ... and into the ISO_Fortran_binding_wrapper.h header, through which the compiler and runtime access its contents. This change ensures that user code that #includes ISO_Fortran_binding.h within 'extern "C" {' doesn't encounter mysterious namespace errors. Full diff: https://github.com/llvm/llvm-project/pull/72909.diff 3 Files Affected:
diff --git a/flang/include/flang/ISO_Fortran_binding.h b/flang/include/flang/ISO_Fortran_binding.h
index 51d6219427cce5e..dd384c516263e5f 100644
--- a/flang/include/flang/ISO_Fortran_binding.h
+++ b/flang/include/flang/ISO_Fortran_binding.h
@@ -10,7 +10,14 @@
#ifndef CFI_ISO_FORTRAN_BINDING_H_
#define CFI_ISO_FORTRAN_BINDING_H_
+/* When this header is included into the compiler and runtime implementations,
+ * it does so by means of a wrapper header that establishes namespaces and
+ * a macro for extra function attributes (RT_API_ATTRS).
+ */
+#ifndef FORTRAN_ISO_FORTRAN_BINDING_WRAPPER_H_
#include <stddef.h>
+#define FORTRAN_ISO_NAMESPACE_
+#endif
/* Standard interface to Fortran from C and C++.
* These interfaces are named in subclause 18.5 of the Fortran 2018
@@ -22,12 +29,6 @@
#define RT_API_ATTRS
#endif
-#ifdef __cplusplus
-namespace Fortran {
-namespace ISO {
-inline namespace Fortran_2018 {
-#endif
-
/* 18.5.4 */
#define CFI_VERSION 20180515
@@ -169,7 +170,8 @@ template <int r> struct CdescStorage : public CFI_cdesc_t {
template <> struct CdescStorage<1> : public CFI_cdesc_t {};
template <> struct CdescStorage<0> : public CFI_cdesc_t {};
} // namespace cfi_internal
-#define CFI_CDESC_T(rank) ::Fortran::ISO::cfi_internal::CdescStorage<rank>
+#define CFI_CDESC_T(rank) \
+ FORTRAN_ISO_NAMESPACE_::cfi_internal::CdescStorage<rank>
#else
#define CFI_CDESC_T(_RANK) \
struct { \
@@ -199,9 +201,6 @@ RT_API_ATTRS int CFI_setpointer(
CFI_cdesc_t *, const CFI_cdesc_t *source, const CFI_index_t lower_bounds[]);
#ifdef __cplusplus
} // extern "C"
-} // inline namespace Fortran_2018
-} // namespace ISO
-} // namespace Fortran
#endif
#endif /* CFI_ISO_FORTRAN_BINDING_H_ */
diff --git a/flang/include/flang/ISO_Fortran_binding_wrapper.h b/flang/include/flang/ISO_Fortran_binding_wrapper.h
index c810ebccdbcadd5..83c974365e3439e 100644
--- a/flang/include/flang/ISO_Fortran_binding_wrapper.h
+++ b/flang/include/flang/ISO_Fortran_binding_wrapper.h
@@ -22,8 +22,18 @@
*/
/* clang-format off */
+#include <stddef.h>
#include "Runtime/api-attrs.h"
+#ifdef __cplusplus
+namespace Fortran {
+namespace ISO {
+#define FORTRAN_ISO_NAMESPACE_ ::Fortran::ISO
+#endif /* __cplusplus */
#include "ISO_Fortran_binding.h"
+#ifdef __cplusplus
+} // namespace ISO
+} // namespace Fortran
+#endif /* __cplusplus */
/* clang-format on */
#endif /* FORTRAN_ISO_FORTRAN_BINDING_WRAPPER_H_ */
diff --git a/flang/lib/Optimizer/CodeGen/DescriptorModel.h b/flang/lib/Optimizer/CodeGen/DescriptorModel.h
index 39427a42f0f4d66..ed35caef9301493 100644
--- a/flang/lib/Optimizer/CodeGen/DescriptorModel.h
+++ b/flang/lib/Optimizer/CodeGen/DescriptorModel.h
@@ -113,7 +113,7 @@ getModel<Fortran::ISO::cfi_internal::FlexibleArray<Fortran::ISO::CFI_dim_t>>() {
/// Get the type model of the field number `Field` in an ISO CFI descriptor.
template <int Field>
static constexpr TypeBuilderFunc getDescFieldTypeModel() {
- Fortran::ISO::Fortran_2018::CFI_cdesc_t dummyDesc{};
+ Fortran::ISO::CFI_cdesc_t dummyDesc{};
// check that the descriptor is exactly 8 fields as specified in CFI_cdesc_t
// in flang/include/flang/ISO_Fortran_binding.h.
auto [a, b, c, d, e, f, g, h] = dummyDesc;
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Peter Klausler (klausler) Changes…ortran_binding.h ... and into the ISO_Fortran_binding_wrapper.h header, through which the compiler and runtime access its contents. This change ensures that user code that #includes ISO_Fortran_binding.h within 'extern "C" {' doesn't encounter mysterious namespace errors. Full diff: https://github.com/llvm/llvm-project/pull/72909.diff 3 Files Affected:
diff --git a/flang/include/flang/ISO_Fortran_binding.h b/flang/include/flang/ISO_Fortran_binding.h
index 51d6219427cce5e..dd384c516263e5f 100644
--- a/flang/include/flang/ISO_Fortran_binding.h
+++ b/flang/include/flang/ISO_Fortran_binding.h
@@ -10,7 +10,14 @@
#ifndef CFI_ISO_FORTRAN_BINDING_H_
#define CFI_ISO_FORTRAN_BINDING_H_
+/* When this header is included into the compiler and runtime implementations,
+ * it does so by means of a wrapper header that establishes namespaces and
+ * a macro for extra function attributes (RT_API_ATTRS).
+ */
+#ifndef FORTRAN_ISO_FORTRAN_BINDING_WRAPPER_H_
#include <stddef.h>
+#define FORTRAN_ISO_NAMESPACE_
+#endif
/* Standard interface to Fortran from C and C++.
* These interfaces are named in subclause 18.5 of the Fortran 2018
@@ -22,12 +29,6 @@
#define RT_API_ATTRS
#endif
-#ifdef __cplusplus
-namespace Fortran {
-namespace ISO {
-inline namespace Fortran_2018 {
-#endif
-
/* 18.5.4 */
#define CFI_VERSION 20180515
@@ -169,7 +170,8 @@ template <int r> struct CdescStorage : public CFI_cdesc_t {
template <> struct CdescStorage<1> : public CFI_cdesc_t {};
template <> struct CdescStorage<0> : public CFI_cdesc_t {};
} // namespace cfi_internal
-#define CFI_CDESC_T(rank) ::Fortran::ISO::cfi_internal::CdescStorage<rank>
+#define CFI_CDESC_T(rank) \
+ FORTRAN_ISO_NAMESPACE_::cfi_internal::CdescStorage<rank>
#else
#define CFI_CDESC_T(_RANK) \
struct { \
@@ -199,9 +201,6 @@ RT_API_ATTRS int CFI_setpointer(
CFI_cdesc_t *, const CFI_cdesc_t *source, const CFI_index_t lower_bounds[]);
#ifdef __cplusplus
} // extern "C"
-} // inline namespace Fortran_2018
-} // namespace ISO
-} // namespace Fortran
#endif
#endif /* CFI_ISO_FORTRAN_BINDING_H_ */
diff --git a/flang/include/flang/ISO_Fortran_binding_wrapper.h b/flang/include/flang/ISO_Fortran_binding_wrapper.h
index c810ebccdbcadd5..83c974365e3439e 100644
--- a/flang/include/flang/ISO_Fortran_binding_wrapper.h
+++ b/flang/include/flang/ISO_Fortran_binding_wrapper.h
@@ -22,8 +22,18 @@
*/
/* clang-format off */
+#include <stddef.h>
#include "Runtime/api-attrs.h"
+#ifdef __cplusplus
+namespace Fortran {
+namespace ISO {
+#define FORTRAN_ISO_NAMESPACE_ ::Fortran::ISO
+#endif /* __cplusplus */
#include "ISO_Fortran_binding.h"
+#ifdef __cplusplus
+} // namespace ISO
+} // namespace Fortran
+#endif /* __cplusplus */
/* clang-format on */
#endif /* FORTRAN_ISO_FORTRAN_BINDING_WRAPPER_H_ */
diff --git a/flang/lib/Optimizer/CodeGen/DescriptorModel.h b/flang/lib/Optimizer/CodeGen/DescriptorModel.h
index 39427a42f0f4d66..ed35caef9301493 100644
--- a/flang/lib/Optimizer/CodeGen/DescriptorModel.h
+++ b/flang/lib/Optimizer/CodeGen/DescriptorModel.h
@@ -113,7 +113,7 @@ getModel<Fortran::ISO::cfi_internal::FlexibleArray<Fortran::ISO::CFI_dim_t>>() {
/// Get the type model of the field number `Field` in an ISO CFI descriptor.
template <int Field>
static constexpr TypeBuilderFunc getDescFieldTypeModel() {
- Fortran::ISO::Fortran_2018::CFI_cdesc_t dummyDesc{};
+ Fortran::ISO::CFI_cdesc_t dummyDesc{};
// check that the descriptor is exactly 8 fields as specified in CFI_cdesc_t
// in flang/include/flang/ISO_Fortran_binding.h.
auto [a, b, c, d, e, f, g, h] = dummyDesc;
|
psteinfeld
approved these changes
Nov 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ortran_binding.h
... and into the ISO_Fortran_binding_wrapper.h header, through which the compiler and runtime access its contents. This change ensures that user code that #includes ISO_Fortran_binding.h within 'extern "C" {' doesn't encounter mysterious namespace errors.