jpenix-quic added a comment.
> BTW, can you continue working on the lowering of the convert argument of open
> statement?
I will take a look at this!
================
Comment at: flang/lib/Lower/Bridge.cpp:2757
+ if (funit.isMainProgram()) {
+ auto conversion = bridge.getConversion();
----------------
peixin wrote:
> I think this should be moved into `void
> lowerFunc(Fortran::lower::pft::FunctionLikeUnit &funit)`.
I ended moving this again while changing approaches--the equivalent line is
above at lines 243-252. Does the new location seem reasonable? I had to add the
bool to indirectly track if a main program was specified, but the new location
seemed the most fitting to me.
================
Comment at: flang/runtime/main.cpp:55
+
+ if (auto convert{Fortran::runtime::GetConvertFromInt(i)}) {
+ Fortran::runtime::executionEnvironment.conversion = *convert;
----------------
peixin wrote:
> Nit: Is it better to to check the range of i before getting it from
> `GetConvertFromInt`?
> ```
> if (i < static_cast<int>(Convert::Unknown) || i >
> static_cast<int>(Convert::Swap)) {
> crash
> } else {
> Fortran::runtime::executionEnvironment.conversion = static_cast<Convert>(i);
> }
> ```
I ended up not needing `GetConvertFromInt` when reworking the approach, so this
has been removed!
================
Comment at: flang/runtime/main.cpp:58
+ } else {
+ Fortran::runtime::Terminator{__FILE__, __LINE__}.Crash(
+ "Invalid convert option passed to ConvertOption");
----------------
peixin wrote:
> Should `__FILE__, __LINE__` be passed as argument in lowering to point to the
> file name and line of the source file? Or is this (__FILE__, __LINE__)
> pointing the the file name and line of the source file?
I removed this instance of `__FILE__, __LINE__`, but I added another similar
one in `environment.cpp` line 33!
The error points to `environment.cpp` line 33 in the new revision. I'm not sure
if this is the best way of handling it, but given that there isn't a Fortran
source line associated with either the environment list or the old runtime call
(and as I think both are closer to implementation details than user-facing
features), pointing to the runtime file seemed like the least confusing option
for the error location.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130513/new/
https://reviews.llvm.org/D130513
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
