r186253 - PR16540: ASTVector::insert(Context, Iter, Element) doesn't compile

[David Blaikie]

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. It's a
bit unnecessary to be sending email (when buildbots already send out
failure mail anyway) when it's already being actively discussed)


> llvm[4]: Linking Release+Asserts unit test ASTUndefined 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/20130713/d0ff56bf/attachment.html>