On Thu, Feb 23, 2023 at 10:53 PM Harald Anlauf wrote: > the patch is mostly fine, but there is a minor style issue: > > + if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK)) > + gfc_error ("Symbol %qs at %L has the WEAK attribute but is a %s", > + sym->name, &sym->declared_at, sym->attr.dummy > + ? "dummy argument" : "local variable"); > + > > It is my understanding that this is not translation-friendly. > Please use separate error texts for either case instead. Interesting, I was under the impression this was fixed with OO-inlines around the *.c rename. In any case, adjusted in v2 to use: + if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK)) + { + if (sym->attr.dummy) + gfc_error ("Symbol %qs at %L has the WEAK attribute but is a " + "dummy argument", sym->name, &sym->declared_at); + else + gfc_error ("Symbol %qs at %L has the WEAK attribute but is a " + "local variable", sym->name, &sym->declared_at); + } > Do we need to really have that many separate files for all > the tests? Note that each separate file contributes to the > time developers wait on regtesting to complete. Some of the > files essentially test only minor variations, like weak-2.f90 > and weak-3.f90. These testcases are dg-compile and do not go through the "-O0 -O1 -O2 -O3 -Os" options like dg-run. Combining the testcases does not reduce gfortran.sum a lot: -PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?impl -PASS: gfortran.dg/weak-2.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-3.f90 -O scan-assembler weak[^ \t]*[ \t]_?bar__ -PASS: gfortran.dg/weak-3.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-4.f90 -O scan-assembler weak[^ \t]*[ \t]_?__foo_MOD_abc -PASS: gfortran.dg/weak-4.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-5.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-6.f90 -O (test for errors, line 3) -PASS: gfortran.dg/weak-6.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-7.f90 -O (test for errors, line 10) -PASS: gfortran.dg/weak-7.f90 -O (test for errors, line 6) -PASS: gfortran.dg/weak-7.f90 -O (test for excess errors) -PASS: gfortran.dg/weak-8.f90 -O (test for errors, line 3) -PASS: gfortran.dg/weak-8.f90 -O (test for errors, line 7) -PASS: gfortran.dg/weak-8.f90 -O (test for excess errors) +PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?__foo_MOD_abc +PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?bar__ +PASS: gfortran.dg/weak-2.f90 -O scan-assembler weak[^ \t]*[ \t]_?impl1 +PASS: gfortran.dg/weak-2.f90 -O (test for excess errors) +PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 14) +PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 18) +PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 24) +PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 28) +PASS: gfortran.dg/weak-3.f90 -O (test for errors, line 5) +PASS: gfortran.dg/weak-3.f90 -O (test for excess errors) Only benefit is a bit less gfortran/f951 binaries invocations at expense of potentially introducing issues in what was intended to be tested. There exists a partial(intentionally or not) sequential file-scope namespace (like in C) how gfortran parses different units in the same file. Swapping unit order in the file can affect not only code generation but diagnostic counts reported. I tend to avoid having more than one unit per source to avoid dealing with "borrowing". However with part3 now implemented after debugging, I guess, samples could be combined to "accepts" + "rejects" two testcases, Done in v2. > What is the purpose of testcase weak-5.f90? It's valid > Fortran, the common block /c/ shows in the assembler and > does not interfere with the module variable c. Removed. Issue is not directly related to only the WEAK attributes. Will be addressed in the future. > Finally, please do not forget to CC patches to gcc-patches@ > so that others can see them. Out of curiosity, what is the purpose of CC patches to gcc-patches too? Attachments are even available in web mailing list too, like in: https://gcc.gnu.org/pipermail/fortran/2023-February/058953.html Regards, Rimvydas