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

[Duncan P. N. Exon Smith]

> 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.

>   // 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