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

[David Dean]

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/   -internal-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