[PATCH] D50232: Fix modules build with different technique to suppress Knuth debugging

[Raphael Isemann via Phabricator via llvm-commits]

teemperor accepted this revision.
teemperor added a comment.

We could also land the original version of the patch that introduced this. It didn't contain the push/pop_macro and wouldn't change the functionality to people actually using KNUTH_DEBUG. https://reviews.llvm.org/D40404?id=124114 But I'm fine with both.

> Unfortunately this is incompatible with enabling modules while building LLVM (via LLVM_ENABLE_MODULES=ON) because the macro then has incompatible definitions in a single translation unit.

I don't think that's what's going on here. The problem is that the first use of LLVM_DEBUG is from inside the push_macro pragma. But we don't update the LLVM_DEBUG IdentifierInfo with the data from the modules before pushing it, which means we just push the undefined identifier on the stack which later will be popped and used for the rest of the translation unit. This would be fixed by https://reviews.llvm.org/D33004 or by just manually updating the identifier from the push_macro code through calling `updateOutOfDateIdentifier`.

Thanks for fixing this! LGTM, but if possible update the explanation in the commit message.


Repository:
  rL LLVM

https://reviews.llvm.org/D50232