[llvm] d881fdf - Revert "Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2"
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 12 11:22:43 PST 2022
Thanks Roman for sending these reminders.
I'd also add that the commit message "recommitting <hash>" isn't
especially informative - usually I submit changes like that as
"Recommit "<original commit message>"" (it sort of mirrors the "Revert
"<...>"" syntax) and then include both the original commit and revert
hash in the full description "This is a recommit of <hash> which was
reverted in <hash>" and the full original commit message along with a
separate paragraph about how the reasons for revert were addressed
(changes made in the patch, external factors, precommitting some
cleanup/fixes, etc).
On Fri, Dec 9, 2022 at 4:28 AM Roman Lebedev via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Reminder to please always mention the reason for the revert in the
> commit message.
>
> On Fri, Dec 9, 2022 at 12:15 PM via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> >
> > Author: serge-sans-paille
> > Date: 2022-12-09T10:15:41+01:00
> > New Revision: d881fdf72047fd18b88c6a65d0966cad542c95cd
> >
> > URL: https://github.com/llvm/llvm-project/commit/d881fdf72047fd18b88c6a65d0966cad542c95cd
> > DIFF: https://github.com/llvm/llvm-project/commit/d881fdf72047fd18b88c6a65d0966cad542c95cd.diff
> >
> > LOG: Revert "Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2"
> >
> > This reverts commit 4faf00006cf989f3ae212912994022c0486a2dc4.
> >
> > Added:
> >
> >
> > Modified:
> > clang/lib/Driver/ToolChains/Gnu.cpp
> > lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> > llvm/include/llvm/Option/OptTable.h
> > llvm/lib/Option/OptTable.cpp
> > llvm/unittests/Option/OptionMarshallingTest.cpp
> > llvm/utils/TableGen/OptParserEmitter.cpp
> >
> > Removed:
> >
> >
> >
> > ################################################################################
> > diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
> > index 60d62e2b9c5c1..4621850f13772 100644
> > --- a/clang/lib/Driver/ToolChains/Gnu.cpp
> > +++ b/clang/lib/Driver/ToolChains/Gnu.cpp
> > @@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
> > if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
> > const Driver &D = TC.getDriver();
> > const llvm::opt::OptTable &Opts = D.getOpts();
> > - StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
> > - StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
> > + const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
> > + const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
> > D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
> > }
> > return HasStaticPIE;
> >
> > diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> > index fde098840be4b..9d89148616be1 100644
> > --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> > +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
> > @@ -1055,7 +1055,7 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
> > // Only add the version-min options if we got a version from somewhere
> > if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
> > #define OPTION(PREFIX, NAME, VAR, ...) \
> > - llvm::StringRef opt_##VAR = NAME; \
> > + const char *opt_##VAR = NAME; \
> > (void)opt_##VAR;
> > #include "clang/Driver/Options.inc"
> > #undef OPTION
> >
> > diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h
> > index 390462d762e61..07d9870f71b33 100644
> > --- a/llvm/include/llvm/Option/OptTable.h
> > +++ b/llvm/include/llvm/Option/OptTable.h
> > @@ -102,7 +102,9 @@ class OptTable {
> > const Option getOption(OptSpecifier Opt) const;
> >
> > /// Lookup the name of the given option.
> > - StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
> > + const char *getOptionName(OptSpecifier id) const {
> > + return getInfo(id).Name;
> > + }
> >
> > /// Get the kind of the given option.
> > unsigned getOptionKind(OptSpecifier id) const {
> >
> > diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
> > index f4426bc34b3f9..ef4873eb7f9c4 100644
> > --- a/llvm/lib/Option/OptTable.cpp
> > +++ b/llvm/lib/Option/OptTable.cpp
> > @@ -36,10 +36,16 @@ namespace opt {
> > // Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
> > // with an exception. '\0' comes at the end of the alphabet instead of the
> > // beginning (thus options precede any other options which prefix them).
> > -static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
> > - size_t MinSize = std::min(A.size(), B.size());
> > - if (int Res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize)))
> > - return Res;
> > +static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
> > + const char *X = A, *Y = B;
> > + char a = tolower(*A), b = tolower(*B);
> > + while (a == b) {
> > + if (a == '\0')
> > + return 0;
> > +
> > + a = tolower(*++X);
> > + b = tolower(*++Y);
> > + }
> >
> > if (a == '\0') // A is a prefix of B.
> > return 1;
> > @@ -54,7 +60,7 @@ static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
> > static int StrCmpOptionName(const char *A, const char *B) {
> > if (int N = StrCmpOptionNameIgnoreCase(A, B))
> > return N;
> > - return A.compare(B);
> > + return strcmp(A, B);
> > }
> >
> > static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) {
> > @@ -180,7 +186,7 @@ static unsigned matchOption(const OptTable::Info *I, StringRef Str,
> > bool Matched = IgnoreCase ? Rest.startswith_insensitive(I->Name)
> > : Rest.startswith(I->Name);
> > if (Matched)
> > - return Prefix.size() + I->Name.size();
> > + return Prefix.size() + StringRef(I->Name).size();
> > }
> > }
> > return 0;
> > @@ -341,8 +347,8 @@ std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,
> >
> > const Info *End = OptionInfos.data() + OptionInfos.size();
> > StringRef Name = Str.ltrim(PrefixChars);
> > - const Info *Start =
> > - std::lower_bound(OptionInfos.data() + FirstSearchableIndex, End, Name);
> > + const Info *Start = std::lower_bound(
> > + OptionInfos.data() + FirstSearchableIndex, End, Name.data());
> > const Info *Fallback = nullptr;
> > unsigned Prev = Index;
> >
> > @@ -397,20 +403,19 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
> > unsigned FlagsToInclude,
> > unsigned FlagsToExclude) const {
> > unsigned Prev = Index;
> > - StringRef Str = Args.getArgString(Index);
> > + const char *Str = Args.getArgString(Index);
> >
> > // Anything that doesn't start with PrefixesUnion is an input, as is '-'
> > // itself.
> > if (isInput(PrefixesUnion, Str))
> > - return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++,
> > - Str.data());
> > + return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str);
> >
> > const Info *Start = OptionInfos.data() + FirstSearchableIndex;
> > const Info *End = OptionInfos.data() + OptionInfos.size();
> > StringRef Name = StringRef(Str).ltrim(PrefixChars);
> >
> > // Search for the first next option which could be a prefix.
> > - Start = std::lower_bound(Start, End, Name);
> > + Start = std::lower_bound(Start, End, Name.data());
> >
> > // Options are stored in sorted order, with '\0' at the end of the
> > // alphabet. Since the only options which can accept a string must
> > @@ -450,11 +455,9 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
> > // If we failed to find an option and this arg started with /, then it's
> > // probably an input path.
> > if (Str[0] == '/')
> > - return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++,
> > - Str.data());
> > + return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str);
> >
> > - return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++,
> > - Str.data());
> > + return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++, Str);
> > }
> >
> > InputArgList OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
> >
> > diff --git a/llvm/unittests/Option/OptionMarshallingTest.cpp b/llvm/unittests/Option/OptionMarshallingTest.cpp
> > index c4d48acc7a3f9..85f0d86a86968 100644
> > --- a/llvm/unittests/Option/OptionMarshallingTest.cpp
> > +++ b/llvm/unittests/Option/OptionMarshallingTest.cpp
> > @@ -6,11 +6,10 @@
> > //
> > //===----------------------------------------------------------------------===//
> >
> > -#include "llvm/ADT/StringRef.h"
> > #include "gtest/gtest.h"
> >
> > struct OptionWithMarshallingInfo {
> > - llvm::StringRef Name;
> > + const char *Name;
> > const char *KeyPath;
> > const char *ImpliedCheck;
> > const char *ImpliedValue;
> > @@ -28,10 +27,10 @@ static const OptionWithMarshallingInfo MarshallingTable[] = {
> > };
> >
> > TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
> > - ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
> > - ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
> > - ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
> > - ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
> > + ASSERT_STREQ(MarshallingTable[0].Name, "marshalled-flag-d");
> > + ASSERT_STREQ(MarshallingTable[1].Name, "marshalled-flag-c");
> > + ASSERT_STREQ(MarshallingTable[2].Name, "marshalled-flag-b");
> > + ASSERT_STREQ(MarshallingTable[3].Name, "marshalled-flag-a");
> > }
> >
> > TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
> >
> > diff --git a/llvm/utils/TableGen/OptParserEmitter.cpp b/llvm/utils/TableGen/OptParserEmitter.cpp
> > index 375d7eda28009..9fff65ec1a2f8 100644
> > --- a/llvm/utils/TableGen/OptParserEmitter.cpp
> > +++ b/llvm/utils/TableGen/OptParserEmitter.cpp
> > @@ -54,10 +54,9 @@ static std::string getOptionSpelling(const Record &R) {
> >
> > static void emitNameUsingSpelling(raw_ostream &OS, const Record &R) {
> > size_t PrefixLength;
> > - OS << "llvm::StringRef(&";
> > + OS << "&";
> > write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
> > - OS << "[" << PrefixLength << "], " << R.getValueAsString("Name").size()
> > - << ")";
> > + OS << "[" << PrefixLength << "]";
> > }
> >
> > class MarshallingInfo {
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list