Migrate 3C regression tests from %clang_cc1 to %clang. #596
Conversation
This requires replacing `%clang_cc1 -verify` with `%clang -Xclang -verify` in the tests that use it. This is one part of the RUN command cleanup that we've wanted to do for a while in #346, and now it is blocking part of #576. This is because the single behavior difference we know of between %clang_cc1 and %clang (other than the need for `-Xclang`) is that %clang_cc1 turns off the system headers, and we want two of our tests that currently use %clang_cc1 to be able to get checked declarations from the system headers. As recently as dac3f97 (August 2020), all of the tests that used %clang_cc1 used -verify. Since then, a few tests have been added that use %clang_cc1 and not -verify. This lends support to my theory that the only reason we used %clang_cc1 was that we were unaware that -verify could be used with %clang via -Xclang, and some subsequently added tests copied the use of %clang_cc1 only because the authors didn't know any better. Thus, I believe that the risk of the migration affecting the validity of the tests (e.g., causing them to falsely pass) is low.
|
I absolutely just copied the |
|
My only comment is that we should have documentation somewhere that explains why use the options we use. This will hopefully prevent people in the future from blindly copying options. Do we have this? |
The thread of #346 is probably the best source of information right now. I just added a brief "Writing regression tests" section to our 3C Google document that links to that thread. IMO, it wouldn't be a good use of time to write more formal documentation right now. If you feel strongly that we should start writing better developer documentation, I'm open to that, but I would want such a policy to be applied fairly to all of our work and not just to this PR. Realistically, copy and paste is going to remain a significant development technique for us, so sometimes just removing all (or almost all) instances of the deprecated code pattern is the most important thing to do. |
This requires replacing
%clang_cc1 -verifywith%clang -Xclang -verifyin the tests that use it.This is one part of the RUN command cleanup that we've wanted to do for a while in #346, and now it is blocking part of #576. This is because the single behavior difference we know of between %clang_cc1 and %clang (other than the need for
-Xclang) is that %clang_cc1 turns off the system headers, and we want two of our tests that currently use %clang_cc1 to be able to get checked declarations from the system headers.As recently as dac3f97 (August 2020), all of the tests that used %clang_cc1 used -verify. Since then, a few tests have been added that use %clang_cc1 and not -verify. This lends support to my theory that the only reason we used %clang_cc1 was that we were unaware that -verify could be used with %clang via -Xclang, and some subsequently added tests copied the use of %clang_cc1 only because the authors didn't know any better. Thus, I believe that the risk of the migration affecting the validity of the tests (e.g., causing them to falsely pass) is low.
Aaron, it looks like you added all of the tests that use
%clang_cc1and not-verify(seegit grep '%clang_cc1' origin | grep -v -e '-verify') except forptrtoconstarr.c. Did you have a reason for using%clang_cc1? Since I want to ask you this question anyway, I propose that you would be a good person to review the whole PR.