r200096 - unittests: explicit stringify StringRefs for conversion

[David Blaikie]

On Mon, Jan 27, 2014 at 9:52 PM, Saleem Abdulrasool
<compnerd at compnerd.org>wrote:

> On Mon, Jan 27, 2014 at 9:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>>
>> On Mon, Jan 27, 2014 at 9:16 PM, Saleem Abdulrasool <
>> compnerd at compnerd.org> wrote:
>>
>>> 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.
>>>
>>
>> Where is the function declared, then? (how do we end up calling a
>> function that's not declared?)
>>
>
> Its a template specialisation of std::ostream & operator<<(std::ostream &,
> T &); (from iostream) used to construct the assertion message (the
> StringRef is pushed into a std::ostringstream to construct an assertion
> message).
>

An explicit specialization? The declaration of it should still be exposed
somewhere to the clang unit test library that was having the linking
problem.

If it's an implicit specialization, then why didn't it just get compiled
into the clang library (from a definition in a header somewhere)


>
>
>>
>>
>>>
>>> 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
>>>
>>
>>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140127/c7520213/attachment.html>