[SYCL] Don't diagnose checks in a discarded statement. #3714
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For the checks we do in SemaSYCL via the callgraph, we don't want to diagnose when it is in a discarded statement. This adds the infrastructure for another bug, where we visit constexpr during this too.
erichkeane
commented
May 7, 2021
clang/lib/Analysis/CallGraph.cpp
Outdated
| } | ||
| } | ||
|
|
||
| VisitChildren(If); |
There was a problem hiding this comment.
This line PERHAPS (i haven't thought enough through?) should be the StmtVisitor::VisitStmt(If); call instead... I think it ends up changing nothing, but is probably the right thing to do.
vladimirlaz
pushed a commit
that referenced
this pull request
Jun 23, 2021
This patch removes ConstexprDepthRAII which incorrectly maintained constexpr context. Instead we don't traverse statements which are forced constant expressions. This patch fixes the following bugs - constexpr int j = test_constexpr_context(recfn(1)); int k; if constexpr (false) k = recfn(1); Here, recfn() is a recursive function. The usage 1. should not diagnose in SYCL kernel since the function is called in constexpr context. Similarly 2. should not diagnose in SYCL kernel since the recursive function call is in a discarded branch. Includes commits in PR - #3714 Signed-off-by: Elizabeth Andrews <elizabeth.andrews@intel.com> Co-authored-by: Erich Keane <erich.keane@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For the checks we do in SemaSYCL via the callgraph, we don't want to
diagnose when it is in a discarded statement. This adds the
infrastructure for another bug, where we visit constexpr during this
too.
Note this is a 'draft' since I don't find myself with the time to implement the "rest" of this (tests + all the rest of the constexpr stuff). I believe @elizabethandrews was assigned the constexpr-call bug after @mibintc gave up on it ( :D), so this might be of interest to her!
The change to the CallGraph.cpp (CGBuilder) should be all that is necessary, we simply have to implement the 'visit' functions for every situation we can come up with for constant-evaluation. I believe the ill-planned ConstexprRAII (which likely needs removing once we fix this?) has at least most of the list implemented, so that should be at least a head start.