necto wrote: @steakhal > Speaking of hardcoded USRs in the tests, have you explored if we could make a > tool/utility that could create these instead of hardcoding these? > > I expect that in the future we might need to change the mangling there, that > would be also non backward compatible. An indirection would sidestep these > hardcoded USR issues.
I have explored a little and found no easy and portable way. Likely one could use c-index-test or clang_extdef_map. I agree it would make the tests better. Unfortunately, I have no time to do that at the moment. > I think the way CTU is tested using these thousands of Input files is just > some legacy because we didn't have split-file to actually have these input > files in a single test file. > I also dislike the hard-coded USRs, and I think I'll claude something to make > this prettier unless you say I shouldn't because you have forks/or downstream > patches where such changes would interfere with. In that case I'd of course > try not to mess with you. I agree and would welcome the improvement. It should not be too hard to port downstream patches @balazske > I like adding these tests and improve error reporting. One issue with these > tests can be that if the import "silently fails" we can not verify if there > was really the expected type of error (or no error), but this is less > important. (But I would like more if these cases are somehow shown to the > user.) Probably the rename and move part of this change can be moved to > another PR. The new failure tests can be moved again to new sub-directory (or > have a common name). I agree here as well, and again for the time constraints I will not be able to demonstrate that the added test cases actually triggering the error path they are supposed to trigger. In one of the follow-ups I would like to actually report diagnostics for these cases (i.e., address the FIXME in `CrossTranslationUnitContext::emitCrossTUDiagnostics`), unless it is not welcome in the upstream. I have addressed your comment, @steakhal in 231317175365 fixup! Add test cases for various CTU-import failure modes And I have renamed the files after your approval. If you approve, I plan to split this PR into two as suggested by @balazske : - add a bunch cases for CTU import failures - move all CTU-related cases into clang/test/Analysis/ctu directory https://github.com/llvm/llvm-project/pull/188524 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
