[llvm] r186319 - [Option] Store arg strings in a set backed by a BumpPtrAllocator

Reid Kleckner rnk at google.com
Mon Jul 15 09:44:05 PDT 2013


I went ahead and reverted in r186329.  This test passes for me on Linux,
and is excluded on Windows due to the shell glob usage.  I'm still not sure
what the root cause is.


On Mon, Jul 15, 2013 at 12:00 PM, David Dean <david_dean at apple.com> wrote:

> This has broken one of the tests. Please have a look and fix/revert as
> soon as you can.
>
>
> http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/4406
>
> ******************** TEST 'Clang :: Driver/crash-report.c' FAILED
> ********************
> Script:
> --
> rm -rf
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp
> mkdir
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp
> not env
> TMPDIR=/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp
> TEMP=/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp
> TMP=/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp
> RC_DEBUG_OPTIONS=1
>  /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/Release+Asserts/bin/clang
>  -fsyntax-only
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c
>   -F/tmp/ -I /tmp/ -idirafter /tmp/ -iquote /tmp/ -isystem /tmp/   -iprefix
> /the/prefix -iwithprefix /tmp -iwithprefixbefore /tmp/   -inter!
>  nal-isystem /tmp/ -internal-externc-isystem /tmp/   -DFOO=BAR 2>&1 |
> FileCheck
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c
> cat
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp/crash-report-*.c
> | FileCheck --check-prefix=CHECKSRC
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c
> cat
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/Driver/Output/crash-report.c.tmp/crash-report-*.sh
> | FileCheck --check-prefix=CHECKSH
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c
> not env FORCE_CLANG_DIAGNOSTICS_CRASH=1
>  /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/Release+Asserts/bin/clang
>  -fsyntax-only -x c /dev/null 2>&1 | FileCheck
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c
> --
> Exit Code: 1
> Command Output (stderr):
> --
> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/Driver/crash-report.c:18:11:
> error: expected string not found in input
> // CHECK: Preprocessed source(s) and associated run script(s) are located
> at:
>           ^
> <stdin>:1:1: note: scanning from here
> clang: warning: argument unused during compilation: '-internal-isystem
> /tmp/'
> ^
> <stdin>:29:112: note: possible intended match here
> clang: note: diagnostic msg: PLEASE submit a bug report to
> http://llvm.org/bugs/ and include the crash backtrace, preprocessed
> source, and associated run script.
>
>                                      ^
> --
>
> ********************
>
> On 15 Jul 2013, at 6:46 AM, Reid Kleckner <reid at kleckner.net> wrote:
>
> > Author: rnk
> > Date: Mon Jul 15 08:46:24 2013
> > New Revision: 186319
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=186319&view=rev
> > Log:
> > [Option] Store arg strings in a set backed by a BumpPtrAllocator
> >
> > No functionality change.
> >
> > This is preparing to move response file parsing into lib/Option so it
> > can be shared between clang and lld.  This change isn't just a
> > micro-optimization.  Clang's driver uses a std::set<std::string> to
> > unique arguments while parsing response files, so this matches that.
> >
> > Modified:
> >    llvm/trunk/include/llvm/Option/ArgList.h
> >    llvm/trunk/lib/Option/ArgList.cpp
> >
> > Modified: llvm/trunk/include/llvm/Option/ArgList.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Option/ArgList.h?rev=186319&r1=186318&r2=186319&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/Option/ArgList.h (original)
> > +++ llvm/trunk/include/llvm/Option/ArgList.h Mon Jul 15 08:46:24 2013
> > @@ -12,9 +12,10 @@
> >
> > #include "llvm/ADT/SmallVector.h"
> > #include "llvm/ADT/StringRef.h"
> > +#include "llvm/ADT/StringSet.h"
> > #include "llvm/Option/OptSpecifier.h"
> > #include "llvm/Option/Option.h"
> > -#include <list>
> > +#include "llvm/Support/Allocator.h"
> > #include <string>
> > #include <vector>
> >
> > @@ -298,7 +299,7 @@ private:
> >   /// This is mutable since we treat the ArgList as being the list
> >   /// of Args, and allow routines to add new strings (to have a
> >   /// convenient place to store the memory) via MakeIndex.
> > -  mutable std::list<std::string> SynthesizedStrings;
> > +  mutable StringSet<BumpPtrAllocator> SynthesizedStrings;
> >
> >   /// The number of original input argument strings.
> >   unsigned NumInputArgStrings;
> >
> > Modified: llvm/trunk/lib/Option/ArgList.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Option/ArgList.cpp?rev=186319&r1=186318&r2=186319&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Option/ArgList.cpp (original)
> > +++ llvm/trunk/lib/Option/ArgList.cpp Mon Jul 15 08:46:24 2013
> > @@ -323,9 +323,18 @@ InputArgList::~InputArgList() {
> > unsigned InputArgList::MakeIndex(StringRef String0) const {
> >   unsigned Index = ArgStrings.size();
> >
> > +  // If necessary, make a copy so we can null terminate it.
> > +  std::string NullTerminated;
> > +  if (String0.back() != '\0') {
> > +    NullTerminated.append(String0.data(), String0.size());
> > +    NullTerminated.push_back('\0');
> > +    String0 = StringRef(&NullTerminated[0], NullTerminated.size());
> > +  }
> > +
> >   // Tuck away so we have a reliable const char *.
> > -  SynthesizedStrings.push_back(String0);
> > -  ArgStrings.push_back(SynthesizedStrings.back().c_str());
> > +  String0 = SynthesizedStrings.GetOrCreateValue(String0).getKey();
> > +  assert(String0.back() == '\0');
> > +  ArgStrings.push_back(String0.data());
> >
> >   return Index;
> > }
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> -David
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130715/1087add8/attachment.html>


More information about the llvm-commits mailing list