This is an archive of the discontinued LLVM Phabricator instance.

[flang] Make ISO_Fortran_binding.h a standalone header again.
ClosedPublic

Authored by vzakhari on Aug 22 2023, 1:58 PM.

Details

Summary

This implements the proposal from
https://discourse.llvm.org/t/adding-flang-specific-header-files-to-clang/72442/6
Since ISO_Fortran_binding.h is supposed to be included from users'
C/C++ codes, it would better have no dependencies on other header
files.

Diff Detail

Event Timeline

vzakhari created this revision.Aug 22 2023, 1:58 PM
Herald added a project: Restricted Project. · View Herald Transcript
vzakhari requested review of this revision.Aug 22 2023, 1:58 PM
klausler added inline comments.Aug 22 2023, 2:05 PM
flang/include/flang/iso_fortran_binding_wrapper.h
1 ↗(On Diff #552500)

Why not capitalize ISO?

24 ↗(On Diff #552500)

Doesn't api-attrs.h need to be included before ISO_Fortran_binding.h?

vzakhari added inline comments.Aug 22 2023, 2:10 PM
flang/include/flang/iso_fortran_binding_wrapper.h
1 ↗(On Diff #552500)

I do not have any preference here, so I followed the simplest typing pattern. I will capitalize it.

24 ↗(On Diff #552500)

Thanks for catching! Pre-upload clang-format did this.

PeteSteinfeld accepted this revision.Aug 22 2023, 2:44 PM

I agree with Peter on the naming of the file.

Otherwise, all builds and tests correctly and looks good.

flang/include/flang/iso_fortran_binding_wrapper.h
24 ↗(On Diff #552500)

Isn't it just alphabetizing to include "ISO..." before "Run..."?

This revision is now accepted and ready to land.Aug 22 2023, 2:44 PM
vzakhari updated this revision to Diff 552514.Aug 22 2023, 2:46 PM
  • Fixed review remarks.
flang/include/flang/iso_fortran_binding_wrapper.h
24 ↗(On Diff #552500)

Yes, it does.