rovka added inline comments.
================ Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" ---------------- awarzynski wrote: > rovka wrote: > > awarzynski wrote: > > > rovka wrote: > > > > mmuetzel wrote: > > > > > Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional? > > > > Yes, I don't want those to appear either before or after the Fortran > > > > libs. I guess if we wanted to be pedantic we'd also check that they > > > > don't appear after the object_file, or between the libs and the > > > > subsystem, but that seems a bit much. > > > Based on the [[ > > > https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-same-directive > > > | docs ]], I'd say that this would be the idiomatic way to do this: > > > ```lang=bash > > > ! MSVC-LABEL: > > > ! MSVC-NOT: > > > ! MSVC-SAME: > > > ``` > > > IIUC, the following would only be needed if there's a potential for > > > `libcmt` or `oldnames` to appear on a separate line: > > > ``` > > > ```lang=bash > > > ! MSVC-LABEL: > > > ! MSVC-NOT: > > > ! MSVC-SAME: > > > ! MSVC-NOT: > > > ``` > > > But this wouldn't happen, right? (there's going to be only one linker > > > invocation). Also, you could just use [[ > > > https://llvm.org/docs/CommandGuide/FileCheck.html#options | > > > --implicit-check-not ]] :) > > > Based on the [[ > > > https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-same-directive > > > | docs ]], I'd say that this would be the idiomatic way to do this: > > > ```lang=bash > > > ! MSVC-LABEL: > > > ! MSVC-NOT: > > > ! MSVC-SAME: > > > ``` > > > > Based on the same docs, I would say it shouldn't be enough to mention it > > just once. But that's just what I expect, the docs are completely unhelpful > > about the actual behaviour here. For instance, I would expect to be able to > > write > > ``` > > ! MSVC-NOT: should-only-come-after-X > > ! MSVC-SAME: X > > ``` > > If the MSVC-NOT applies to the whole line, then lines with 'X > > should-come-after-X' get rejected, but imo they should be accepted (I'll > > admit I didn't actually verify this, and anyway the implicit-check-not > > makes the whole discussion moot). > > > > > IIUC, the following would only be needed if there's a potential for > > > `libcmt` or `oldnames` to appear on a separate line: > > > ``` > > > ```lang=bash > > > ! MSVC-LABEL: > > > ! MSVC-NOT: > > > ! MSVC-SAME: > > > ! MSVC-NOT: > > > ``` > > > > I agree that if there's no MSVC-SAME after the last MSVC-NOT, then the > > MSVC-NOT would apply to the following line. > > > > > But this wouldn't happen, right? (there's going to be only one linker > > > invocation). Also, you could just use [[ > > > https://llvm.org/docs/CommandGuide/FileCheck.html#options | > > > --implicit-check-not ]] :) > > > > Wooow 😍 I didn't know about that one, I'll definitely update the test to > > use it, thanks! > > Based on the same docs, I would say it shouldn't be enough to mention it > > just once. > > I'm re-rereading the docs and agree. Sounds like `--implicit-check-not` is > the best option. > > > For instance, I would expect to be able to write > > ``` > > ! MSVC-NOT: should-only-come-after-X > > ! MSVC-SAME: X > > ``` > > If the MSVC-NOT applies to the whole line, then lines with 'X > > should-come-after-X' get rejected > > I think that it //should// and //is// accepted. Example below: > > **file.f90** > ```lang=bash > ! RUN: cat %S/test.f90 | FileCheck %s > > ! CHECK-LABEL: my-label > ! CHECK-NOT: should-only-come-after-X > ! CHECK-SAME: X > ! CHECK-SAME: should-only-come-after-X > ``` > > **test.f90** > ```lang=bash > my-label X should-only-come-after-X > ``` > I copied the above into the Driver test dir and run like this: > ``` > $ bin/llvm-lit -va ../../flang/test/Driver/file.f90 > -- Testing: 1 tests, 1 workers -- > PASS: Flang :: Driver/file.f90 (1 of 1) > > Testing Time: 0.03s > Passed: 1 > ``` > > Does this agree with your experiments? > > #filecheck-is-confusing :) Yep, that's what I was saying :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
