aaron.ballman marked an inline comment as done. aaron.ballman added inline comments.
================ Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:547 + // there is no syntax I could find that would allow it). However, the option + // is handled properly on a real invocation. See: Clang::ConstructJob(). + ASSERT_THAT(GeneratedArgs, Contains(HasSubstr("-sycl-std="))); ---------------- jansvoboda11 wrote: > The `generateCC1CommandLine` function is used when CC1 gets invoked with > `-round-trip-args`. It is also going to be used in `clang-scan-deps`. > > Instead of putting a fixme here, I think it would make more sense to decide > how to handle the option properly. There are a couple of approaches: > * Instead of removing `ShouldParseIf<fsycl.KeyPath>` from `sycl_std_EQ`, it > could be replaced with `ShouldParseIf<!strconcat(fsycl_is_device.KeyPath, > "||", fsycl_is_host.KeyPath)>`. > * Remove `ShouldParseIf<fsycl.KeyPath>` from `sycl_std_EQ`, but issue > warning/error in `CompilerInvocation::fixupInvocation` whenever `SYCLVersion > != LangOptions::SYCL_None && !(SYCLIsDevice || SYCLIsHost)`. > * Remove the marshalling annotation from `sycl_std_EQ` and handle its > parsing/generation including the device/host checks manually in > `CompilerInvocation`. (I'm personally not fan of moving simple logic like > this from tablegen marshalling annotations to `CompilerInvocation` though.) > > See > https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option > for more information. Thank you for the suggestions! I took the first suggestion and applied it in this latest revision and I really like the results. 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