<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi John,<div><br><div><div>On Jul 19, 2013, at 1:18 PM, Thompson, John <<a href="mailto:John_Thompson@playstation.sony.com">John_Thompson@playstation.sony.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">The enclosed patch implements a much reworked new feature in the modularize tool for checking for inconsistent macro expansions and inconsistent condition expression evaluations in conditional directives with respect to C++ modules. It also includes a modified test for it.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">This is an implementation of the proposal I posted previously (but received no feedback), written up in this Google Doc: <a href="https://docs.google.com/document/d/1dxXKI8r-3NwQDR6Zm-JT9kh-hWrruw8tUKmG9emY8AU/edit?usp=sharing" style="color: purple; text-decoration: underline;">https://docs.google.com/document/d/1dxXKI8r-3NwQDR6Zm-JT9kh-hWrruw8tUKmG9emY8AU/edit?usp=sharing</a><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">For details about the implementation in prose form, I’ve written the following Google Docs document, with comments enabled:<span class="Apple-converted-space"> </span><a href="https://docs.google.com/document/d/1TBQF3nzk66Sw0hJV0cCsNFvwF_zQuO5WbjoMmPtEfHU/edit?usp=sharing" style="color: purple; text-decoration: underline;">https://docs.google.com/document/d/1TBQF3nzk66Sw0hJV0cCsNFvwF_zQuO5WbjoMmPtEfHU/edit?usp=sharing</a><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">The patch is relative to the “extra” directory at tools/clang/extra.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">I’m sorry it’s such a large patch. I’ve tried to put as much as I can of the new feature code in a new .h interface and .cpp implementation file, with just the bare minimum API to connect it to Modularize.cpp.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">I’d appreciate your review feedback for this patch and/or permission to check in.</div></div></div></blockquote><br></div></div><div>This looks great! The detail in these diagnostics will come in quite handy for others working to clean up headers for modules. </div><div><br></div><div>I looked through the latest version of the patch, and I don't have much in the way of specific feedback: the architecture looks solid, and I didn't see any issues with the implementation. Go ahead and commit, thanks!</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><div><br></div></body></html>