[DO NOT MERGE][Testing] Review Clang patches #20552
Draft
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
Comment out invalidating cache
Comment out isOpen()
This reverts commit 1324b32.
Co-authored-by: Jonas Hahnfeld <[email protected]>
The parser (or rather the BalancedDelimiterTracker) was doing a Parser::cutOffParsing() because of an overflow of open ( - because we never reset the count.
Reverts part of upstream LLVM commit llvm/llvm-project@84df7a0 The newer logic triggers an assertion when building ROOT.
This deals with the fact that our modulemaps include headers which can vary across library versions and that attribute is a way to express this.
Right now the ExternalASTSources in LLVM have an awkward way of implementing their generation counters, which provide an incrementing UID for versioning the AST when it is changed lazily by the ExternalASTSource (for example when more decls are). The current implementation is based on having an counter in each ExternalASTSource, but each ExternalASTSource actually only refers to the top most ExternalASTSource of the current ASTContext, which means that the counter suddenly resets when we add any kind of new ExternalASTSource (and we have no way to work around this). Also, some ExternalASTSources like the ASTReader make assumptions that they are the top most ExternalASTSource which means that as soon as we overwrite the ASTReader, we suddenly have two counters running providing conflicting information to anyone querying the counters. This patch merges all these counters into one counter which is in the ASTContext. This should get rid of any more counter desyncronization problems when we attach our own external sources or when parts of the code make invalid assumptions about which external source is currently the top most one in the ASTContext. Patch is upstreamed via LLVM phabricator review D39714.
Test Results 14 files 14 suites 2d 9h 38m 0s ⏱️ For more details on these failures, see this check. Results for commit cf31991. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This Pull request:
Attempts to get ROOT building with minimal clang patches. This helps us reorder patches in-order of priority, from patches that are critical for ROOT to build and patches that are nice to have.
A lot of tests are expected to fail.
Changes or fixes:
Checklist:
This PR fixes #