C++: Support x-macros that are #undef'ed in header #1836
Merged
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
This fixes a false positive on https://github.com/zduka/tpp.
|
Changes LGTM but yes, I need to find some problematic snapshots and make sure performance is still OK... |
geoffw0
approved these changes
Aug 28, 2019
There was a problem hiding this comment.
I tried the original (pre-#313) version of the query on 16 projects, the only ones that took over 60 seconds were:
Chromium - original 594s (218 results), master 556s (218 results), PR 558s (215 results)
Libreoffice - original 201s (219 results), master 189s (219 results), PR 188s (215 results)
Linux (full) - original 283s (676 results), master 282s (676 results), PR 280s (674 results)
UnrealEngine - original exec failed; master exec failed; PR exec failed
(timeout was only set to 600s)
It looks like I've failed to find a problematic snapshot that the first PR solves (it's also possible the modern optimizer copes with all versions of the query equally now). Nevertheless, performance is no worse in my testing, and I think the original issue was caused by exposing the macroName variable in the query itself, which your version does not do. So we should be good.
FWIW, I also investigated some of the lost results and they matched your intended pattern. 👍
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.
This change should address the FP reported at https://discuss.lgtm.com/t/false-positive-in-c-header/2265.
The changed predicates take negligible time on all snapshots I've tried. @geoffw0, you must have had some problematic snapshots when you wrote #313. Can you test on those?
I'll create a change note file for 1.23 in a separate PR.