<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 6, 2015 at 2:55 PM, Thompson, John <span dir="ltr"><<a href="mailto:John_Thompson@playstation.sony.com" target="_blank">John_Thompson@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div><span class="">
<p class="MsoNormal">> Seems out of place in a function "getAddDependenciesAdjuster".<u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
</span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Yeah, I should have changed the name. Will do.<u></u><u></u></span></p><span class="">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal">> Also it seems egregious to change the default language, what's the rationale?<u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
</span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Because modularize is compiling .h files, the compiler apparently defaults to C parsing for .h files, making modularize users most of the time pass in a “-x
c++” option, something you pointed out before, I think. This just flips the logic and forces C users to specify “-x c”, assuming C++ will be the more common case.</span></p></div></div></blockquote><div><br></div><div>I think a better rationale (and one that we can mention in the documentation) is that most C headers are also parse-able as C++, so things will generally work, while the reverse is likely to always fail.</div><div><br></div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-John<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Sean Silva<br>
<b>Sent:</b> Wednesday, May 06, 2015 12:52 PM<br>
<b>To:</b> John Thompson<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [clang-tools-extra] r236625 - Changed option processing to implicitly use -x c++ if no other -x option specified. Added implicit -w option to disable compilation warnings, in particular to avoid warning on pragma once.<u></u><u></u></span></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Seems out of place in a function "getAddDependenciesAdjuster". Also it seems egregious to change the default language, what's the rationale?<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Wed, May 6, 2015 at 11:43 AM, John Thompson <<a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">Author: jtsoftware<br>
Date: Wed May 6 13:43:01 2015<br>
New Revision: 236625<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=236625&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=236625&view=rev</a><br>
Log:<br>
Changed option processing to implicitly use -x c++ if no other -x option specified. Added implicit -w option to disable compilation warnings, in particular to avoid warning on pragma once.<br>
<br>
Modified:<br>
clang-tools-extra/trunk/docs/ModularizeUsage.rst<br>
clang-tools-extra/trunk/modularize/Modularize.cpp<br>
<br>
Modified: clang-tools-extra/trunk/docs/ModularizeUsage.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=236625&r1=236624&r2=236625&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=236625&r1=236624&r2=236625&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/ModularizeUsage.rst (original)<br>
+++ clang-tools-extra/trunk/docs/ModularizeUsage.rst Wed May 6 13:43:01 2015<br>
@@ -37,9 +37,10 @@ directory.<br>
<br>
``<front-end-options>`` is a place-holder for regular Clang<br>
front-end arguments, which must follow the <include-files-list>.<br>
-Note that by default, the underlying Clang front end assumes .h files<br>
-contain C source, so you might need to specify the ``-x c++`` Clang option<br>
-to tell Clang that the header contains C++ definitions.<br>
+Note that by default, modularize assumes .h files<br>
+contain C++ source, so if you are using a different language,<br>
+you might need to use a ``-x`` option to tell Clang that the<br>
+header contains another language, i.e.: ``-x c``<br>
<br>
Note also that because modularize does not use the clang driver,<br>
you will likely need to pass in additional compiler front-end<br>
<br>
Modified: clang-tools-extra/trunk/modularize/Modularize.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=236625&r1=236624&r2=236625&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=236625&r1=236624&r2=236625&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Wed May 6 13:43:01 2015<br>
@@ -73,9 +73,9 @@<br>
// you will likely need to pass in additional compiler front-end<br>
// arguments to match those passed in by default by the driver.<br>
//<br>
-// Note that by default, the underlying Clang front end assumes .h files<br>
-// contain C source. If your .h files in the file list contain C++ source,<br>
-// you should append the following to your command lines: -x c++<br>
+// Note that by default, the modularize assumes .h files contain C++ source.<br>
+// If your .h files in the file list contain another language, you should<br>
+// append an appropriate -x option to your command line, i.e.: -x c<br>
//<br>
// Modularization Issue Checks<br>
//<br>
@@ -331,7 +331,8 @@ static std::string findInputFile(const C<br>
}<br>
<br>
// This arguments adjuster inserts "-include (file)" arguments for header<br>
-// dependencies.<br>
+// dependencies. It also insertts a "-w" option and a "-x c++",<br>
+// if no other "-x" option is present.<br>
static ArgumentsAdjuster<br>
getAddDependenciesAdjuster(DependencyMap &Dependencies) {<br>
return [&Dependencies](const CommandLineArguments &Args) {<br>
@@ -346,6 +347,13 @@ getAddDependenciesAdjuster(DependencyMap<br>
NewArgs.push_back(FileDependents[Index]);<br>
}<br>
}<br>
+ // Ignore warnings. (Insert after "clang_tool" at beginning.)<br>
+ NewArgs.insert(NewArgs.begin() + 1, "-w");<br>
+ // Since we are compiling .h files, assume C++ unless given a -x option.<br>
+ if (std::find(NewArgs.begin(), NewArgs.end(), "-x") == NewArgs.end()) {<br>
+ NewArgs.insert(NewArgs.begin() + 2, "-x");<br>
+ NewArgs.insert(NewArgs.begin() + 3, "c++");<br>
+ }<br>
return NewArgs;<br>
};<br>
}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div></div></div>
</div>
</blockquote></div><br></div></div>