jansvoboda11 added inline comments.
================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:547
TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagNotPresent) {
const char *Args[] = {"-fsycl"};
----------------
This test originally checked that enabling SYCL without specifying the version
is fine and results in `LangOptions::SYCL_None`.
I'd like to keep the semantics of this test intact (it exercises the underlying
tablegen machinery of `ShouldParseIf`).
I suggested only replacing `-fsycl` with `-fsycl-is-host` here and in the
assert below.
================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:556
- ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl")));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl"))));
ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std="))));
----------------
================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:565
- ASSERT_FALSE(Diags->hasErrorOccurred());
- ASSERT_TRUE(Invocation.getLangOpts()->SYCL);
- ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(),
LangOptions::SYCL_2017);
+ ASSERT_TRUE(Diags->hasErrorOccurred());
+ ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(),
LangOptions::SYCL_None);
----------------
Here, too, I'd like to keep the test mostly intact, only updating `-fsycl` to
`-fsycl-is-host` or `-fsycl-is-device`.
You already test that `-fsycl` passed to CC1 results in an error in
`clang/test/Frontend/sycl.cpp`, so I don't think we need an unit test for that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97717/new/
https://reviews.llvm.org/D97717
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
