yaxunl added a comment. In D155539#4528137 <https://reviews.llvm.org/D155539#4528137>, @yaxunl wrote:
> In D155539#4528094 <https://reviews.llvm.org/D155539#4528094>, @probinson > wrote: > >> In D155539#4524543 <https://reviews.llvm.org/D155539#4524543>, @yaxunl wrote: >> >>> In D155539#4524189 <https://reviews.llvm.org/D155539#4524189>, @probinson >>> wrote: >>> >>>> This change to lang-std.cpp causes it not to verify _which_ language >>>> standard is the default. It only verifies that cuda and hip don't _change_ >>>> it. >>>> If you run FileCheck on one of those output files, it would preserve that >>>> property. >>> >>> It is intentional since we just want to make sure CUDA/HIP uses the same >>> default language standard as C++. We do not want to update the test when >>> C++ changes its default language standard. >> >> I agree you don't want to change the CUDA/HIP part. But we do want to >> preserve the check for what the actual default is. That was the purpose when >> the test was introduced in D131465 <https://reviews.llvm.org/D131465>. As it >> stands, there is no test for the actual default; you have repurposed the >> test from "check language standard defaults" to "check that CUDA/HIP don't >> affect the default" and that's losing test coverage. >> >> If you want to extract the CUDA/HIP part into a separate test, that's fine. >> But please don't lose the coverage that we had. > > Sorry, I misunderstood the purpose of this test. I will add the check back > and create a review for that. Thanks. https://reviews.llvm.org/D156127 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155539/new/ https://reviews.llvm.org/D155539 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
