aaron.ballman added a comment. In https://reviews.llvm.org/D54349#1294325, @vmiklos wrote:
> > What do you think about code like: > > > > #if FOO == 4 > > #if FOO == 4 > > #endif > > #endif > > > > #if defined(FOO) > > #if defined(FOO) > > #endif > > #endif > > > > #if !defined(FOO) > > #if !defined(FOO) > > #endif > > #endif > > I looked at supporting these, but it's not clear to me how to get e.g. the > 'FOO == 4' string (the condition) from the `If()` callback. So far I only see > how to get the start location of the condition and the whole range till the > end of endif. Do you have a code pointer how to do this, please? (Or OK if I > leave this for a follow-up patch?) 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. >> #if defined(FOO) >> #if !defined(FOO) >> #endif >> #endif >> >> #if !defined(FOO) >> #if defined(FOO) >> #endif >> #endif > > I expect handling these correctly is more complex -- I would prefer focusing > on matching conditons in this patch, and get back to inverted conditions in a > follow-up patch. Is that OK? If we handle inverted conditions, then handling > `a && b` and `!a || !b` would make sense as well for example. 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. https://reviews.llvm.org/D54349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits