vmiklos marked an inline comment as done. vmiklos added inline comments.
================ Comment at: test/clang-tidy/readability-redundant-preprocessor.cpp:53 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +#if FOO == 3 + 1 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here ---------------- aaron.ballman wrote: > I didn't describe my test scenario very well -- can you change this line to > FOO == 4 but leave the preceding conditional check as FOO == 3 + 1? The goal > is to test that this isn't a token-by-token check, but uses the symbolic > values instead. That doesn't work at the moment, since `PreprocessorEntry::Condition` is just a string, so `3 + 1` won't equal to `4`. I think we discussed this above already, and you said: > I think it requires manual work. There's a FIXME in `PPCallbacks::If()` to > pass a list/tree of tokens that would make implementing this reasonable. I'd > say it's fine as a follow-up patch. But later you wrote: > I agree that it's more complex, but that's why I was asking for it -- I don't > think the design you have currently will extend for these sort of cases, and > I was hoping to cover more utility with the check to hopefully shake out > those forward-looking design considerations. As it stands, I feel like this > check is a bit too simplistic to have much value, but if you covered some of > these other cases, it would alleviate that worry for me. My hope is that the check with its current "feature set" already has some value, but let me know if I'm too optimistic. :-) That being said, if I want to support simple cases like "realize that `3 + 1` and `4` is the same" -- do you imagine that would be manually handled in this check or there is already some reusable building block in the codebase to evaluate if two expressions have the same integer value? I guess doing this manually is a lot of work, e.g. the answer for `FOO + 4` would be "it depends", so that should not equal to `8`, even if FOO happens to be `4`, etc. https://reviews.llvm.org/D54349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
