r186253 - PR16540: ASTVector::insert(Context, Iter, Element) doesn't compile
David Blaikie
dblaikie at gmail.com
Sun Jul 14 08:41:41 PDT 2013
On Jul 14, 2013 8:30 AM, "Quentin Colombet" <qcolombet at apple.com> wrote:
>
> Hi David,
>
> On Jul 13, 2013, at 11:54 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>>
>> On Sat, Jul 13, 2013 at 11:49 AM, Quentin Colombet <qcolombet at apple.com>
wrote:
>>>
>>> Hi David,
>>>
>>> This breaks a buildbot:
>>>
http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/4351
>>
>>
>> Yes, I'm aware of the issue (it might help if buildcops such as yourself
were on IRC for such things - bots report into the channel & a lot of live
triage happens there for time-sensitive issues such as build breaks.
>
> I didn't know that. Thanks for the information.
No worries - might be something worth suggesting/adding to the apple
buildcop process/documentation.
>
>> It's a bit unnecessary to be sending email (when buildbots already send
out failure mail anyway) when it's already being actively discussed)
>
> Sorry, sometimes people missed that email and I wanted to be sure you
were aware of the problem and working on it. SInce I was not checking IRC,
I guess it is my fault for not knowing you were already working on the
issue.
> My apologies.
Yeah, people do have a tendency to miss the build email (I do sometimes). I
think there are some things we could do to improve that:
* more builds (to ensure precise blame) - using more hardware, incremental
builds, etc.
* less email spam - or make it easier to triage the mail. I guess its
because I'm on lab mailing lists, but getting email for every buildbreak
and buildfix adds a fair bit of noise, I probably need to figure out how to
triage those emails better - but I question their value in general. Knowing
the tree is broken is only important 'now', not historically if I check my
email an hour from now - and the website should be a better indicator of
current state (though it is pull not push)
* possibly having the build failure emails sent as replies to the commit
that caused them, maybe even on-list. This would automate what you and
other apple build cops are doing already (which does add some value,
informing the community of the breakm etc - but is redundant with other
notifications (email and otherwise) while also providing more context in
the form of the actual build failure email (though these emails could be
improved too, they often lack the required context and have a lot of
boilerplate to wade through)
>
>>
>>>
>>> llvm[4]: Linking Release+Asserts unit test AST
>>> Undefined symbols for architecture x86_64:
>>> "operator new[](unsigned long, clang::ASTContext const&, unsigned
long)", referenced from:
>>> clang::ASTVector<int>::insert(clang::ASTContext&, int*, int
const&) in ASTVectorTest.o
>>> ld: symbol(s) not found for architecture x86_64
>>> clang: error: linker command failed with exit code 1 (use -v to see
invocation)
>>> make[4]: *** [Release+Asserts/ASTTests] Error 1
>>> make[3]: *** [AST/.makeall] Error 2
>>> make[3]: *** Waiting for unfinished jobs....
>>>
>>> Could you fix it please?
>>
>>
>> I'd love to, but having some trouble understanding how (short of
reverting the patch, which I can do) - care to offer any insight on how
this might be failing to link an inline function? (& this is building
cleanly for me with CMake+Ninja)
>>
>>>
>>>
>>> Thanks,
>>>
>>> -Quentin
>>> On Jul 13, 2013, at 11:08 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>> Author: dblaikie
>>>> Date: Sat Jul 13 13:08:59 2013
>>>> New Revision: 186253
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=186253&view=rev
>>>> Log:
>>>> PR16540: ASTVector::insert(Context, Iter, Element) doesn't compile
>>>>
>>>> Fix some uninstantiable code in ASTVector::insert. I've added a
>>>> cheap-and-dirty compile test for this, because I don't have the time to
>>>> figure out a nice way to get a real ASTContext to implement executable
>>>> tests - but we probably should have them for this ADT.
>>>>
>>>> Added:
>>>> cfe/trunk/unittests/AST/ASTVectorTest.cpp
>>>> Modified:
>>>> cfe/trunk/include/clang/AST/ASTVector.h
>>>> cfe/trunk/unittests/AST/CMakeLists.txt
>>>>
>>>> Modified: cfe/trunk/include/clang/AST/ASTVector.h
>>>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTVector.h?rev=186253&r1=186252&r2=186253&view=diff
>>>>
==============================================================================
>>>> --- cfe/trunk/include/clang/AST/ASTVector.h (original)
>>>> +++ cfe/trunk/include/clang/AST/ASTVector.h Sat Jul 13 13:08:59 2013
>>>> @@ -216,11 +216,11 @@ public:
>>>>
>>>> iterator insert(ASTContext &C, iterator I, const T &Elt) {
>>>> if (I == this->end()) { // Important special case for empty
vector.
>>>> - push_back(Elt);
>>>> + push_back(Elt, C);
>>>> return this->end()-1;
>>>> }
>>>>
>>>> - if (this->EndX < this->CapacityX) {
>>>> + if (this->End < this->Capacity) {
>>>> Retry:
>>>> new (this->end()) T(this->back());
>>>> this->setEnd(this->end()+1);
>>>>
>>>> Added: cfe/trunk/unittests/AST/ASTVectorTest.cpp
>>>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTVectorTest.cpp?rev=186253&view=auto
>>>>
==============================================================================
>>>> --- cfe/trunk/unittests/AST/ASTVectorTest.cpp (added)
>>>> +++ cfe/trunk/unittests/AST/ASTVectorTest.cpp Sat Jul 13 13:08:59 2013
>>>> @@ -0,0 +1,26 @@
>>>> +//===- unittests/AST/DeclTest.cpp --- Declaration tests
-------------------===//
>>>> +//
>>>> +// The LLVM Compiler Infrastructure
>>>> +//
>>>> +// This file is distributed under the University of Illinois Open
Source
>>>> +// License. See LICENSE.TXT for details.
>>>> +//
>>>>
+//===----------------------------------------------------------------------===//
>>>> +//
>>>> +// Unit tests for the ASTVector container.
>>>> +//
>>>>
+//===----------------------------------------------------------------------===//
>>>> +
>>>> +#include "llvm/Support/Compiler.h"
>>>> +#include "clang/AST/ASTVector.h"
>>>> +#include "clang/Basic/TargetInfo.h"
>>>> +#include "clang/Frontend/CompilerInstance.h"
>>>> +#include "gtest/gtest.h"
>>>> +
>>>> +using namespace clang;
>>>> +
>>>> +LLVM_ATTRIBUTE_UNUSED void CompileTest() {
>>>> + ASTContext *C = 0;
>>>> + ASTVector<int> V;
>>>> + V.insert(*C, V.begin(), 0);
>>>> +}
>>>>
>>>> Modified: cfe/trunk/unittests/AST/CMakeLists.txt
>>>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CMakeLists.txt?rev=186253&r1=186252&r2=186253&view=diff
>>>>
==============================================================================
>>>> --- cfe/trunk/unittests/AST/CMakeLists.txt (original)
>>>> +++ cfe/trunk/unittests/AST/CMakeLists.txt Sat Jul 13 13:08:59 2013
>>>> @@ -1,6 +1,7 @@
>>>> add_clang_unittest(ASTTests
>>>> ASTContextParentMapTest.cpp
>>>> ASTTypeTraitsTest.cpp
>>>> + ASTVectorTest.cpp
>>>> CommentLexer.cpp
>>>> CommentParser.cpp
>>>> DeclPrinterTest.cpp
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130714/a8190eb0/attachment.html>
More information about the cfe-commits
mailing list