<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri","sans-serif";
color:windowtext;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri","sans-serif";}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’m having trouble getting a review for this patch, so if you have an interest in modules, perhaps you could take a look and either give some feedback or a thumbs up?<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">It's a potential problem for modules (and headers in general) if '#include' directives occur inside 'extern "C/C++" {}' or 'namespace (name) {}' blocks, because that can produce different underlying definitions if other sources include
the header outside of such blocks. This patch to modularize checks for these cases, treating them as errors, and produces error message referencing both the offending '#include' and the enclosing 'extern "C/C++" {}' or 'namespace (name) {}' block.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">This is something I hit several times in using modularize on a set of some platform standard headers. I got a whole bunch of errors from modularize, but it took me a while to figure out it was because some headers were including other
headers inside extern “C” {} blocks, which of course changes the underlying definitions. I think there were also some cases where #includes were inside a namespace std {} block (because of the macro wrappers for this, i.e. _STD_BEGIN, it probably wasn’t obvious
to the developer). This enhancement points out these potential problems directly.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">There are legitimate cases where this will give errors, i.e. such as if you repeatedly include the same header to facilitate groups of similar definitions:<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">namespace foo {<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">#define FOO int<o:p></o:p></p>
<p class="MsoPlainText">#include “usefoo.h”<o:p></o:p></p>
<p class="MsoPlainText">#undef FOO<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">#define FOO float<o:p></o:p></p>
<p class="MsoPlainText">#include “usefoo.h”<o:p></o:p></p>
<p class="MsoPlainText">#undef FOO<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">}<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">But I think you can just either ignore the errors, or move the namespace inside foo.h.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Here’s the Phabricator page for reviewing the changes:<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><a href="http://llvm-reviews.chandlerc.com/D1648">http://llvm-reviews.chandlerc.com/D1648</a><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Files:<o:p></o:p></p>
<p class="MsoPlainText"> test/modularize/Inputs/Empty.h<o:p></o:p></p>
<p class="MsoPlainText"> test/modularize/Inputs/IncludeInExtern.h<o:p></o:p></p>
<p class="MsoPlainText"> test/modularize/Inputs/IncludeInNamespace.h<o:p></o:p></p>
<p class="MsoPlainText"> test/modularize/ProblemsExternC.modularize<o:p></o:p></p>
<p class="MsoPlainText"> test/modularize/ProblemsNamespace.modularize<o:p></o:p></p>
<p class="MsoPlainText"> modularize/PreprocessorTracker.cpp<o:p></o:p></p>
<p class="MsoPlainText"> modularize/PreprocessorTracker.h<o:p></o:p></p>
<p class="MsoPlainText"> modularize/Modularize.cpp<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-John<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>