<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 21, 2015, at 6:13 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" 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;" class=""><br class="Apple-interchange-newline">On 2015-Jan-21, at 17:49, Chris Bieneman <<a href="mailto:beanz@apple.com" class="">beanz@apple.com</a>> wrote:<br class=""><br class="">Author: cbieneman<br class="">Date: Wed Jan 21 19:49:59 2015<br class="">New Revision: 226762<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226762&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=226762&view=rev</a><br class="">Log:<br class="">Assigning and copying command line option objects shouldn't be allowed.<br class=""><br class="">Summary:<br class="">The default copy and assignment operators for these objects probably don't actually do what the clients intend, so they should be deleted.<br class=""><br class="">Places using the assignment operator to set the value of an option should cast to the option's data type first to call into the override for operator=. Places using the copy constructor just need to be changed to not copy (i.e. passing by const reference instead of value).<br class=""><br class="">Reviewers: dexonsmith, chandlerc<br class=""><br class="">Subscribers: llvm-commits<br class=""><br class="">Differential Revision: <a href="http://reviews.llvm.org/D7114" class="">http://reviews.llvm.org/D7114</a><br class=""><br class="">Modified:<br class=""> llvm/trunk/include/llvm/Support/CommandLine.h<br class=""> llvm/trunk/tools/lli/lli.cpp<br class=""> llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp<br class=""> llvm/trunk/tools/llvm-size/llvm-size.cpp<br class=""><br class="">Modified: llvm/trunk/include/llvm/Support/CommandLine.h<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=226762&r1=226761&r2=226762&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=226762&r1=226761&r2=226762&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/Support/CommandLine.h (original)<br class="">+++ llvm/trunk/include/llvm/Support/CommandLine.h Wed Jan 21 19:49:59 2015<br class="">@@ -1180,6 +1180,10 @@ public:<br class=""> return this->getValue();<br class=""> }<br class=""><br class="">+ // Command line options should not be copyable<br class="">+ opt(const opt &) LLVM_DELETED_FUNCTION;<br class="">+ opt &operator=(const opt &) LLVM_DELETED_FUNCTION;<br class="">+<br class=""></blockquote><br 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;" class=""><span 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; float: none; display: inline !important;" class="">Can you move these to the `private` section? That'll give better</span><br 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;" class=""><span 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; float: none; display: inline !important;" class="">diagnostics when `= deleted` isn't available.</span><br 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;" class=""></div></blockquote><div><br class=""></div>Yep. Done with r226769.</div><div><br class=""></div><div>-Chris</div><div><br class=""><blockquote type="cite" class=""><div class=""><br 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;" class=""><blockquote type="cite" 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;" class=""> // One option...<br class=""> template <class M0t><br class=""> explicit opt(const M0t &M0)<br class="">@@ -1374,6 +1378,10 @@ public:<br class=""><br class=""> void setNumAdditionalVals(unsigned n) { Option::setNumAdditionalVals(n); }<br class=""><br class="">+ // Command line options should not be copyable<br class="">+ list(const list &) LLVM_DELETED_FUNCTION;<br class="">+ list &operator=(const list &) LLVM_DELETED_FUNCTION;<br class="">+<br class=""> // One option...<br class=""> template <class M0t><br class=""> explicit list(const M0t &M0)<br class="">@@ -1592,6 +1600,10 @@ public:<br class=""> return Positions[optnum];<br class=""> }<br class=""><br class="">+ // Command line options should not be copyable<br class="">+ bits(const bits &) LLVM_DELETED_FUNCTION;<br class="">+ bits &operator=(const bits &) LLVM_DELETED_FUNCTION;<br class="">+<br class=""> // One option...<br class=""> template <class M0t><br class=""> explicit bits(const M0t &M0)<br class="">@@ -1725,6 +1737,10 @@ public:<br class=""> AliasFor = &O;<br class=""> }<br class=""><br class="">+ // Command line options should not be copyable<br class="">+ alias(const alias &) LLVM_DELETED_FUNCTION;<br class="">+ alias &operator=(const alias &) LLVM_DELETED_FUNCTION;<br class="">+<br class=""> // One option...<br class=""> template <class M0t><br class=""> explicit alias(const M0t &M0)<br class=""><br class="">Modified: llvm/trunk/tools/lli/lli.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=226762&r1=226761&r2=226762&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=226762&r1=226761&r2=226762&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/tools/lli/lli.cpp (original)<br class="">+++ llvm/trunk/tools/lli/lli.cpp Wed Jan 21 19:49:59 2015<br class="">@@ -556,7 +556,7 @@ int main(int argc, char **argv, char * c<br class=""> // If the user specifically requested an argv[0] to pass into the program,<br class=""> // do it now.<br class=""> if (!FakeArgv0.empty()) {<br class="">- InputFile = FakeArgv0;<br class="">+ InputFile = static_cast<std::string>(FakeArgv0);<br class=""> } else {<br class=""> // Otherwise, if there is a .bc suffix on the executable strip it off, it<br class=""> // might confuse the program.<br class=""><br class="">Modified: llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=226762&r1=226761&r2=226762&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=226762&r1=226761&r2=226762&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp (original)<br class="">+++ llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp Wed Jan 21 19:49:59 2015<br class="">@@ -38,7 +38,8 @@ static void exitWithError(const Twine &M<br class=""><br class="">enum ProfileKinds { instr, sample };<br class=""><br class="">-void mergeInstrProfile(cl::list<std::string> Inputs, StringRef OutputFilename) {<br class="">+void mergeInstrProfile(const cl::list<std::string> &Inputs,<br class="">+ StringRef OutputFilename) {<br class=""> if (OutputFilename.compare("-") == 0)<br class=""> exitWithError("Cannot write indexed profdata format to stdout.");<br class=""><br class="">@@ -64,7 +65,8 @@ void mergeInstrProfile(cl::list<std::str<br class=""> Writer.write(Output);<br class="">}<br class=""><br class="">-void mergeSampleProfile(cl::list<std::string> Inputs, StringRef OutputFilename,<br class="">+void mergeSampleProfile(const cl::list<std::string> &Inputs,<br class="">+ StringRef OutputFilename,<br class=""> sampleprof::SampleProfileFormat OutputFormat) {<br class=""> using namespace sampleprof;<br class=""> auto WriterOrErr = SampleProfileWriter::create(OutputFilename, OutputFormat);<br class=""><br class="">Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=226762&r1=226761&r2=226762&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=226762&r1=226761&r2=226762&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)<br class="">+++ llvm/trunk/tools/llvm-size/llvm-size.cpp Wed Jan 21 19:49:59 2015<br class="">@@ -709,7 +709,7 @@ int main(int argc, char **argv) {<br class=""><br class=""> ToolName = argv[0];<br class=""> if (OutputFormatShort.getNumOccurrences())<br class="">- OutputFormat = OutputFormatShort;<br class="">+ OutputFormat = static_cast<OutputFormatTy>(OutputFormatShort);<br class=""> if (RadixShort.getNumOccurrences())<br class=""> Radix = RadixShort;<br class=""><br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</blockquote></div></blockquote></div><br class=""></body></html>