Okay. Yeah I don't see us needing to support the equal with trailing nothing.
-Todd > On Sep 22, 2016, at 9:23 AM, Zachary Turner <ztur...@google.com> wrote: > > yea I mostly just wanted to know if we needed to specificlaly distinguish > between ARG=\0 and ARG\0. Because if so the second parameter would need to > be Optional<StringRef> since StringRef has no way to differentiate between "I > don't refer to anything" versus "I refer to the empty string". (Technically > it kinda does, but I think it relies on an implementation detail so shouldn't > be used) > >> On Thu, Sep 22, 2016 at 9:20 AM Todd Fiala <todd.fi...@gmail.com> wrote: >> Primarily because we pass them verbatim to posix_spawn and other launchers, >> and there it is legitimate to not have an equal with trailing nothingness. >> On the Xcode side, we use a ton of environment variables. >> >> As to whether there is a difference between ARG1=\0 and ARG1\0, I'm not sure. >> >> On Thu, Sep 22, 2016 at 9:12 AM, Zachary Turner <ztur...@google.com> wrote: >> Thanks for the test. Is there any practical difference between "ARGS=" and >> "ARGS"? >> >> On Thu, Sep 22, 2016 at 9:08 AM Todd Fiala via lldb-commits >> <lldb-commits@lists.llvm.org> wrote: >> Author: tfiala >> Date: Thu Sep 22 11:00:01 2016 >> New Revision: 282171 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=282171&view=rev >> Log: >> added environment variable-related Args gtests >> >> Also fixed up a couple misbehaving functions. It is perfectly >> legal to have env vars with no values (i.e. the '=' and following >> need not be present). >> >> Modified: >> lldb/trunk/source/Interpreter/Args.cpp >> lldb/trunk/unittests/Interpreter/TestArgs.cpp >> >> Modified: lldb/trunk/source/Interpreter/Args.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=282171&r1=282170&r2=282171&view=diff >> ============================================================================== >> --- lldb/trunk/source/Interpreter/Args.cpp (original) >> +++ lldb/trunk/source/Interpreter/Args.cpp Thu Sep 22 11:00:01 2016 >> @@ -976,13 +976,15 @@ void Args::LongestCommonPrefix(std::stri >> >> void Args::AddOrReplaceEnvironmentVariable(llvm::StringRef env_var_name, >> llvm::StringRef new_value) { >> - if (env_var_name.empty() || new_value.empty()) >> + if (env_var_name.empty()) >> return; >> >> // Build the new entry. >> std::string var_string(env_var_name); >> - var_string += "="; >> - var_string += new_value; >> + if (!new_value.empty()) { >> + var_string += "="; >> + var_string += new_value; >> + } >> >> size_t index = 0; >> if (ContainsEnvironmentVariable(env_var_name, &index)) { >> @@ -1006,7 +1008,7 @@ bool Args::ContainsEnvironmentVariable(l >> >> llvm::StringRef name, value; >> std::tie(name, value) = arg_value.split('='); >> - if (name == env_var_name && !value.empty()) { >> + if (name == env_var_name) { >> if (argument_index) >> *argument_index = i; >> return true; >> >> Modified: lldb/trunk/unittests/Interpreter/TestArgs.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/TestArgs.cpp?rev=282171&r1=282170&r2=282171&view=diff >> ============================================================================== >> --- lldb/trunk/unittests/Interpreter/TestArgs.cpp (original) >> +++ lldb/trunk/unittests/Interpreter/TestArgs.cpp Thu Sep 22 11:00:01 2016 >> @@ -11,6 +11,9 @@ >> >> #include "lldb/Interpreter/Args.h" >> >> +#include <limits> >> +#include <sstream> >> + >> using namespace lldb_private; >> >> TEST(ArgsTest, TestSingleArg) { >> @@ -153,3 +156,85 @@ TEST(ArgsTest, StringToScriptLanguage) { >> } >> >> TEST(ArgsTest, StringToVersion) {} >> + >> +// Environment Variable Tests >> + >> +class EnvVarFixture: public ::testing::Test { >> +protected: >> + >> + void SetUp() { >> + args.AppendArgument(llvm::StringRef("Arg1=foo")); >> + args.AppendArgument(llvm::StringRef("Arg2")); >> + args.AppendArgument(llvm::StringRef("Arg3=bar")); >> + } >> + >> + size_t GetIndexForEnvVar(llvm::StringRef envvar_name) { >> + size_t argument_index = std::numeric_limits<size_t>::max(); >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name, >> + &argument_index)); >> + EXPECT_LT(argument_index, args.GetArgumentCount()); >> + return argument_index; >> + } >> + >> + Args args; >> +}; >> + >> + >> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNoValue) { >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg2"))); >> +} >> + >> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableWithValue) { >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg3"))); >> +} >> + >> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNonExistentVariable) { >> + auto nonexistent_envvar = llvm::StringRef("ThisEnvVarShouldNotExist"); >> + EXPECT_FALSE(args.ContainsEnvironmentVariable(nonexistent_envvar)); >> +} >> + >> +TEST_F(EnvVarFixture, >> TestReplaceEnvironmentVariableInitialNoValueWithNoValue) { >> + auto envvar_name = llvm::StringRef("Arg2"); >> + auto argument_index = GetIndexForEnvVar(envvar_name); >> + >> + args.AddOrReplaceEnvironmentVariable(envvar_name, llvm::StringRef("")); >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); >> + EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index)); >> +} >> + >> +TEST_F(EnvVarFixture, >> TestReplaceEnvironmentVariableInitialNoValueWithValue) { >> + auto envvar_name = llvm::StringRef("Arg2"); >> + auto argument_index = GetIndexForEnvVar(envvar_name); >> + >> + auto new_value = llvm::StringRef("NewValue"); >> + args.AddOrReplaceEnvironmentVariable(envvar_name, new_value); >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); >> + >> + std::stringstream stream; >> + stream << envvar_name.str() << '=' << new_value.str(); >> + EXPECT_EQ(llvm::StringRef(stream.str()), >> + args.GetArgumentAtIndex(argument_index)); >> +} >> + >> +TEST_F(EnvVarFixture, >> TestReplaceEnvironmentVariableInitialValueWithNoValue) { >> + auto envvar_name = llvm::StringRef("Arg1"); >> + auto argument_index = GetIndexForEnvVar(envvar_name); >> + >> + args.AddOrReplaceEnvironmentVariable(envvar_name, llvm::StringRef("")); >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); >> + EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index)); >> +} >> + >> +TEST_F(EnvVarFixture, TestReplaceEnvironmentVariableInitialValueWithValue) { >> + auto envvar_name = llvm::StringRef("Arg1"); >> + auto argument_index = GetIndexForEnvVar(envvar_name); >> + >> + auto new_value = llvm::StringRef("NewValue"); >> + args.AddOrReplaceEnvironmentVariable(envvar_name, new_value); >> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); >> + >> + std::stringstream stream; >> + stream << envvar_name.str() << '=' << new_value.str(); >> + EXPECT_EQ(llvm::StringRef(stream.str()), >> + args.GetArgumentAtIndex(argument_index)); >> +} >> >> >> _______________________________________________ >> lldb-commits mailing list >> lldb-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> >> >> >> -- >> -Todd
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
