[llvm] r187819 - Change private functions of LTOCodeGenerator from ret-false-on-succ to ret-true-on-succ.

[Eric Christopher]

On Tue, Aug 6, 2013 at 3:14 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
> Chris's comment has been authorative. His comment was "I don't have strong
> opinion", meaning
> I can take advantage of any loophole. I don't care if we codify this std or
> not.
>

I suppose that works as "authoritative". And you may not care, but if
you want consistency then you should propose something. Also, if
you're just changing the private functions then that means that code
base isn't consistent, just the private ones are. That's even more
confusing than just documenting what each function returns on success.

-eric

>
> On 8/6/13 3:08 PM, Eric Christopher wrote:
>>
>> Should probably add something into the coding standard if we're going
>> to start moving existing code over to one or the other instead of just
>> changing it, I didn't see anything authorative on the thread.
>>
>> -eric
>>
>> On Tue, Aug 6, 2013 at 3:05 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>>>
>>> This was discussed the other day.  Most people don't like
>>> return-false-on-succ.
>>> My code are all return-true-on-succ, when integrate with the
>>> LTOCodeGenerator,
>>> it will be extremely confusing.
>>>
>>>
>>> On 8/6/13 3:00 PM, Eric Christopher wrote:
>>>>
>>>> ... why?
>>>>
>>>> -eric
>>>>
>>>> On Tue, Aug 6, 2013 at 2:51 PM, Shuxin Yang <shuxin.llvm at gmail.com>
>>>> wrote:
>>>>>
>>>>> Author: shuxin_yang
>>>>> Date: Tue Aug  6 16:51:21 2013
>>>>> New Revision: 187819
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=187819&view=rev
>>>>> Log:
>>>>> Change private functions of LTOCodeGenerator from ret-false-on-succ to
>>>>> ret-true-on-succ.
>>>>>
>>>>> Modified:
>>>>>       llvm/trunk/tools/lto/LTOCodeGenerator.cpp
>>>>>
>>>>> Modified: llvm/trunk/tools/lto/LTOCodeGenerator.cpp
>>>>> URL:
>>>>>
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lto/LTOCodeGenerator.cpp?rev=187819&r1=187818&r2=187819&view=diff
>>>>>
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/tools/lto/LTOCodeGenerator.cpp (original)
>>>>> +++ llvm/trunk/tools/lto/LTOCodeGenerator.cpp Tue Aug  6 16:51:21 2013
>>>>> @@ -159,7 +159,7 @@ bool LTOCodeGenerator::setCodePICModel(l
>>>>>
>>>>>    bool LTOCodeGenerator::writeMergedModules(const char *path,
>>>>>                                              std::string &errMsg) {
>>>>> -  if (determineTarget(errMsg))
>>>>> +  if (!determineTarget(errMsg))
>>>>>        return true;
>>>>>
>>>>>      // Run the verifier on the merged modules.
>>>>> @@ -213,7 +213,7 @@ bool LTOCodeGenerator::compile_to_file(c
>>>>>      }
>>>>>
>>>>>      objFile.keep();
>>>>> -  if (genResult) {
>>>>> +  if (!genResult) {
>>>>>        sys::fs::remove(Twine(Filename));
>>>>>        return true;
>>>>>      }
>>>>> @@ -252,7 +252,7 @@ const void* LTOCodeGenerator::compile(si
>>>>>
>>>>>    bool LTOCodeGenerator::determineTarget(std::string &errMsg) {
>>>>>      if (_target != NULL)
>>>>> -    return false;
>>>>> +    return true;
>>>>>
>>>>>      // if options were requested, set them
>>>>>      if (!_codegenOptions.empty())
>>>>> @@ -267,7 +267,7 @@ bool LTOCodeGenerator::determineTarget(s
>>>>>      // create target machine from info for merged modules
>>>>>      const Target *march = TargetRegistry::lookupTarget(TripleStr,
>>>>> errMsg);
>>>>>      if (march == NULL)
>>>>> -    return true;
>>>>> +    return false;
>>>>>
>>>>>      // The relocation model is actually a static member of
>>>>> TargetMachine
>>>>> and
>>>>>      // needs to be set before the TargetMachine is instantiated.
>>>>> @@ -300,7 +300,7 @@ bool LTOCodeGenerator::determineTarget(s
>>>>>      _target = march->createTargetMachine(TripleStr, _mCpu, FeatureStr,
>>>>> Options,
>>>>>                                           RelocModel,
>>>>> CodeModel::Default,
>>>>>                                           CodeGenOpt::Aggressive);
>>>>> -  return false;
>>>>> +  return true;
>>>>>    }
>>>>>
>>>>>    void LTOCodeGenerator::
>>>>> @@ -391,8 +391,8 @@ void LTOCodeGenerator::applyScopeRestric
>>>>>    /// Optimize merged modules using various IPO passes
>>>>>    bool LTOCodeGenerator::generateObjectFile(raw_ostream &out,
>>>>>                                              std::string &errMsg) {
>>>>> -  if (this->determineTarget(errMsg))
>>>>> -    return true;
>>>>> +  if (!this->determineTarget(errMsg))
>>>>> +    return false;
>>>>>
>>>>>      Module* mergedModule = _linker.getModule();
>>>>>
>>>>> @@ -435,7 +435,7 @@ bool LTOCodeGenerator::generateObjectFil
>>>>>      if (_target->addPassesToEmitFile(codeGenPasses, Out,
>>>>>                                       TargetMachine::CGFT_ObjectFile))
>>>>> {
>>>>>        errMsg = "target file type not supported";
>>>>> -    return true;
>>>>> +    return false;
>>>>>      }
>>>>>
>>>>>      // Run our queue of passes all at once now, efficiently.
>>>>> @@ -444,7 +444,7 @@ bool LTOCodeGenerator::generateObjectFil
>>>>>      // Run the code generator, and write assembly file
>>>>>      codeGenPasses.run(*mergedModule);
>>>>>
>>>>> -  return false; // success
>>>>> +  return true;
>>>>>    }
>>>>>
>>>>>    /// setCodeGenDebugOptions - Set codegen debugging options to aid in
>>>>> debugging
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>