LGTM

Similarly, have you considered adding a differential to test C1609 and C1610?

Have you looked into whether there's a useful way to test constraint C1607, .e.g, with

type(lock_type)=, parameter :: const[*]=0

which seems not so useful because a parameter can't be be a coarray and a lock_type entity must be a coarray, but it's possible there's better way to test the constraint or it's possible the constraint is redundant because of C1608.

In order check constraint C1608 in the Fortran 2018 standard, let's add a check that flang accepts the following with and without the allocatable attribute

  use iso_fortran_env
  type foo
    type(lock_type), allocatable :: bar
  end type
  type(foo) foobar[*]
end

which gfortran rejects when the allocatable attribute is present but the NAG (nagfor) compiler correctly accepts either way.

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

On Mon, May 30, 2022 at 2:39 AM Andrzej Warzynski via Phabricator <reviews@reviews.llvm.org> wrote:

This update fixes the standard-conforming this_image() invocations in which the optional dim argument is not present.

I have access to the NAG Fortran compiler and find it very helpful for checking standard conformance. I was unaware of the NAG test suite and don't see any mention of it online. Is it an official product that can be purchased?

In D125788#3534482, @sscalpone wrote:

My belief is that we should wait until f18 hits a reasonable quality bar for executables before making flang-new the default. My reasoning seems not to have carried the day.

Add XFAIL directive and responding to one review comment by adding a keyword argument.

pushed to main

I think this is an exciting step. I hope it gets approved. Although it's technically true that this could appear to be a regression for current flang script users, the ultimate compilation currently happens by invoking an external compiler so most current flang script users can eliminate the regression by simply calling the external compiler. The exception would be if the current flang script user specifically wants flang's parsing capabilities for checking code correctness (do we know if anyone is using the flang script that way?) or for testing the parser (which I presume can still happen by other means). It might be nice to simply rename the current script so those who want it can simply change the name they use to invoke it.

• Fixed left-hand-side of assignments where array is required because dim argument is not present.
• Test requirement that the kind parameters be an integer constant.
• Test violation of rank requirement for dim argument.

Remove unimplemented "!ERROR: ..." directives.

@awarzynski I'll echo your comment that "... most people new to LLVM Flang use flang rather than flang-new and most (all?) find it very confusing." I've known of flang in various forms (including a flang that predated classic flang) and yet I'm still finding new information that clears up confusion on my part. I only just realized today that the phrase "LLVM flang" is used to differentiate from "classic flang." And although I've known about armflang for several years and AOCC (which seemingly should be AOCCF or some such) for a month or so, I only just realized today that they are based on classic flang, which finally makes sense because I couldn't figure out how they could be based on what is currently on the main branch of LLVM flang. For users and testers, it will be really nice if the community coalesces around the main branch of LLVM flang as the default meaning when someone says "flang" and if that default flang produces executables even if it's buggy.

@awarzynski based on @h-vetinari's optimism about adding flags, I still like option 2. If it's not possible to add flags, then I would prefer option 1. If we go with option 3, then I am at least encouraged by reviewing the upstreaming project status links that @kiranchandramohan posted in the Slack flang workspace #general channel. I don't know the time required to complete the remaining tasks, but the following issues show 216 of 235 tasks completed with 2 in progress:

I also very much like the second option. I think it prevents a naive user from stumbling into treacherous territory. Also, as someone who is funded to write tests for flang, making it easier to build executables would expand the options for testing. Currently, we're focused on semantics tests primarily because of the complications associated with generating executables.

Add keyword arguments to two non-conforming calls.

Add semantically correct but not-yet-supported team_type argument checks.

Added variable declarations and expanded the variety of standard-conforming calls.

Added required variable declaration.

1. Reorganized and regrouped checks.
2. Increased the coverage of checks for non-standard-conforming code.

I forgot to put the differential number in my commit message for commit e065570. How do I close this differential?

1. Fixed on typo: removed a trailing comma.
2. Added check for missing mandatory 'a' argument.
3. Added test for 'stat' argument intent(out) .
4. Added check for 'errmsg' argument intent(inout).
5. Replaced non-standard single-quote characters.

1. Fixed on typo: removed a trailing comma.
2. Added check for missing mandatory 'a' argument.
3. Added test for 'stat' argument intent(out) .
4. Added check for 'errmsg' argument intent(inout).
5. Replaced non-standard single-quote characters.

Expand coverage of SYNC ALL statement semantics test:

1. Add checks for both constraints in the standard.
2. Check more valid and invalid statement forms.

This patch makes the co_broadcast test more similar to the other collectives tests by

Update co_min test to be uniform with co_max test:

1. Expand types tested for argument 'a'.
2. Vary the types tested more throughout the test.
3. Replace several "to be determined" error messages.
4. Fix typo: remove extraneous comma.
5. Clarify the "To Do" comment.

1. Expand the coverage of argument types to include characters.
2. Vary the types of argument 'a' more throughout the test.
3. Group like calls into blocks with a leading comment describing the block.

Handled all review comments:

1. Fixed two grammatical errors in comments.
2. Clarified the To Do comment.

The latest version of the patch addresses all reviewer comments.

@ekieri Thanks for the helpful suggestions. The latest update to the patch includes a check for polymorphic 'A' and a check for matching type parameters for the arguments and result of 'operation'. The latter test has both arguments and the result all not matching a kind-type parameter of 'A'. I will upload one more patch soon with checks for the semantics of 'result_image', 'stat', and 'errmsg'.

updated patch based on comments from @ekieri

All comments have been addressed:

1. Trailing whitespace has been removed.
2. The section of the standard that defines the co_reduce interface is cited in a comment.
3. All statistically checkable restrictions are now checked.

As suggested in Diff D113086, the updated diff

1. Removes trailing whitespace if present.
2. Cites the section of the standard that defines the co_broadcast interface.
3. Adds a check for an error in which the first co_broadcast argument is coindexed.

As suggested in Diff D113086, the updated diff

1. Removes trailing whitespace if present.
2. Cites the section of the standard that defines the co_max interface.
3. Adds a check for an error in which the first co_max argument is coindexed.

Rebased off of main and ensured that the whole file appears in the diff.

@kiranchandramohan with your approval and that of @ktras, I just pushed differential D113076. Could you review and approve this one along with differential D113077, D113083, and D113084? Also, could you point me to a policy on what approvals are needed in order to push commits to main?