r200096 - unittests: explicit stringify StringRefs for conversion

[Saleem Abdulrasool]

On Mon, Jan 27, 2014 at 9:05 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
>
> On Mon, Jan 27, 2014 at 8:51 PM, Saleem Abdulrasool <compnerd at compnerd.org
> > wrote:
>
>> On Mon, Jan 27, 2014 at 8:38 AM, David Blaikie <dblaikie at gmail.com>wrote:
>>
>>>
>>>
>>>
>>> On Sat, Jan 25, 2014 at 3:07 PM, Saleem Abdulrasool <
>>> compnerd at compnerd.org> wrote:
>>>
>>>> On Sat, Jan 25, 2014 at 12:38 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>>>
>>>>> On Sat, Jan 25, 2014 at 12:04 PM, Saleem Abdulrasool <
>>>>> compnerd at compnerd.org> wrote:
>>>>>
>>>>>> Author: compnerd
>>>>>> Date: Sat Jan 25 14:04:44 2014
>>>>>> New Revision: 200096
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=200096&view=rev
>>>>>> Log:
>>>>>> unittests: explicit stringify StringRefs for conversion
>>>>>>
>>>>>> When clang is built outside of the LLVM tree (against a corresponding
>>>>>> version),
>>>>>> there is no definition providing for operator<<(std::ostream &,
>>>>>> StringRef) which
>>>>>> is required for the assertion routines in google-test tests.
>>>>>
>>>>>
>>>>> Hmm, why is this different for an in-tree vs an out-of-tree build?
>>>>>
>>>>
>>>>  Ugh, I should have phrased that better.  I meant a stand-alone vs
>>>> combined clang build.
>>>>
>>>
>>> The question, I think, still stands, doesn't it? Why is this different
>>> and can we make it not different instead of working around the difference?
>>>
>>
>> With this change, it no longer is different, we always convert the string
>> ref to a string before printing.
>>
>
> Yes - but there's still some difference in the build system that lead to
> this problem and could lead to others. That's not good... we don't want to
> keep introducing/chasing down those problems. We should make the build
> systems similar and not necessitate this change.
>
>
>>  The overload is provided by the unit-tests in LLVM (as that is the only
>> place that the overload is actually used), and when clang is built
>> standalone, the routine is not available.
>>
>
> I'm still not following. What do you mean by "the overload is provided by
> the unit-tests"? Is this declared in a unit test library and defined there
> - how does that declaration get found by the clang unit tests?
>

Yeah, it is implemented in a unit test library that makes its way into the
clang unit test link (I didn't track down the exact path that led to the
library getting included into the clang build).  It is not the declaration
that it is looking for, only the definition.

