yaxunl added a comment.

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.


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

Reply via email to