[llvm] r226762 - Assigning and copying command line option objects shouldn't be allowed.

[Chris Bieneman]

> On Jan 21, 2015, at 6:13 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> On 2015-Jan-21, at 17:49, Chris Bieneman <beanz at apple.com> wrote:
>> 
>> Author: cbieneman
>> Date: Wed Jan 21 19:49:59 2015
>> New Revision: 226762
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=226762&view=rev
>> Log:
>> Assigning and copying command line option objects shouldn't be allowed.
>> 
>> Summary:
>> The default copy and assignment operators for these objects probably don't actually do what the clients intend, so they should be deleted.
>> 
>> 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).
>> 
>> Reviewers: dexonsmith, chandlerc
>> 
>> Subscribers: llvm-commits
>> 
>> Differential Revision: http://reviews.llvm.org/D7114
>> 
>> Modified:
>>   llvm/trunk/include/llvm/Support/CommandLine.h
>>   llvm/trunk/tools/lli/lli.cpp
>>   llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
>>   llvm/trunk/tools/llvm-size/llvm-size.cpp
>> 
>> Modified: llvm/trunk/include/llvm/Support/CommandLine.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=226762&r1=226761&r2=226762&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/CommandLine.h (original)
>> +++ llvm/trunk/include/llvm/Support/CommandLine.h Wed Jan 21 19:49:59 2015
>> @@ -1180,6 +1180,10 @@ public:
>>    return this->getValue();
>>  }
>> 
>> +  // Command line options should not be copyable
>> +  opt(const opt &) LLVM_DELETED_FUNCTION;
>> +  opt &operator=(const opt &) LLVM_DELETED_FUNCTION;
>> +
> 
> Can you move these to the `private` section?  That'll give better
> diagnostics when `= deleted` isn't available.

Yep. Done with r226769.

-Chris

> 
>>  // One option...
>>  template <class M0t>
>>  explicit opt(const M0t &M0)
>> @@ -1374,6 +1378,10 @@ public:
>> 
>>  void setNumAdditionalVals(unsigned n) { Option::setNumAdditionalVals(n); }
>> 
>> +  // Command line options should not be copyable
>> +  list(const list &) LLVM_DELETED_FUNCTION;
>> +  list &operator=(const list &) LLVM_DELETED_FUNCTION;
>> +
>>  // One option...
>>  template <class M0t>
>>  explicit list(const M0t &M0)
>> @@ -1592,6 +1600,10 @@ public:
>>    return Positions[optnum];
>>  }
>> 
>> +  // Command line options should not be copyable
>> +  bits(const bits &) LLVM_DELETED_FUNCTION;
>> +  bits &operator=(const bits &) LLVM_DELETED_FUNCTION;
>> +
>>  // One option...
>>  template <class M0t>
>>  explicit bits(const M0t &M0)
>> @@ -1725,6 +1737,10 @@ public:
>>    AliasFor = &O;
>>  }
>> 
>> +  // Command line options should not be copyable
>> +  alias(const alias &) LLVM_DELETED_FUNCTION;
>> +  alias &operator=(const alias &) LLVM_DELETED_FUNCTION;
>> +
>>  // One option...
>>  template <class M0t>
>>  explicit alias(const M0t &M0)
>> 
>> Modified: llvm/trunk/tools/lli/lli.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=226762&r1=226761&r2=226762&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/lli/lli.cpp (original)
>> +++ llvm/trunk/tools/lli/lli.cpp Wed Jan 21 19:49:59 2015
>> @@ -556,7 +556,7 @@ int main(int argc, char **argv, char * c
>>  // If the user specifically requested an argv[0] to pass into the program,
>>  // do it now.
>>  if (!FakeArgv0.empty()) {
>> -    InputFile = FakeArgv0;
>> +    InputFile = static_cast<std::string>(FakeArgv0);
>>  } else {
>>    // Otherwise, if there is a .bc suffix on the executable strip it off, it
>>    // might confuse the program.
>> 
>> Modified: llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=226762&r1=226761&r2=226762&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp (original)
>> +++ llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp Wed Jan 21 19:49:59 2015
>> @@ -38,7 +38,8 @@ static void exitWithError(const Twine &M
>> 
>> enum ProfileKinds { instr, sample };
>> 
>> -void mergeInstrProfile(cl::list<std::string> Inputs, StringRef OutputFilename) {
>> +void mergeInstrProfile(const cl::list<std::string> &Inputs,
>> +                       StringRef OutputFilename) {
>>  if (OutputFilename.compare("-") == 0)
>>    exitWithError("Cannot write indexed profdata format to stdout.");
>> 
>> @@ -64,7 +65,8 @@ void mergeInstrProfile(cl::list<std::str
>>  Writer.write(Output);
>> }
>> 
>> -void mergeSampleProfile(cl::list<std::string> Inputs, StringRef OutputFilename,
>> +void mergeSampleProfile(const cl::list<std::string> &Inputs,
>> +                        StringRef OutputFilename,
>>                        sampleprof::SampleProfileFormat OutputFormat) {
>>  using namespace sampleprof;
>>  auto WriterOrErr = SampleProfileWriter::create(OutputFilename, OutputFormat);
>> 
>> Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=226762&r1=226761&r2=226762&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)
>> +++ llvm/trunk/tools/llvm-size/llvm-size.cpp Wed Jan 21 19:49:59 2015
>> @@ -709,7 +709,7 @@ int main(int argc, char **argv) {
>> 
>>  ToolName = argv[0];
>>  if (OutputFormatShort.getNumOccurrences())
>> -    OutputFormat = OutputFormatShort;
>> +    OutputFormat = static_cast<OutputFormatTy>(OutputFormatShort);
>>  if (RadixShort.getNumOccurrences())
>>    Radix = RadixShort;
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150121/83d07928/attachment.html>