I suppose that we could provide the extra overload specifically for making
>> the clang unit-tests compile standalone.
>>
>>
>>>
>>>>
>>>>>  Avoid the
>>>>>> compilation failure by explicitly stringifying the StringRef prior to
>>>>>> use.
>>>>>>
>>>>>> Modified:
>>>>>>     cfe/trunk/unittests/AST/DeclPrinterTest.cpp
>>>>>>     cfe/trunk/unittests/AST/StmtPrinterTest.cpp
>>>>>>     cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
>>>>>>
>>>>>> Modified: cfe/trunk/unittests/AST/DeclPrinterTest.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/DeclPrinterTest.cpp?rev=200096&r1=200095&r2=200096&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/unittests/AST/DeclPrinterTest.cpp (original)
>>>>>> +++ cfe/trunk/unittests/AST/DeclPrinterTest.cpp Sat Jan 25 14:04:44
>>>>>> 2014
>>>>>> @@ -77,7 +77,8 @@ public:
>>>>>>    OwningPtr<FrontendActionFactory>
>>>>>> Factory(newFrontendActionFactory(&Finder));
>>>>>>
>>>>>>    if (!runToolOnCodeWithArgs(Factory->create(), Code, Args,
>>>>>> FileName))
>>>>>> -    return testing::AssertionFailure() << "Parsing error in \"" <<
>>>>>> Code << "\"";
>>>>>> +    return testing::AssertionFailure()
>>>>>> +      << "Parsing error in \"" << Code.str() << "\"";
>>>>>>
>>>>>>    if (Printer.getNumFoundDecls() == 0)
>>>>>>      return testing::AssertionFailure()
>>>>>> @@ -90,8 +91,8 @@ public:
>>>>>>
>>>>>>    if (Printer.getPrinted() != ExpectedPrinted)
>>>>>>      return ::testing::AssertionFailure()
>>>>>> -      << "Expected \"" << ExpectedPrinted << "\", "
>>>>>> -         "got \"" << Printer.getPrinted() << "\"";
>>>>>> +      << "Expected \"" << ExpectedPrinted.str() << "\", "
>>>>>> +         "got \"" << Printer.getPrinted().str() << "\"";
>>>>>>
>>>>>>    return ::testing::AssertionSuccess();
>>>>>>  }
>>>>>>
>>>>>> Modified: cfe/trunk/unittests/AST/StmtPrinterTest.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/StmtPrinterTest.cpp?rev=200096&r1=200095&r2=200096&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/unittests/AST/StmtPrinterTest.cpp (original)
>>>>>> +++ cfe/trunk/unittests/AST/StmtPrinterTest.cpp Sat Jan 25 14:04:44
>>>>>> 2014
>>>>>> @@ -76,7 +76,8 @@ public:
>>>>>>    OwningPtr<FrontendActionFactory>
>>>>>> Factory(newFrontendActionFactory(&Finder));
>>>>>>
>>>>>>    if (!runToolOnCodeWithArgs(Factory->create(), Code, Args))
>>>>>> -    return testing::AssertionFailure() << "Parsing error in \"" <<
>>>>>> Code << "\"";
>>>>>> +    return testing::AssertionFailure()
>>>>>> +      << "Parsing error in \"" << Code.str() << "\"";
>>>>>>
>>>>>>    if (Printer.getNumFoundStmts() == 0)
>>>>>>      return testing::AssertionFailure()
>>>>>> @@ -89,8 +90,8 @@ public:
>>>>>>
>>>>>>    if (Printer.getPrinted() != ExpectedPrinted)
>>>>>>      return ::testing::AssertionFailure()
>>>>>> -      << "Expected \"" << ExpectedPrinted << "\", "
>>>>>> -         "got \"" << Printer.getPrinted() << "\"";
>>>>>> +      << "Expected \"" << ExpectedPrinted.str() << "\", "
>>>>>> +         "got \"" << Printer.getPrinted().str() << "\"";
>>>>>>
>>>>>>    return ::testing::AssertionSuccess();
>>>>>>  }
>>>>>>
>>>>>> Modified: cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp?rev=200096&r1=200095&r2=200096&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp (original)
>>>>>> +++ cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp Sat Jan
>>>>>> 25 14:04:44 2014
>>>>>> @@ -24,7 +24,7 @@ static void expectFailure(StringRef JSON
>>>>>>    std::string ErrorMessage;
>>>>>>    EXPECT_EQ(NULL,
>>>>>> JSONCompilationDatabase::loadFromBuffer(JSONDatabase,
>>>>>>
>>>>>>  ErrorMessage))
>>>>>> -    << "Expected an error because of: " << Explanation;
>>>>>> +    << "Expected an error because of: " << Explanation.str();
>>>>>>  }
>>>>>>
>>>>>>  TEST(JSONCompilationDatabase, ErrsOnInvalidFormat) {
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Saleem Abdulrasool
>>>> compnerd (at) compnerd (dot) org
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>
>>
>>
>> --
>> Saleem Abdulrasool
>> compnerd (at) compnerd (dot) org
>>
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140127/6b17d743/attachment.html>